debugger: expose shared memories to gdbstub - #14087
Conversation
|
@cfallin would you be able to take a look at this? |
cfallin
left a comment
There was a problem hiding this comment.
Thanks for the PR. I have a number of comments below, as well as one top-level thought:
Lack of support for shared memories is downstream of the bigger issue that we don't support debugging multithreaded programs. That is because wasi-threads has an instance+store-per-thread model, and the debugger controls only one debuggee store. A good debugging experience on a multithreaded program must pause all threads, and that's what a debugger like LLDB expects. The fact that the rest-of-the-world may keep running is visible implicitly in this patch: you need to cast the *UnsafeCell<u8> representing a byte in the shared memory to an *AtomicU8 and do an atomic access to avoid UB on the access because another thread may also be mutating it. But that's a bad debug experience, violates LLDB's assumptions, and so I don't think it makes sense to expose.
Aside from all that, the gdbstub protocol has a bunch of stuff to report and control thread state; I'd expect we would want all that as well if we truly supported multithreaded debugging.
So a question: do you have a plan for those broader questions? Or if not, what is the use-case for attaching a debugger to just one thread and observing its shared memory?
|
Thanks @cfallin I addressed the comments. Shared memories now use a stable debugger ID, the extra unit test and manual qXfer packet are gone, and the LLDB test now reads the memory directly. Local tests pass. This only covers inspecting shared memory in the current single-store debugger; multi-store wasi-threads debugging remains separate. Thanks again fir the review. |
shared linear memories were omitted from the synthetic Wasm address space; this maps both memory types, implements shared-memory access, and adds an LLDB regression test proving address 0 reads correctly.