Page MenuHomePhabricator

[LLDB] Add support for WebAssembly debugging
Needs ReviewPublic

Authored by paolosev on Apr 27 2020, 6:17 PM.

Details

Reviewers
labath
clayborg
Summary

This patch is a refactoring of https://reviews.llvm.org/D78801. As suggested, I have created a separate patch to more easily compare the implementations.
Even here, the goal is to use the GDB-remote protocol to connect to a Wasm engine that implements a GDB-remote stub that offers the ability to access the engine runtime internal state.

Here I am removing the interface IWasmProcess and the new class WasmProcessGDBRemote and moving most of the Wasm logic in ProcessGDBRemote.
I am still adding the new Unwind class UnwindWasm, which now however is in core and not in plugins code. Having a Unwind-derived class for Wasm unwinding seems to be the cleanest solution.

The advantage with this new design is that the required changes are smaller and isolated in a few places.
The drawback is that three 'core' classes (UnwindWasm, Value and DWARFExpression) have now a dependency on ProcessGDBRemote and/or ThreadGDBRemote. This could be avoided re-introducing a core interface like IWasmProcess. We could add a couple of virtual method bool IsWasmProcess() and IWasmProcess* AsWasmProcess() to Process, which would be overridden by ProcessGDBRemote. Then these classes could just query this interface, removing direct dependencies on GDBRemote.

I am also adding a new qSupported flag named "wasm" that should tell LLDB whether the remote supports Wasm debugging.

Diff Detail

Event Timeline

paolosev created this revision.Apr 27 2020, 6:17 PM
paolosev edited the summary of this revision. (Show Details)Apr 27 2020, 6:18 PM

Is there any progress about such patch and D78801

I have implemented the debugging feature in our Wasm VM based on https://reviews.llvm.org/D78801, and it already work to attach, set breakpoint, step, show variable value, backtrace...

I am not sure if I need to change LLDB part to this one, or keep using D78801.

But if both patches wont be merged, I have to maintain a private LLDB version....

Is there any progress about such patch and D78801

I have implemented the debugging feature in our Wasm VM based on https://reviews.llvm.org/D78801, and it already work to attach, set breakpoint, step, show variable value, backtrace...

I am not sure if I need to change LLDB part to this one, or keep using D78801.

But if both patches wont be merged, I have to maintain a private LLDB version....

Unfortunately I have not received any more feedback for this patch, or the alternative version D78801, so I assumed it won't move forward :(
Meanwhile, I have created a personal fork (https://github.com/paolosevMSFT/llvm-project/tree/WasmDbg).

I still think that it would be very useful to add support for Wasm debugging to LLDB. Especially in scenarios where Wasm is not used as part of a web app, but server side (node.js) or running on micro runtime for IoT devices.
I know that there is the problem that LLDB does not support segmented address spaces, and so this patch requires a couple of tiny but smelly changes to core code.
From the mailing list I seem to remember that somebody was working to add support for segmented addresses, but I don't know what is the current state of the initiative.

I'd be happy to keep working on this, please let me know what I could do to progress toward an acceptable solution.

Is there any progress about such patch and D78801

I have implemented the debugging feature in our Wasm VM based on https://reviews.llvm.org/D78801, and it already work to attach, set breakpoint, step, show variable value, backtrace...

I am not sure if I need to change LLDB part to this one, or keep using D78801.

But if both patches wont be merged, I have to maintain a private LLDB version....

Unfortunately I have not received any more feedback for this patch, or the alternative version D78801, so I assumed it won't move forward :(
Meanwhile, I have created a personal fork (https://github.com/paolosevMSFT/llvm-project/tree/WasmDbg).

I still think that it would be very useful to add support for Wasm debugging to LLDB. Especially in scenarios where Wasm is not used as part of a web app, but server side (node.js) or running on micro runtime for IoT devices.
I know that there is the problem that LLDB does not support segmented address spaces, and so this patch requires a couple of tiny but smelly changes to core code.
From the mailing list I seem to remember that somebody was working to add support for segmented addresses, but I don't know what is the current state of the initiative.

I'd be happy to keep working on this, please let me know what I could do to progress toward an acceptable solution.

Usually the thing is to ping the review thread at most weekly & maybe search around for specific reviewers to ask if you're met with a lot of silence.

Usually the thing is to ping the review thread at most weekly & maybe search around for specific reviewers to ask if you're met with a lot of silence.

There was not a lot of feedback on this thread, but if you look at https://reviews.llvm.org/D78801, the discussion there was huge :)
But it's a good point! I'll post the same message there.

What's the testing story for WASM going to be?

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
300

Nit:

/ WebAssembly-specific commands
/ \{
...
/// \}

Even better would be doxygen comments for all functions.

What's the testing story for WASM going to be?

Thanks for the feedback @aprantl!
This patch was also create to show a possible alternative to https://reviews.llvm.org/D78801, but D78801 has evolved since then and I should probably close/abandon this one.

I had not looked at the testing story for Wasm yet because it seemed that the changes to the core code could have been a showstopper, but we would need to test a process connected to LLDB through a GDBRemote connection.
I see that this is normally done using the lldbsuite package, GdbRemoteTestCaseBase, which actually launches the debuggee process.
We cannot do the same for Wasm, we cannot run a JS engine that implements a GDB Stub here. We should instead write code similar to GdbRemoteTestCaseBase that simulates a Wasm process, implementing a GDB stub, handling GDBRemote messages (both generic and Wasm-specific).
(This is the opposite of the work done to test the implementation of the GDB Stub in V8: there we had the process and we had to simulate the debugger).

Furthermore, we could also extend:

  • test\Shell\Reproducer\TestGDBRemoteRepro.test
  • test\API\functionalities\gdb_remote_client\TestGDBRemoteClient.py

with other Wasm-specific tests.

Thanks for the explanation!