llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Felipe de Azevedo Piovezan (felipepiovezan) <details> <summary>Changes</summary> 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. --- Full diff: https://github.com/llvm/llvm-project/pull/152796.diff 3 Files Affected: - (modified) lldb/include/lldb/Target/StackID.h (+5-5) - (modified) lldb/source/Target/StackFrame.cpp (+10-6) - (modified) lldb/source/Target/StackID.cpp (+18) ``````````diff 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", `````````` </details> https://github.com/llvm/llvm-project/pull/152796 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
