Skip to content

Commit 01bcbea

Browse files
committed
[lldb/Target] Track containing StackFrameList to avoid circular dependencies
This change adds tracking of the StackFrameList that created each frame by storing a weak pointer (m_frame_list_wp) in both StackFrame and ExecutionContextRef. When resolving frames through `ExecutionContextRef::GetFrameSP`, the code now first attempts to use the remembered frame list instead of immediately calling `Thread::GetStackFrameList`. This breaks circular dependencies that can occur during frame provider initialization, where creating a frame provider might trigger ExecutionContext resolution, which would then call back into `Thread::GetStackFrameList`, creating an infinite loop. The StackFrameList now sets m_frame_list_wp on every frame it creates, and a new `GetContainingStackFrameList` method that allows frames to expose their source list. Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
1 parent 9324dae commit 01bcbea

File tree

6 files changed

+48
-5
lines changed

6 files changed

+48
-5
lines changed

lldb/include/lldb/Target/ExecutionContext.h

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,10 @@ class ExecutionContextRef {
268268
m_tid = LLDB_INVALID_THREAD_ID;
269269
}
270270

271-
void ClearFrame() { m_stack_id.Clear(); }
271+
void ClearFrame() {
272+
m_stack_id.Clear();
273+
m_frame_list_wp.reset();
274+
}
272275

273276
protected:
274277
// Member variables
@@ -279,7 +282,14 @@ class ExecutionContextRef {
279282
///< object refers to in case the
280283
/// backing object changes
281284
StackID m_stack_id; ///< The stack ID that this object refers to in case the
282-
///backing object changes
285+
///< backing object changes
286+
mutable lldb::StackFrameListWP
287+
m_frame_list_wp; ///< Weak reference to the
288+
///< frame list that contains
289+
///< this frame. If we can create a valid
290+
///< StackFrameListSP from it, we must use it to resolve
291+
///< the StackID, otherwise, we should ask the Thread's
292+
///< StackFrameList.
283293
};
284294

285295
/// \class ExecutionContext ExecutionContext.h

lldb/include/lldb/Target/StackFrame.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -532,6 +532,19 @@ class StackFrame : public ExecutionContextScope,
532532

533533
lldb::RecognizedStackFrameSP GetRecognizedFrame();
534534

535+
/// Get the StackFrameList that contains this frame.
536+
///
537+
/// Returns the StackFrameList that contains this frame, allowing
538+
/// frames to resolve execution contexts without calling
539+
/// Thread::GetStackFrameList(), which can cause circular dependencies
540+
/// during frame provider initialization.
541+
///
542+
/// \return
543+
/// The StackFrameList that contains this frame, or nullptr if not set.
544+
virtual lldb::StackFrameListSP GetContainingStackFrameList() const {
545+
return m_frame_list_wp.lock();
546+
}
547+
535548
protected:
536549
friend class StackFrameList;
537550

@@ -574,6 +587,7 @@ class StackFrame : public ExecutionContextScope,
574587
/// well as any other frame with the same trait.
575588
bool m_behaves_like_zeroth_frame;
576589
lldb::VariableListSP m_variable_list_sp;
590+
lldb::StackFrameListWP m_frame_list_wp;
577591
/// Value objects for each variable in m_variable_list_sp.
578592
ValueObjectList m_variable_list_value_objects;
579593
std::optional<lldb::RecognizedStackFrameSP> m_recognized_frame_sp;

