Skip to content

[lldb] Move "SelectMostRelevantFrame" from Thread::WillStop #6622

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

[lldb] Move "SelectMostRelevantFrame" from Thread::WillStop #6622

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions lldb/include/lldb/Target/StackFrameList.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class StackFrameList {
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.
Expand Down Expand Up @@ -110,6 +110,8 @@ class StackFrameList {

void SetCurrentInlinedDepth(uint32_t new_depth);

void SelectMostRelevantFrame();

typedef std::vector<lldb::StackFrameSP> collection;
typedef collection::iterator iterator;
typedef collection::const_iterator const_iterator;
Expand All @@ -134,8 +136,10 @@ class StackFrameList {
/// 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<uint32_t> m_selected_frame_idx;

/// The number of concrete frames fetched while filling the frame list. This
/// is only used when synthetic frames are enabled.
Expand Down
2 changes: 0 additions & 2 deletions lldb/include/lldb/Target/Thread.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,6 @@ class Thread : public std::enable_shared_from_this<Thread>,

virtual void RefreshStateAfterStop() = 0;

void SelectMostRelevantFrame();

std::string GetStopDescription();

std::string GetStopDescriptionRaw();
Expand Down
58 changes: 52 additions & 6 deletions lldb/source/Target/StackFrameList.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -38,7 +39,7 @@ StackFrameList::StackFrameList(Thread &thread,
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) {
Expand Down Expand Up @@ -252,7 +253,7 @@ struct CallDescriptor {
using CallSequence = std::vector<CallDescriptor>;

/// 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,
Expand Down Expand Up @@ -776,9 +777,46 @@ bool StackFrameList::SetFrameAtIndex(uint32_t idx, StackFrameSP &frame_sp) {
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<std::recursive_mutex> 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) {
Expand All @@ -787,17 +825,19 @@ uint32_t StackFrameList::SetSelectedFrame(lldb_private::StackFrame *frame) {
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) {
Expand Down Expand Up @@ -825,10 +865,16 @@ void StackFrameList::SetDefaultFileAndLineToSelectedFrame() {

// 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<std::recursive_mutex> guard(m_mutex);
m_frames.clear();
m_concrete_frames_fetched = 0;
m_selected_frame_idx.reset();
}

lldb::StackFrameSP
Expand Down
27 changes: 0 additions & 27 deletions lldb/source/Target/Thread.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -606,36 +606,9 @@ std::string Thread::GetStopDescriptionRaw() {
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.

Expand Down