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

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to