https://github.com/felipepiovezan updated https://github.com/llvm/llvm-project/pull/157498
>From 7ab0dfad28de552f796aaf9c6f2a021d4ad766a7 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan <fpiove...@apple.com> Date: Mon, 8 Sep 2025 08:30:19 -0700 Subject: [PATCH 1/2] [lldb] Track CFA pointer metadata in StackID 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. --- lldb/include/lldb/Target/StackID.h | 11 +- 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 | 11 +- .../Makefile | 11 + .../TestArmPointerMetadataCFADwarfExpr.py | 25 ++ .../main.s | 226 ++++++++++++++++++ 9 files changed, 281 insertions(+), 17 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 c2a5d733dcd69..754f80d001647 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; } @@ -59,9 +63,12 @@ class StackID { /// The call frame address (stack pointer) value at the beginning of the /// function that uniquely identifies this frame (along with m_symbol_scope - /// below) + /// 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 b6724bb0c4119..42dbed490a33d 100644 --- a/lldb/source/API/SBFrame.cpp +++ b/lldb/source/API/SBFrame.cpp @@ -267,7 +267,7 @@ lldb::addr_t SBFrame::GetCFA() const { } if (StackFrame *frame = exe_ctx->GetFramePtr()) - 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 332cf2c86024a..5040351f4975b 100644 --- a/lldb/source/Expression/DWARFExpression.cpp +++ b/lldb/source/Expression/DWARFExpression.cpp @@ -2195,7 +2195,7 @@ llvm::Expected<Value> 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 bcf1297f2114f..5ec2eb9c2cb6a 100644 --- a/lldb/source/Target/RegisterContextUnwind.cpp +++ b/lldb/source/Target/RegisterContextUnwind.cpp @@ -1995,8 +1995,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, @@ -2018,8 +2016,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( @@ -2056,9 +2052,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; @@ -2099,7 +2092,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 d83c90582ba9c..3ed73a3b0afdb 100644 --- a/lldb/source/Target/StackFrameList.cpp +++ b/lldb/source/Target/StackFrameList.cpp @@ -449,7 +449,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 f879276527dda..137c776a84d2f 100644 --- a/lldb/source/Target/StackID.cpp +++ b/lldb/source/Target/StackID.cpp @@ -17,7 +17,8 @@ using namespace lldb_private; 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); @@ -29,6 +30,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; } @@ -49,7 +51,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(); @@ -67,8 +70,8 @@ bool lldb_private::operator!=(const StackID &lhs, const StackID &rhs) { } bool lldb_private::operator<(const StackID &lhs, const StackID &rhs) { - 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..acde260beb9b8 --- /dev/null +++ b/lldb/test/API/macosx/arm-pointer-metadata-cfa-dwarf-expr/TestArmPointerMetadataCFADwarfExpr.py @@ -0,0 +1,25 @@ +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]! + thread.StepInstruction(False) + thread.StepInstruction(False) + + 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) 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 970eedd0ccfade0250189ee5b6bf256eb69568c1 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan <fpiove...@apple.com> Date: Wed, 10 Sep 2025 14:48:00 -0700 Subject: [PATCH 2/2] fixup! format python tesT --- .../TestArmPointerMetadataCFADwarfExpr.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index acde260beb9b8..a757ec6b84f81 100644 --- 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 @@ -18,7 +18,7 @@ def test(self): thread.StepInstruction(False) thread.StepInstruction(False) - argv_addr = thread.frames[1].GetValueForVariablePath("&argv"); + argv_addr = thread.frames[1].GetValueForVariablePath("&argv") self.assertTrue(argv_addr.IsValid()) argv_addr_uint = argv_addr.GetValueAsUnsigned() _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits