[Lldb-commits] [PATCH] D78801: [LLDB] Add class WasmProcess for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev added a comment.





================
Comment at: lldb/source/Plugins/Process/wasm/WasmProcess.cpp:71-76
+  if (vm_addr < 0x100000000) {
+    if (WasmReadMemory(0 /*frame_index*/, vm_addr, buf, size)) {
+      return size;
+    }
+    return 0;
+  } else
----------------
clayborg wrote:
> This seems flaky to me. How are you ever going to get the frame index right unless we encode it into the "vm_addr" using bitfields like we spoke about before. And if we encode it all into the 64 bit address, then we don't need this special read. Seems like we need to figure out if we are going to encode everything into an uint64_t or not. That will be the easiest way to integrate this into LLDB as all memory reads take a "lldb::addr_t" right now (no memory space information). We would change ReadMemory and WriteMemory to start taking a more complex type like:
> 
> ```
> AddressSpecifier {
>   lldb::addr_t addr;
>   uint64_t segment;
> };
> ```
> But that will be a huge change to the LLDB source code that should be done in a separate patch before we do anything here.
I look forward to implementing more cleanly in the future with AddressSpecifiers in ReadMemory and WriteMemory; for the moment I have implemented this with bitfields as suggested:
```
enum WasmAddressType {
  Code = 0x00,
  Data = 0x01,
  Memory = 0x02,
  Invalid = 0x03
};

struct wasm_addr_t {
  uint64_t type : 2;
  uint64_t module_id : 30;
  uint64_t offset : 32;
};
``` 
I still need to override `ReadMemory` here because it is not always possible to generate a full `wasm_addr_t` without making too many changes. Sometimes, for example in `ValueObjectChild::UpdateValue -> ValueObject::GetPointerValue` the existing code generates addresses without the knowledge of module_ids. But this is not a big problem because all these reads always refer to the Wasm Memory and the module_id can easily be retrieved from the current execution context. This is always true because a Wasm module can never read/write the memory of another module in the same process, at most Memories can be shared, but this is transparent to the debugger.

However, this requires a small changes to `ReadMemory`: I would introduce `ExecutionContext *exe_ctx` as an optional final parameter, null by default, passed by `Value` and `ValueObject`, ignored by all other process classes and utilized by ProcessWasm to deduce the module.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78801