[Lldb-commits] [lldb] [lldb] Be conversative about setting highmem address masks (PR #90533)
Jason Molenda via lldb-commits
lldb-commits at lists.llvm.org
Mon Apr 29 18:30:51 PDT 2024
https://github.com/jasonmolenda updated https://github.com/llvm/llvm-project/pull/90533
>From 3c272e99326a287f0a61c1f8c2c7ee790e1aeb48 Mon Sep 17 00:00:00 2001
From: Jason Molenda <jason at molenda.com>
Date: Mon, 29 Apr 2024 16:45:36 -0700
Subject: [PATCH 1/2] [lldb] Be conversative about setting highmem address
masks
The most common case with address masks/addressable bits is that
we have one value/mask that applies across the entire address space,
for code and data. On AArch64, we can have separate masks for high
and low address spaces. In the normal "one mask for all memory"
case, we use the low memory masks in Process, and we use the
`virtual-addressable-bits` setting for the user to override that
value. When high memory has a different set of masks, those become
active in Process and the user can use `highmem-virtual-addressable-bits`
to override only the highmem value, if it is wrong.
This patch is handling a case where a gdb stub sent incorrect address
masks, for both high and low memory:
```
(lldb) process plugin packet send qHostInfo
packet: qHostInfo
response: cputype:16777228;cpusubtype:0;endian:little;ptrsize:8;low_mem_addressing_bits:64;high_mem_addressing_bits:64;
```
When in fact this target was using 47 bits of addressing. This
qHostInfo response set the Process low and high address masks for
code and data so that no bit-clearing is performed. The user tried
to override this with the virtual-addressable-bits setting, and was
surprised when it had no affect on code running in high memory.
Because the high memory code mask now has a setting (all bits are
addressable) and they needed to use the very-uncommon
highmem-virtual-addressable-bits setting.
When we have an *unset* high memory address mask, and we are told
to set low- and high-memory to the same new address mask, maintain
the high memory mask as unset in Process. The same thing is done
with the SBProcess::SetAddressMask API when the user specifies
lldb.eAddressMaskRangeAll.
Added a new test case to TestAddressMasks.py to test that the
low-memory override works correctly. Also fixed a bug in the
testsuite that I did where I added `@skipIf(archs=["arm"])`
to avoid a problem on the Linaro 32-bit arm Ubuntu CI bot. I
forgot that this is a regex, and so the tests were being skipped
entirely on any arm.* system.
---
lldb/source/API/SBProcess.cpp | 26 ++++++++++++++++---
lldb/source/Target/Process.cpp | 20 +++++++++++---
.../process/address-masks/TestAddressMasks.py | 21 ++++++++++++---
3 files changed, 56 insertions(+), 11 deletions(-)
diff --git a/lldb/source/API/SBProcess.cpp b/lldb/source/API/SBProcess.cpp
index c37c111c5a58e0..a0654d23e67efd 100644
--- a/lldb/source/API/SBProcess.cpp
+++ b/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->GetHighmemCodeAddressMask() !=
+ 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->GetHighmemDataAddressMask() !=
+ 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->GetHighmemDataAddressMask() !=
+ LLDB_INVALID_ADDRESS_MASK ||
+ process_sp->GetHighmemCodeAddressMask() !=
+ 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);
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 30c240b064b59c..a75d7b12520742 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -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);
+ }
}
}
diff --git a/lldb/test/API/python_api/process/address-masks/TestAddressMasks.py b/lldb/test/API/python_api/process/address-masks/TestAddressMasks.py
index 152776efc726f2..7908a46c0fb38c 100644
--- a/lldb/test/API/python_api/process/address-masks/TestAddressMasks.py
+++ b/lldb/test/API/python_api/process/address-masks/TestAddressMasks.py
@@ -19,7 +19,7 @@ def reset_all_masks(self, process):
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 +80,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 +112,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 +131,19 @@ 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))
>From 897cad96c98144636f0d1621c04de1c25040268b Mon Sep 17 00:00:00 2001
From: Jason Molenda <jason at molenda.com>
Date: Mon, 29 Apr 2024 18:30:26 -0700
Subject: [PATCH 2/2] Fix a corner case with the SBProcess set address mask
APIs
---
lldb/include/lldb/Target/Process.h | 14 ++++++++++++++
lldb/source/API/SBProcess.cpp | 8 ++++----
lldb/source/Target/Process.cpp | 8 ++++----
.../process/address-masks/TestAddressMasks.py | 14 +++++++++++++-
4 files changed, 35 insertions(+), 9 deletions(-)
diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index aac0cf51680a9e..a75d27a013fa13 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/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
diff --git a/lldb/source/API/SBProcess.cpp b/lldb/source/API/SBProcess.cpp
index a0654d23e67efd..81a6ca49d039ee 100644
--- a/lldb/source/API/SBProcess.cpp
+++ b/lldb/source/API/SBProcess.cpp
@@ -1301,7 +1301,7 @@ void SBProcess::SetAddressMask(AddressMaskType type, addr_t 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->GetHighmemCodeAddressMask() !=
+ if (process_sp->GetConcreteHighmemCodeAddressMask() !=
LLDB_INVALID_ADDRESS_MASK)
process_sp->SetHighmemCodeAddressMask(mask);
} else if (addr_range == eAddressMaskRangeHigh) {
@@ -1316,7 +1316,7 @@ void SBProcess::SetAddressMask(AddressMaskType type, addr_t 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->GetHighmemDataAddressMask() !=
+ if (process_sp->GetConcreteHighmemDataAddressMask() !=
LLDB_INVALID_ADDRESS_MASK)
process_sp->SetHighmemDataAddressMask(mask);
} else if (addr_range == eAddressMaskRangeHigh) {
@@ -1332,9 +1332,9 @@ void SBProcess::SetAddressMask(AddressMaskType type, addr_t 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->GetHighmemDataAddressMask() !=
+ if (process_sp->GetConcreteHighmemDataAddressMask() !=
LLDB_INVALID_ADDRESS_MASK ||
- process_sp->GetHighmemCodeAddressMask() !=
+ process_sp->GetConcreteHighmemCodeAddressMask() !=
LLDB_INVALID_ADDRESS_MASK) {
process_sp->SetHighmemCodeAddressMask(mask);
process_sp->SetHighmemDataAddressMask(mask);
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index a75d7b12520742..1ae424d4ac3426 100644
--- a/lldb/source/Target/Process.cpp
+++ b/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);
diff --git a/lldb/test/API/python_api/process/address-masks/TestAddressMasks.py b/lldb/test/API/python_api/process/address-masks/TestAddressMasks.py
index 7908a46c0fb38c..bee890a269e0ce 100644
--- a/lldb/test/API/python_api/process/address-masks/TestAddressMasks.py
+++ b/lldb/test/API/python_api/process/address-masks/TestAddressMasks.py
@@ -14,7 +14,12 @@ 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")
@@ -147,3 +152,10 @@ def test_address_unset_highmem_masks_stay_unset(self):
)
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))
More information about the lldb-commits
mailing list