[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D72751#1837764 <https://reviews.llvm.org/D72751#1837764>, @paolosev wrote:

> Thanks for the explanation! I wasn't quite clear on "executable module" here, but after your comments I realized that `Target::SetExecutableModule()` should not probably be called also for Wasm modules.
>  The point is that `ObjectFileWasm::CalculateType()` should return `eTypeSharedLibrary`, not `eTypeExecutable`.
>  With this change the first issue is easily solved: we just need to call `Target::SetSectionLoadAddress()` once, in `ObjectFileWasm::SetLoadAddress()` because `Target::SetExecutableModule() -> Target::ClearModules() -> SectionLoadList::Clear()` is not called, and `DynamicLoaderWasmDYLD::DidAttach()` can be simplified to just call `ProcessGDBRemote::LoadModules()`.
>  Does this solution work for you?


I am fine that. I don't really know enough about wasm to say if you should have something like the "main" module, but I don't think it should make a big difference to lldb anyway. And another benefit to that is that we can say we stick to the qXfer "spec" and only send the shared libraries over.

> If so, we should look at the second point, the need to initialize `m_file_addr = m_file_offset = 0` for the "code" Section in order to make the DWARF symbols work...

Yes, let's do that. Can you check what happens if you just move the `file_offset = sect_info.offset & 0xffffffff;` line in ObjectFileWasm outside of the `if(!code)` block? My guess is that you'll just need to replace some section->GetFileOffset() calls with ->GetFileAddress(). I'm pretty sure those calls will be only in wasm code because other object formats don't have sections at file offset zero, and everything is fine with that.



================
Comment at: lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.cpp:56-69
+/// In WebAssembly, linear memory is disjointed from code space. The VM can load
+/// multiple instances of a module, which logically share the same code.
+/// Currently we only support wasm32, which uses a 32-bit address space for the
+/// code.
+/// We represent a code address in LLDB as a 64-bit address with the format:
+/// 63            32 31             0
+/// +---------------+---------------+
----------------
This comment is probably not that useful anymore...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72751/new/

https://reviews.llvm.org/D72751