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

Fri May 29 12:00:38 PDT 2020

jingham added a comment.

In D78801#2062403 <https://reviews.llvm.org/D78801#2062403>, @labath wrote:

> In D78801#2050449 <https://reviews.llvm.org/D78801#2050449>, @paolosev wrote:
>
> > Hello @clayborg , @labath,
> >  Any thoughts on this latest patch? :-)
>
>
> Sorry about the delay. I think that in terms of the design we've come as far as we can without making substantial changes to a lot of lldb interfaces. The question on my mind is.. is that enough?
>
> Here, I am mainly thinking about the introduction of the ExecutionContext argument to `ReadMemory`. In a universe with static flat address spaces, the argument looks flat out wrong. If one thinks of "memory" as something more dynamic, it does not seem to be that bad, as it can be viewed as the context in which to interpret the memory addresses. However, I am having trouble assigning a semantic to it besides saying "it does what webassembly needs". Maybe it's just because I live in a flat universe and lack address space intuition...
>
> Anyway, I think it would be good to get more people's opinions on this. For start, I nominate Jim. :)
>
> The problem with "looking forward to implementing more cleanly in the future with AddressSpecifiers" is that _everyone_ is looking forward to having address spaces, but noone is actually working on implementing them. And I want to be careful about accumulating technical debt like this upstream, because it's the technical debt which makes future implementations hard.

I wasn't following very closely, sorry...

Before getting to far along, I'd just like to toss out something...

I always thought it was a shame that we ever introduced Process::ReadMemory(addr_t addr, ...)  We already have a nice Address object.  Seems to me the only reason we don't use everywhere is so we can do math on addresses more simply.   For instance, it seems like the Address class is the proper place to pass along Address Space information.  After all, when you make the address you are planning on fetching data from, you have to know what address space you were targeting, so you could naturally encode it at that point.

What about replacing ProcessReadMemory(addr_t addr, ...) with ProcessReadMemory(Address addr, ...), or even banning the use of lldb::addr_t on everywhere except in the bowels of Process subclasses and as an interchange for getting addresses as text from users.  You might want to store lldb::addr_t's for Symbol values for space concerns, but presumably the Symbol File would know the relevant address space, so it would fix that up and always hand out Addresses.

This would be a pretty big but mostly formal change.  It would seem more natural, Address is our abstraction for addresses in a process.  Instead we have this odd mix of API's that take lldb::addr_t and some that take Address.

> 
> 
> In D78801#2026636 <https://reviews.llvm.org/D78801#2026636>, @paolosev wrote:
> 
>> Note that WasmDWARFEvaluator::Evaluate() not only needs to handle Wasm-specific opcodes like DW_OP_WASM_location, but might also handle in a Wasm-specific way some "standard" opcodes. For example, static variables are generally encoded with `DW_OP_addr`, but for WebAssembly the corresponding offset is the offset in the module Data section, which is mapped into an area of the Memory section when the module is loaded. So, memory reads related to DW_OP_addr refer to a different space and should be dealt differently by the runtime.
>>  This makes `target variable *global_ptr` work when we have multiple modules., because we can retrieve the module_id from the Module associated to the DWARFExpression.
> 
> 
> I am confused by this line of reasoning. For a global variable, the DW_OP_addr describes the location of the variable itself (the value of `&global_ptr`). That is what makes it possible to display the value of the pointer (`global_ptr`). Displaying the value of the pointed-to object (`*global_ptr`) is a different thing, and AFAIK it completely bypasses any dwarf expressions. Are you saying that the value of the pointer somehow inherits the "address space" of the memory where the pointer itself is located?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78801