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

Greg Clayton via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Tue Jul 25 20:42:13 PDT 2023

clayborg added inline comments.

Comment at: lldb/include/lldb/API/SBProcess.h:415-416
+  /// (unsigned long long) 0xfffffffffffffc00
+  lldb::addr_t GetCodeAddressMask();
+  lldb::addr_t GetDataAddressMask();
Will there only ever be two variants of this for code and data? If we think there might be another address mask type later it might be a good idea to use an enumeration for these two functions and all functions below? I was thinking of something like:
enum AddressMaskType {
  eTypeCode = 0,
lldb::addr_t GetAddressMask(AddressMaskType mask_type);

This enum could be used in all subsequent calls. I am just thinking of any needed extra API changes needed in the future. If we need another address mask type in the future we just add another enum and all functions stay the same if we do it as suggested above. Else we will need to add many more API functions if we need a new type later.

Comment at: lldb/include/lldb/API/SBProcess.h:430-434
+  /// If the low ten bits are the only valid bits for addressing,
+  /// this would be the mask returned:
+  ///
+  /// (lldb) p/x ~((1ULL << 10) - 1)
+  /// (unsigned long long) 0xfffffffffffffc00
Is this comment needed for the set accessors? These functions don't return anything

Comment at: lldb/include/lldb/API/SBProcess.h:451-453
+  lldb::addr_t FixCodeAddress(lldb::addr_t addr);
+  lldb::addr_t FixDataAddress(lldb::addr_t addr);
+  lldb::addr_t FixAnyAddress(lldb::addr_t addr);
Should the parameter be named "load_addr" to ensure people know it is a load address?

  rG LLVM Github Monorepo



More information about the lldb-commits mailing list