Skip to content

Commit b3114e3

Browse files
felipepiovezanmedismailben
authored andcommitted
Make StackID call Fix{Code,Data} pointers (llvm#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 llvm#150537. This was tested running the LLDB test suite on arm64e. (cherry picked from commit 9c8e716)
1 parent f05bc88 commit b3114e3

File tree

3 files changed

+37
-12
lines changed

3 files changed

+37
-12
lines changed

lldb/include/lldb/Target/StackID.h

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,18 @@
1414

1515
namespace lldb_private {
1616

17+
class Process;
18+
1719
class StackID {
1820
public:
1921
// Constructors and Destructors
2022
StackID() = default;
2123

2224
StackID(const StackID &rhs) = default;
2325

26+
explicit StackID(lldb::addr_t pc, lldb::addr_t cfa,
27+
SymbolContextScope *symbol_scope, Process *process);
28+
2429
~StackID() = default;
2530

2631
lldb::addr_t GetPC() const { return m_pc; }
@@ -68,11 +73,8 @@ class StackID {
6873
protected:
6974
friend class StackFrame;
7075

71-
explicit StackID(lldb::addr_t pc, lldb::addr_t cfa) : m_pc(pc), m_cfa(cfa) {}
72-
73-
void SetPC(lldb::addr_t pc) { m_pc = pc; }
74-
75-
void SetCFA(lldb::addr_t cfa) { m_cfa = cfa; }
76+
void SetPC(lldb::addr_t pc, Process *process);
77+
void SetCFA(lldb::addr_t cfa, Process *process);
7678

7779
lldb::addr_t m_pc =
7880
LLDB_INVALID_ADDRESS; // The pc value for the function/symbol for this

lldb/source/Target/StackFrame.cpp

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,8 @@ StackFrame::StackFrame(const ThreadSP &thread_sp, user_id_t frame_idx,
6464
const SymbolContext *sc_ptr)
6565
: m_thread_wp(thread_sp), m_frame_index(frame_idx),
6666
m_concrete_frame_index(unwind_frame_index), m_reg_context_sp(),
67-
m_id(pc, cfa), m_frame_code_addr(pc), m_sc(), m_flags(), m_frame_base(),
67+
m_id(pc, cfa, nullptr, thread_sp->GetProcess().get()),
68+
m_frame_code_addr(pc), m_sc(), m_flags(), m_frame_base(),
6869
m_frame_base_error(), m_cfa_is_valid(cfa_is_valid),
6970
m_stack_frame_kind(kind),
7071
m_behaves_like_zeroth_frame(behaves_like_zeroth_frame),
@@ -74,7 +75,7 @@ StackFrame::StackFrame(const ThreadSP &thread_sp, user_id_t frame_idx,
7475
// recursive functions properly aren't confused with one another on a history
7576
// stack.
7677
if (IsHistorical() && !m_cfa_is_valid) {
77-
m_id.SetCFA(m_frame_index);
78+
m_id.SetCFA(m_frame_index, thread_sp->GetProcess().get());
7879
}
7980

8081
if (sc_ptr != nullptr) {
@@ -90,9 +91,11 @@ StackFrame::StackFrame(const ThreadSP &thread_sp, user_id_t frame_idx,
9091
const SymbolContext *sc_ptr)
9192
: m_thread_wp(thread_sp), m_frame_index(frame_idx),
9293
m_concrete_frame_index(unwind_frame_index),
93-
m_reg_context_sp(reg_context_sp), m_id(pc, cfa), m_frame_code_addr(pc),
94-
m_sc(), m_flags(), m_frame_base(), m_frame_base_error(),
95-
m_cfa_is_valid(true), m_stack_frame_kind(StackFrame::Kind::Regular),
94+
m_reg_context_sp(reg_context_sp),
95+
m_id(pc, cfa, nullptr, thread_sp->GetProcess().get()),
96+
m_frame_code_addr(pc), m_sc(), m_flags(), m_frame_base(),
97+
m_frame_base_error(), m_cfa_is_valid(true),
98+
m_stack_frame_kind(StackFrame::Kind::Regular),
9699
m_behaves_like_zeroth_frame(behaves_like_zeroth_frame),
97100
m_variable_list_sp(), m_variable_list_value_objects(),
98101
m_recognized_frame_sp(), m_disassembly(), m_mutex() {
@@ -116,7 +119,8 @@ StackFrame::StackFrame(const ThreadSP &thread_sp, user_id_t frame_idx,
116119
: m_thread_wp(thread_sp), m_frame_index(frame_idx),
117120
m_concrete_frame_index(unwind_frame_index),
118121
m_reg_context_sp(reg_context_sp),
119-
m_id(pc_addr.GetLoadAddress(thread_sp->CalculateTarget().get()), cfa),
122+
m_id(pc_addr.GetLoadAddress(thread_sp->CalculateTarget().get()), cfa,
123+
nullptr, thread_sp->GetProcess().get()),
120124
m_frame_code_addr(pc_addr), m_sc(), m_flags(), m_frame_base(),
121125
m_frame_base_error(), m_cfa_is_valid(true),
122126
m_stack_frame_kind(StackFrame::Kind::Regular),
@@ -2002,7 +2006,9 @@ void StackFrame::UpdatePreviousFrameFromCurrentFrame(StackFrame &curr_frame) {
20022006
std::lock_guard<std::recursive_mutex> guard(m_mutex);
20032007
assert(GetStackID() ==
20042008
curr_frame.GetStackID()); // TODO: remove this after some testing
2005-
m_id.SetPC(curr_frame.m_id.GetPC()); // Update the Stack ID PC value
2009+
m_id.SetPC(
2010+
curr_frame.m_id.GetPC(),
2011+
curr_frame.CalculateProcess().get()); // Update the Stack ID PC value
20062012
assert(GetThread() == curr_frame.GetThread());
20072013
m_frame_index = curr_frame.m_frame_index;
20082014
m_concrete_frame_index = curr_frame.m_concrete_frame_index;

lldb/source/Target/StackID.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,23 @@ bool StackID::IsCFAOnStack(Process &process) const {
3232
return m_cfa_on_stack == eLazyBoolYes;
3333
}
3434

35+
StackID::StackID(lldb::addr_t pc, lldb::addr_t cfa,
36+
SymbolContextScope *symbol_scope, Process *process)
37+
: m_pc(pc), m_cfa(cfa), m_symbol_scope(symbol_scope) {
38+
if (process) {
39+
m_pc = process->FixCodeAddress(m_pc);
40+
m_cfa = process->FixDataAddress(m_cfa);
41+
}
42+
}
43+
44+
void StackID::SetPC(lldb::addr_t pc, Process *process) {
45+
m_pc = process ? process->FixCodeAddress(pc) : pc;
46+
}
47+
48+
void StackID::SetCFA(lldb::addr_t cfa, Process *process) {
49+
m_cfa = process ? process->FixDataAddress(cfa) : cfa;
50+
}
51+
3552
void StackID::Dump(Stream *s) {
3653
s->Printf("StackID (pc = 0x%16.16" PRIx64 ", cfa = 0x%16.16" PRIx64
3754
", cfa_on_stack = %d, symbol_scope = %p",

0 commit comments

Comments
 (0)