From fb7aed7ea2e1942bd7f40e17690c2086ccefa3b2 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Fri, 8 Aug 2025 14:07:30 -0700 Subject: [PATCH 01/20] [lldb] Make StackID call Fix{Code,Data} pointers In architectures where pointers may contain metadata, such as arm64e, it is important to ignore those bits when comparing two different StackIDs, as the metadata may not be the same even if the pointers are. This patch is a step towards allowing consumers of pointers to decide whether they want to keep or remove metadata, as opposed to discarding metadata at the moment pointers are created. See https://github.com/llvm/llvm-project/pull/150537. This was tested running the LLDB test suite on arm64e. (cherry picked from commit d662d7703a1a0b04a78387204b3bea3c2ee93120) --- lldb/include/lldb/Target/StackID.h | 12 +++++++----- lldb/source/Target/StackFrame.cpp | 20 +++++++++++++------- lldb/source/Target/StackID.cpp | 18 ++++++++++++++++++ 3 files changed, 38 insertions(+), 12 deletions(-) diff --git a/lldb/include/lldb/Target/StackID.h b/lldb/include/lldb/Target/StackID.h index a965c3f6c7225..1d14db52c6849 100644 --- a/lldb/include/lldb/Target/StackID.h +++ b/lldb/include/lldb/Target/StackID.h @@ -14,11 +14,16 @@ namespace lldb_private { +class Process; + class StackID { public: // Constructors and Destructors StackID() = default; + explicit StackID(lldb::addr_t pc, lldb::addr_t cfa, + SymbolContextScope *symbol_scope, Process *process); + StackID(const StackID &rhs) = default; ~StackID() = default; @@ -68,11 +73,8 @@ class StackID { protected: friend class StackFrame; - explicit StackID(lldb::addr_t pc, lldb::addr_t cfa) : m_pc(pc), m_cfa(cfa) {} - - void SetPC(lldb::addr_t pc) { m_pc = pc; } - - void SetCFA(lldb::addr_t cfa) { m_cfa = cfa; } + void SetPC(lldb::addr_t pc, Process *process); + void SetCFA(lldb::addr_t cfa, Process *process); lldb::addr_t m_pc = LLDB_INVALID_ADDRESS; // The pc value for the function/symbol for this diff --git a/lldb/source/Target/StackFrame.cpp b/lldb/source/Target/StackFrame.cpp index 1df8b16da8a3d..1530827deab7c 100644 --- a/lldb/source/Target/StackFrame.cpp +++ b/lldb/source/Target/StackFrame.cpp @@ -64,7 +64,8 @@ StackFrame::StackFrame(const ThreadSP &thread_sp, user_id_t frame_idx, const SymbolContext *sc_ptr) : m_thread_wp(thread_sp), m_frame_index(frame_idx), m_concrete_frame_index(unwind_frame_index), m_reg_context_sp(), - m_id(pc, cfa), m_frame_code_addr(pc), m_sc(), m_flags(), m_frame_base(), + m_id(pc, cfa, nullptr, thread_sp->GetProcess().get()), + m_frame_code_addr(pc), m_sc(), m_flags(), m_frame_base(), m_frame_base_error(), m_cfa_is_valid(cfa_is_valid), m_stack_frame_kind(kind), m_behaves_like_zeroth_frame(behaves_like_zeroth_frame), @@ -74,7 +75,7 @@ StackFrame::StackFrame(const ThreadSP &thread_sp, user_id_t frame_idx, // recursive functions properly aren't confused with one another on a history // stack. if (IsHistorical() && !m_cfa_is_valid) { - m_id.SetCFA(m_frame_index); + m_id.SetCFA(m_frame_index, thread_sp->GetProcess().get()); } if (sc_ptr != nullptr) { @@ -90,9 +91,11 @@ StackFrame::StackFrame(const ThreadSP &thread_sp, user_id_t frame_idx, const SymbolContext *sc_ptr) : m_thread_wp(thread_sp), m_frame_index(frame_idx), m_concrete_frame_index(unwind_frame_index), - m_reg_context_sp(reg_context_sp), m_id(pc, cfa), m_frame_code_addr(pc), - m_sc(), m_flags(), m_frame_base(), m_frame_base_error(), - m_cfa_is_valid(true), m_stack_frame_kind(StackFrame::Kind::Regular), + m_reg_context_sp(reg_context_sp), + m_id(pc, cfa, nullptr, thread_sp->GetProcess().get()), + m_frame_code_addr(pc), m_sc(), m_flags(), m_frame_base(), + m_frame_base_error(), m_cfa_is_valid(true), + m_stack_frame_kind(StackFrame::Kind::Regular), m_behaves_like_zeroth_frame(behaves_like_zeroth_frame), m_variable_list_sp(), m_variable_list_value_objects(), m_recognized_frame_sp(), m_disassembly(), m_mutex() { @@ -116,7 +119,8 @@ StackFrame::StackFrame(const ThreadSP &thread_sp, user_id_t frame_idx, : m_thread_wp(thread_sp), m_frame_index(frame_idx), m_concrete_frame_index(unwind_frame_index), m_reg_context_sp(reg_context_sp), - m_id(pc_addr.GetLoadAddress(thread_sp->CalculateTarget().get()), cfa), + m_id(pc_addr.GetLoadAddress(thread_sp->CalculateTarget().get()), cfa, + nullptr, thread_sp->GetProcess().get()), m_frame_code_addr(pc_addr), m_sc(), m_flags(), m_frame_base(), m_frame_base_error(), m_cfa_is_valid(true), m_stack_frame_kind(StackFrame::Kind::Regular), @@ -2002,7 +2006,9 @@ void StackFrame::UpdatePreviousFrameFromCurrentFrame(StackFrame &curr_frame) { std::lock_guard guard(m_mutex); assert(GetStackID() == curr_frame.GetStackID()); // TODO: remove this after some testing - m_id.SetPC(curr_frame.m_id.GetPC()); // Update the Stack ID PC value + m_id.SetPC( + curr_frame.m_id.GetPC(), + curr_frame.CalculateProcess().get()); // Update the Stack ID PC value assert(GetThread() == curr_frame.GetThread()); m_frame_index = curr_frame.m_frame_index; m_concrete_frame_index = curr_frame.m_concrete_frame_index; diff --git a/lldb/source/Target/StackID.cpp b/lldb/source/Target/StackID.cpp index 19989d731d42c..e0340f523a846 100644 --- a/lldb/source/Target/StackID.cpp +++ b/lldb/source/Target/StackID.cpp @@ -14,6 +14,7 @@ #include "lldb/Target/Process.h" #include "lldb/Target/Thread.h" #include "lldb/Utility/LLDBLog.h" +#include "lldb/Target/Process.h" #include "lldb/Utility/Stream.h" using namespace lldb_private; @@ -32,6 +33,23 @@ bool StackID::IsCFAOnStack(Process &process) const { return m_cfa_on_stack == eLazyBoolYes; } +StackID::StackID(lldb::addr_t pc, lldb::addr_t cfa, + SymbolContextScope *symbol_scope, Process *process) + : m_pc(pc), m_cfa(cfa), m_symbol_scope(symbol_scope) { + if (process) { + m_pc = process->FixCodeAddress(m_pc); + m_cfa = process->FixDataAddress(m_cfa); + } +} + +void StackID::SetPC(lldb::addr_t pc, Process *process) { + m_pc = process ? process->FixCodeAddress(pc) : pc; +} + +void StackID::SetCFA(lldb::addr_t cfa, Process *process) { + m_cfa = process ? process->FixDataAddress(cfa) : cfa; +} + void StackID::Dump(Stream *s) { s->Printf("StackID (pc = 0x%16.16" PRIx64 ", cfa = 0x%16.16" PRIx64 ", cfa_on_stack = %d, symbol_scope = %p", From c52fe951dd1a3f0afb6b95b2c97ac654a3533d70 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Tue, 2 Sep 2025 14:00:06 -0700 Subject: [PATCH 02/20] [lldb] Fix StackIDTest compilation issue Commit 9c8e71644227 updated some APIs that are used by this downstream-only test. (cherry picked from commit 5d30e9cd32210082cd6ff95793b36d50c30959ca) --- lldb/unittests/StackID/StackIDTest.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lldb/unittests/StackID/StackIDTest.cpp b/lldb/unittests/StackID/StackIDTest.cpp index de62f3cd9b4d2..9d5ab45fb8c97 100644 --- a/lldb/unittests/StackID/StackIDTest.cpp +++ b/lldb/unittests/StackID/StackIDTest.cpp @@ -68,8 +68,8 @@ enum OnStack { Yes, No }; /// Helper class to enable testing StackID::IsYounger. struct MockStackID : StackID { MockStackID(addr_t cfa, OnStack on_stack) : StackID() { - SetPC(0); - SetCFA(cfa); + SetPC(0, nullptr); + SetCFA(cfa, nullptr); m_cfa_on_stack = on_stack == OnStack::Yes ? LazyBool::eLazyBoolYes : LazyBool::eLazyBoolNo; } From 077e3514711e38a0d361422d32a37f9405b9f879 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Thu, 4 Sep 2025 13:05:10 -0700 Subject: [PATCH 03/20] [lldb] Call FixUpPointer in WritePointerToMemory (try 2) (#153585) In architectures where pointers may contain metadata, such as arm64e, the metadata may need to be cleaned prior to sending this pointer to be used in expression evaluation generated code. This patch is a step towards allowing consumers of pointers to decide whether they want to keep or remove metadata, as opposed to discarding metadata at the moment pointers are created. See #150537. This was tested running the LLDB test suite on arm64e. (The first attempt at this patch caused a failure in TestScriptedProcessEmptyMemoryRegion.py. This test exercises a case where IRMemoryMap uses host memory in its allocations; pointers to such allocations should not be fixed, which is what the original patch failed to account for). (cherry picked from commit f88eadda357b0429b390ec0bcf64c361ad1a8f28) --- lldb/source/Expression/IRMemoryMap.cpp | 9 ++++ .../arm-pointer-metadata-stripping/Makefile | 2 + .../TestArmPointerMetadataStripping.py | 48 +++++++++++++++++++ .../extra_symbols.json | 21 ++++++++ .../arm-pointer-metadata-stripping/main.c | 13 +++++ 5 files changed, 93 insertions(+) create mode 100644 lldb/test/API/macosx/arm-pointer-metadata-stripping/Makefile create mode 100644 lldb/test/API/macosx/arm-pointer-metadata-stripping/TestArmPointerMetadataStripping.py create mode 100644 lldb/test/API/macosx/arm-pointer-metadata-stripping/extra_symbols.json create mode 100644 lldb/test/API/macosx/arm-pointer-metadata-stripping/main.c diff --git a/lldb/source/Expression/IRMemoryMap.cpp b/lldb/source/Expression/IRMemoryMap.cpp index 65b5d11413c85..6411a99bbd256 100644 --- a/lldb/source/Expression/IRMemoryMap.cpp +++ b/lldb/source/Expression/IRMemoryMap.cpp @@ -638,6 +638,15 @@ void IRMemoryMap::WritePointerToMemory(lldb::addr_t process_address, lldb::addr_t address, Status &error) { error.Clear(); + /// Only ask the Process to fix the address if this address belongs to the + /// process. An address belongs to the process if the Allocation policy is not + /// eAllocationPolicyHostOnly. + auto it = FindAllocation(address, 1); + if (it == m_allocations.end() || + it->second.m_policy != AllocationPolicy::eAllocationPolicyHostOnly) + if (auto process_sp = GetProcessWP().lock()) + address = process_sp->FixAnyAddress(address); + Scalar scalar(address); WriteScalarToMemory(process_address, scalar, GetAddressByteSize(), error); diff --git a/lldb/test/API/macosx/arm-pointer-metadata-stripping/Makefile b/lldb/test/API/macosx/arm-pointer-metadata-stripping/Makefile new file mode 100644 index 0000000000000..c9319d6e6888a --- /dev/null +++ b/lldb/test/API/macosx/arm-pointer-metadata-stripping/Makefile @@ -0,0 +1,2 @@ +C_SOURCES := main.c +include Makefile.rules diff --git a/lldb/test/API/macosx/arm-pointer-metadata-stripping/TestArmPointerMetadataStripping.py b/lldb/test/API/macosx/arm-pointer-metadata-stripping/TestArmPointerMetadataStripping.py new file mode 100644 index 0000000000000..f61945b3eb4c9 --- /dev/null +++ b/lldb/test/API/macosx/arm-pointer-metadata-stripping/TestArmPointerMetadataStripping.py @@ -0,0 +1,48 @@ +import lldb +import json +import os +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +@skipUnlessDarwin +@skipIf(archs=no_match(["arm64", "arm64e"])) +class TestArmPointerMetadataStripping(TestBase): + # Use extra_symbols.json as a template to add a new symbol whose address + # contains non-zero high order bits set. + def create_symbols_file(self): + template_path = os.path.join(self.getSourceDir(), "extra_symbols.json") + with open(template_path, "r") as f: + symbols_data = json.load(f) + + target = self.dbg.GetSelectedTarget() + symbols_data["triple"] = target.GetTriple() + + module = target.GetModuleAtIndex(0) + symbols_data["uuid"] = module.GetUUIDString() + + json_filename = self.getBuildArtifact("extra_symbols.json") + with open(json_filename, "w") as file: + json.dump(symbols_data, file, indent=4) + + return json_filename + + def test(self): + self.build() + src = lldb.SBFileSpec("main.c") + target, process, thread, bkpt = lldbutil.run_to_source_breakpoint( + self, "break here", src + ) + + symbols_file = self.create_symbols_file() + self.runCmd(f"target module add {symbols_file}") + + # The high order bits should be stripped. + self.expect_expr("get_high_bits(&myglobal_json)", result_value="0") + + # Mark all bits as used for addresses and ensure bits are no longer stripped. + self.runCmd("settings set target.process.virtual-addressable-bits 64") + self.expect_expr( + "get_high_bits(&myglobal_json)", result_value=str(0x1200000000000000) + ) diff --git a/lldb/test/API/macosx/arm-pointer-metadata-stripping/extra_symbols.json b/lldb/test/API/macosx/arm-pointer-metadata-stripping/extra_symbols.json new file mode 100644 index 0000000000000..5c2503d508b42 --- /dev/null +++ b/lldb/test/API/macosx/arm-pointer-metadata-stripping/extra_symbols.json @@ -0,0 +1,21 @@ +{ + "triple": "replace me", + "uuid": "replace me", + "type": "executable", + "sections": [ + { + "name": "__DATA", + "type": "data", + "address": 1297224342667202580, + "size": 16 + } + ], + "symbols": [ + { + "name": "myglobal_json", + "size": 8, + "type": "data", + "address": 1297224342667202580 + } + ] +} diff --git a/lldb/test/API/macosx/arm-pointer-metadata-stripping/main.c b/lldb/test/API/macosx/arm-pointer-metadata-stripping/main.c new file mode 100644 index 0000000000000..05a85133caf72 --- /dev/null +++ b/lldb/test/API/macosx/arm-pointer-metadata-stripping/main.c @@ -0,0 +1,13 @@ +#include + +uintptr_t get_high_bits(void *ptr) { + uintptr_t address_bits = 56; + uintptr_t mask = ~((1ULL << address_bits) - 1); + uintptr_t ptrtoint = (uintptr_t)ptr; + uintptr_t high_bits = ptrtoint & mask; + return high_bits; +} + +int main() { + return 0; // break here +} From da2792c0fe6fb58d68abab094a7afa905c2367fb Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Wed, 30 Jul 2025 14:56:19 -0700 Subject: [PATCH 04/20] [lldb] Call FixAddress from TestSwiftAsyncBacktraceLocals (cherry picked from commit bbdcc4765c2d39805fe42519ea581234c655ded5) --- .../unwind/backtrace_locals/TestSwiftAsyncBacktraceLocals.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lldb/test/API/lang/swift/async/unwind/backtrace_locals/TestSwiftAsyncBacktraceLocals.py b/lldb/test/API/lang/swift/async/unwind/backtrace_locals/TestSwiftAsyncBacktraceLocals.py index 928e6f91cda29..6568eb5543b52 100644 --- a/lldb/test/API/lang/swift/async/unwind/backtrace_locals/TestSwiftAsyncBacktraceLocals.py +++ b/lldb/test/API/lang/swift/async/unwind/backtrace_locals/TestSwiftAsyncBacktraceLocals.py @@ -75,8 +75,8 @@ def run_fibo_tests(self, target, process): # The PC of a logical frame is stored in its "callee" # AsyncContext as the second pointer field. error = lldb.SBError() - ret_addr = process.ReadPointerFromMemory( - cfa[fibonacci_number-1] + target.addr_size, error) + ret_addr = process.FixAddress(process.ReadPointerFromMemory( + cfa[fibonacci_number-1] + target.addr_size, error)) prologue_to_skip = frame.GetFunction().GetPrologueByteSize() self.assertSuccess(error, "Managed to read context memory") self.assertEqual(ret_addr + prologue_to_skip, frame.GetPC()) From a62c77505ef39b0e1047b49d8e2b87ad091eda0c Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Wed, 30 Jul 2025 14:59:35 -0700 Subject: [PATCH 05/20] [lldb] Increase tolerance for skipped instructions in TestSwiftAsyncUnwindAllInstructions In arm64, this is number is 0.102, which is barely above 0.1 (cherry picked from commit bfaa20a0f62cd349363b070e5410982623041b08) --- .../TestSwiftAsyncUnwindAllInstructions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/test/API/lang/swift/async/unwind/unwind_in_all_instructions/TestSwiftAsyncUnwindAllInstructions.py b/lldb/test/API/lang/swift/async/unwind/unwind_in_all_instructions/TestSwiftAsyncUnwindAllInstructions.py index 5ea9ee0144195..e46a8e49307ca 100644 --- a/lldb/test/API/lang/swift/async/unwind/unwind_in_all_instructions/TestSwiftAsyncUnwindAllInstructions.py +++ b/lldb/test/API/lang/swift/async/unwind/unwind_in_all_instructions/TestSwiftAsyncUnwindAllInstructions.py @@ -186,4 +186,4 @@ def test(self): # have some sanity check that we have hit at least a decent chunk of # them. breakpoints_not_hit = len(breakpoints) - self.assertLess(breakpoints_not_hit / num_breakpoints, 0.10) + self.assertLess(breakpoints_not_hit / num_breakpoints, 0.15) From 17e9a972b55514f736bf13f86dcb12183d1628ee Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Thu, 31 Jul 2025 08:02:41 -0700 Subject: [PATCH 06/20] [lldb] Disable TestSwiftAsyncUnwindRecursiveQFunclets for arm64e This test was designed to distinguish the case that doesn't work on this architecture. (cherry picked from commit 04fdf8de1b2ae2b4b440334c78692b32b3b3f722) --- .../TestSwiftAsyncUnwindRecursiveQFunclets.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lldb/test/API/lang/swift/async/unwind/unwind_recursive_q_funclets/TestSwiftAsyncUnwindRecursiveQFunclets.py b/lldb/test/API/lang/swift/async/unwind/unwind_recursive_q_funclets/TestSwiftAsyncUnwindRecursiveQFunclets.py index 09a5603736b38..4a7d0273e1581 100644 --- a/lldb/test/API/lang/swift/async/unwind/unwind_recursive_q_funclets/TestSwiftAsyncUnwindRecursiveQFunclets.py +++ b/lldb/test/API/lang/swift/async/unwind/unwind_recursive_q_funclets/TestSwiftAsyncUnwindRecursiveQFunclets.py @@ -12,6 +12,7 @@ class TestCase(lldbtest.TestBase): @swiftTest @skipIf(oslist=["windows", "linux"]) + @skipIf(archs=["arm64e"]) def test(self): """Test that the debugger can unwind at all instructions of all funclets""" self.build() From 007b2071f57a3285f90c7e41864f1e0131bdc752 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Tue, 2 Sep 2025 14:32:51 -0700 Subject: [PATCH 07/20] [lldb] Fix typo in TestSwiftOtherArchDylib skipIf for arm64e This was disabling the test for the wrong architecture. (cherry picked from commit 5a93cadf65f66ad1407a6f8a226030cea5b56c0c) --- .../API/lang/swift/other_arch_dylib/TestSwiftOtherArchDylib.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/test/API/lang/swift/other_arch_dylib/TestSwiftOtherArchDylib.py b/lldb/test/API/lang/swift/other_arch_dylib/TestSwiftOtherArchDylib.py index 872cfe9d70392..fbeb7c99eafe2 100644 --- a/lldb/test/API/lang/swift/other_arch_dylib/TestSwiftOtherArchDylib.py +++ b/lldb/test/API/lang/swift/other_arch_dylib/TestSwiftOtherArchDylib.py @@ -13,7 +13,7 @@ class TestSwiftOtherArchDylib(TestBase): @skipUnlessDarwin @skipIfDarwinEmbedded @skipIf(archs=no_match(["arm64"])) - @skipIf(archs=["arm64"], bugnumber="the swift.org toolchain cannot produce arm64e binaries") + @skipIf(archs=["arm64e"], bugnumber="the swift.org toolchain cannot produce arm64e binaries") def test(self): """Test module import from dylibs with an architecture that uses a different SDK""" From 52990e25b07169bb5328ce07f9db6e328473eb8d Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Thu, 31 Jul 2025 10:23:47 -0700 Subject: [PATCH 08/20] [lldb] Disable TestDAP_stackTraceMissingFunctionName on arm64e This test jumps to address 0 and expects to get a bad frame on top of the stack. However, on arm64e this will fail while still on main because pointer authentication will fail. (cherry picked from commit 2adf931b2f07b6f3aa43cedc0fe617d35380ccc7) --- .../TestDAP_stackTraceMissingFunctionName.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lldb/test/API/tools/lldb-dap/stackTraceMissingFunctionName/TestDAP_stackTraceMissingFunctionName.py b/lldb/test/API/tools/lldb-dap/stackTraceMissingFunctionName/TestDAP_stackTraceMissingFunctionName.py index f2131d6a82121..bfc88a5275e9b 100644 --- a/lldb/test/API/tools/lldb-dap/stackTraceMissingFunctionName/TestDAP_stackTraceMissingFunctionName.py +++ b/lldb/test/API/tools/lldb-dap/stackTraceMissingFunctionName/TestDAP_stackTraceMissingFunctionName.py @@ -8,6 +8,8 @@ class TestDAP_stackTraceMissingFunctionName(lldbdap_testcase.DAPTestCaseBase): @skipIfWindows + # Jumping to address 0 will fail PAC signing before crashign on a bad frame. + @skipIf(archs=["arm64e"]) def test_missingFunctionName(self): """ Test that the stack frame without a function name is given its pc in the response. From 8cc7914ffb74c177ad6bbf6d3a34b858e278914f Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Mon, 8 Sep 2025 08:47:42 -0700 Subject: [PATCH 09/20] [lldb][NFC] Fix style issues with StackID.h (#157483) Some comments were "suffixed" to member variable declarations; these are moved to before the variable. Some constructors and operators were just defaulted and not necessary. Some comments dividing the class into logical sections, like "// constructors and destructors", were not applied everywhere. These were removed. They are used in some parts of LLDB, but are the exception. An include was not needed. The operator != can be defined in terms of ==. (cherry picked from commit 54b3dc1dad9a28709e880b54a416cdcdb624cad4) --- lldb/include/lldb/Target/StackID.h | 57 +++++++++++------------------- lldb/source/Target/StackID.cpp | 11 +----- 2 files changed, 22 insertions(+), 46 deletions(-) diff --git a/lldb/include/lldb/Target/StackID.h b/lldb/include/lldb/Target/StackID.h index 1d14db52c6849..5c217f5df9398 100644 --- a/lldb/include/lldb/Target/StackID.h +++ b/lldb/include/lldb/Target/StackID.h @@ -10,7 +10,6 @@ #define LLDB_TARGET_STACKID_H #include "lldb/Core/AddressRange.h" -#include "lldb/lldb-private.h" namespace lldb_private { @@ -18,14 +17,11 @@ class Process; class StackID { public: - // Constructors and Destructors StackID() = default; explicit StackID(lldb::addr_t pc, lldb::addr_t cfa, SymbolContextScope *symbol_scope, Process *process); - StackID(const StackID &rhs) = default; - ~StackID() = default; lldb::addr_t GetPC() const { return m_pc; } @@ -51,17 +47,6 @@ class StackID { void Dump(Stream *s); - // Operators - const StackID &operator=(const StackID &rhs) { - if (this != &rhs) { - m_pc = rhs.m_pc; - m_cfa = rhs.m_cfa; - m_cfa_on_stack = rhs.m_cfa_on_stack; - m_symbol_scope = rhs.m_symbol_scope; - } - return *this; - } - /// Check if the CFA is on the stack, or elsewhere in the process, such as on /// the heap. bool IsCFAOnStack(Process &process) const; @@ -76,28 +61,28 @@ class StackID { void SetPC(lldb::addr_t pc, Process *process); void SetCFA(lldb::addr_t cfa, Process *process); - lldb::addr_t m_pc = - LLDB_INVALID_ADDRESS; // The pc value for the function/symbol for this - // frame. This will - // only get used if the symbol scope is nullptr (the code where we are - // stopped is not represented by any function or symbol in any shared - // library). - lldb::addr_t m_cfa = - LLDB_INVALID_ADDRESS; // The call frame address (stack pointer) value - // at the beginning of the function that uniquely - // identifies this frame (along with m_symbol_scope - // below) - // True if the CFA is an address on the stack, false if it's an address - // elsewhere (ie heap). + /// The pc value for the function/symbol for this frame. This will only get + /// used if the symbol scope is nullptr (the code where we are stopped is not + /// represented by any function or symbol in any shared library). + lldb::addr_t m_pc = LLDB_INVALID_ADDRESS; + + /// The call frame address (stack pointer) value at the beginning of the + /// function that uniquely identifies this frame (along with m_symbol_scope + /// below) + lldb::addr_t m_cfa = LLDB_INVALID_ADDRESS; + + /// If nullptr, there is no block or symbol for this frame. If not nullptr, + /// this will either be the scope for the lexical block for the frame, or the + /// scope for the symbol. Symbol context scopes are always be unique pointers + /// since the are part of the Block and Symbol objects and can easily be used + /// to tell if a stack ID is the same as another. + SymbolContextScope *m_symbol_scope = nullptr; + + // BEGIN SWIFT + /// True if the CFA is an address on the stack, false if it's an address + /// elsewhere (ie heap). mutable LazyBool m_cfa_on_stack = eLazyBoolCalculate; - SymbolContextScope *m_symbol_scope = - nullptr; // If nullptr, there is no block or symbol for this frame. - // If not nullptr, this will either be the scope for the - // lexical block for the frame, or the scope for the - // symbol. Symbol context scopes are always be unique - // pointers since the are part of the Block and Symbol - // objects and can easily be used to tell if a stack ID - // is the same as another. + // END SWIFT }; bool operator==(const StackID &lhs, const StackID &rhs); diff --git a/lldb/source/Target/StackID.cpp b/lldb/source/Target/StackID.cpp index e0340f523a846..cd8eda6ebefd4 100644 --- a/lldb/source/Target/StackID.cpp +++ b/lldb/source/Target/StackID.cpp @@ -81,16 +81,7 @@ bool lldb_private::operator==(const StackID &lhs, const StackID &rhs) { } bool lldb_private::operator!=(const StackID &lhs, const StackID &rhs) { - if (lhs.GetCallFrameAddress() != rhs.GetCallFrameAddress()) - return true; - - SymbolContextScope *lhs_scope = lhs.GetSymbolContextScope(); - SymbolContextScope *rhs_scope = rhs.GetSymbolContextScope(); - - if (lhs_scope == nullptr && rhs_scope == nullptr) - return lhs.GetPC() != rhs.GetPC(); - - return lhs_scope != rhs_scope; + return !(lhs == rhs); } // BEGIN SWIFT From d695eb00c865cfff926f088d44bb64fecb9607d6 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Tue, 9 Sep 2025 07:27:52 -0700 Subject: [PATCH 10/20] [lldb][nfc] Rename WritePointerToMemory argument's name (#157566) One of those arguments should be called `pointer` to correlate it to the name of the function and to distinguish it from the address where it will be written. (cherry picked from commit d367c7d01620f723981652f72256df7c953d871d) --- lldb/include/lldb/Expression/IRMemoryMap.h | 2 +- lldb/source/Expression/IRMemoryMap.cpp | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lldb/include/lldb/Expression/IRMemoryMap.h b/lldb/include/lldb/Expression/IRMemoryMap.h index abec5442793c6..deac6e00f04c8 100644 --- a/lldb/include/lldb/Expression/IRMemoryMap.h +++ b/lldb/include/lldb/Expression/IRMemoryMap.h @@ -59,7 +59,7 @@ class IRMemoryMap { size_t size, Status &error); void WriteScalarToMemory(lldb::addr_t process_address, Scalar &scalar, size_t size, Status &error); - void WritePointerToMemory(lldb::addr_t process_address, lldb::addr_t address, + void WritePointerToMemory(lldb::addr_t process_address, lldb::addr_t pointer, Status &error); void ReadMemory(uint8_t *bytes, lldb::addr_t process_address, size_t size, Status &error); diff --git a/lldb/source/Expression/IRMemoryMap.cpp b/lldb/source/Expression/IRMemoryMap.cpp index 6411a99bbd256..12cfbeb36df79 100644 --- a/lldb/source/Expression/IRMemoryMap.cpp +++ b/lldb/source/Expression/IRMemoryMap.cpp @@ -635,19 +635,19 @@ void IRMemoryMap::WriteScalarToMemory(lldb::addr_t process_address, } void IRMemoryMap::WritePointerToMemory(lldb::addr_t process_address, - lldb::addr_t address, Status &error) { + lldb::addr_t pointer, Status &error) { error.Clear(); - /// Only ask the Process to fix the address if this address belongs to the + /// Only ask the Process to fix `pointer` if the address belongs to the /// process. An address belongs to the process if the Allocation policy is not /// eAllocationPolicyHostOnly. - auto it = FindAllocation(address, 1); + auto it = FindAllocation(pointer, 1); if (it == m_allocations.end() || it->second.m_policy != AllocationPolicy::eAllocationPolicyHostOnly) if (auto process_sp = GetProcessWP().lock()) - address = process_sp->FixAnyAddress(address); + pointer = process_sp->FixAnyAddress(pointer); - Scalar scalar(address); + Scalar scalar(pointer); WriteScalarToMemory(process_address, scalar, GetAddressByteSize(), error); } From 75070b05a2b3dd3ddffa1f6181a2c995c9630105 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Thu, 11 Sep 2025 07:04:18 -0700 Subject: [PATCH 11/20] [lldb] Make TestSwiftAsyncFrameVarMultipleFrames consider stripped pointers By always stripping pointers in the test driver, the test will work regardless of architecture. (cherry picked from commit 761ac92cc187e21ac6150b6e91f1929907ffcf52) --- .../TestSwiftAsyncFrameVarMultipleFrames.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lldb/test/API/lang/swift/async/frame/variables_multiple_frames/TestSwiftAsyncFrameVarMultipleFrames.py b/lldb/test/API/lang/swift/async/frame/variables_multiple_frames/TestSwiftAsyncFrameVarMultipleFrames.py index 6a8daf44db31f..37805ebe4eee0 100644 --- a/lldb/test/API/lang/swift/async/frame/variables_multiple_frames/TestSwiftAsyncFrameVarMultipleFrames.py +++ b/lldb/test/API/lang/swift/async/frame/variables_multiple_frames/TestSwiftAsyncFrameVarMultipleFrames.py @@ -17,11 +17,14 @@ def read_ptr_from_memory(self, process, addr): # Check that the CFA chain is correctly built def check_cfas(self, async_frames, process): - async_cfas = list(map(lambda frame: frame.GetCFA(), async_frames)) + async_cfas = list( + map(lambda frame: process.FixAddress(frame.GetCFA()), async_frames) + ) expected_cfas = [async_cfas[0]] # The CFA chain ends in nullptr. while expected_cfas[-1] != 0: - expected_cfas.append(self.read_ptr_from_memory(process, expected_cfas[-1])) + indirect_ctx = self.read_ptr_from_memory(process, expected_cfas[-1]) + expected_cfas.append(process.FixAddress(indirect_ctx)) self.assertEqual(async_cfas, expected_cfas[:-1]) @@ -43,13 +46,10 @@ def check_async_regs_one_frame(self, frame, process): cfa = frame.GetCFA() is_indirect = "await resume" in frame.GetFunctionName() - async_register = frame.FindRegister(async_reg_name).GetValueAsUnsigned() - + async_ctx = frame.FindRegister(async_reg_name).GetValueAsUnsigned() if is_indirect: - deref_async_reg = self.read_ptr_from_memory(process, async_register) - self.assertEqual(deref_async_reg, cfa) - else: - self.assertEqual(async_register, cfa) + async_ctx = self.read_ptr_from_memory(process, async_ctx) + self.assertEqual(process.FixAddress(async_ctx), process.FixAddress(cfa)) def check_async_regs(self, async_frames, process): for frame in async_frames: From 5ecc9f13904177e2d3bc8c831872a6ed754aec73 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Thu, 11 Sep 2025 11:24:56 -0700 Subject: [PATCH 12/20] [lldb] Strip pointer metadata when in Swift Task formatters This is mostly a cosmetic choice (and also to fix test expectations). Users don't need the metadata when running `task` commands that expect a pointer. (cherry picked from commit 92fb650a6fee587fe70bf84fcf56d3f2e83242e7) --- lldb/source/Plugins/Language/Swift/SwiftFormatters.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lldb/source/Plugins/Language/Swift/SwiftFormatters.cpp b/lldb/source/Plugins/Language/Swift/SwiftFormatters.cpp index afc5d0c740bb2..58ecdf799cc5a 100644 --- a/lldb/source/Plugins/Language/Swift/SwiftFormatters.cpp +++ b/lldb/source/Plugins/Language/Swift/SwiftFormatters.cpp @@ -863,6 +863,8 @@ class TaskSyntheticFrontEnd : public SyntheticChildrenFrontEnd { m_ts->GetTypeFromMangledTypename(ConstString("$sSVD")); addr_t value = m_task_ptr; + if (auto process_sp = m_backend.GetProcessSP()) + value = process_sp->FixDataAddress(value); DataExtractor data{reinterpret_cast(&value), sizeof(value), endian::InlHostByteOrder(), sizeof(void *)}; @@ -903,7 +905,7 @@ class TaskSyntheticFrontEnd : public SyntheticChildrenFrontEnd { parent_addr = 0; } - addr_t value = parent_addr; + addr_t value = process_sp->FixDataAddress(parent_addr); DataExtractor data{reinterpret_cast(&value), sizeof(value), endian::InlHostByteOrder(), sizeof(void *)}; From c507ec1b1550cfa6608a0e78e6fb2046de404969 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Fri, 12 Sep 2025 09:17:48 -0700 Subject: [PATCH 13/20] [lldb] Track CFA pointer metadata in StackID (#157498) In this commit: 9c8e71644227 [lldb] Make StackID call Fix{Code,Data} pointers (#152796) We made StackID keep track of the CFA without any pointer metadata in it. This is necessary when comparing two StackIDs to determine which one is "younger". However, the CFA inside StackIDs is also used in other contexts through the method StackID::GetCallFrameAddress. One notable case is DWARFExpression: the computation of `DW_OP_call_frame_address` is done using StackID. This feeds into many other places, e.g. expression evaluation may require the address of a variable that is computed from the CFA; to access the variable without faulting, we may need to preserve the pointer metadata. As such, StackID must be able to provide both versions of the CFA. In the spirit of allowing consumers of pointers to decide what to do with pointer metadata, this patch changes StackID to store both versions of the cfa pointer. Two getter methods are provided, and all call sites except DWARFExpression preserve their existing behavior (stripped pointer). Other alternatives were considered: * Just store the raw pointer. This would require changing the comparisong operator `<` to also receive a Process, as the comparison requires stripped pointers. It wasn't clear if all call-sites had a non-null process, whereas we know we have a process when creating a StackID. * Store a weak pointer to the process inside the class, and then strip metadata as needed. This would require a `weak_ptr::lock` in many operations of LLDB, and it felt wasteful. It also prevents stripping of the pointer if the process has gone away. This patch also changes RegisterContextUnwind::ReadFrameAddress, which is the method computing the CFA fed into StackID, to also preserve the signature pointers. (cherry picked from commit 5d088ba30440d37f180f6b2e2f2fcc25d5c77018) --- lldb/include/lldb/Target/StackID.h | 9 +- lldb/source/API/SBFrame.cpp | 2 +- lldb/source/Expression/DWARFExpression.cpp | 2 +- lldb/source/Target/RegisterContextUnwind.cpp | 8 - lldb/source/Target/StackFrameList.cpp | 2 +- lldb/source/Target/StackID.cpp | 17 +- .../Makefile | 11 + .../TestArmPointerMetadataCFADwarfExpr.py | 35 +++ .../main.s | 226 ++++++++++++++++++ 9 files changed, 293 insertions(+), 19 deletions(-) create mode 100644 lldb/test/API/macosx/arm-pointer-metadata-cfa-dwarf-expr/Makefile create mode 100644 lldb/test/API/macosx/arm-pointer-metadata-cfa-dwarf-expr/TestArmPointerMetadataCFADwarfExpr.py create mode 100644 lldb/test/API/macosx/arm-pointer-metadata-cfa-dwarf-expr/main.s diff --git a/lldb/include/lldb/Target/StackID.h b/lldb/include/lldb/Target/StackID.h index 5c217f5df9398..7dccf918cc22f 100644 --- a/lldb/include/lldb/Target/StackID.h +++ b/lldb/include/lldb/Target/StackID.h @@ -26,7 +26,11 @@ class StackID { lldb::addr_t GetPC() const { return m_pc; } - lldb::addr_t GetCallFrameAddress() const { return m_cfa; } + lldb::addr_t GetCallFrameAddressWithMetadata() const { + return m_cfa_with_metadata; + } + + lldb::addr_t GetCallFrameAddressWithoutMetadata() const { return m_cfa; } SymbolContextScope *GetSymbolContextScope() const { return m_symbol_scope; } @@ -71,6 +75,9 @@ class StackID { /// below) lldb::addr_t m_cfa = LLDB_INVALID_ADDRESS; + /// The cfa with metadata (i.e. prior to Process::FixAddress). + lldb::addr_t m_cfa_with_metadata = LLDB_INVALID_ADDRESS; + /// If nullptr, there is no block or symbol for this frame. If not nullptr, /// this will either be the scope for the lexical block for the frame, or the /// scope for the symbol. Symbol context scopes are always be unique pointers diff --git a/lldb/source/API/SBFrame.cpp b/lldb/source/API/SBFrame.cpp index 6963da3ce88bf..135440b0eb035 100644 --- a/lldb/source/API/SBFrame.cpp +++ b/lldb/source/API/SBFrame.cpp @@ -318,7 +318,7 @@ lldb::addr_t SBFrame::GetCFA() const { StackFrame *frame = exe_ctx.GetFramePtr(); if (frame) - return frame->GetStackID().GetCallFrameAddress(); + return frame->GetStackID().GetCallFrameAddressWithoutMetadata(); return LLDB_INVALID_ADDRESS; } diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp index 536091be0284e..53d151041dc6d 100644 --- a/lldb/source/Expression/DWARFExpression.cpp +++ b/lldb/source/Expression/DWARFExpression.cpp @@ -2279,7 +2279,7 @@ llvm::Expected DWARFExpression::Evaluate( // Note that we don't have to parse FDEs because this DWARF expression // is commonly evaluated with a valid stack frame. StackID id = frame->GetStackID(); - addr_t cfa = id.GetCallFrameAddress(); + addr_t cfa = id.GetCallFrameAddressWithMetadata(); if (cfa != LLDB_INVALID_ADDRESS) { stack.push_back(Scalar(cfa)); stack.back().SetValueType(Value::ValueType::LoadAddress); diff --git a/lldb/source/Target/RegisterContextUnwind.cpp b/lldb/source/Target/RegisterContextUnwind.cpp index c40e0d59911e4..67c2bc15a467d 100644 --- a/lldb/source/Target/RegisterContextUnwind.cpp +++ b/lldb/source/Target/RegisterContextUnwind.cpp @@ -2002,8 +2002,6 @@ bool RegisterContextUnwind::ReadFrameAddress( reg_info, cfa_reg_contents, reg_info->byte_size, reg_value); if (error.Success()) { address = reg_value.GetAsUInt64(); - if (abi_sp) - address = abi_sp->FixCodeAddress(address); UnwindLogMsg( "CFA value via dereferencing reg %s (%d): reg has val 0x%" PRIx64 ", CFA value is 0x%" PRIx64, @@ -2024,8 +2022,6 @@ bool RegisterContextUnwind::ReadFrameAddress( RegisterNumber cfa_reg(m_thread, row_register_kind, fa.GetRegisterNumber()); if (ReadGPRValue(cfa_reg, cfa_reg_contents)) { - if (abi_sp) - cfa_reg_contents = abi_sp->FixDataAddress(cfa_reg_contents); if (cfa_reg_contents == LLDB_INVALID_ADDRESS || cfa_reg_contents == 0 || cfa_reg_contents == 1) { UnwindLogMsg( @@ -2060,9 +2056,6 @@ bool RegisterContextUnwind::ReadFrameAddress( dwarfexpr.Evaluate(&exe_ctx, this, 0, nullptr, nullptr); if (result) { address = result->GetScalar().ULongLong(); - if (ABISP abi_sp = m_thread.GetProcess()->GetABI()) - address = abi_sp->FixCodeAddress(address); - UnwindLogMsg("CFA value set by DWARF expression is 0x%" PRIx64, address); return true; @@ -2102,7 +2095,6 @@ bool RegisterContextUnwind::ReadFrameAddress( } case UnwindPlan::Row::FAValue::isConstant: { address = fa.GetConstant(); - address = m_thread.GetProcess()->FixDataAddress(address); UnwindLogMsg("CFA value set by constant is 0x%" PRIx64, address); return true; } diff --git a/lldb/source/Target/StackFrameList.cpp b/lldb/source/Target/StackFrameList.cpp index 4af34a61355da..be33e45889ba3 100644 --- a/lldb/source/Target/StackFrameList.cpp +++ b/lldb/source/Target/StackFrameList.cpp @@ -448,7 +448,7 @@ bool StackFrameList::FetchFramesUpTo(uint32_t end_idx, } } else { unwind_frame_sp = m_frames.front(); - cfa = unwind_frame_sp->m_id.GetCallFrameAddress(); + cfa = unwind_frame_sp->m_id.GetCallFrameAddressWithoutMetadata(); } } else { // Check for interruption when building the frames. diff --git a/lldb/source/Target/StackID.cpp b/lldb/source/Target/StackID.cpp index cd8eda6ebefd4..640baf4c984e0 100644 --- a/lldb/source/Target/StackID.cpp +++ b/lldb/source/Target/StackID.cpp @@ -35,7 +35,8 @@ bool StackID::IsCFAOnStack(Process &process) const { StackID::StackID(lldb::addr_t pc, lldb::addr_t cfa, SymbolContextScope *symbol_scope, Process *process) - : m_pc(pc), m_cfa(cfa), m_symbol_scope(symbol_scope) { + : m_pc(pc), m_cfa(cfa), m_cfa_with_metadata(cfa), + m_symbol_scope(symbol_scope) { if (process) { m_pc = process->FixCodeAddress(m_pc); m_cfa = process->FixDataAddress(m_cfa); @@ -47,6 +48,7 @@ void StackID::SetPC(lldb::addr_t pc, Process *process) { } void StackID::SetCFA(lldb::addr_t cfa, Process *process) { + m_cfa_with_metadata = cfa; m_cfa = process ? process->FixDataAddress(cfa) : cfa; } @@ -67,7 +69,8 @@ void StackID::Dump(Stream *s) { } bool lldb_private::operator==(const StackID &lhs, const StackID &rhs) { - if (lhs.GetCallFrameAddress() != rhs.GetCallFrameAddress()) + if (lhs.GetCallFrameAddressWithoutMetadata() != + rhs.GetCallFrameAddressWithoutMetadata()) return false; SymbolContextScope *lhs_scope = lhs.GetSymbolContextScope(); @@ -134,8 +137,8 @@ CompareHeapCFAs(const StackID &lhs, const StackID &rhs, Process &process) { if (!lhs_cfa_on_stack && rhs_cfa_on_stack) return HeapCFAComparisonResult::Older; - const lldb::addr_t lhs_cfa = lhs.GetCallFrameAddress(); - const lldb::addr_t rhs_cfa = rhs.GetCallFrameAddress(); + const lldb::addr_t lhs_cfa = lhs.GetCallFrameAddressWithoutMetadata(); + const lldb::addr_t rhs_cfa = rhs.GetCallFrameAddressWithoutMetadata(); // If the cfas are the same, fallback to the usual scope comparison. if (lhs_cfa == rhs_cfa) return HeapCFAComparisonResult::NoOpinion; @@ -170,9 +173,9 @@ bool StackID::IsYounger(const StackID &lhs, const StackID &rhs, break; } // END SWIFT - - const lldb::addr_t lhs_cfa = lhs.GetCallFrameAddress(); - const lldb::addr_t rhs_cfa = rhs.GetCallFrameAddress(); + // + const lldb::addr_t lhs_cfa = lhs.GetCallFrameAddressWithoutMetadata(); + const lldb::addr_t rhs_cfa = rhs.GetCallFrameAddressWithoutMetadata(); // FIXME: We are assuming that the stacks grow downward in memory. That's not // necessary, but true on diff --git a/lldb/test/API/macosx/arm-pointer-metadata-cfa-dwarf-expr/Makefile b/lldb/test/API/macosx/arm-pointer-metadata-cfa-dwarf-expr/Makefile new file mode 100644 index 0000000000000..f0de8ffca59fc --- /dev/null +++ b/lldb/test/API/macosx/arm-pointer-metadata-cfa-dwarf-expr/Makefile @@ -0,0 +1,11 @@ +ASM_SOURCES := main.s + +# This is to appease Makefile.rules, there is no main.c +C_SOURCES := main.c + +ASM_OBJS := $(ASM_SOURCES:.s=.o) + +%.o: %.s + $(CC) -c -x assembler $< -o $@ + +include Makefile.rules diff --git a/lldb/test/API/macosx/arm-pointer-metadata-cfa-dwarf-expr/TestArmPointerMetadataCFADwarfExpr.py b/lldb/test/API/macosx/arm-pointer-metadata-cfa-dwarf-expr/TestArmPointerMetadataCFADwarfExpr.py new file mode 100644 index 0000000000000..839e0e1a4fc4d --- /dev/null +++ b/lldb/test/API/macosx/arm-pointer-metadata-cfa-dwarf-expr/TestArmPointerMetadataCFADwarfExpr.py @@ -0,0 +1,35 @@ +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +@skipUnlessDarwin +@skipIf(archs=no_match(["arm64"])) +class TestArmPointerMetadataStripping(TestBase): + def test(self): + self.build() + target, process, thread, bkpt = lldbutil.run_to_name_breakpoint(self, "foo") + + # Step over the first two instructions of foo in order to + # toggle the bit of fp and save it on the stack: + # orr x29, x29, #0x1000000000000000 + # stp x29, x30, [sp, #-16]! + # This is effectively adding metadata to the CFA of the caller frame (main). + thread.StepInstruction(False) + thread.StepInstruction(False) + + # The location of `argv` has been artificially made equal to the CFA of the frame. + # As such, it should have the metadata artificially set previously. + argv_addr = thread.frames[1].GetValueForVariablePath("&argv") + self.assertTrue(argv_addr.IsValid()) + argv_addr_uint = argv_addr.GetValueAsUnsigned() + self.assertNotEqual((argv_addr_uint & (1 << 60)), 0) + + # GetCFA strips metadata. + cfa = thread.frames[1].GetCFA() + self.assertEqual((cfa & (1 << 60)), 0) + + # If the test worked correctly, the cfa and the location should be identical, + # modulo the metadata. + self.assertEqual(cfa | (1 << 60), argv_addr_uint) diff --git a/lldb/test/API/macosx/arm-pointer-metadata-cfa-dwarf-expr/main.s b/lldb/test/API/macosx/arm-pointer-metadata-cfa-dwarf-expr/main.s new file mode 100644 index 0000000000000..0825c5ddd08b5 --- /dev/null +++ b/lldb/test/API/macosx/arm-pointer-metadata-cfa-dwarf-expr/main.s @@ -0,0 +1,226 @@ +; The assembly below corresponds to this program: +; __attribute__((nodebug)) +; int foo() { +; return 10; +; } +; int main(int argc, char **argv) { +; foo(); +; return 0; +; } +; +; The assembly was edited in two places (search for "EDIT"): +; 1. A "orr x29, x29, #0x1000000000000000" instruction was added in foo. This +; effectively changes the CFA value of the frame above foo (i.e. main). +; 2. In main, the DWARF location of `argv` was changed to DW_AT_call_frame_cfa. +; +; This allows us to stop in foo, go to frame 1 (main) and do `v &argv`, +; obtaining the result of evaluating DW_AT_call_frame_cfa. + + .section __TEXT,__text,regular,pure_instructions + .globl _foo ; -- Begin function foo + .p2align 2 +_foo: ; @foo +Lfunc_begin0: + .cfi_startproc + orr x29, x29, #0x1000000000000000 ; EDIT: Set top byte of fp. + stp x29, x30, [sp, #-16]! ; 16-byte Folded Spill + mov x29, sp + .cfi_def_cfa w29, 16 + .cfi_offset w30, -8 + .cfi_offset w29, -16 + mov w0, #10 ; =0xa + ldp x29, x30, [sp], #16 ; 16-byte Folded Reload + ret +Lfunc_end0: + .cfi_endproc + ; -- End function + .globl _main ; -- Begin function main + .p2align 2 +_main: ; @main +Lfunc_begin1: + .file 1 "/test" "test.c" + .loc 1 6 0 ; test.c:6:0 + .cfi_startproc + sub sp, sp, #48 + stp x29, x30, [sp, #32] ; 16-byte Folded Spill + add x29, sp, #32 + .cfi_def_cfa w29, 16 + .cfi_offset w30, -8 + .cfi_offset w29, -16 + mov w8, #0 ; =0x0 + str w8, [sp, #12] ; 4-byte Folded Spill + stur wzr, [x29, #-4] + stur w0, [x29, #-8] + str x1, [sp, #16] +Ltmp0: + bl _foo + ldr w0, [sp, #12] ; 4-byte Folded Reload + ldp x29, x30, [sp, #32] ; 16-byte Folded Reload + add sp, sp, #48 + ret +Ltmp1: +Lfunc_end1: + .cfi_endproc + ; -- End function + .section __DWARF,__debug_abbrev,regular,debug +Lsection_abbrev: + .byte 1 ; Abbreviation Code + .byte 17 ; DW_TAG_compile_unit + .byte 1 ; DW_CHILDREN_yes + .byte 37 ; DW_AT_producer + .byte 14 ; DW_FORM_strp + .byte 19 ; DW_AT_language + .byte 5 ; DW_FORM_data2 + .byte 3 ; DW_AT_name + .byte 14 ; DW_FORM_strp + .ascii "\202|" ; DW_AT_LLVM_sysroot + .byte 14 ; DW_FORM_strp + .ascii "\357\177" ; DW_AT_APPLE_sdk + .byte 14 ; DW_FORM_strp + .byte 16 ; DW_AT_stmt_list + .byte 23 ; DW_FORM_sec_offset + .byte 27 ; DW_AT_comp_dir + .byte 14 ; DW_FORM_strp + .byte 17 ; DW_AT_low_pc + .byte 1 ; DW_FORM_addr + .byte 18 ; DW_AT_high_pc + .byte 6 ; DW_FORM_data4 + .byte 0 ; EOM(1) + .byte 0 ; EOM(2) + .byte 2 ; Abbreviation Code + .byte 46 ; DW_TAG_subprogram + .byte 1 ; DW_CHILDREN_yes + .byte 17 ; DW_AT_low_pc + .byte 1 ; DW_FORM_addr + .byte 18 ; DW_AT_high_pc + .byte 6 ; DW_FORM_data4 + .byte 64 ; DW_AT_frame_base + .byte 24 ; DW_FORM_exprloc + .byte 3 ; DW_AT_name + .byte 14 ; DW_FORM_strp + .byte 58 ; DW_AT_decl_file + .byte 11 ; DW_FORM_data1 + .byte 59 ; DW_AT_decl_line + .byte 11 ; DW_FORM_data1 + .byte 39 ; DW_AT_prototyped + .byte 25 ; DW_FORM_flag_present + .byte 73 ; DW_AT_type + .byte 19 ; DW_FORM_ref4 + .byte 63 ; DW_AT_external + .byte 25 ; DW_FORM_flag_present + .byte 0 ; EOM(1) + .byte 0 ; EOM(2) + .byte 3 ; Abbreviation Code + .byte 5 ; DW_TAG_formal_parameter + .byte 0 ; DW_CHILDREN_no + .byte 2 ; DW_AT_location + .byte 24 ; DW_FORM_exprloc + .byte 3 ; DW_AT_name + .byte 14 ; DW_FORM_strp + .byte 58 ; DW_AT_decl_file + .byte 11 ; DW_FORM_data1 + .byte 59 ; DW_AT_decl_line + .byte 11 ; DW_FORM_data1 + .byte 73 ; DW_AT_type + .byte 19 ; DW_FORM_ref4 + .byte 0 ; EOM(1) + .byte 0 ; EOM(2) + .byte 4 ; Abbreviation Code + .byte 36 ; DW_TAG_base_type + .byte 0 ; DW_CHILDREN_no + .byte 3 ; DW_AT_name + .byte 14 ; DW_FORM_strp + .byte 62 ; DW_AT_encoding + .byte 11 ; DW_FORM_data1 + .byte 11 ; DW_AT_byte_size + .byte 11 ; DW_FORM_data1 + .byte 0 ; EOM(1) + .byte 0 ; EOM(2) + .byte 5 ; Abbreviation Code + .byte 15 ; DW_TAG_pointer_type + .byte 0 ; DW_CHILDREN_no + .byte 73 ; DW_AT_type + .byte 19 ; DW_FORM_ref4 + .byte 0 ; EOM(1) + .byte 0 ; EOM(2) + .byte 0 ; EOM(3) + .section __DWARF,__debug_info,regular,debug +Lsection_info: +Lcu_begin0: +.set Lset0, Ldebug_info_end0-Ldebug_info_start0 ; Length of Unit + .long Lset0 +Ldebug_info_start0: + .short 4 ; DWARF version number +.set Lset1, Lsection_abbrev-Lsection_abbrev ; Offset Into Abbrev. Section + .long Lset1 + .byte 8 ; Address Size (in bytes) + .byte 1 ; Abbrev [1] 0xb:0x76 DW_TAG_compile_unit + .long 0 ; DW_AT_producer + .short 12 ; DW_AT_language + .long 47 ; DW_AT_name + .long 54 ; DW_AT_LLVM_sysroot + .long 165 ; DW_AT_APPLE_sdk +.set Lset2, Lline_table_start0-Lsection_line ; DW_AT_stmt_list + .long Lset2 + .long 180 ; DW_AT_comp_dir + .quad Lfunc_begin1 ; DW_AT_low_pc +.set Lset3, Lfunc_end1-Lfunc_begin1 ; DW_AT_high_pc + .long Lset3 + .byte 2 ; Abbrev [2] 0x32:0x36 DW_TAG_subprogram + .quad Lfunc_begin1 ; DW_AT_low_pc +.set Lset4, Lfunc_end1-Lfunc_begin1 ; DW_AT_high_pc + .long Lset4 + .byte 1 ; DW_AT_frame_base + .byte 109 + .long 247 ; DW_AT_name + .byte 1 ; DW_AT_decl_file + .byte 6 ; DW_AT_decl_line + ; DW_AT_prototyped + .long 107 ; DW_AT_type + ; DW_AT_external + .byte 3 ; Abbrev [3] 0x4b:0xe DW_TAG_formal_parameter + .byte 2 ; DW_AT_location + .byte 145 + .byte 120 + .long 256 ; DW_AT_name + .byte 1 ; DW_AT_decl_file + .byte 6 ; DW_AT_decl_line + .long 103 ; DW_AT_type + .byte 3 ; Abbrev [3] 0x59:0xe DW_TAG_formal_parameter + .byte 1 ; DW_AT_location + .byte 0x9c ; EDIT: DW_AT_call_frame_cfa + .long 261 ; DW_AT_name + .byte 1 ; DW_AT_decl_file + .byte 6 ; DW_AT_decl_line + .long 110 ; DW_AT_type + .byte 0 ; End Of Children Mark + .byte 4 ; Abbrev [4] 0x68:0x7 DW_TAG_base_type + .long 252 ; DW_AT_name + .byte 5 ; DW_AT_encoding + .byte 4 ; DW_AT_byte_size + .byte 5 ; Abbrev [5] 0x6f:0x5 DW_TAG_pointer_type + .long 115 ; DW_AT_type + .byte 5 ; Abbrev [5] 0x74:0x5 DW_TAG_pointer_type + .long 120 ; DW_AT_type + .byte 4 ; Abbrev [4] 0x79:0x7 DW_TAG_base_type + .long 266 ; DW_AT_name + .byte 6 ; DW_AT_encoding + .byte 1 ; DW_AT_byte_size + .byte 0 ; End Of Children Mark +Ldebug_info_end0: + .section __DWARF,__debug_str,regular,debug +Linfo_string: + .asciz "Apple clang " ; string offset=0 + .asciz "test.c" ; string offset=47 + .asciz "/Applications/Xcode..........................................................................................." ; string offset=54 + .asciz ".............." ; string offset=165 + .asciz "......................................................../llvm_src1" ; string offset=180 + .asciz "main" ; string offset=247 + .asciz "int" ; string offset=252 + .asciz "argc" ; string offset=256 + .asciz "argv" ; string offset=261 + .asciz "char" ; string offset=266 +.subsections_via_symbols + .section __DWARF,__debug_line,regular,debug +Lsection_line: +Lline_table_start0: From 778cf9828b7a9f50a0dfb7d2f697acc27a6dc9c0 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Mon, 15 Sep 2025 09:49:23 -0700 Subject: [PATCH 14/20] [lldb] Fix TestSwiftAsyncFrameVarMultipleFrames under arm64e We needed to strip more pointers in the test. (cherry picked from commit 62d9ce285c1905b579b8f3c904492752bd1286aa) --- .../TestSwiftAsyncFrameVarMultipleFrames.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lldb/test/API/lang/swift/async/frame/variables_multiple_frames/TestSwiftAsyncFrameVarMultipleFrames.py b/lldb/test/API/lang/swift/async/frame/variables_multiple_frames/TestSwiftAsyncFrameVarMultipleFrames.py index 37805ebe4eee0..8e2578ee8a10b 100644 --- a/lldb/test/API/lang/swift/async/frame/variables_multiple_frames/TestSwiftAsyncFrameVarMultipleFrames.py +++ b/lldb/test/API/lang/swift/async/frame/variables_multiple_frames/TestSwiftAsyncFrameVarMultipleFrames.py @@ -39,7 +39,10 @@ def check_pcs(self, async_frames, process, target): # with the funclet's prologue skipped. parent_frame = async_frames[idx + 1] prologue_to_skip = parent_frame.GetFunction().GetPrologueByteSize() - self.assertEqual(continuation_ptr + prologue_to_skip, parent_frame.GetPC()) + self.assertEqual( + process.FixAddress(continuation_ptr) + prologue_to_skip, + parent_frame.GetPC(), + ) def check_async_regs_one_frame(self, frame, process): async_reg_name = "r14" if self.getArchitecture() == "x86_64" else "x22" From 92b7d63a62911772be91916f7b7ef25fa12f197d Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Wed, 17 Sep 2025 19:19:50 -0700 Subject: [PATCH 15/20] Reland "Revert "[lldb] Fix OP_deref evaluation for large integer results (#159460)"" The original had an issue on "AArch-less" bots. Fixed it with some ifdefs around the presence of the AArch ABI plugin. Note for the cherry-pick: the test was removed as the related test file in this branch is too old. This reverts commit 1a4685d. Cherry-picked from 40eb97623032ccf0a6a8f1adaea6e37014827f34. --- lldb/source/Expression/DWARFExpression.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp index 53d151041dc6d..19235c919a8bb 100644 --- a/lldb/source/Expression/DWARFExpression.cpp +++ b/lldb/source/Expression/DWARFExpression.cpp @@ -1131,8 +1131,6 @@ llvm::Expected DWARFExpression::Evaluate( lldb::addr_t pointer_value = process->ReadPointerFromMemory(pointer_addr, error); if (pointer_value != LLDB_INVALID_ADDRESS) { - if (ABISP abi_sp = process->GetABI()) - pointer_value = abi_sp->FixCodeAddress(pointer_value); stack.back().GetScalar() = pointer_value; stack.back().ClearContext(); } else { From 9613d7fa5d9ff45d6c17d40225554659ae537119 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Thu, 18 Sep 2025 07:04:35 -0700 Subject: [PATCH 16/20] [lldb] Skip TestArmPointerMetadataStripping This requires fixes from upstream llvm to work. --- .../TestArmPointerMetadataStripping.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lldb/test/API/macosx/arm-pointer-metadata-stripping/TestArmPointerMetadataStripping.py b/lldb/test/API/macosx/arm-pointer-metadata-stripping/TestArmPointerMetadataStripping.py index f61945b3eb4c9..132d302d8da60 100644 --- a/lldb/test/API/macosx/arm-pointer-metadata-stripping/TestArmPointerMetadataStripping.py +++ b/lldb/test/API/macosx/arm-pointer-metadata-stripping/TestArmPointerMetadataStripping.py @@ -6,6 +6,7 @@ from lldbsuite.test import lldbutil +@skip @skipUnlessDarwin @skipIf(archs=no_match(["arm64", "arm64e"])) class TestArmPointerMetadataStripping(TestBase): From 618f94402ceb28654cad9854de20a8cc23a1a842 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Thu, 18 Sep 2025 10:33:51 -0700 Subject: [PATCH 17/20] [lldb][nfc] Remove no-op calls to Fix*Address (#159586) The first call, in InitializeNonZerothFrame, is inside a logging branch. For that, it's better to log the real value instead of the fixed one. The second call, inside RegisterContextUnwind::ReadFrameAddress, is computing an address which is then passed to ReadRegisterValueFromMemory, which boils down to a Process::ReadMemory, which fixes the address if it wants to. The current variable names are misleading, making readers believe it is the cfa value itself that is being passed to Fix*Address; that's not the case. This commit renames the variable to make this abundantly clear. (cherry picked from commit 113357f1a8feb0bfa337bb8a0b9d1d6eaa2d4f0f) --- lldb/source/Target/RegisterContextUnwind.cpp | 33 +++++++++----------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/lldb/source/Target/RegisterContextUnwind.cpp b/lldb/source/Target/RegisterContextUnwind.cpp index 67c2bc15a467d..09636542d9368 100644 --- a/lldb/source/Target/RegisterContextUnwind.cpp +++ b/lldb/source/Target/RegisterContextUnwind.cpp @@ -362,16 +362,10 @@ void RegisterContextUnwind::InitializeNonZerothFrame() { if (log) { UnwindLogMsg("pc = 0x%" PRIx64, pc); addr_t reg_val; - if (ReadGPRValue(eRegisterKindGeneric, LLDB_REGNUM_GENERIC_FP, reg_val)) { - if (abi_sp) - reg_val = abi_sp->FixDataAddress(reg_val); + if (ReadGPRValue(eRegisterKindGeneric, LLDB_REGNUM_GENERIC_FP, reg_val)) UnwindLogMsg("fp = 0x%" PRIx64, reg_val); - } - if (ReadGPRValue(eRegisterKindGeneric, LLDB_REGNUM_GENERIC_SP, reg_val)) { - if (abi_sp) - reg_val = abi_sp->FixDataAddress(reg_val); + if (ReadGPRValue(eRegisterKindGeneric, LLDB_REGNUM_GENERIC_SP, reg_val)) UnwindLogMsg("sp = 0x%" PRIx64, reg_val); - } } // A pc of 0x0 means it's the end of the stack crawl unless we're above a trap @@ -1989,30 +1983,31 @@ bool RegisterContextUnwind::ReadFrameAddress( switch (fa.GetValueType()) { case UnwindPlan::Row::FAValue::isRegisterDereferenced: { - RegisterNumber cfa_reg(m_thread, row_register_kind, - fa.GetRegisterNumber()); - if (ReadGPRValue(cfa_reg, cfa_reg_contents)) { + RegisterNumber regnum_to_deref(m_thread, row_register_kind, + fa.GetRegisterNumber()); + addr_t reg_to_deref_contents; + if (ReadGPRValue(regnum_to_deref, reg_to_deref_contents)) { const RegisterInfo *reg_info = - GetRegisterInfoAtIndex(cfa_reg.GetAsKind(eRegisterKindLLDB)); + GetRegisterInfoAtIndex(regnum_to_deref.GetAsKind(eRegisterKindLLDB)); RegisterValue reg_value; if (reg_info) { - if (abi_sp) - cfa_reg_contents = abi_sp->FixDataAddress(cfa_reg_contents); Status error = ReadRegisterValueFromMemory( - reg_info, cfa_reg_contents, reg_info->byte_size, reg_value); + reg_info, reg_to_deref_contents, reg_info->byte_size, reg_value); if (error.Success()) { address = reg_value.GetAsUInt64(); UnwindLogMsg( "CFA value via dereferencing reg %s (%d): reg has val 0x%" PRIx64 ", CFA value is 0x%" PRIx64, - cfa_reg.GetName(), cfa_reg.GetAsKind(eRegisterKindLLDB), - cfa_reg_contents, address); + regnum_to_deref.GetName(), + regnum_to_deref.GetAsKind(eRegisterKindLLDB), + reg_to_deref_contents, address); return true; } else { UnwindLogMsg("Tried to deref reg %s (%d) [0x%" PRIx64 "] but memory read failed.", - cfa_reg.GetName(), cfa_reg.GetAsKind(eRegisterKindLLDB), - cfa_reg_contents); + regnum_to_deref.GetName(), + regnum_to_deref.GetAsKind(eRegisterKindLLDB), + reg_to_deref_contents); } } } From 5462859ca6bfb23f9e7d35245945eed0cda55406 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Thu, 18 Sep 2025 13:49:56 -0700 Subject: [PATCH 18/20] [lldb][NFC] Simplify logic in ABIMacOSX_arm64::FixDataAddress (#159612) I've intentionally split this into two commits to make it easier that this is an NFC patch; don't think we need to preserve them separately though upon merging. (cherry picked from commit a6662866e88a887ab125c4d533659d27c4134108) --- .../Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp | 57 +++++++++---------- 1 file changed, 27 insertions(+), 30 deletions(-) diff --git a/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp b/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp index 49a9a42eab709..912d549cbb8ef 100644 --- a/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp +++ b/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp @@ -811,42 +811,39 @@ ValueObjectSP ABIMacOSX_arm64::GetReturnValueObjectImpl( return return_valobj_sp; } -addr_t ABIMacOSX_arm64::FixCodeAddress(addr_t pc) { - addr_t pac_sign_extension = 0x0080000000000000ULL; - addr_t tbi_mask = 0xff80000000000000ULL; - addr_t mask = 0; - - if (ProcessSP process_sp = GetProcessSP()) { - mask = process_sp->GetCodeAddressMask(); - if (pc & pac_sign_extension) { - addr_t highmem_mask = process_sp->GetHighmemCodeAddressMask(); - if (highmem_mask != LLDB_INVALID_ADDRESS_MASK) - mask = highmem_mask; - } - } +constexpr addr_t tbi_mask = 0xff80000000000000ULL; +constexpr addr_t pac_sign_extension = 0x0080000000000000ULL; + +/// Consults the process for its {code, data} address masks and applies it to +/// `addr`. +static addr_t DoFixAddr(addr_t addr, bool is_code, ProcessSP process_sp) { + if (!process_sp) + return addr; + + addr_t mask = is_code ? process_sp->GetCodeAddressMask() + : process_sp->GetDataAddressMask(); if (mask == LLDB_INVALID_ADDRESS_MASK) mask = tbi_mask; - return (pc & pac_sign_extension) ? pc | mask : pc & (~mask); + if (addr & pac_sign_extension) { + addr_t highmem_mask = is_code ? process_sp->GetHighmemCodeAddressMask() + : process_sp->GetHighmemCodeAddressMask(); + if (highmem_mask != LLDB_INVALID_ADDRESS_MASK) + return addr | highmem_mask; + return addr | mask; + } + + return addr & (~mask); } -addr_t ABIMacOSX_arm64::FixDataAddress(addr_t pc) { - addr_t pac_sign_extension = 0x0080000000000000ULL; - addr_t tbi_mask = 0xff80000000000000ULL; - addr_t mask = 0; - - if (ProcessSP process_sp = GetProcessSP()) { - mask = process_sp->GetDataAddressMask(); - if (pc & pac_sign_extension) { - addr_t highmem_mask = process_sp->GetHighmemDataAddressMask(); - if (highmem_mask != LLDB_INVALID_ADDRESS_MASK) - mask = highmem_mask; - } - } - if (mask == LLDB_INVALID_ADDRESS_MASK) - mask = tbi_mask; +addr_t ABIMacOSX_arm64::FixCodeAddress(addr_t pc) { + ProcessSP process_sp = GetProcessSP(); + return DoFixAddr(pc, true /*is_code*/, GetProcessSP()); +} - return (pc & pac_sign_extension) ? pc | mask : pc & (~mask); +addr_t ABIMacOSX_arm64::FixDataAddress(addr_t addr) { + ProcessSP process_sp = GetProcessSP(); + return DoFixAddr(addr, false /*is_code*/, GetProcessSP()); } void ABIMacOSX_arm64::Initialize() { From 1c84c9bf068d0fb7193c29489d3d56132967d1a4 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Fri, 19 Sep 2025 07:45:08 -0700 Subject: [PATCH 19/20] [lldb] Don't call FixDataAddress when reading fp in ReadGPRValue (#159606) Based on testing on processors that use pointer metadata, and with all the work done to delay calls to FixDataAddress, this is no longer necessary. Note that, with debugserver in particular, this is an NFC change: the code path here is for frame zero, and debugserver will strip metadata when reading fp from frame zero anyway. (cherry picked from commit bce48c89a04df5d5918d787e8b481cdfb0707307) --- lldb/source/Target/RegisterContextUnwind.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/lldb/source/Target/RegisterContextUnwind.cpp b/lldb/source/Target/RegisterContextUnwind.cpp index 09636542d9368..d167d135eee87 100644 --- a/lldb/source/Target/RegisterContextUnwind.cpp +++ b/lldb/source/Target/RegisterContextUnwind.cpp @@ -2182,9 +2182,6 @@ bool RegisterContextUnwind::ReadGPRValue(lldb::RegisterKind register_kind, if (generic_regnum == LLDB_REGNUM_GENERIC_PC || generic_regnum == LLDB_REGNUM_GENERIC_RA) value = abi_sp->FixCodeAddress(value); - if (generic_regnum == LLDB_REGNUM_GENERIC_SP || - generic_regnum == LLDB_REGNUM_GENERIC_FP) - value = abi_sp->FixDataAddress(value); } return true; } From 31724b11382726d03ba67cfa102a931962d20721 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Thu, 18 Sep 2025 14:45:00 -0700 Subject: [PATCH 20/20] [lldb] Introduce Process::FixAnyAddressPreservingAuthentication This is yet another variant of the Fix{Code,Data}Address methods, but tailored for pointers that both: 1. Are going to be used in-process, 2. Require authentication metadata. Currently, the callsite inside IRMemoryMap::WritePointerToMemory is an example of 1; the pointer written to memory will be used by JITed code during expression evaluation. An example of (2) can be found in the MTE extension on arm processors. An MTE-tagged pointer must preserve its normal bits in order for load instructions to complete without faulting. However, PAC bits must be stripped, as codegen for some expressions may generate regular load instructions for accesses to those (instead of the special PAC instructions). (cherry picked from commit 37ad33e39ac960178e4cf02e5598db35a279ae21) --- lldb/include/lldb/Target/ABI.h | 4 ++++ lldb/include/lldb/Target/Process.h | 5 +++++ lldb/source/Expression/IRMemoryMap.cpp | 2 +- lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp | 9 +++++++++ lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.h | 1 + lldb/source/Target/Process.cpp | 6 ++++++ .../TestArmPointerMetadataStripping.py | 9 ++++++++- 7 files changed, 34 insertions(+), 2 deletions(-) diff --git a/lldb/include/lldb/Target/ABI.h b/lldb/include/lldb/Target/ABI.h index dd941d1c905c1..4275dd6c9273b 100644 --- a/lldb/include/lldb/Target/ABI.h +++ b/lldb/include/lldb/Target/ABI.h @@ -140,6 +140,10 @@ class ABI : public PluginInterface { return FixDataAddress(pc); } + virtual lldb::addr_t FixAnyAddressPreservingAuthentication(lldb::addr_t pc) { + return FixAnyAddress(pc); + } + llvm::MCRegisterInfo &GetMCRegisterInfo() { return *m_mc_register_info_up; } virtual void diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h index 657fa0d5a6d72..77122d7d9a9b5 100644 --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -1463,6 +1463,11 @@ class Process : public std::enable_shared_from_this, /// platforms where there is a difference (only Arm Thumb at this time). lldb::addr_t FixAnyAddress(lldb::addr_t pc); + /// Strip pointer metadata except for the bits necessary to authenticate a + /// memory access. This is useful, for example, if `address` requires + /// authentication and it is going to be consumed in JITed code. + lldb::addr_t FixAnyAddressPreservingAuthentication(lldb::addr_t address); + /// Get the Modification ID of the process. /// /// \return diff --git a/lldb/source/Expression/IRMemoryMap.cpp b/lldb/source/Expression/IRMemoryMap.cpp index 12cfbeb36df79..1bf1897e2fa06 100644 --- a/lldb/source/Expression/IRMemoryMap.cpp +++ b/lldb/source/Expression/IRMemoryMap.cpp @@ -645,7 +645,7 @@ void IRMemoryMap::WritePointerToMemory(lldb::addr_t process_address, if (it == m_allocations.end() || it->second.m_policy != AllocationPolicy::eAllocationPolicyHostOnly) if (auto process_sp = GetProcessWP().lock()) - pointer = process_sp->FixAnyAddress(pointer); + pointer = process_sp->FixAnyAddressPreservingAuthentication(pointer); Scalar scalar(pointer); diff --git a/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp b/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp index 912d549cbb8ef..d33fef106e340 100644 --- a/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp +++ b/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp @@ -846,6 +846,15 @@ addr_t ABIMacOSX_arm64::FixDataAddress(addr_t addr) { return DoFixAddr(addr, false /*is_code*/, GetProcessSP()); } +addr_t ABIMacOSX_arm64::FixAnyAddressPreservingAuthentication(addr_t addr) { + // Save the old MTE tag and restore it later. + constexpr addr_t mte_mask = 0x0f00000000000000ULL; + addr_t old_mte_tag = addr & mte_mask; + + addr_t fixed_addr = FixDataAddress(addr); + return old_mte_tag | (fixed_addr & (~mte_mask)); +} + void ABIMacOSX_arm64::Initialize() { PluginManager::RegisterPlugin(GetPluginNameStatic(), pluginDesc, CreateInstance); diff --git a/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.h b/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.h index 025a7a3fc368b..763f07452bb0c 100644 --- a/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.h +++ b/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.h @@ -64,6 +64,7 @@ class ABIMacOSX_arm64 : public ABIAArch64 { lldb::addr_t FixCodeAddress(lldb::addr_t pc) override; lldb::addr_t FixDataAddress(lldb::addr_t pc) override; + lldb::addr_t FixAnyAddressPreservingAuthentication(lldb::addr_t pc) override; // Static Functions diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 294b7fbd1ee6b..36a572df840e5 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -6048,6 +6048,12 @@ addr_t Process::FixAnyAddress(addr_t addr) { return addr; } +addr_t Process::FixAnyAddressPreservingAuthentication(addr_t addr) { + if (ABISP abi_sp = GetABI()) + addr = abi_sp->FixAnyAddressPreservingAuthentication(addr); + return addr; +} + void Process::DidExec() { Log *log = GetLog(LLDBLog::Process); LLDB_LOGF(log, "Process::%s()", __FUNCTION__); diff --git a/lldb/test/API/macosx/arm-pointer-metadata-stripping/TestArmPointerMetadataStripping.py b/lldb/test/API/macosx/arm-pointer-metadata-stripping/TestArmPointerMetadataStripping.py index 132d302d8da60..2fb83485542c0 100644 --- a/lldb/test/API/macosx/arm-pointer-metadata-stripping/TestArmPointerMetadataStripping.py +++ b/lldb/test/API/macosx/arm-pointer-metadata-stripping/TestArmPointerMetadataStripping.py @@ -39,8 +39,15 @@ def test(self): symbols_file = self.create_symbols_file() self.runCmd(f"target module add {symbols_file}") + # The address of myglobal_json is: 0x1200AAAAAAAB1014 # The high order bits should be stripped. - self.expect_expr("get_high_bits(&myglobal_json)", result_value="0") + # On Darwin platforms, the lower nibble of the most significant byte is preserved. + if platform.system() == "Darwin": + expected_value = str(0x200000000000000) + else: + expected_value = "0" + + self.expect_expr("get_high_bits(&myglobal_json)", result_value=expected_value) # Mark all bits as used for addresses and ensure bits are no longer stripped. self.runCmd("settings set target.process.virtual-addressable-bits 64")