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

[Michael137]

This attempts to improve user-experience when LLDB stops on a verbose_trap. Currently if a __builtin_verbose_trap triggers, we display the first frame above the call to the verbose_trap. So in the newly added test case, we would've previously stopped here:

(lldb) run
Process 28095 launched: '/Users/michaelbuch/a.out' (arm64)
Process 28095 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = Bounds error: out-of-bounds access
    frame #1: 0x0000000100003f5c a.out`std::__1::vector<int>::operator[](this=0x000000016fdfebef size=0, (null)=10) at verbose_trap.cpp:6:9
   3    template <typename T>
   4    struct vector {
   5        void operator[](unsigned) {
-> 6            __builtin_verbose_trap("Bounds error", "out-of-bounds access");
   7        }
   8    };

After this patch, we would stop in the first non-std frame:

(lldb) run
Process 27843 launched: '/Users/michaelbuch/a.out' (arm64)
Process 27843 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = Bounds error: out-of-bounds access
    frame #2: 0x0000000100003f44 a.out`g() at verbose_trap.cpp:14:5
   11  
   12   void g() {
   13       std::vector<int> v;
-> 14       v[10];
   15   }
   16  

rdar://134490328

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

This attempts to improve user-experience when LLDB stops on a verbose_trap. Currently if a __builtin_verbose_trap triggers, we display the first frame above the call to the verbose_trap. So in the newly added test case, we would've previously stopped here:

(lldb) run
Process 28095 launched: '/Users/michaelbuch/a.out' (arm64)
Process 28095 stopped
* thread #<!-- -->1, queue = 'com.apple.main-thread', stop reason = Bounds error: out-of-bounds access
    frame #<!-- -->1: 0x0000000100003f5c a.out`std::__1::vector&lt;int&gt;::operator[](this=0x000000016fdfebef size=0, (null)=10) at verbose_trap.cpp:6:9
   3    template &lt;typename T&gt;
   4    struct vector {
   5        void operator[](unsigned) {
-&gt; 6            __builtin_verbose_trap("Bounds error", "out-of-bounds access");
   7        }
   8    };

After this patch, we would stop in the first non-std frame:

(lldb) run
Process 27843 launched: '/Users/michaelbuch/a.out' (arm64)
Process 27843 stopped
* thread #<!-- -->1, queue = 'com.apple.main-thread', stop reason = Bounds error: out-of-bounds access
    frame #<!-- -->2: 0x0000000100003f44 a.out`g() at verbose_trap.cpp:14:5
   11  
   12   void g() {
   13       std::vector&lt;int&gt; v;
-&gt; 14       v[10];
   15   }
   16  

---

Full diff: https://github.com/llvm/llvm-project/pull/108825.diff

7 Files Affected:

• (modified) lldb/source/Target/VerboseTrapFrameRecognizer.cpp (+26-1)
• (added) lldb/test/Shell/Recognizer/Inputs/verbose_trap-in-stl-callback.cpp (+22)
• (added) lldb/test/Shell/Recognizer/Inputs/verbose_trap-in-stl-nested.cpp (+21)
• (added) lldb/test/Shell/Recognizer/Inputs/verbose_trap-in-stl.cpp (+17)
• (added) lldb/test/Shell/Recognizer/verbose_trap-in-stl-callback.test (+13)
• (added) lldb/test/Shell/Recognizer/verbose_trap-in-stl-nested.test (+13)
• (added) lldb/test/Shell/Recognizer/verbose_trap-in-stl.test (+13)

diff --git a/lldb/source/Target/VerboseTrapFrameRecognizer.cpp b/lldb/source/Target/VerboseTrapFrameRecognizer.cpp
index de710fcda54064..530a8941c8d504 100644
--- a/lldb/source/Target/VerboseTrapFrameRecognizer.cpp
+++ b/lldb/source/Target/VerboseTrapFrameRecognizer.cpp
@@ -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) {
+    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) {
@@ -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);
diff --git a/lldb/test/Shell/Recognizer/Inputs/verbose_trap-in-stl-callback.cpp b/lldb/test/Shell/Recognizer/Inputs/verbose_trap-in-stl-callback.cpp
new file mode 100644
index 00000000000000..23beed4c62c3b3
--- /dev/null
+++ b/lldb/test/Shell/Recognizer/Inputs/verbose_trap-in-stl-callback.cpp
@@ -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;
+}
diff --git a/lldb/test/Shell/Recognizer/Inputs/verbose_trap-in-stl-nested.cpp b/lldb/test/Shell/Recognizer/Inputs/verbose_trap-in-stl-nested.cpp
new file mode 100644
index 00000000000000..67fa65c9ceae22
--- /dev/null
+++ b/lldb/test/Shell/Recognizer/Inputs/verbose_trap-in-stl-nested.cpp
@@ -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;
+}
diff --git a/lldb/test/Shell/Recognizer/Inputs/verbose_trap-in-stl.cpp b/lldb/test/Shell/Recognizer/Inputs/verbose_trap-in-stl.cpp
new file mode 100644
index 00000000000000..4f01827944e166
--- /dev/null
+++ b/lldb/test/Shell/Recognizer/Inputs/verbose_trap-in-stl.cpp
@@ -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;
+}
diff --git a/lldb/test/Shell/Recognizer/verbose_trap-in-stl-callback.test b/lldb/test/Shell/Recognizer/verbose_trap-in-stl-callback.test
new file mode 100644
index 00000000000000..6ea397d3ee700b
--- /dev/null
+++ b/lldb/test/Shell/Recognizer/verbose_trap-in-stl-callback.test
@@ -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
diff --git a/lldb/test/Shell/Recognizer/verbose_trap-in-stl-nested.test b/lldb/test/Shell/Recognizer/verbose_trap-in-stl-nested.test
new file mode 100644
index 00000000000000..81a492d1ed5791
--- /dev/null
+++ b/lldb/test/Shell/Recognizer/verbose_trap-in-stl-nested.test
@@ -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
+
+run
+# CHECK: thread #{{.*}}stop reason = Bounds error: out-of-bounds access
+frame info
+# CHECK: frame #{{.*}}`g() at verbose_trap-in-stl-nested.cpp
+q
diff --git a/lldb/test/Shell/Recognizer/verbose_trap-in-stl.test b/lldb/test/Shell/Recognizer/verbose_trap-in-stl.test
new file mode 100644
index 00000000000000..dd08290174e3af
--- /dev/null
+++ b/lldb/test/Shell/Recognizer/verbose_trap-in-stl.test
@@ -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

[ldionne]

I'm not qualified to review the implementation, but I like the result!

[egorzhdan]

This is a great UX improvement, thanks!

[adrian-prantl]

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.

[adrian-prantl]

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

[Michael137]

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.