diff --git a/lldb/include/lldb/Target/StackFrameList.h b/lldb/include/lldb/Target/StackFrameList.h --- a/lldb/include/lldb/Target/StackFrameList.h +++ b/lldb/include/lldb/Target/StackFrameList.h @@ -46,7 +46,7 @@ uint32_t SetSelectedFrame(lldb_private::StackFrame *frame); /// Get the currently selected frame index. - uint32_t GetSelectedFrameIndex() const; + uint32_t GetSelectedFrameIndex(); /// Mark a stack frame as the currently selected frame using the frame index /// \p idx. Like \ref GetFrameAtIndex, invisible frames cannot be selected. @@ -110,6 +110,8 @@ void SetCurrentInlinedDepth(uint32_t new_depth); + void SelectMostRelevantFrame(); + typedef std::vector collection; typedef collection::iterator iterator; typedef collection::const_iterator const_iterator; @@ -134,8 +136,10 @@ /// changes. collection m_frames; - /// The currently selected frame. - uint32_t m_selected_frame_idx; + /// The currently selected frame. An optional is used to record whether anyone + /// has set the selected frame on this stack yet. We only let recognizers + /// change the frame if this is the first time GetSelectedFrame is called. + std::optional m_selected_frame_idx; /// The number of concrete frames fetched while filling the frame list. This /// is only used when synthetic frames are enabled. diff --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h --- a/lldb/include/lldb/Target/Thread.h +++ b/lldb/include/lldb/Target/Thread.h @@ -217,8 +217,6 @@ virtual void RefreshStateAfterStop() = 0; - void SelectMostRelevantFrame(); - std::string GetStopDescription(); std::string GetStopDescriptionRaw(); diff --git a/lldb/source/Target/StackFrameList.cpp b/lldb/source/Target/StackFrameList.cpp --- a/lldb/source/Target/StackFrameList.cpp +++ b/lldb/source/Target/StackFrameList.cpp @@ -18,6 +18,7 @@ #include "lldb/Target/Process.h" #include "lldb/Target/RegisterContext.h" #include "lldb/Target/StackFrame.h" +#include "lldb/Target/StackFrameRecognizer.h" #include "lldb/Target/StopInfo.h" #include "lldb/Target/Target.h" #include "lldb/Target/Thread.h" @@ -38,7 +39,7 @@ const lldb::StackFrameListSP &prev_frames_sp, bool show_inline_frames) : m_thread(thread), m_prev_frames_sp(prev_frames_sp), m_mutex(), m_frames(), - m_selected_frame_idx(0), m_concrete_frames_fetched(0), + m_selected_frame_idx(), m_concrete_frames_fetched(0), m_current_inlined_depth(UINT32_MAX), m_current_inlined_pc(LLDB_INVALID_ADDRESS), m_show_inlined_frames(show_inline_frames) { @@ -252,7 +253,7 @@ using CallSequence = std::vector; /// Find the unique path through the call graph from \p begin (with return PC -/// \p return_pc) to \p end. On success this path is stored into \p path, and +/// \p return_pc) to \p end. On success this path is stored into \p path, and /// on failure \p path is unchanged. static void FindInterveningFrames(Function &begin, Function &end, ExecutionContext &exe_ctx, Target &target, @@ -781,9 +782,46 @@ return false; // resize failed, out of memory? } -uint32_t StackFrameList::GetSelectedFrameIndex() const { +void StackFrameList::SelectMostRelevantFrame() { + // Don't call into the frame recognizers on the private state thread as + // they can cause code to run in the target, and that can cause deadlocks + // when fetching stop events for the expression. + if (m_thread.GetProcess()->CurrentThreadIsPrivateStateThread()) + return; + + Log *log = GetLog(LLDBLog::Thread); + + // Only the top frame should be recognized. + StackFrameSP frame_sp = GetFrameAtIndex(0); + if (!frame_sp) { + LLDB_LOG(log, "Failed to construct Frame #0"); + return; + } + + RecognizedStackFrameSP recognized_frame_sp = frame_sp->GetRecognizedFrame(); + + if (!recognized_frame_sp) { + LLDB_LOG(log, "Frame #0 not recognized"); + return; + } + + if (StackFrameSP most_relevant_frame_sp = + recognized_frame_sp->GetMostRelevantFrame()) { + LLDB_LOG(log, "Found most relevant frame at index {0}", + most_relevant_frame_sp->GetFrameIndex()); + SetSelectedFrame(most_relevant_frame_sp.get()); + } else { + LLDB_LOG(log, "No relevant frame!"); + } +} + +uint32_t StackFrameList::GetSelectedFrameIndex() { std::lock_guard guard(m_mutex); - return m_selected_frame_idx; + if (!m_selected_frame_idx) + SelectMostRelevantFrame(); + if (!m_selected_frame_idx) + m_selected_frame_idx = 0; + return *m_selected_frame_idx; } uint32_t StackFrameList::SetSelectedFrame(lldb_private::StackFrame *frame) { @@ -792,17 +830,19 @@ const_iterator begin = m_frames.begin(); const_iterator end = m_frames.end(); m_selected_frame_idx = 0; + for (pos = begin; pos != end; ++pos) { if (pos->get() == frame) { m_selected_frame_idx = std::distance(begin, pos); uint32_t inlined_depth = GetCurrentInlinedDepth(); if (inlined_depth != UINT32_MAX) - m_selected_frame_idx -= inlined_depth; + m_selected_frame_idx = *m_selected_frame_idx - inlined_depth; break; } } + SetDefaultFileAndLineToSelectedFrame(); - return m_selected_frame_idx; + return *m_selected_frame_idx; } bool StackFrameList::SetSelectedFrameByIndex(uint32_t idx) { @@ -830,10 +870,16 @@ // The thread has been run, reset the number stack frames to zero so we can // determine how many frames we have lazily. +// Note, we don't actually re-use StackFrameLists, we always make a new +// StackFrameList every time we stop, and then copy frame information frame +// by frame from the old to the new StackFrameList. So the comment above, +// does not describe how StackFrameLists are currently used. +// Clear is currently only used to clear the list in the destructor. void StackFrameList::Clear() { std::lock_guard guard(m_mutex); m_frames.clear(); m_concrete_frames_fetched = 0; + m_selected_frame_idx.reset(); } lldb::StackFrameSP diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp --- a/lldb/source/Target/Thread.cpp +++ b/lldb/source/Target/Thread.cpp @@ -588,36 +588,9 @@ return raw_stop_description; } -void Thread::SelectMostRelevantFrame() { - Log *log = GetLog(LLDBLog::Thread); - - auto frames_list_sp = GetStackFrameList(); - - // Only the top frame should be recognized. - auto frame_sp = frames_list_sp->GetFrameAtIndex(0); - - auto recognized_frame_sp = frame_sp->GetRecognizedFrame(); - - if (!recognized_frame_sp) { - LLDB_LOG(log, "Frame #0 not recognized"); - return; - } - - if (StackFrameSP most_relevant_frame_sp = - recognized_frame_sp->GetMostRelevantFrame()) { - LLDB_LOG(log, "Found most relevant frame at index {0}", - most_relevant_frame_sp->GetFrameIndex()); - SetSelectedFrame(most_relevant_frame_sp.get()); - } else { - LLDB_LOG(log, "No relevant frame!"); - } -} - void Thread::WillStop() { ThreadPlan *current_plan = GetCurrentPlan(); - SelectMostRelevantFrame(); - // FIXME: I may decide to disallow threads with no plans. In which // case this should go to an assert.