Author: Felipe de Azevedo Piovezan Date: 2026-07-15T11:05:10Z New Revision: 6b3b6a88905cfb7e098d3c4a790ae8a196cfeab2
URL: https://github.com/llvm/llvm-project/commit/6b3b6a88905cfb7e098d3c4a790ae8a196cfeab2 DIFF: https://github.com/llvm/llvm-project/commit/6b3b6a88905cfb7e098d3c4a790ae8a196cfeab2.diff LOG: [lldb][NFC] Create StackID::IsYounger (#209402) Currently, StackIDs are compared with a custom operator <. There are some issues with that: 1. It's not clear what "<" means in this context. It really is a test of "frame A is on top of frame B or not comparable". But this notion is not expressed through "<". 2. This is not a real operator "<" in the sense that if "!<" does not imply ">=". In particular, frames may not always be comparable (i.e. (they are not part of the same stack). To address these issues, this commit replaces `<` with a custom function "IsYounger", which makes explicit what is being tested. Added: Modified: lldb/include/lldb/Target/StackID.h lldb/source/Target/StackFrameList.cpp lldb/source/Target/StackID.cpp lldb/source/Target/ThreadPlanStepInstruction.cpp lldb/source/Target/ThreadPlanStepOut.cpp lldb/source/Target/ThreadPlanStepRange.cpp lldb/source/Target/ThreadPlanStepUntil.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Target/StackID.h b/lldb/include/lldb/Target/StackID.h index 3f6a83b5e2fa5..00676241d292c 100644 --- a/lldb/include/lldb/Target/StackID.h +++ b/lldb/include/lldb/Target/StackID.h @@ -50,6 +50,11 @@ class StackID { void Dump(Stream *s); + /// Returns true if this StackID corresponds to a frame younger (i.e. higher + /// on the call stack) than `other`, and false otherwise (including when the + /// frames are not comparable, in some cases where this can be detected). + bool IsYoungerThan(const StackID &other) const; + protected: friend class StackFrame; friend class SyntheticStackFrameList; @@ -80,10 +85,6 @@ class StackID { bool operator==(const StackID &lhs, const StackID &rhs); bool operator!=(const StackID &lhs, const StackID &rhs); - -// frame_id_1 < frame_id_2 means "frame_id_1 is YOUNGER than frame_id_2" -bool operator<(const StackID &lhs, const StackID &rhs); - } // namespace lldb_private #endif // LLDB_TARGET_STACKID_H diff --git a/lldb/source/Target/StackFrameList.cpp b/lldb/source/Target/StackFrameList.cpp index 465d1af11add0..b1e82eecb0131 100644 --- a/lldb/source/Target/StackFrameList.cpp +++ b/lldb/source/Target/StackFrameList.cpp @@ -762,7 +762,7 @@ StackFrameList::GetFrameWithConcreteFrameIndex(uint32_t unwind_idx) { static bool CompareStackID(const StackFrameSP &stack_sp, const StackID &stack_id) { - return stack_sp->GetStackID() < stack_id; + return stack_sp->GetStackID().IsYoungerThan(stack_id); } StackFrameSP StackFrameList::GetFrameWithStackID(const StackID &stack_id) { diff --git a/lldb/source/Target/StackID.cpp b/lldb/source/Target/StackID.cpp index 137c776a84d2f..756b7e2338bd9 100644 --- a/lldb/source/Target/StackID.cpp +++ b/lldb/source/Target/StackID.cpp @@ -69,9 +69,9 @@ bool lldb_private::operator!=(const StackID &lhs, const StackID &rhs) { return !(lhs == rhs); } -bool lldb_private::operator<(const StackID &lhs, const StackID &rhs) { - const lldb::addr_t lhs_cfa = lhs.GetCallFrameAddressWithoutMetadata(); - const lldb::addr_t rhs_cfa = rhs.GetCallFrameAddressWithoutMetadata(); +bool StackID::IsYoungerThan(const StackID &other) const { + const lldb::addr_t lhs_cfa = GetCallFrameAddressWithoutMetadata(); + const lldb::addr_t rhs_cfa = other.GetCallFrameAddressWithoutMetadata(); // FIXME: We are assuming that the stacks grow downward in memory. That's not // necessary, but true on @@ -85,8 +85,8 @@ bool lldb_private::operator<(const StackID &lhs, const StackID &rhs) { if (lhs_cfa != rhs_cfa) return lhs_cfa < rhs_cfa; - SymbolContextScope *lhs_scope = lhs.GetSymbolContextScope(); - SymbolContextScope *rhs_scope = rhs.GetSymbolContextScope(); + SymbolContextScope *lhs_scope = GetSymbolContextScope(); + SymbolContextScope *rhs_scope = other.GetSymbolContextScope(); if (lhs_scope != nullptr && rhs_scope != nullptr) { // Same exact scope, lhs is not less than (younger than rhs) diff --git a/lldb/source/Target/ThreadPlanStepInstruction.cpp b/lldb/source/Target/ThreadPlanStepInstruction.cpp index e9db3171a5bef..7da8a195bdee9 100644 --- a/lldb/source/Target/ThreadPlanStepInstruction.cpp +++ b/lldb/source/Target/ThreadPlanStepInstruction.cpp @@ -110,7 +110,7 @@ bool ThreadPlanStepInstruction::IsPlanStale() { SetPlanComplete(); } return (thread.GetRegisterContext()->GetPC(0) != m_instruction_addr); - } else if (cur_frame_id < m_stack_id) { + } else if (cur_frame_id.IsYoungerThan(m_stack_id)) { // If the current frame is younger than the start frame and we are stepping // over, then we need to continue, but if we are doing just one step, we're // done. @@ -137,7 +137,8 @@ bool ThreadPlanStepInstruction::ShouldStop(Event *event_ptr) { StackID cur_frame_zero_id = cur_frame_sp->GetStackID(); - if (cur_frame_zero_id == m_stack_id || m_stack_id < cur_frame_zero_id) { + if (cur_frame_zero_id == m_stack_id || + m_stack_id.IsYoungerThan(cur_frame_zero_id)) { if (thread.GetRegisterContext()->GetPC(0) != m_instruction_addr) { if (--m_iteration_count <= 0) { SetPlanComplete(); diff --git a/lldb/source/Target/ThreadPlanStepOut.cpp b/lldb/source/Target/ThreadPlanStepOut.cpp index 7af4de6e71ec1..8e80ba3c84a2f 100644 --- a/lldb/source/Target/ThreadPlanStepOut.cpp +++ b/lldb/source/Target/ThreadPlanStepOut.cpp @@ -341,12 +341,12 @@ bool ThreadPlanStepOut::DoPlanExplainsStop(Event *event_ptr) { if (m_step_out_to_id == frame_zero_id) done = true; - else if (m_step_out_to_id < frame_zero_id) { + else if (m_step_out_to_id.IsYoungerThan(frame_zero_id)) { // Either we stepped past the breakpoint, or the stack ID calculation // was incorrect and we should probably stop. done = true; } else { - done = (m_immediate_step_from_id < frame_zero_id); + done = (m_immediate_step_from_id.IsYoungerThan(frame_zero_id)); } if (done) { @@ -405,7 +405,7 @@ bool ThreadPlanStepOut::ShouldStop(Event *event_ptr) { StopInfoSP stop_info_sp = GetPrivateStopInfo(); if (stop_info_sp && stop_info_sp->GetStopReason() == eStopReasonBreakpoint) { StackID frame_zero_id = GetThread().GetStackFrameAtIndex(0)->GetStackID(); - done = !(frame_zero_id < m_step_out_to_id); + done = !(frame_zero_id.IsYoungerThan(m_step_out_to_id)); } } @@ -565,5 +565,5 @@ bool ThreadPlanStepOut::IsPlanStale() { // then there's something for us to do. Otherwise, we're stale. StackID frame_zero_id = GetThread().GetStackFrameAtIndex(0)->GetStackID(); - return !(frame_zero_id < m_step_out_to_id); + return !(frame_zero_id.IsYoungerThan(m_step_out_to_id)); } diff --git a/lldb/source/Target/ThreadPlanStepRange.cpp b/lldb/source/Target/ThreadPlanStepRange.cpp index b57754a180e72..92225906b601e 100644 --- a/lldb/source/Target/ThreadPlanStepRange.cpp +++ b/lldb/source/Target/ThreadPlanStepRange.cpp @@ -223,7 +223,7 @@ lldb::FrameComparison ThreadPlanStepRange::CompareCurrentFrameToStartFrame() { if (cur_frame_id == m_stack_id) { frame_order = eFrameCompareEqual; - } else if (cur_frame_id < m_stack_id) { + } else if (cur_frame_id.IsYoungerThan(m_stack_id)) { frame_order = eFrameCompareYounger; } else { StackFrameSP cur_parent_frame = thread.GetStackFrameAtIndex(1); diff --git a/lldb/source/Target/ThreadPlanStepUntil.cpp b/lldb/source/Target/ThreadPlanStepUntil.cpp index db2b930c1085c..5629217eb3c59 100644 --- a/lldb/source/Target/ThreadPlanStepUntil.cpp +++ b/lldb/source/Target/ThreadPlanStepUntil.cpp @@ -173,7 +173,7 @@ void ThreadPlanStepUntil::AnalyzeStop() { bool done; StackID cur_frame_zero_id; - done = (m_stack_id < cur_frame_zero_id); + done = (m_stack_id.IsYoungerThan(cur_frame_zero_id)); if (done) { m_stepped_out = true; @@ -199,7 +199,7 @@ void ThreadPlanStepUntil::AnalyzeStop() { if (frame_zero_id == m_stack_id) done = true; - else if (frame_zero_id < m_stack_id) + else if (frame_zero_id.IsYoungerThan(m_stack_id)) done = false; else { StackFrameSP older_frame_sp = thread.GetStackFrameAtIndex(1); _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
