https://github.com/felipepiovezan updated https://github.com/llvm/llvm-project/pull/209402
>From c410840f7af29cfc029eef514592e13e9c24cfe8 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan <[email protected]> Date: Tue, 14 Jul 2026 09:12:23 +0100 Subject: [PATCH 1/2] [lldb][NFC] Create StackID::IsYounger 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. --- lldb/include/lldb/Target/StackID.h | 9 +++++---- lldb/source/Target/StackFrameList.cpp | 2 +- lldb/source/Target/StackID.cpp | 2 +- lldb/source/Target/ThreadPlanStepInstruction.cpp | 5 +++-- lldb/source/Target/ThreadPlanStepOut.cpp | 8 ++++---- lldb/source/Target/ThreadPlanStepRange.cpp | 2 +- lldb/source/Target/ThreadPlanStepUntil.cpp | 4 ++-- 7 files changed, 17 insertions(+), 15 deletions(-) diff --git a/lldb/include/lldb/Target/StackID.h b/lldb/include/lldb/Target/StackID.h index 3f6a83b5e2fa5..2292bef5e49be 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 `lhs` corresponds to a frame younger (i.e. higher on the + /// call stack) than `rhs`, and false otherwise (including when the frames are + /// not comparable). + static bool IsYounger(const StackID &lhs, const StackID &rhs); + 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..b124ef1c199c9 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 StackID::IsYounger(stack_sp->GetStackID(), 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..c5d8e5a3e6644 100644 --- a/lldb/source/Target/StackID.cpp +++ b/lldb/source/Target/StackID.cpp @@ -69,7 +69,7 @@ bool lldb_private::operator!=(const StackID &lhs, const StackID &rhs) { return !(lhs == rhs); } -bool lldb_private::operator<(const StackID &lhs, const StackID &rhs) { +bool StackID::IsYounger(const StackID &lhs, const StackID &rhs) { const lldb::addr_t lhs_cfa = lhs.GetCallFrameAddressWithoutMetadata(); const lldb::addr_t rhs_cfa = rhs.GetCallFrameAddressWithoutMetadata(); diff --git a/lldb/source/Target/ThreadPlanStepInstruction.cpp b/lldb/source/Target/ThreadPlanStepInstruction.cpp index e9db3171a5bef..0e062bf6d7a34 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 (StackID::IsYounger(cur_frame_id, 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 || + StackID::IsYounger(m_stack_id, 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..9d830f13947c7 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 (StackID::IsYounger(m_step_out_to_id, 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 = (StackID::IsYounger(m_immediate_step_from_id, 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 = !(StackID::IsYounger(frame_zero_id, 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 !(StackID::IsYounger(frame_zero_id, m_step_out_to_id)); } diff --git a/lldb/source/Target/ThreadPlanStepRange.cpp b/lldb/source/Target/ThreadPlanStepRange.cpp index b57754a180e72..00f5d70ee00ff 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 (StackID::IsYounger(cur_frame_id, 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..43d068162c3b2 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 = (StackID::IsYounger(m_stack_id, 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 (StackID::IsYounger(frame_zero_id, m_stack_id)) done = false; else { StackFrameSP older_frame_sp = thread.GetStackFrameAtIndex(1); >From 772d5bdeff2dbb9933fdb9c830fb5677b2903f60 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan <[email protected]> Date: Tue, 14 Jul 2026 10:35:47 +0100 Subject: [PATCH 2/2] fixup! Make it a member function --- lldb/include/lldb/Target/StackID.h | 8 ++++---- lldb/source/Target/StackFrameList.cpp | 2 +- lldb/source/Target/StackID.cpp | 10 +++++----- lldb/source/Target/ThreadPlanStepInstruction.cpp | 4 ++-- lldb/source/Target/ThreadPlanStepOut.cpp | 8 ++++---- lldb/source/Target/ThreadPlanStepRange.cpp | 2 +- lldb/source/Target/ThreadPlanStepUntil.cpp | 4 ++-- 7 files changed, 19 insertions(+), 19 deletions(-) diff --git a/lldb/include/lldb/Target/StackID.h b/lldb/include/lldb/Target/StackID.h index 2292bef5e49be..57e4386b39ad4 100644 --- a/lldb/include/lldb/Target/StackID.h +++ b/lldb/include/lldb/Target/StackID.h @@ -50,10 +50,10 @@ class StackID { void Dump(Stream *s); - /// Returns true if `lhs` corresponds to a frame younger (i.e. higher on the - /// call stack) than `rhs`, and false otherwise (including when the frames are - /// not comparable). - static bool IsYounger(const StackID &lhs, const StackID &rhs); + /// 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). + bool IsYoungerThan(const StackID &other) const; protected: friend class StackFrame; diff --git a/lldb/source/Target/StackFrameList.cpp b/lldb/source/Target/StackFrameList.cpp index b124ef1c199c9..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 StackID::IsYounger(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 c5d8e5a3e6644..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 StackID::IsYounger(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 StackID::IsYounger(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 0e062bf6d7a34..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 (StackID::IsYounger(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. @@ -138,7 +138,7 @@ bool ThreadPlanStepInstruction::ShouldStop(Event *event_ptr) { StackID cur_frame_zero_id = cur_frame_sp->GetStackID(); if (cur_frame_zero_id == m_stack_id || - StackID::IsYounger(m_stack_id, cur_frame_zero_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 9d830f13947c7..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 (StackID::IsYounger(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 = (StackID::IsYounger(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 = !(StackID::IsYounger(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 !(StackID::IsYounger(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 00f5d70ee00ff..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 (StackID::IsYounger(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 43d068162c3b2..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 = (StackID::IsYounger(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 (StackID::IsYounger(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
