[lldb] Swift OS plugin #9839
[lldb] Swift OS plugin #9839
Conversation
Currently, an LLDB target option controls whether plugins report all threads. However, it seems natural for this knowledge could come from the plugin itself. To support this, this commits adds a virtual method to the plugin base class, returning an `optional<bool>` to denote whether the plugin has an opinion on whether it reports all threads. The default answer of `noopt` causes Process to query the Target option, maintaining compatibility with the old approach.
| @@ -0,0 +1,164 @@ | |||
| //===-- OperatingSystemSwift.cpp -----------------------------------------===// | |||
There was a problem hiding this comment.
Not important. "OperatingSystemSwift" sounds slightly wrong to me. What about "OperatingSystemSwiftTasks" or "OperatingSystemSwiftConcurrency"?
There was a problem hiding this comment.
Yeah the name seems misleading the full lldb affordance name is "Operating System Thread Plugin"
| return nullptr; | ||
|
|
||
| /// Heuristic to only load the plugin for swift programs. | ||
| FileSpec swift_module_name = FileSpec("libswiftCore.dylib"); |
There was a problem hiding this comment.
This only works in a very narrow subset of Swift programs. You should defer to SwiftLanguageRuntime's helper code.
The cases this doesn't support are: non-Darwin (".dylib"), statically linked Swift stdlib, embedded Swift (no dylibs at all).
There was a problem hiding this comment.
Do you have a helper function mind? The only thing I found is GetLikelySwiftImageNamesForModule, which has some very weird behavior I'm not sure I understand.
There was a problem hiding this comment.
You could also arrange that this can only be accessed by the SwiftLanguageRuntime, and then its loading will be gated on the Runtime loading, which already has to do this sort of computation.
There was a problem hiding this comment.
@jimingham I looked into that option, but AFAICT the SwiftLanguageRuntime is loaded regardless of modules.
I.e. the first call to "load runtimes" will always load it, even if the relevant libraries haven't been loaded yet
| swift::tls_get_key(swift::tls_key::concurrency_task) * ptr_size; | ||
| // Offset of a Task ID inside a Task data structure, guaranteed by the ABI. | ||
| // See Job in swift/RemoteInspection/RuntimeInternals.h. | ||
| m_task_id_offset = 4 * ptr_size + 4; |
There was a problem hiding this comment.
You should check that the swift concurrency version is still 1 here.
| } | ||
|
|
||
| // Mask higher bits to avoid conflicts with core thread IDs. | ||
| uint64_t masked_task_id = 0xdeadbeef00000000 | *task_id; |
There was a problem hiding this comment.
Btw it is probably fine to not do this, TIDs are generally higher numbers and there might be value in exposing the task id directly. If we do mask, we should use some other mask
| } | ||
|
|
||
| // Mask higher bits to avoid conflicts with core thread IDs. | ||
| uint64_t masked_task_id = 0xdeadbeef00000000 | *task_id; |
There was a problem hiding this comment.
This will be the visible "TID" for running tasks, right? And what I'd have to type if I wanted to set a "task specific" breakpoint? If so, we might not want it to be deadbeef. That's cute if you know the reference, but otherwise somewhat off-putting...
There was a problem hiding this comment.
Oh yeah, I'm actually looking for suggestions here, if we should mask it at all and what to use if we do
Deadbeef was just something to help me grep the logs as I debugged.
| ThreadSP swift_thread = [&]() -> ThreadSP { | ||
| // If we already had a thread for this Task in the last stop, re-use it. | ||
| if (ThreadSP old_thread = old_thread_list.FindThreadByID(masked_task_id); | ||
| IsOperatingSystemPluginThread(old_thread)) |
There was a problem hiding this comment.
Do you have to handle the case where when you stopped the first time, this Task was backed by "core thread 1" but the next time you stopped it was backed by a different core thread? I don't see where you are changing the backing thread in this case?
|
I note that the only swift specific bits of this are finding the task on a particular thread, and a few checks about whether this should be used that I think belong more properly in the LanguageRuntime that owns the OS plugin... Would it be possible to make an |
No description provided.