lldb/include/lldb/Target/StackFrameList.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ namespace lldb_private {
2020

2121
class ScriptedThread;
2222

23-
class StackFrameList {
23+
class StackFrameList : public std::enable_shared_from_this<StackFrameList> {
2424
public:
2525
// Constructors and Destructors
2626
StackFrameList(Thread &thread, const lldb::StackFrameListSP &prev_frames_sp,

lldb/include/lldb/lldb-forward.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,7 @@ typedef std::unique_ptr<lldb_private::SourceManager> SourceManagerUP;
440440
typedef std::shared_ptr<lldb_private::StackFrame> StackFrameSP;
441441
typedef std::weak_ptr<lldb_private::StackFrame> StackFrameWP;
442442
typedef std::shared_ptr<lldb_private::StackFrameList> StackFrameListSP;
443+
typedef std::weak_ptr<lldb_private::StackFrameList> StackFrameListWP;
443444
typedef std::shared_ptr<lldb_private::StackFrameRecognizer>
444445
StackFrameRecognizerSP;
445446
typedef std::unique_ptr<lldb_private::StackFrameRecognizerManager>

lldb/source/Target/ExecutionContext.cpp

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -466,10 +466,13 @@ operator=(const ExecutionContext &exe_ctx) {
466466
else
467467
m_tid = LLDB_INVALID_THREAD_ID;
468468
lldb::StackFrameSP frame_sp(exe_ctx.GetFrameSP());
469-
if (frame_sp)
469+
if (frame_sp) {
470470
m_stack_id = frame_sp->GetStackID();
471-
else
471+
m_frame_list_wp = frame_sp->GetContainingStackFrameList();
472+
} else {
472473
m_stack_id.Clear();
474+
m_frame_list_wp.reset();
475+
}
473476
return *this;
474477
}
475478

@@ -511,6 +514,7 @@ void ExecutionContextRef::SetThreadSP(const lldb::ThreadSP &thread_sp) {
511514
void ExecutionContextRef::SetFrameSP(const lldb::StackFrameSP &frame_sp) {
512515
if (frame_sp) {
513516
m_stack_id = frame_sp->GetStackID();
517+
m_frame_list_wp = frame_sp->GetContainingStackFrameList();
514518
SetThreadSP(frame_sp->GetThread());
515519
} else {
516520
ClearFrame();
@@ -638,6 +642,15 @@ lldb::ThreadSP ExecutionContextRef::GetThreadSP() const {
638642

639643
lldb::StackFrameSP ExecutionContextRef::GetFrameSP() const {
640644
if (m_stack_id.IsValid()) {
645+
// Try the remembered frame list first to avoid circular dependencies
646+
// during frame provider initialization.
647+
if (auto frame_list_sp = m_frame_list_wp.lock()) {
648+
if (auto frame_sp = frame_list_sp->GetFrameWithStackID(m_stack_id))
649+
return frame_sp;
650+
}
651+
652+
// Fallback: ask the thread, which might re-trigger the frame provider
653+
// initialization.
641654
lldb::ThreadSP thread_sp(GetThreadSP());
642655
if (thread_sp)
643656
return thread_sp->GetFrameWithStackID(m_stack_id);

lldb/source/Target/StackFrameList.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,7 @@ void StackFrameList::SynthesizeTailCallFrames(StackFrame &next_frame) {
330330
m_thread.shared_from_this(), frame_idx, concrete_frame_idx, cfa,
331331
cfa_is_valid, pc, StackFrame::Kind::Regular, artificial,
332332
behaves_like_zeroth_frame, &sc);
333+
synth_frame->m_frame_list_wp = shared_from_this();
333334
m_frames.push_back(synth_frame);
334335
LLDB_LOG(log, "Pushed frame {0} at {1:x}", callee->GetDisplayName(), pc);
335336
}
@@ -445,6 +446,7 @@ bool StackFrameList::FetchFramesUpTo(uint32_t end_idx,
445446
unwind_frame_sp = std::make_shared<StackFrame>(
446447
m_thread.shared_from_this(), m_frames.size(), idx, reg_ctx_sp,
447448
cfa, pc, behaves_like_zeroth_frame, nullptr);
449+
unwind_frame_sp->m_frame_list_wp = shared_from_this();
448450
m_frames.push_back(unwind_frame_sp);
449451
}
450452
} else {
@@ -479,6 +481,7 @@ bool StackFrameList::FetchFramesUpTo(uint32_t end_idx,
479481
// although its concrete index will stay the same.
480482
SynthesizeTailCallFrames(*unwind_frame_sp.get());
481483

484+
unwind_frame_sp->m_frame_list_wp = shared_from_this();
482485
m_frames.push_back(unwind_frame_sp);
483486
}
484487

@@ -503,6 +506,7 @@ bool StackFrameList::FetchFramesUpTo(uint32_t end_idx,
503506
unwind_frame_sp->GetRegisterContextSP(), cfa, next_frame_address,
504507
behaves_like_zeroth_frame, &next_frame_sc));
505508

509+
frame_sp->m_frame_list_wp = shared_from_this();
506510
m_frames.push_back(frame_sp);
507511
unwind_sc = next_frame_sc;
508512
curr_frame_address = next_frame_address;
@@ -559,6 +563,7 @@ bool StackFrameList::FetchFramesUpTo(uint32_t end_idx,
559563
prev_frame->UpdatePreviousFrameFromCurrentFrame(*curr_frame);
560564
// Now copy the fixed up previous frame into the current frames so the
561565
// pointer doesn't change.
566+
prev_frame_sp->m_frame_list_wp = shared_from_this();
562567
m_frames[curr_frame_idx] = prev_frame_sp;
563568

564569
#if defined(DEBUG_STACK_FRAMES)

0 commit comments

Comments
 (0)