[lldb] Be conversative about setting highmem address masks

lldb/include/lldb/Target/Process.h

@@ -1465,6 +1465,20 @@ class Process : public std::enable_shared_from_this<Process>,
   /// platforms where there is a difference (only Arm Thumb at this time).
   lldb::addr_t FixAnyAddress(lldb::addr_t pc);
 
+  /// Retrieve the actual address masks for high memory code/data,
+  /// without the normal fallbacks of returning the low-memory masks
+  /// or the user settings.  Needed by SBProcess to determine if
+  /// the high address masks have actually been set, and should
+  /// be updated when "set all masks" is requested.  In most cases,
+  /// the highmem masks should be left to have LLDB_INVALID_ADDRESS_MASK,
+  /// unset.
+  lldb::addr_t GetConcreteHighmemCodeAddressMask() {
+    return m_highmem_code_address_mask;
+  }
+  lldb::addr_t GetConcreteHighmemDataAddressMask() {
+    return m_highmem_data_address_mask;
+  }
+
   /// Get the Modification ID of the process.
   ///
   /// \return

lldb/source/API/SBProcess.cpp

@@ -1298,7 +1298,12 @@ void SBProcess::SetAddressMask(AddressMaskType type, addr_t mask,
     case eAddressMaskTypeCode:
       if (addr_range == eAddressMaskRangeAll) {
         process_sp->SetCodeAddressMask(mask);
-        process_sp->SetHighmemCodeAddressMask(mask);
+        // If the highmem mask is the default-invalid,
+        // don't change it, keep everything consistently
+        // using the lowmem all-address-space mask.
+        if (process_sp->GetConcreteHighmemCodeAddressMask() !=
+            LLDB_INVALID_ADDRESS_MASK)
+          process_sp->SetHighmemCodeAddressMask(mask);
       } else if (addr_range == eAddressMaskRangeHigh) {
         process_sp->SetHighmemCodeAddressMask(mask);
       } else {
@@ -1308,7 +1313,12 @@ void SBProcess::SetAddressMask(AddressMaskType type, addr_t mask,
     case eAddressMaskTypeData:
       if (addr_range == eAddressMaskRangeAll) {
         process_sp->SetDataAddressMask(mask);
-        process_sp->SetHighmemDataAddressMask(mask);
+        // If the highmem mask is the default-invalid,
+        // don't change it, keep everything consistently
+        // using the lowmem all-address-space mask.
+        if (process_sp->GetConcreteHighmemDataAddressMask() !=
+            LLDB_INVALID_ADDRESS_MASK)
+          process_sp->SetHighmemDataAddressMask(mask);
       } else if (addr_range == eAddressMaskRangeHigh) {
         process_sp->SetHighmemDataAddressMask(mask);
       } else {
@@ -1319,8 +1329,16 @@ void SBProcess::SetAddressMask(AddressMaskType type, addr_t mask,
       if (addr_range == eAddressMaskRangeAll) {
         process_sp->SetCodeAddressMask(mask);
         process_sp->SetDataAddressMask(mask);
-        process_sp->SetHighmemCodeAddressMask(mask);
-        process_sp->SetHighmemDataAddressMask(mask);
+        // If the highmem masks are the default-invalid,
+        // don't change them, keep everything consistently
+        // using the lowmem all-address-space masks.
+        if (process_sp->GetConcreteHighmemDataAddressMask() !=
+                LLDB_INVALID_ADDRESS_MASK ||
+            process_sp->GetConcreteHighmemCodeAddressMask() !=
+                LLDB_INVALID_ADDRESS_MASK) {
+          process_sp->SetHighmemCodeAddressMask(mask);
+          process_sp->SetHighmemDataAddressMask(mask);
+        }
       } else if (addr_range == eAddressMaskRangeHigh) {
         process_sp->SetHighmemCodeAddressMask(mask);
         process_sp->SetHighmemDataAddressMask(mask);

lldb/source/Target/Process.cpp

@@ -5681,21 +5681,21 @@ void Process::Flush() {
   m_queue_list_stop_id = 0;
 }
 
-lldb::addr_t Process::GetCodeAddressMask() {
+addr_t Process::GetCodeAddressMask() {
   if (uint32_t num_bits_setting = GetVirtualAddressableBits())
     return AddressableBits::AddressableBitToMask(num_bits_setting);
 
   return m_code_address_mask;
 }
 
-lldb::addr_t Process::GetDataAddressMask() {
+addr_t Process::GetDataAddressMask() {
   if (uint32_t num_bits_setting = GetVirtualAddressableBits())
     return AddressableBits::AddressableBitToMask(num_bits_setting);
 
   return m_data_address_mask;
 }
 
-lldb::addr_t Process::GetHighmemCodeAddressMask() {
+addr_t Process::GetHighmemCodeAddressMask() {
   if (uint32_t num_bits_setting = GetHighmemVirtualAddressableBits())
     return AddressableBits::AddressableBitToMask(num_bits_setting);
 
@@ -5704,7 +5704,7 @@ lldb::addr_t Process::GetHighmemCodeAddressMask() {
   return GetCodeAddressMask();
 }
 
-lldb::addr_t Process::GetHighmemDataAddressMask() {
+addr_t Process::GetHighmemDataAddressMask() {
   if (uint32_t num_bits_setting = GetHighmemVirtualAddressableBits())
     return AddressableBits::AddressableBitToMask(num_bits_setting);
 
@@ -6473,9 +6473,21 @@ void Process::SetAddressableBitMasks(AddressableBits bit_masks) {
   }
 
   if (high_memory_addr_bits != 0) {
-    addr_t high_addr_mask =
-        AddressableBits::AddressableBitToMask(high_memory_addr_bits);
-    SetHighmemCodeAddressMask(high_addr_mask);
-    SetHighmemDataAddressMask(high_addr_mask);
+    // If the same high and low mem address bits were specified,
+    // and we don't have a highmem setting for code and data currently,
+    // don't set the highmem masks.
+    // When we have separate high- and low- masks, the user
+    // setting `virtual-addressable-bits` only overrides the low
+    // memory masks, which most users would be surprised by.
+    // Leave the high memory masks unset, to make it clear that only the
+    // low memory masks are active.
+    if (high_memory_addr_bits != low_memory_addr_bits ||
+        m_highmem_code_address_mask != LLDB_INVALID_ADDRESS_MASK ||
+        m_highmem_data_address_mask != LLDB_INVALID_ADDRESS_MASK) {
+      addr_t high_addr_mask =
+          AddressableBits::AddressableBitToMask(high_memory_addr_bits);
+      SetHighmemCodeAddressMask(high_addr_mask);
+      SetHighmemDataAddressMask(high_addr_mask);
+    }
   }
 }

lldb/test/API/python_api/process/address-masks/TestAddressMasks.py

@@ -14,12 +14,17 @@ def reset_all_masks(self, process):
         process.SetAddressMask(
             lldb.eAddressMaskTypeAll,
             lldb.LLDB_INVALID_ADDRESS_MASK,
-            lldb.eAddressMaskRangeAll,
+            lldb.eAddressMaskRangeLow,
+        )
+        process.SetAddressMask(
+            lldb.eAddressMaskTypeAll,
+            lldb.LLDB_INVALID_ADDRESS_MASK,
+            lldb.eAddressMaskRangeHigh,
         )
         self.runCmd("settings set target.process.virtual-addressable-bits 0")
         self.runCmd("settings set target.process.highmem-virtual-addressable-bits 0")
 
-    @skipIf(archs=["arm"])  # 32-bit arm ABI hardcodes Code mask, is 32-bit
+    @skipIf(archs=["arm$"])  # 32-bit arm ABI hardcodes Code mask, is 32-bit
     def test_address_masks(self):
         self.build()
         (target, process, t, bp) = lldbutil.run_to_source_breakpoint(
@@ -80,7 +85,6 @@ def test_address_masks(self):
     # AArch64 can have different address masks for high and low memory, when different
     # page tables are set up.
     @skipIf(archs=no_match(["arm64", "arm64e", "aarch64"]))
-    @skipIf(archs=["arm"])  # 32-bit arm ABI hardcodes Code mask, is 32-bit
     def test_address_masks_target_supports_highmem_tests(self):
         self.build()
         (target, process, t, bp) = lldbutil.run_to_source_breakpoint(
@@ -113,7 +117,7 @@ def test_address_masks_target_supports_highmem_tests(self):
     # On most targets where we have a single mask for all address range, confirm
     # that the high memory masks are ignored.
     @skipIf(archs=["arm64", "arm64e", "aarch64"])
-    @skipIf(archs=["arm"])  # 32-bit arm ABI hardcodes Code mask, is 32-bit
+    @skipIf(archs=["arm$"])  # 32-bit arm ABI hardcodes Code mask, is 32-bit
     def test_address_masks_target_no_highmem(self):
         self.build()
         (target, process, t, bp) = lldbutil.run_to_source_breakpoint(
@@ -132,3 +136,26 @@ def test_address_masks_target_no_highmem(self):
         self.runCmd("settings set target.process.highmem-virtual-addressable-bits 42")
         self.assertEqual(0x0000000000007694, process.FixAddress(0x00265E950001F694))
         self.assertEqual(0xFFFFFFFFFFFFF694, process.FixAddress(0xFFA65E950000F694))
+
+    # On most targets where we have a single mask for all address range, confirm
+    # that the high memory masks are ignored.
+    @skipIf(archs=no_match(["arm64", "arm64e", "aarch64"]))
+    def test_address_unset_highmem_masks_stay_unset(self):
+        self.build()
+        (target, process, t, bp) = lldbutil.run_to_source_breakpoint(
+            self, "break here", lldb.SBFileSpec("main.c")
+        )
+        self.reset_all_masks(process)
+
+        process.SetAddressableBits(
+            lldb.eAddressMaskTypeAll, 64, lldb.eAddressMaskRangeLow
+        )
+        self.runCmd("settings set target.process.virtual-addressable-bits 47")
+        self.assertEqual(0xFFFFFE0044580BC4, process.FixAddress(0xFFE8FE0044580BC4))
+
+        self.reset_all_masks(process)
+        process.SetAddressableBits(
+            lldb.eAddressMaskTypeAll, 64, lldb.eAddressMaskRangeAll
+        )
+        self.runCmd("settings set target.process.virtual-addressable-bits 47")
+        self.assertEqual(0xFFFFFE0044580BC4, process.FixAddress(0xFFE8FE0044580BC4))