eclipse-cdt-cloud / vscode-memory-inspector

vscode memory inspector
https://open-vsx.org/extension/eclipse-cdt/memory-inspector
Eclipse Public License 2.0
6 stars 10 forks source link

Support different memory address representations for variables #107

Open martin-fleck-at opened 7 months ago

martin-fleck-at commented 7 months ago

(based on https://github.com/eclipse-cdt-cloud/vscode-memory-inspector/pull/96#discussion_r1523567127)

When querying the memory address for a variable, we currently only support hexadecimal addresses in our default implementation of the C Tracker. However, the ReadMemoryResponse and DisassembledInstruction also support the decimal format as an argument. We should at least think about supporting those two formats and may consider supporting other formats such as octal or binary as well. In those cases, we need to consider whether we do want to convert them to hex or decimal for further processing as they are the only formats mentioned explicitly by the DAP.

martin-fleck-at commented 7 months ago

Furthermore, the current address matching returns the first match but does not require the whole string to be an address. I'd open that up for discussion and we should comment the code accordingly to the decision.

martin-fleck-at commented 7 months ago

@colin-grant-work @jreineckearm An initial version of this that also supports decimal addresses was added as part of https://github.com/eclipse-cdt-cloud/vscode-memory-inspector/pull/96. However, I think we still need to discuss whether we also want to support other data formats (even though the DAP does not mention them explicitly) and whether we expect an address to be returned directly as opposed to extracting the address from the returned textual value as we do now.

jreineckearm commented 7 months ago

I believe we should be good with decimal and hexadecimal for the time being in terms of number formats. I haven't seen yet the usage of octal, float, binary, etc on such interfaces. Normally, you use hexadecimal address values in embedded (and other) debuggers. Or decimal if integers are passed as a string without bothering to format as hexadecimal.

What might be interesting to explore is if we gain anything from resolving textual memory references as they are (with evaluate requests) rather than using address-of on the variable name. Thinking here of expressions for structs like struct_base_address + member_offset. It may help to cover more use cases rather than assuming size-of and address-of operations are present in the debug adapters expression evaluator. That would be in addition to the current approach to preserve size-/address-of as last resort fallback. I am only worried about debug adapters that log multiple errors for each if those attempts. We may in fact enter territory here that is better covered by debug adapter contributions to the Memory Inspector extension. See discussions #68 and #77 .

colin-grant-work commented 7 months ago

I think that if we're ok supporting just bases 10 and 16, the work added in #96 should suffice: it saves the address-of request if we think we already know the address. Making other requests or using metadata that is specific to certain debug adapters is best handled by providing an alternative to the base implementation either upstream or downstream.