https://github.com/felipepiovezan created https://github.com/llvm/llvm-project/pull/152796
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. >From d662d7703a1a0b04a78387204b3bea3c2ee93120 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan <fpiove...@apple.com> Date: Fri, 8 Aug 2025 14:07:30 -0700 Subject: [PATCH] [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. --- lldb/include/lldb/Target/StackID.h | 10 +++++----- lldb/source/Target/StackFrame.cpp | 16 ++++++++++------ lldb/source/Target/StackID.cpp | 18 ++++++++++++++++++ 3 files changed, 33 insertions(+), 11 deletions(-) diff --git a/lldb/include/lldb/Target/StackID.h b/lldb/include/lldb/Target/StackID.h index 09392792cb501..fddbc8e48dfdc 100644 --- a/lldb/include/lldb/Target/StackID.h +++ b/lldb/include/lldb/Target/StackID.h @@ -14,14 +14,15 @@ 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) - : m_pc(pc), m_cfa(cfa), m_symbol_scope(symbol_scope) {} + SymbolContextScope *symbol_scope, Process *process); StackID(const StackID &rhs) : m_pc(rhs.m_pc), m_cfa(rhs.m_cfa), m_symbol_scope(rhs.m_symbol_scope) {} @@ -63,9 +64,8 @@ class StackID { protected: friend class StackFrame; - 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 d97a814952186..40049d40aa413 100644 --- a/lldb/source/Target/StackFrame.cpp +++ b/lldb/source/Target/StackFrame.cpp @@ -61,8 +61,9 @@ 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, nullptr), m_frame_code_addr(pc), m_sc(), m_flags(), - m_frame_base(), m_frame_base_error(), m_cfa_is_valid(cfa_is_valid), + 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), m_variable_list_sp(), m_variable_list_value_objects(), @@ -71,7 +72,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) { @@ -87,7 +88,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(reg_context_sp), m_id(pc, cfa, nullptr), + 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), @@ -115,7 +117,7 @@ StackFrame::StackFrame(const ThreadSP &thread_sp, user_id_t 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, - nullptr), + 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), @@ -1995,7 +1997,9 @@ void StackFrame::UpdatePreviousFrameFromCurrentFrame(StackFrame &curr_frame) { std::lock_guard<std::recursive_mutex> 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 410d5b7e820a5..b1795970802ae 100644 --- a/lldb/source/Target/StackID.cpp +++ b/lldb/source/Target/StackID.cpp @@ -10,10 +10,28 @@ #include "lldb/Symbol/Block.h" #include "lldb/Symbol/Symbol.h" #include "lldb/Symbol/SymbolContext.h" +#include "lldb/Target/Process.h" #include "lldb/Utility/Stream.h" 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) { + 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 ", symbol_scope = %p", _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
