[Lldb-commits] [PATCH] D155905: lldb RFC: Exposing set/get address masks, Fix*Address methods in SBProcess

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

The only part of this that's maybe tricky is the HighMem parts of it. Given the niche use case it should be ok if they turn out a bit awkward for general use.

You are missing a way to fix a high mem address, is that intentional? Or would you get the high mem mask, change the setting, fix the address, as needed by context.

The rest are passing on existing calls we know are useful within lldb.



================
Comment at: lldb/include/lldb/API/SBProcess.h:401
 
+  /// Get the current address mask that may be applied to addresses
+  /// before reading from memory.  The bits which are not relevant/valid
----------------
may -> should?

Though for API uses that only run on simple systems, calling this would just be unnecessary overhead.


================
Comment at: lldb/include/lldb/API/SBProcess.h:402-403
+  /// Get the current address mask that may be applied to addresses
+  /// before reading from memory.  The bits which are not relevant/valid
+  /// for addressing should be set.
+  ///
----------------
"should be set in the mask". Or in reverse "should be cleared by the mask". Just be clear whether it's the mask or the address.


================
Comment at: lldb/include/lldb/API/SBProcess.h:416
+  lldb::addr_t GetCodeAddressMask();
+  lldb::addr_t GetDataAddressMask();
+
----------------
Should we note somewhere, maybe below where the `any` method is, the logic behind the code/data split?

Such as it is. Overall it's that data (load store) and code (fetch) addresses may have a different arrangement of addressing bits, so choose the one that makes sense in context. If we don't know, use any. It'd be an obvious question from anyone integrating this into an IDE.


================
Comment at: lldb/include/lldb/API/SBProcess.h:453
+  lldb::addr_t FixDataAddress(lldb::addr_t addr);
+  lldb::addr_t FixAnyAddress(lldb::addr_t addr);
+
----------------
Here is a good place for a note about which to pick.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155905