[lldb] Add GetCodeAddressMask and GetDataAddressMask to Process

[JDevlieghere]

Summary

Implement Jason's suggestion from D99944 to have both a code and data mask, while still honoring the existing setting.

[jasonmolenda]

Thanks Jonas, I think this change to Process can cover

Omair's use case, where the number of bits are used hardcoded.

The Darwin use case were ProcessGDBRemote, DynamicLoaderDarwinKernel, and ProcessMachCore can all set the number of bits used in addressing programmatically, detected at runtime.

The firmware use case where people (often in a python script in a dSYM) can set the number of bits manually so it is correct in their environment, when the dynamic detection schemes are not available.

Justin's case where ptrace(PTRACE_GETREGSET) gets us a code and data address mask.

Canonicalizing them as masks in Process, accepting either form, and providing the value as a mask, should make it easy to use in the ABI plugins where we have a FixCodeAddress and FixDataAddress.

On Darwin, we use the same number of bits for both code and data, but given the way ptrace() behaves on Linux, I'm guessing this may not be the case everywhere. Should we store both masks, and add FixCodeAddress + FixDataAddress methods in the ABI's, Justin? What do you think?

The Darwin support can be adapted to this centralized scheme that Jonas has written easily. Everyone else OK with this?

I don't feel strongly about the method name. Justin used Process::GetPointerAuthenticationAddressMask which makes it sound specific to ARM v8.3 ptrauth. The ARM v8.5 MTE tagging might be another use case (or TBI as Omair has noted).

[DavidSpickett]

I don't feel strongly about the method name. Justin used Process::GetPointerAuthenticationAddressMask which makes it sound specific to ARM v8.3 ptrauth. The ARM v8.5 MTE tagging might be another use case (or TBI as Omair has noted).

I'd go with the generic code/address name for the full masks (which would remove MTE/PAC/TBI all at once) and if we find situations where you only need one, add those as separate methods.
(maybe we already have a few of those situations, just my guess that most callers want to remove all the non address bits)

For MTE removing only tags is currently in a plugin and we only really need to do it in certain commands. For everything else removing all the non address bits would be fine.

[omjavaid]

In D100515#2690225, @jasonmolenda wrote:

Thanks Jonas, I think this change to Process can cover

Omair's use case, where the number of bits are used hardcoded.

The Darwin use case were ProcessGDBRemote, DynamicLoaderDarwinKernel, and ProcessMachCore can all set the number of bits used in addressing programmatically, detected at runtime.

The firmware use case where people (often in a python script in a dSYM) can set the number of bits manually so it is correct in their environment, when the dynamic detection schemes are not available.

This looks very good and I have couple of highlights to make from Linux/AArch64 perspective:
qHostInfo is not sufficient for mask calculation on Linux because both data/code ptr auth masks are read by thread register context (both masks are currently same and they same for all the threads of a process).
IMO moving the mask calculation to ABI GetCodeAddress method may be a better option. We can pull in information of addressable bits from process and also read register context for code/data masks by passing threadsp to ABI::GetCodeAddress. But in case we dont get agreement on moving mask calculation to ABI we can calculate mask somewhere inside ProcessGDBRemote and set it from there.... similar to what I have done in D99947.

Justin's case where ptrace(PTRACE_GETREGSET) gets us a code and data address mask.

Canonicalizing them as masks in Process, accepting either form, and providing the value as a mask, should make it easy to use in the ABI plugins where we have a FixCodeAddress and FixDataAddress.

On Darwin, we use the same number of bits for both code and data, but given the way ptrace() behaves on Linux, I'm guessing this may not be the case everywhere. Should we store both masks, and add FixCodeAddress + FixDataAddress methods in the ABI's, Justin? What do you think?

The Darwin support can be adapted to this centralized scheme that Jonas has written easily. Everyone else OK with this?

I don't feel strongly about the method name. Justin used Process::GetPointerAuthenticationAddressMask which makes it sound specific to ARM v8.3 ptrauth. The ARM v8.5 MTE tagging might be another use case (or TBI as Omair has noted).

[pcc]

