Skip to content
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.

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

[lldb][FrameRecognizer] Display the first non-std frame on verbose_trap #108825

Merged
merged 3 commits into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
27 changes: 26 additions & 1 deletion lldb/source/Target/VerboseTrapFrameRecognizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,31 @@ using namespace llvm;
using namespace lldb;
using namespace lldb_private;

/// The 0th frame is the artificial inline frame generated to store
/// the verbose_trap message. So, starting with the current parent frame,
/// find the first frame that's not inside of the STL.
static StackFrameSP FindMostRelevantFrame(Thread &selected_thread) {
StackFrameSP most_relevant_frame_sp = selected_thread.GetStackFrameAtIndex(1);
while (most_relevant_frame_sp) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add an upper bound for the loop? I'm afraid that this recognizer might be triggered when looking at the backtrace of an infinite recursion, for example, and then LLDB just hangs.

auto const &sc =
most_relevant_frame_sp->GetSymbolContext(eSymbolContextEverything);
ConstString frame_name = sc.GetFunctionName();
if (!frame_name)
return nullptr;

// Found a frame outside of the `std` namespace. That's the
// first frame in user-code that ended up triggering the
// verbose_trap. Hence that's the one we want to display.
if (!frame_name.GetStringRef().starts_with("std::"))
return most_relevant_frame_sp;

most_relevant_frame_sp = selected_thread.GetStackFrameAtIndex(
most_relevant_frame_sp->GetFrameIndex() + 1);
}

return nullptr;
}

VerboseTrapRecognizedStackFrame::VerboseTrapRecognizedStackFrame(
StackFrameSP most_relevant_frame_sp, std::string stop_desc)
: m_most_relevant_frame(most_relevant_frame_sp) {
Expand All @@ -30,7 +55,7 @@ VerboseTrapFrameRecognizer::RecognizeFrame(lldb::StackFrameSP frame_sp) {
ThreadSP thread_sp = frame_sp->GetThread();
ProcessSP process_sp = thread_sp->GetProcess();

StackFrameSP most_relevant_frame_sp = thread_sp->GetStackFrameAtIndex(1);
StackFrameSP most_relevant_frame_sp = FindMostRelevantFrame(*thread_sp);

if (!most_relevant_frame_sp) {
Log *log = GetLog(LLDBLog::Unwind);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
namespace std {
void definitely_aborts() { __builtin_verbose_trap("Failed", "Invariant violated"); }

void aborts_soon() { definitely_aborts(); }
} // namespace std

void g() { std::aborts_soon(); }

namespace std {
namespace detail {
void eventually_aborts() { g(); }
} // namespace detail

inline namespace __1 {
void eventually_aborts() { detail::eventually_aborts(); }
} // namespace __1
} // namespace std

int main() {
std::eventually_aborts();
return 0;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
namespace std {
namespace detail {
void function_that_aborts() { __builtin_verbose_trap("Bounds error", "out-of-bounds access"); }
} // namespace detail

inline namespace __1 {
template <typename T> struct vector {
void operator[](unsigned) { detail::function_that_aborts(); }
};
} // namespace __1
} // namespace std

void g() {
std::vector<int> v;
v[10];
}

int main() {
g();
return 0;
}
17 changes: 17 additions & 0 deletions lldb/test/Shell/Recognizer/Inputs/verbose_trap-in-stl.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
namespace std {
inline namespace __1 {
template <typename T> struct vector {
void operator[](unsigned) { __builtin_verbose_trap("Bounds error", "out-of-bounds access"); }
};
} // namespace __1
} // namespace std

void g() {
std::vector<int> v;
v[10];
}

int main() {
g();
return 0;
}
13 changes: 13 additions & 0 deletions lldb/test/Shell/Recognizer/verbose_trap-in-stl-callback.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Tests that we show the first non-STL frame when
# a verbose_trap triggers from within the STL.

# UNSUPPORTED: system-windows
#
# RUN: %clang_host -g -O0 %S/Inputs/verbose_trap-in-stl-callback.cpp -o %t.out
# RUN: %lldb -b -s %s %t.out | FileCheck %s --check-prefixes=CHECK

run
# CHECK: thread #{{.*}}stop reason = Failed: Invariant violated
frame info
# CHECK: frame #{{.*}}`g() at verbose_trap-in-stl-callback.cpp
q
13 changes: 13 additions & 0 deletions lldb/test/Shell/Recognizer/verbose_trap-in-stl-nested.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Tests that we show the first non-STL frame when
# a verbose_trap triggers from within the STL.

# UNSUPPORTED: system-windows
#
# RUN: %clang_host -g -O0 %S/Inputs/verbose_trap-in-stl-nested.cpp -o %t.out
# RUN: %lldb -b -s %s %t.out | FileCheck %s --check-prefixes=CHECK
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a good reason why this isn't an API test?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find it much easier to understand what the test is trying to check when it's done as a Shell test. Especially for when we just want to inspect the format of the frame. (FWIW, all the other frame format tests are also Shell tests).

From the Testing FAQ:

API tests: Integration tests that interact with the debugger through the SB API. These are written in Python and use LLDB’s dotest.py testing framework on top of Python’s unittest.

And later:

A good rule of thumb is to prefer shell tests when what is being tested is relatively simple. Expressivity is limited compared to the API tests, which means that you have to have a well-defined test scenario that you can easily match with FileCheck.

IMO these tests fit into the latter category.

But I'm happy to change these to API tests if that is more appropriate.


run
# CHECK: thread #{{.*}}stop reason = Bounds error: out-of-bounds access
frame info
# CHECK: frame #{{.*}}`g() at verbose_trap-in-stl-nested.cpp
q
13 changes: 13 additions & 0 deletions lldb/test/Shell/Recognizer/verbose_trap-in-stl.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Tests that we show the first non-STL frame when
# a verbose_trap triggers from within the STL.

# UNSUPPORTED: system-windows
#
# RUN: %clang_host -g -O0 %S/Inputs/verbose_trap-in-stl.cpp -o %t.out
# RUN: %lldb -b -s %s %t.out | FileCheck %s --check-prefixes=CHECK

run
# CHECK: thread #{{.*}}stop reason = Bounds error: out-of-bounds access
frame info
# CHECK: frame #{{.*}}`g() at verbose_trap-in-stl.cpp
q
Loading