[lldb] Add support for Task names #10684

kastiglione · 2025-05-14T21:44:11Z

This updates both OperatingSystemSwiftTasks and Task_SummaryProvider to include a
Task's name, if available. See SE-0469 for the introduction of task names.

The result is that threads representing Swift Tasks will show the given name, and when
printing a task, its name will be shown in the summary string.

rdar://146940592

This updates both `OperatingSystemSwiftTasks` and `Task_SummaryProvider` to include a Task's name, if available. See [SE-0469][] for the introduction of task names. The result is that threads representing Swift Tasks will show the given name, and when printing a task, its name will be shown in the summary string. [SE-0469]: https://github.com/swiftlang/swift-evolution/blob/main/proposals/0469-task-names.md

kastiglione · 2025-05-14T21:44:13Z

@swift-ci test

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp

adrian-prantl · 2025-05-14T22:22:12Z

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp

+    return {};
+  }
+
+  TaskStatusRecord getParent(Status &status) {


For future-proofing you could convert these functions to Expected like so:

llvm::Expected<TaskStatusRecord> getParent() { addr_t parent = LLDB_INVALID_ADDRESS; Status status; if (*this) parent = process->ReadPointerFromMemory(addr + ParentOffset, status); if (Status.Fail()) return Status.takeError(); return {process, parent}; }

Because these functions perform a number of Read* calls to process, I kept Status as the means of communicating errors. It makes the code more concise while still having the error handling/control. After the set of Reads (a logical transaction of sorts) have been completed (or failure occurred), the Status is then converted to Expected. This pattern I used elsewhere in support of inspection of other concurrency types.

I'll leave a comment describing this.

I must admit that this pattern also gave me a hard time convincing me everything was correct. Especially reading something like if (getKind(status) != Kind::TaskName), it reads like we're extracting the kind from the status

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp

adrian-prantl · 2025-05-14T22:24:22Z

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp

+
+}; // namespace
+
+llvm::Expected<std::optional<std::string>> GetTaskName(lldb::addr_t task_addr,


Does the optional add anything over just using an empty string for "success, but no name"?

My testing shows that I can provide the empty string as a task name, which should be distinguished from no task name.

To elaborate, using std::optional<std::string> allows both a task name of the empty string (which lldb should show), and a task not having a name.

adrian-prantl · 2025-05-14T22:25:53Z

lldb/source/Plugins/OperatingSystem/SwiftTasks/OperatingSystemSwiftTasks.cpp

+OperatingSystemSwiftTasks::FindTaskName(addr_t task_addr, Process *process) {
+  auto task_name_or_err = GetTaskName(task_addr, process);
+  if (auto err = task_name_or_err.takeError()) {
+    LLDB_LOG_ERROR(GetLog(LLDBLog::OS), std::move(err),


instead of returning optional and logging the error, you can return Expected and return llvm::createStringError(...). And let the call site do logging or error reporting.

or actually, here just return task_name_or_err

ah I guess the entire function becomes pointless then :-)

lldb/source/Plugins/OperatingSystem/SwiftTasks/OperatingSystemSwiftTasks.h

felipepiovezan · 2025-05-14T22:27:38Z

lldb/source/Plugins/OperatingSystem/SwiftTasks/OperatingSystemSwiftTasks.h

+
+  /// Find the (optional) name of the Task at the given address, if any.
+  std::optional<std::string> FindTaskName(lldb::addr_t task_addr,
+                                          Process *process);


This method doesn't need to me a member, it can be declared and defined inside the cpp file

lldb/source/Plugins/OperatingSystem/SwiftTasks/OperatingSystemSwiftTasks.h

felipepiovezan · 2025-05-14T22:29:12Z

lldb/source/Plugins/OperatingSystem/SwiftTasks/OperatingSystemSwiftTasks.h

-  /// Find the Task ID of the task being executed by `thread`, if any.
-  std::optional<uint64_t> FindTaskId(Thread &thread);
+  /// Find the address of the Task being executed by `thread`, if any.
+  std::optional<lldb::addr_t> FindTaskAddress(Thread &thread);


This method doesn't need to be a member

lldb/source/Plugins/OperatingSystem/SwiftTasks/OperatingSystemSwiftTasks.cpp

felipepiovezan · 2025-05-14T22:39:06Z

lldb/source/Plugins/OperatingSystem/SwiftTasks/OperatingSystemSwiftTasks.cpp

+  if (task_name)
+    name = llvm::formatv("{0} (Task {1})", *task_name, task_id);
+  else
+    name = llvm::formatv("Task {0}", task_id);


Similar comment here: let's strive to always initialize at the declaration. A ternary operator should work fine here, and if this ever becomes more complicated we move it to a helper function

In this case I see a ternary as worse readability. As written above, I consider the initialization to be clearly one of two ways. I'm not aware of any standard conventions or strong arguments that code should be bent to ensure variables are always initialized.

I strongly disagree, but won't insist in the interest of timeliness. But please ponder how would you implement this in a language that does not allow for uninitialized variables? (swift, rust, zig, all the modern ones, and why do they make that choice)

Swift does not allow uninitialized variables, but does permit separate declaration and initialization:

var name: String if whatever { name = oneName() } else { name = anotherName() }

kastiglione · 2025-05-15T01:50:52Z

@swift-ci test

…hange UpdateThreadList to handle task failures more granularly

kastiglione · 2025-05-15T15:43:02Z

@swift-ci test

kastiglione · 2025-05-15T17:26:11Z

@swift-ci test

kastiglione · 2025-05-15T19:54:51Z

@swift-ci test

kastiglione · 2025-05-15T19:57:31Z

@swift-ci test

felipepiovezan

LGTM! I think there is still one function that was forgotten in this version, but we can address it afterwards

kastiglione · 2025-05-15T21:11:47Z

@swift-ci test

kastiglione · 2025-05-15T21:12:03Z

Thanks, I updated that function.

kastiglione · 2025-05-15T22:31:22Z

@swift-ci test

adrian-prantl · 2025-05-16T19:37:57Z

@swift-ci test

kastiglione · 2025-05-17T03:42:39Z

@swift-ci test macOS

kastiglione · 2025-05-17T03:42:49Z

@swift-ci test windows

kastiglione · 2025-05-17T21:38:06Z

@swift-ci test macOS

* [lldb] Add support for Task names This updates both `OperatingSystemSwiftTasks` and `Task_SummaryProvider` to include a Task's name, if available. See [SE-0469][] for the introduction of task names. The result is that threads representing Swift Tasks will show the given name, and when printing a task, its name will be shown in the summary string. [SE-0469]: https://github.com/swiftlang/swift-evolution/blob/main/proposals/0469-task-names.md * Add tests * Add docstrings; Switch to process reference; Make functions static; Change UpdateThreadList to handle task failures more granularly * Another switch from pointer to reference; Use GetAddressByteSize * Support 32 bit in TaskStatusRecord * Fix bug in handling of non-Task threads * Change int type of TaskStatusRecord offsets * Make FindTaskId static * Refactor test to (hopefully) work on Linux * Disable test_thread_contains_name on Linux (cherry-picked from commit 262cfab)

kastiglione requested a review from a team as a code owner May 14, 2025 21:44

kastiglione requested review from adrian-prantl and felipepiovezan May 14, 2025 21:45

adrian-prantl reviewed May 14, 2025

View reviewed changes

felipepiovezan reviewed May 14, 2025

View reviewed changes

lldb/source/Plugins/OperatingSystem/SwiftTasks/OperatingSystemSwiftTasks.h Outdated Show resolved Hide resolved

felipepiovezan reviewed May 14, 2025

View reviewed changes

lldb/source/Plugins/OperatingSystem/SwiftTasks/OperatingSystemSwiftTasks.h Outdated Show resolved Hide resolved

felipepiovezan reviewed May 14, 2025

View reviewed changes

Add tests

3e558af

Add docstrings; Switch to process reference; Make functions static; C…

67fc472

…hange UpdateThreadList to handle task failures more granularly

Another switch from pointer to reference; Use GetAddressByteSize

1808c33

kastiglione added 2 commits May 15, 2025 11:05

Support 32 bit in TaskStatusRecord

80758af

Fix bug in handling of non-Task threads

6baa56a

kastiglione requested review from adrian-prantl and felipepiovezan May 15, 2025 19:55

Change int type of TaskStatusRecord offsets

184d150

felipepiovezan approved these changes May 15, 2025

View reviewed changes

Make FindTaskId static

a3187cb

adrian-prantl approved these changes May 15, 2025

View reviewed changes

Refactor test to (hopefully) work on Linux

6ce018a

Disable test_thread_contains_name on Linux

116816e

adrian-prantl merged commit 262cfab into swift/release/6.2 May 19, 2025
3 checks passed

adrian-prantl deleted the dl/lldb-Add-support-for-Task-names branch May 19, 2025 15:43


		}; // namespace

		llvm::Expected<std::optional<std::string>> GetTaskName(lldb::addr_t task_addr,

[lldb] Add support for Task names #10684

[lldb] Add support for Task names #10684

Uh oh!

Conversation

kastiglione commented May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kastiglione commented May 14, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kastiglione May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

felipepiovezan May 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kastiglione commented May 15, 2025

Uh oh!

kastiglione commented May 15, 2025

Uh oh!

kastiglione commented May 15, 2025

Uh oh!

kastiglione commented May 15, 2025

Uh oh!

kastiglione commented May 15, 2025

Uh oh!

felipepiovezan left a comment

Choose a reason for hiding this comment

Uh oh!

kastiglione commented May 15, 2025

Uh oh!

kastiglione commented May 15, 2025

Uh oh!

kastiglione commented May 15, 2025

Uh oh!

adrian-prantl commented May 16, 2025

Uh oh!

kastiglione commented May 17, 2025

Uh oh!

kastiglione commented May 17, 2025

Uh oh!

kastiglione commented May 17, 2025

Uh oh!

Uh oh!

Uh oh!

kastiglione commented May 14, 2025 •

edited

Loading

kastiglione May 14, 2025 •

edited

Loading

felipepiovezan May 15, 2025 •

edited

Loading