lldb/source/Target/Process.cpp

5569–5570

Is this part correct? (Same below.) In D100521 you have

if (pc & pac_sign_extension)
  return pc | mask;
return pc & (~mask);

So it looks like this would cause the pc to be set to 0 (or -1)?

[jasonmolenda]

In D100515#2690972, @omjavaid wrote:

This looks very good and I have couple of highlights to make from Linux/AArch64 perspective:
qHostInfo is not sufficient for mask calculation on Linux because both data/code ptr auth masks are read by thread register context (both masks are currently same and they same for all the threads of a process).

Jonas was handling the Darwin case where debugserver returns the number of bits used in addressing in the qHostInfo packet here -- and we use the same value for both. On Linux, we would want lldb-server to fetch the masks and send them up as well. Given that they are fetched with ptrace(), it would make more sense to put these masks in the qProcessInfo response where we're attached to a process. I think we always send that packet after we've launched/attached to something.

You would get masks, so those would be set in Process directly.

IMO moving the mask calculation to ABI GetCodeAddress method may be a better option.

I believe Jonas still wants to have an ABI::FixCodeAddress and ABI::FixDataAddress (I'm still not SUPER thrilled with the name "Fix", but whatever, we can refine it easily if someone has a better suggestion that isn't super long). But the Process will maintain the dynamically-determined masks and the user-specified override setting. The ABI will get the current mask from Process when running these methods.

We're going to have the mask format be the canonical one in Process, so FixCodeAddress and FixDataAddress methods in ABI will get it in that form from Process.

[justincohen]

On Darwin, we use the same number of bits for both code and data, but given the way ptrace() behaves on Linux, I'm guessing this may not be the case everywhere. Should we store both masks, and add FixCodeAddress + FixDataAddress methods in the ABI's, Justin? What do you think?

This all LGTM! I don't have a strong opinion on naming. My understanding is both code and data will be necessary in case TBID0 is set on Linux.

[jasonmolenda]

lldb/source/Target/Process.cpp

5569–5570

I get confused so I like to do this by hand quickly to make sure I understand.

given mask of 1110000 and addr of xxx1011 where 'x' are PAC bits,

b55 == 1: m | a == 1111011
b55 == 0: ~m & a == 0001011

given mask of 1111111, low address 0001011 and high address 1111011,

b55 == 1: m & ha == ha
b55 == 0: ~m | la == la

am I not thinking of something that could unify these? I can confuse myself so easily with these things.

We could also detect a mask of -1 and just return the original address in FixCodeAddress/FixDataAddress, right. That would be very simple.

[JDevlieghere]

lldb/source/Target/Process.cpp

5569–5570

I've added checking for -1 in D100521

[jasonmolenda]

I think we've heard from everyone involved with this & it's good to land. Thanks for bringing this home.

[pcc]

lldb/source/Target/Process.cpp

5569–5570

With a mask of 1111111 isn't it

b55 == 1: m | ha == 1111111
b55 == 0: ~m & la == 0000000

I think you can just remove lines 5569-5570 here as well as lines 819-820 from D100521.

[jasonmolenda]

lldb/source/Target/Process.cpp

5569–5570

urk. my caveat that I often confuse myself with these has been proven true. :)

So you're suggesting the default, uninitialized, mask is 0. If we have that, then

mask 0, low address 0001011, high address 1111011
b55 == 1: m | ha == 1111011
b55 == 0: ~m & la == 0001011

In the mask, any bit set to 0 is passed through as-is. Any bit set to 1 is going to be cleared or set in these FixAddress methods. I see.

[pcc]

lldb/source/Target/Process.cpp

5569–5570

Yes, exactly.

[justincohen]

Out of curiosity: Typically should one be able to set target.process.virtual-addressable-bits after the target has been created? Or is it expected that users will need to run in the following order only:

settings set target.process.virtual-addressable-bits ...
target create -c ....

Setting virtual-addressable-bits won't do anythin after the target has been created (and perhaps that is working as intended?)

[JDevlieghere]

In D100515#2695395, @justincohen wrote:

