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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