Out of curiosity: Typically should one be able to set target.process.virtual-addressable-bits after the target has been created? Or is it expected that users will need to run in the following order only:

settings set target.process.virtual-addressable-bits ...
target create -c ....

Setting virtual-addressable-bits won't do anythin after the target has been created (and perhaps that is working as intended?)

Yep, that was exactly why I'm always reading the setting if the mask isn't set yet (as opposed to setting it once in the constructor).

[jasonmolenda]

In D100515#2695559, @JDevlieghere wrote:

In D100515#2695395, @justincohen wrote:

Out of curiosity: Typically should one be able to set target.process.virtual-addressable-bits after the target has been created? Or is it expected that users will need to run in the following order only:

settings set target.process.virtual-addressable-bits ...
target create -c ....

Setting virtual-addressable-bits won't do anythin after the target has been created (and perhaps that is working as intended?)

Yep, that was exactly why I'm always reading the setting if the mask isn't set yet (as opposed to setting it once in the constructor).

And it's an open question of what's the correct behavior if there's a dynamically-set value AND a user specified setting. The patch currently prefers the dynamically-set value and I think that's the right choice, but it'll be interesting to see if there's a platform/system where that doesn't work for some users.

lldb/include/lldb/Target/Process.h

public:
 
   ~ProcessProperties() override;
 
   bool GetDisableMemoryCache() const;
   uint64_t GetMemoryCacheLineSize() const;
   Args GetExtraStartupCommands() const;
   void SetExtraStartupCommands(const Args &args);
   FileSpec GetPythonOSPluginPath() const;
+  uint32_t GetVirtualAddressableBits() const;
+  void SetVirtualAddressableBits(uint32_t bits);
   void SetPythonOSPluginPath(const FileSpec &file);
   bool GetIgnoreBreakpointsInExpressions() const;
   void SetIgnoreBreakpointsInExpressions(bool ignore);
   bool GetUnwindOnErrorInExpressions() const;
   void SetUnwindOnErrorInExpressions(bool ignore);
   bool GetStopOnSharedLibraryEvents() const;
   void SetStopOnSharedLibraryEvents(bool stop);
   bool GetDisableLangRuntimeUnwindPlans() const;
/// \see Process::Notifications
   ///
   /// \return
   ///     The textual description of why the process exited, or nullptr
   ///     if there is no description available.
   const char *GetExitDescription();
 
   virtual void DidExit() {}
 
+  lldb::addr_t GetCodeAddressMask();
+  lldb::addr_t GetDataAddressMask();
+
+  void SetCodeAddressMask(lldb::addr_t code_address_mask) {
+    m_code_address_mask = code_address_mask;
+  }
+
+  void SetDataAddressMask(lldb::addr_t data_address_mask) {
+    m_data_address_mask = data_address_mask;
+  }
+
   /// Get the Modification ID of the process.
   ///
   /// \return
   ///     The modification ID of the process.
   ProcessModID GetModID() const { return m_mod_id; }
 
   const ProcessModID &GetModIDRef() const { return m_mod_id; }
 
bool m_resume_requested; // If m_currently_handling_event or
                            // m_currently_handling_do_on_removals are true,
                            // Resume will only request a resume, using this
                            // flag to check.
 
   /// This is set at the beginning of Process::Finalize() to stop functions
   /// from looking up or creating things during or after a finalize call.
   std::atomic<bool> m_finalizing;
 
+  /// Mask for code an data addresses. The default value (0) means no mask is
+  /// set.
+  /// @{
+  lldb::addr_t m_code_address_mask = 0;
+  lldb::addr_t m_data_address_mask = 0;
+  /// @}
+
   bool m_clear_thread_plans_on_stop;
   bool m_force_next_event_delivery;
   lldb::StateType m_last_broadcast_state; /// This helps with the Public event
                                           /// coalescing in
                                           /// ShouldBroadcastEvent.
   std::map<lldb::addr_t, lldb::addr_t> m_resolved_indirect_addresses;
   bool m_destroy_in_process;
   bool m_can_interpret_function_calls;  // Some targets, e.g the OSX kernel,

lldb/source/Target/Process.cpp

void ProcessProperties::SetExtraStartupCommands(const Args &args) {
   m_collection_sp->SetPropertyAtIndexFromArgs(nullptr, idx, args);
 }
 
 FileSpec ProcessProperties::GetPythonOSPluginPath() const {
   const uint32_t idx = ePropertyPythonOSPluginPath;
   return m_collection_sp->GetPropertyAtIndexAsFileSpec(nullptr, idx);
 }
 
+uint32_t ProcessProperties::GetVirtualAddressableBits() const {
+  const uint32_t idx = ePropertyVirtualAddressableBits;
+  return m_collection_sp->GetPropertyAtIndexAsUInt64(
+      nullptr, idx, g_process_properties[idx].default_uint_value);
+}
+
+void ProcessProperties::SetVirtualAddressableBits(uint32_t bits) {
+  const uint32_t idx = ePropertyVirtualAddressableBits;
+  m_collection_sp->SetPropertyAtIndexAsUInt64(nullptr, idx, bits);
+}
 void ProcessProperties::SetPythonOSPluginPath(const FileSpec &file) {
   const uint32_t idx = ePropertyPythonOSPluginPath;
   m_collection_sp->SetPropertyAtIndexAsFileSpec(nullptr, idx, file);
 }
 
 bool ProcessProperties::GetIgnoreBreakpointsInExpressions() const {
   const uint32_t idx = ePropertyIgnoreBreakpointsInExpressions;
   return m_collection_sp->GetPropertyAtIndexAsBoolean(

 void Process::Flush() {
   m_thread_list.Flush();
   m_extended_thread_list.Flush();
   m_extended_thread_stop_id = 0;
   m_queue_list.Clear();
   m_queue_list_stop_id = 0;
 }
 
+lldb::addr_t Process::GetCodeAddressMask() {
+  if (m_code_address_mask == 0) {
+    if (uint32_t number_of_addressable_bits = GetVirtualAddressableBits()) {
+      lldb::addr_t address_mask = ~((1ULL << number_of_addressable_bits) - 1);
+      SetCodeAddressMask(address_mask);
+    }
+  }
+  return m_code_address_mask;
+}
+
+lldb::addr_t Process::GetDataAddressMask() {
+  if (m_data_address_mask == 0) {
+    if (uint32_t number_of_addressable_bits = GetVirtualAddressableBits()) {
+      lldb::addr_t address_mask = ~((1ULL << number_of_addressable_bits) - 1);
+      SetDataAddressMask(address_mask);
+    }
+  }
+  return m_data_address_mask;
+}
+
 void Process::DidExec() {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
   LLDB_LOGF(log, "Process::%s()", __FUNCTION__);
 
   Target &target = GetTarget();
   target.CleanupProcess();
   target.ClearModules(false);
   m_dynamic_checkers_up.reset();

lldb/source/Target/TargetProperties.td

def StopOnExec: Property<"stop-on-exec", "Boolean">,
     DefaultTrue,
     Desc<"If true, stop when a shared library is loaded or unloaded.">;
   def UtilityExpressionTimeout: Property<"utility-expression-timeout", "UInt64">,
     DefaultUnsignedValue<15>,
     Desc<"The time in seconds to wait for LLDB-internal utility expressions.">;
   def SteppingRunsAllThreads: Property<"run-all-threads", "Boolean">,
     DefaultFalse,
     Desc<"If true, stepping operations will run all threads.  This is equivalent to setting the run-mode option to 'all-threads'.">;
+  def VirtualAddressableBits: Property<"virtual-addressable-bits", "UInt64">,
+    DefaultUnsignedValue<0>,
+    Desc<"The number of bits used for addressing. If the value is 39, then bits 0..38 are used for addressing. The default value of 0 means unspecified.">;
 }
 
 let Definition = "platform" in {
   def UseModuleCache: Property<"use-module-cache", "Boolean">,
     Global,
     DefaultTrue,
     Desc<"Use module cache.">;
   def ModuleCacheDirectory: Property<"module-cache-directory", "FileSpec">,