[lldb] Add frame recognizer for __builtin_verbose_trap #80368

Michael137 · 2024-02-02T00:55:02Z

This patch adds a frame recognizer for Clang's
__builtin_verbose_trap, which behaves like a
__builtin_trap, but emits a failure-reason string into debug-info in order for debuggers to display
it to a user.

The frame recognizer triggers when we encounter
a frame with a function name that begins with
__llvm_verbose_trap, which is the magic prefix
Clang emits into debug-info for verbose traps.
Once such frame is encountered we display the
frame function name as the Stop Reason and display that frame to the user.

Example output:

(lldb) run
warning: a.out was compiled with optimization - stepping may behave oddly; variables may not be available.
Process 35942 launched: 'a.out' (arm64)
Process 35942 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = __llvm_verbose_trap: Function is not implemented
    frame #1: 0x0000000100003fa4 a.out`main [inlined] Dummy::func(this=<unavailable>) at verbose_trap.cpp:3:5 [opt]
   1    struct Dummy {
   2      void func() {
-> 3        __builtin_verbose_trap("Function is not implemented");
   4      }
   5    };
   6
   7    int main() {
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = __llvm_verbose_trap: Function is not implemented
    frame #0: 0x0000000100003fa4 a.out`main [inlined] __llvm_verbose_trap: Function is not implemented at verbose_trap.cpp:0 [opt]
  * frame #1: 0x0000000100003fa4 a.out`main [inlined] Dummy::func(this=<unavailable>) at verbose_trap.cpp:3:5 [opt]
    frame #2: 0x0000000100003fa4 a.out`main at verbose_trap.cpp:8:13 [opt]
    frame #3: 0x0000000189d518b4 dyld`start + 1988

llvmbot · 2024-02-02T00:55:35Z

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

This patch adds a frame recognizer for Clang's
__builtin_verbose_trap, which behaves like a
__builtin_trap, but emits a failure-reason string into debug-info in order for debuggers to display
it to a user.

The frame recognizer triggers when we encounter
a frame with a function name that begins with
__llvm_verbose_trap, which is the magic prefix
Clang emits into debug-info for verbose traps.
Once such frame is encountered we display the
frame function name as the Stop Reason and display that frame to the user.

Example output:

(lldb) run
warning: a.out was compiled with optimization - stepping may behave oddly; variables may not be available.
Process 35942 launched: 'a.out' (arm64)
Process 35942 stopped
* thread #<!-- -->1, queue = 'com.apple.main-thread', stop reason = __llvm_verbose_trap: Function is not implemented
    frame #<!-- -->1: 0x0000000100003fa4 a.out`main [inlined] Dummy::func(this=&lt;unavailable&gt;) at verbose_trap.cpp:3:5 [opt]
   1    struct Dummy {
   2      void func() {
-&gt; 3        __builtin_verbose_trap("Function is not implemented");
   4      }
   5    };
   6
   7    int main() {
(lldb) bt
* thread #<!-- -->1, queue = 'com.apple.main-thread', stop reason = __llvm_verbose_trap: Function is not implemented
    frame #<!-- -->0: 0x0000000100003fa4 a.out`main [inlined] __llvm_verbose_trap: Function is not implemented at verbose_trap.cpp:0 [opt]
  * frame #<!-- -->1: 0x0000000100003fa4 a.out`main [inlined] Dummy::func(this=&lt;unavailable&gt;) at verbose_trap.cpp:3:5 [opt]
    frame #<!-- -->2: 0x0000000100003fa4 a.out`main at verbose_trap.cpp:8:13 [opt]
    frame #<!-- -->3: 0x0000000189d518b4 dyld`start + 1988

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

6 Files Affected:

(added) lldb/include/lldb/Target/VerboseTrapFrameRecognizer.h (+39)
(modified) lldb/source/Target/CMakeLists.txt (+1)
(modified) lldb/source/Target/Process.cpp (+2)
(added) lldb/source/Target/VerboseTrapFrameRecognizer.cpp (+85)
(added) lldb/test/Shell/Recognizer/Inputs/verbose_trap.cpp (+8)
(added) lldb/test/Shell/Recognizer/verbose_trap.test (+9)

diff --git a/lldb/include/lldb/Target/VerboseTrapFrameRecognizer.h b/lldb/include/lldb/Target/VerboseTrapFrameRecognizer.h
new file mode 100644
index 0000000000000..7e045760a28be
--- /dev/null
+++ b/lldb/include/lldb/Target/VerboseTrapFrameRecognizer.h
@@ -0,0 +1,39 @@
+#ifndef LLDB_TARGET_VERBOSETRAPFRAMERECOGNIZER_H
+#define LLDB_TARGET_VERBOSETRAPFRAMERECOGNIZER_H
+
+#include "lldb/Target/StackFrameRecognizer.h"
+
+namespace lldb_private {
+
+void RegisterVerboseTrapFrameRecognizer(Process &process);
+
+/// Holds the stack frame that caused the Verbose trap and the inlined stop
+/// reason message.
+class VerboseTrapRecognizedStackFrame : public RecognizedStackFrame {
+public:
+  VerboseTrapRecognizedStackFrame(lldb::StackFrameSP most_relevant_frame_sp,
+                                  std::string stop_desc);
+
+  lldb::StackFrameSP GetMostRelevantFrame() override;
+
+private:
+  lldb::StackFrameSP m_most_relevant_frame;
+};
+
+/// When a thread stops, it checks the current frame contains a
+/// Verbose Trap diagnostic. If so, it returns a \a
+/// VerboseTrapRecognizedStackFrame holding the diagnostic a stop reason
+/// description with and the parent frame as the most relavant frame.
+class VerboseTrapFrameRecognizer : public StackFrameRecognizer {
+public:
+  std::string GetName() override {
+    return "Verbose Trap StackFrame Recognizer";
+  }
+
+  lldb::RecognizedStackFrameSP
+  RecognizeFrame(lldb::StackFrameSP frame) override;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_TARGET_VERBOSETRAPFRAMERECOGNIZER_H
diff --git a/lldb/source/Target/CMakeLists.txt b/lldb/source/Target/CMakeLists.txt
index cf4818eae3eb8..8186ccbea27d4 100644
--- a/lldb/source/Target/CMakeLists.txt
+++ b/lldb/source/Target/CMakeLists.txt
@@ -78,6 +78,7 @@ add_lldb_library(lldbTarget
   UnixSignals.cpp
   UnwindAssembly.cpp
   UnwindLLDB.cpp
+  VerboseTrapFrameRecognizer.cpp
 
   LINK_LIBS
     lldbBreakpoint
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 23a8a66645c02..04b00aaa1fac9 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -63,6 +63,7 @@
 #include "lldb/Target/ThreadPlanCallFunction.h"
 #include "lldb/Target/ThreadPlanStack.h"
 #include "lldb/Target/UnixSignals.h"
+#include "lldb/Target/VerboseTrapFrameRecognizer.h"
 #include "lldb/Utility/Event.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
@@ -497,6 +498,7 @@ Process::Process(lldb::TargetSP target_sp, ListenerSP listener_sp,
     value_sp->SetValueAs(platform_cache_line_size);
 
   RegisterAssertFrameRecognizer(this);
+  RegisterVerboseTrapFrameRecognizer(*this);
 }
 
 Process::~Process() {
diff --git a/lldb/source/Target/VerboseTrapFrameRecognizer.cpp b/lldb/source/Target/VerboseTrapFrameRecognizer.cpp
new file mode 100644
index 0000000000000..d1a9ec18f8fe4
--- /dev/null
+++ b/lldb/source/Target/VerboseTrapFrameRecognizer.cpp
@@ -0,0 +1,85 @@
+#include "lldb/Target/VerboseTrapFrameRecognizer.h"
+
+#include "lldb/Core/Module.h"
+#include "lldb/Symbol/Function.h"
+#include "lldb/Symbol/SymbolContext.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Target.h"
+
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
+
+using namespace llvm;
+using namespace lldb;
+using namespace lldb_private;
+
+VerboseTrapRecognizedStackFrame::VerboseTrapRecognizedStackFrame(
+    StackFrameSP most_relevant_frame_sp, std::string stop_desc)
+    : m_most_relevant_frame(most_relevant_frame_sp) {
+  m_stop_desc = std::move(stop_desc);
+}
+
+lldb::RecognizedStackFrameSP
+VerboseTrapFrameRecognizer::RecognizeFrame(lldb::StackFrameSP frame_sp) {
+  if (frame_sp->GetFrameIndex())
+    return {};
+
+  ThreadSP thread_sp = frame_sp->GetThread();
+  ProcessSP process_sp = thread_sp->GetProcess();
+
+  StackFrameSP most_relevant_frame_sp = thread_sp->GetStackFrameAtIndex(1);
+
+  if (!most_relevant_frame_sp) {
+    Log *log = GetLog(LLDBLog::Unwind);
+    LLDB_LOG(
+        log,
+        "Failed to find most relevant frame: Hit unwinding bound (1 frame)!");
+    return {};
+  }
+
+  SymbolContext sc = frame_sp->GetSymbolContext(eSymbolContextEverything);
+
+  if (!sc.block)
+    return {};
+
+  // The runtime error is set as the function name in the inlined function info
+  // of frame #0 by the compiler
+  const InlineFunctionInfo *inline_info = nullptr;
+  Block *inline_block = sc.block->GetContainingInlinedBlock();
+
+  if (!inline_block)
+    return {};
+
+  inline_info = sc.block->GetInlinedFunctionInfo();
+
+  if (!inline_info)
+    return {};
+
+  auto runtime_error = inline_info->GetName().GetString();
+
+  if (runtime_error.empty())
+    return {};
+
+  return lldb::RecognizedStackFrameSP(new VerboseTrapRecognizedStackFrame(
+      most_relevant_frame_sp, std::move(runtime_error)));
+}
+
+lldb::StackFrameSP VerboseTrapRecognizedStackFrame::GetMostRelevantFrame() {
+  return m_most_relevant_frame;
+}
+
+namespace lldb_private {
+
+void RegisterVerboseTrapFrameRecognizer(Process &process) {
+  RegularExpressionSP module_regex_sp = nullptr;
+  RegularExpressionSP symbol_regex_sp(
+      new RegularExpression("(__llvm_verbose_trap)"));
+
+  StackFrameRecognizerSP srf_recognizer_sp =
+      std::make_shared<VerboseTrapFrameRecognizer>();
+
+  process.GetTarget().GetFrameRecognizerManager().AddRecognizer(
+      srf_recognizer_sp, module_regex_sp, symbol_regex_sp, false);
+}
+
+} // namespace lldb_private
diff --git a/lldb/test/Shell/Recognizer/Inputs/verbose_trap.cpp b/lldb/test/Shell/Recognizer/Inputs/verbose_trap.cpp
new file mode 100644
index 0000000000000..6867c245e2fc9
--- /dev/null
+++ b/lldb/test/Shell/Recognizer/Inputs/verbose_trap.cpp
@@ -0,0 +1,8 @@
+struct Dummy {
+  void func() { __builtin_verbose_trap("Function is not implemented"); }
+};
+
+int main() {
+  Dummy{}.func();
+  return 0;
+}
diff --git a/lldb/test/Shell/Recognizer/verbose_trap.test b/lldb/test/Shell/Recognizer/verbose_trap.test
new file mode 100644
index 0000000000000..7790354784afd
--- /dev/null
+++ b/lldb/test/Shell/Recognizer/verbose_trap.test
@@ -0,0 +1,9 @@
+# RUN: %clang_host -g -O0 %S/Inputs/verbose_trap.cpp -o %t.out
+# RUN: %lldb -b -s %s %t.out | FileCheck %s
+run
+# CHECK: thread #{{.*}}stop reason = __llvm_verbose_trap: Function is not implemented
+frame info
+# CHECK: frame #{{.*}}`Dummy::func(this={{.*}}) at verbose_trap.cpp
+frame recognizer info 0
+# CHECK: frame 0 is recognized by Verbose Trap StackFrame Recognizer
+q

jimingham · 2024-02-02T01:10:04Z

The patch looks fine to me, but I'm a bit bugged by the fact that it is in Target. In my mind, this is part of the C LanguageRuntime, except we don't yet have a C Language Runtime...
But in any case, somewhere more specific than Target.

clayborg · 2024-02-02T02:43:42Z

This almost seems like a compiler runtime plug-in, but C language would be fine. I would rather see this in plug-in somewhere if possible. It might be nice to have these plug-ins register one or more frame regular expression values that point to the plug-in so we can add more things. It would be nice to also add support for C assert() calls that would unwind the stack to the offinding assert function above the 3 or 4 pthread functions that occur when an assertion is hit

jimingham · 2024-02-02T03:33:05Z

There IS an AssertFrameRecogizer that does just what you suggest, but it is in source/Target. These really should go somewhere else, however, Target is too general for anything but the base recognizer class. Jim

…

On Feb 1, 2024, at 6:43 PM, Greg Clayton ***@***.***> wrote: This almost seems like a compiler runtime plug-in, but C language would be fine. I would rather see this in plug-in somewhere if possible. It might be nice to have these plug-ins register one or more frame regular expression values that point to the plug-in so we can add more things. It would be nice to also add support for C assert() calls that would unwind the stack to the offinding assert function above the 3 or 4 pthread functions that occur when an assertion is hit — Reply to this email directly, view it on GitHub <#80368 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADUPVW3VRHN3DNFRZYNLY2TYRRHGZAVCNFSM6AAAAABCV3DNHSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRSGY4TAOJVGQ>. You are receiving this because you are on a team that was mentioned.

jimingham · 2024-02-02T03:35:39Z

That one's odd, it almost belongs in Platform, after all, there's no guarantee that "assert" is implemented by pthread functions at all. It just happens to be on Unix systems. Jim

…

On Feb 1, 2024, at 7:32 PM, Jim Ingham ***@***.***> wrote: There IS an AssertFrameRecogizer that does just what you suggest, but it is in source/Target. These really should go somewhere else, however, Target is too general for anything but the base recognizer class. Jim > On Feb 1, 2024, at 6:43 PM, Greg Clayton ***@***.***> wrote: > > > This almost seems like a compiler runtime plug-in, but C language would be fine. I would rather see this in plug-in somewhere if possible. It might be nice to have these plug-ins register one or more frame regular expression values that point to the plug-in so we can add more things. It would be nice to also add support for C assert() calls that would unwind the stack to the offinding assert function above the 3 or 4 pthread functions that occur when an assertion is hit > > — > Reply to this email directly, view it on GitHub <#80368 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADUPVW3VRHN3DNFRZYNLY2TYRRHGZAVCNFSM6AAAAABCV3DNHSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRSGY4TAOJVGQ>. > You are receiving this because you are on a team that was mentioned. >

Michael137 · 2024-02-02T09:45:12Z

The patch looks fine to me, but I'm a bit bugged by the fact that it is in Target. In my mind, this is part of the C LanguageRuntime, except we don't yet have a C Language Runtime... But in any case, somewhere more specific than Target.

Seems reasonable to me. To clarify, do you envision a CLanguageRuntime class that CPlusPlusLanguageRuntime and ObjCLanguageRuntime subclass? The CLanguageRuntime then simply registers the frame recognizers given the process passed to the runtime creation callback?

jimingham · 2024-02-02T17:14:59Z

I think that makes the most sense. Jim

…

On Feb 2, 2024, at 1:45 AM, Michael Buch ***@***.***> wrote: The patch looks fine to me, but I'm a bit bugged by the fact that it is in Target. In my mind, this is part of the C LanguageRuntime, except we don't yet have a C Language Runtime... But in any case, somewhere more specific than Target. Seems reasonable to me. Should I create a CLanguageRuntime plugin and move the AssertRecognizedStackFrame there then? I'll open a separate PR for this then — Reply to this email directly, view it on GitHub <#80368 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADUPVW2EKJES7NXFOC27SJDYRSYTPAVCNFSM6AAAAABCV3DNHSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRTGQ2DCMBZGA>. You are receiving this because you are on a team that was mentioned.

adrian-prantl · 2024-02-02T23:57:06Z

What will happen in an Objective-C++ frame then? Will there be two C runtimes because we have both a C++ and an ObjC runtime?

adrian-prantl · 2024-02-02T23:57:55Z

lldb/source/Target/VerboseTrapFrameRecognizer.cpp

+void RegisterVerboseTrapFrameRecognizer(Process &process) {
+  RegularExpressionSP module_regex_sp = nullptr;
+  RegularExpressionSP symbol_regex_sp(
+      new RegularExpression("(__llvm_verbose_trap)"));


Should this be (^__llvm_verbose_trap) so my__llvm_verbose_trap doesn't match?

adrian-prantl · 2024-02-02T23:59:10Z

lldb/test/Shell/Recognizer/verbose_trap.test

+# RUN: %clang_host -g -O0 %S/Inputs/verbose_trap.cpp -o %t.out
+# RUN: %lldb -b -s %s %t.out | FileCheck %s
+run
+# CHECK: thread #{{.*}}stop reason = __llvm_verbose_trap: Function is not implemented


Should we hide the __llvm_verbose_trap implementation detail and display something like "Runtime Error:" instead?

jimingham · 2024-02-03T00:04:54Z

I don't think the C++ or ObjC runtimes would vend their own C language runtimes. I don't think that would reflect a situation any system has (different C runtimes for C++ and ObjC). Rather these would just be three independent plugin instances. Jim

…

On Feb 2, 2024, at 3:57 PM, Adrian Prantl ***@***.***> wrote: What will happen in an Objective-C++ frame then? Will there be two C runtimes because we have both a C++ and an ObjC runtime? — Reply to this email directly, view it on GitHub <#80368 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADUPVW7O3FSZTQIN5KZFRADYRV4OBAVCNFSM6AAAAABCV3DNHSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRUHEYTQMJZHA>. You are receiving this because you are on a team that was mentioned.

JDevlieghere

LGTM with comments addressed.

JDevlieghere · 2024-02-07T16:30:06Z

lldb/include/lldb/Target/VerboseTrapFrameRecognizer.h

@@ -0,0 +1,39 @@
+#ifndef LLDB_TARGET_VERBOSETRAPFRAMERECOGNIZER_H


Missing license header.

JDevlieghere · 2024-02-07T16:31:39Z

lldb/source/Target/VerboseTrapFrameRecognizer.cpp

+  // Replaces "__llvm_verbose_trap: " with "Runtime Error: "
+  auto space_position = error_message.find(" ");
+  if (space_position == std::string::npos) {
+    Log *log = GetLog(LLDBLog::Unwind);


Maybe worth hoisting Log *log = GetLog(LLDBLog::Unwind);, it should be pretty cheap?

JDevlieghere · 2024-02-07T16:31:52Z

lldb/source/Target/VerboseTrapFrameRecognizer.cpp

+  auto space_position = error_message.find(" ");
+  if (space_position == std::string::npos) {
+    Log *log = GetLog(LLDBLog::Unwind);
+    LLDB_LOGF(log,


LLDB_LOG for consistency?

delcypher · 2024-02-07T16:42:38Z

lldb/source/Target/VerboseTrapFrameRecognizer.cpp

+void RegisterVerboseTrapFrameRecognizer(Process &process) {
+  RegularExpressionSP module_regex_sp = nullptr;
+  RegularExpressionSP symbol_regex_sp(
+      new RegularExpression("^__llvm_verbose_trap: "));


Rather than hard coding the name should we be getting it from a header file vended by Clang?

I mentioned this in the corresponding Clang review here.

This wouldn't need to be anything sophisticated. Just a header file under include/clang/CodeGen that is something like

#define CLANG_VERBOSE_TRAP_PREFIX "__llvm_verbose_trap"

and then from LLDB that would be something like:

new RegularExpression("^" CLANG_VERBOSE_TRAP_PREFIX ": ")

That way there is a single source of truth for the name used.

Yup that would be nice

I'll leave it to @ahatanak to do it in #79230 and then use it from LLDB

delcypher · 2024-02-07T16:44:44Z

lldb/test/Shell/Recognizer/verbose_trap.test

+# RUN: %clang_host -g -O0 %S/Inputs/verbose_trap.cpp -o %t.out
+# RUN: %lldb -b -s %s %t.out | FileCheck %s
+run
+# CHECK: thread #{{.*}}stop reason = Runtime Error: Function is not implemented


Runtime Error: is pretty generic. Is there any reason we aren't mentioning that a trap was hit?

How about Runtime trap: ...?

This patch adds a frame recognizer for Clang's `__builtin_verbose_trap`, which behaves like a `__builtin_trap`, but emits a failure-reason string into debug-info in order for debuggers to display it to a user. The frame recognizer triggers when we encounter a frame with a function name that begins with `__llvm_verbose_trap`, which is the magic prefix Clang emits into debug-info for verbose traps. Once such frame is encountered we display the frame function name as the `Stop Reason` and display that frame to the user. Example output: ``` (lldb) run warning: a.out was compiled with optimization - stepping may behave oddly; variables may not be available. Process 35942 launched: 'a.out' (arm64) Process 35942 stopped * thread llvm#1, queue = 'com.apple.main-thread', stop reason = __llvm_verbose_trap: Function is not implemented frame llvm#1: 0x0000000100003fa4 a.out`main [inlined] Dummy::func(this=<unavailable>) at verbose_trap.cpp:3:5 [opt] 1 struct Dummy { 2 void func() { -> 3 __builtin_verbose_trap("Function is not implemented"); 4 } 5 }; 6 7 int main() { (lldb) bt * thread llvm#1, queue = 'com.apple.main-thread', stop reason = __llvm_verbose_trap: Function is not implemented frame #0: 0x0000000100003fa4 a.out`main [inlined] __llvm_verbose_trap: Function is not implemented at verbose_trap.cpp:0 [opt] * frame llvm#1: 0x0000000100003fa4 a.out`main [inlined] Dummy::func(this=<unavailable>) at verbose_trap.cpp:3:5 [opt] frame llvm#2: 0x0000000100003fa4 a.out`main at verbose_trap.cpp:8:13 [opt] frame llvm#3: 0x0000000189d518b4 dyld`start + 1988 ```

delcypher

Looks pretty good. I had a few minor comments

delcypher · 2024-07-12T23:47:54Z

lldb/source/Target/VerboseTrapFrameRecognizer.cpp

+
+  static auto trap_regex =
+      llvm::Regex(llvm::formatv("^{0}\\$(.*)\\$(.*)$", ClangTrapPrefix).str());
+  SmallVector<llvm::StringRef, 2> matches;


If we expect 3 matches shouldn't the stack allocation size be 3 instead of 2?

delcypher · 2024-07-12T23:51:41Z

lldb/source/Target/VerboseTrapFrameRecognizer.cpp

+  ThreadSP thread_sp = frame_sp->GetThread();
+  ProcessSP process_sp = thread_sp->GetProcess();
+
+  StackFrameSP most_relevant_frame_sp = thread_sp->GetStackFrameAtIndex(1);


Is there anyway for there to not be a stack frame at index 1? If that's possible could we bail out early so we don't crash/have bad behavior?

While I don't see it documented explicitly, GetStackFrameAtIndex handles cases where we asked for a frame index that's out-of-bounds. And according to the docs GetStackFrameCount can be expensive to check, so we probably don't want to do that here

Michael137 · 2024-07-13T09:20:25Z

@jimingham @JDevlieghere @clayborg just to confirm, are you still ok with me landing this and address the question of a common CLanguageRuntime plugin as a follow-up?

clayborg · 2024-07-15T22:02:14Z

I am fine with landing as is. Maybe we should add some TODO: comments in the 3 frame recognizers with comments saying what we really should do to put these kinds of frame plug-ins in the right places.

jimingham · 2024-07-15T22:24:19Z

Sure. Jim

…

On Jul 13, 2024, at 2:20 AM, Michael Buch ***@***.***> wrote: @jimingham <https://github.com/jimingham> @JDevlieghere <https://github.com/JDevlieghere> @clayborg <https://github.com/clayborg> just to confirm, are you still ok with me landing this and address the question of a common CLanguageRuntime plugin as a follow-up? — Reply to this email directly, view it on GitHub <#80368 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADUPVW55RDOYPCYBKTTE4ODZMDWPHAVCNFSM6AAAAABCV3DNHSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMRWHAZTINBXGU>. You are receiving this because you were mentioned.

Michael137 · 2024-07-16T03:25:01Z

I am fine with landing as is. Maybe we should add some TODO: comments in the 3 frame recognizers with comments saying what we really should do to put these kinds of frame plug-ins in the right places.

Makes sense, added one to where we register all the recognizers

Summary: This patch adds a frame recognizer for Clang's `__builtin_verbose_trap`, which behaves like a `__builtin_trap`, but emits a failure-reason string into debug-info in order for debuggers to display it to a user. The frame recognizer triggers when we encounter a frame with a function name that begins with `__clang_trap_msg`, which is the magic prefix Clang emits into debug-info for verbose traps. Once such frame is encountered we display the frame function name as the `Stop Reason` and display that frame to the user. Example output: ``` (lldb) run warning: a.out was compiled with optimization - stepping may behave oddly; variables may not be available. Process 35942 launched: 'a.out' (arm64) Process 35942 stopped * thread llvm#1, queue = 'com.apple.main-thread', stop reason = Misc.: Function is not implemented frame llvm#1: 0x0000000100003fa4 a.out`main [inlined] Dummy::func(this=<unavailable>) at verbose_trap.cpp:3:5 [opt] 1 struct Dummy { 2 void func() { -> 3 __builtin_verbose_trap("Misc.", "Function is not implemented"); 4 } 5 }; 6 7 int main() { (lldb) bt * thread llvm#1, queue = 'com.apple.main-thread', stop reason = Misc.: Function is not implemented frame #0: 0x0000000100003fa4 a.out`main [inlined] __clang_trap_msg$Misc.$Function is not implemented$ at verbose_trap.cpp:0 [opt] * frame llvm#1: 0x0000000100003fa4 a.out`main [inlined] Dummy::func(this=<unavailable>) at verbose_trap.cpp:3:5 [opt] frame llvm#2: 0x0000000100003fa4 a.out`main at verbose_trap.cpp:8:13 [opt] frame llvm#3: 0x0000000189d518b4 dyld`start + 1988 ``` Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D59822390

verbose_trap.test, added in llvm#80368, fails on some Windows bots. See https://lab.llvm.org/buildbot/#/builders/141/builds/808.

verbose_trap.test, added in #80368, fails on some Windows bots. See https://lab.llvm.org/buildbot/#/builders/141/builds/808.

verbose_trap.test, added in llvm#80368, fails on some Windows bots. See https://lab.llvm.org/buildbot/#/builders/141/builds/808.

This patch adds a frame recognizer for Clang's `__builtin_verbose_trap`, which behaves like a `__builtin_trap`, but emits a failure-reason string into debug-info in order for debuggers to display it to a user. The frame recognizer triggers when we encounter a frame with a function name that begins with `__clang_trap_msg`, which is the magic prefix Clang emits into debug-info for verbose traps. Once such frame is encountered we display the frame function name as the `Stop Reason` and display that frame to the user. Example output: ``` (lldb) run warning: a.out was compiled with optimization - stepping may behave oddly; variables may not be available. Process 35942 launched: 'a.out' (arm64) Process 35942 stopped * thread #1, queue = 'com.apple.main-thread', stop reason = Misc.: Function is not implemented frame #1: 0x0000000100003fa4 a.out`main [inlined] Dummy::func(this=<unavailable>) at verbose_trap.cpp:3:5 [opt] 1 struct Dummy { 2 void func() { -> 3 __builtin_verbose_trap("Misc.", "Function is not implemented"); 4 } 5 }; 6 7 int main() { (lldb) bt * thread #1, queue = 'com.apple.main-thread', stop reason = Misc.: Function is not implemented frame #0: 0x0000000100003fa4 a.out`main [inlined] __clang_trap_msg$Misc.$Function is not implemented$ at verbose_trap.cpp:0 [opt] * frame #1: 0x0000000100003fa4 a.out`main [inlined] Dummy::func(this=<unavailable>) at verbose_trap.cpp:3:5 [opt] frame #2: 0x0000000100003fa4 a.out`main at verbose_trap.cpp:8:13 [opt] frame #3: 0x0000000189d518b4 dyld`start + 1988 ```

Summary: verbose_trap.test, added in #80368, fails on some Windows bots. See https://lab.llvm.org/buildbot/#/builders/141/builds/808. Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60250944

Michael137 requested review from delcypher, ldionne, adrian-prantl and ahatanak February 2, 2024 00:55

Michael137 requested a review from JDevlieghere as a code owner February 2, 2024 00:55

llvmbot added the lldb label Feb 2, 2024

adrian-prantl reviewed Feb 2, 2024

View reviewed changes

JDevlieghere approved these changes Feb 7, 2024

View reviewed changes

delcypher reviewed Feb 7, 2024

View reviewed changes

Michael137 force-pushed the lldb/verbose-trap-frame-recognizer branch from 1982877 to 587ea97 Compare March 12, 2024 11:12

ahatanaka mentioned this pull request Apr 22, 2024

Add support for builtin_verbose_trap #79230

Merged

Michael137 added 5 commits July 12, 2024 11:22

fixup! simplify regex, replace error prefix with human-readable message

02d69d9

fixup! remove redundant header

c09c6e6

fixup! handle case where function name is in unexpected format

5bb5195

fixup! adjust to new __builtin_verbose_trap message format

4dd2c33

Michael137 force-pushed the lldb/verbose-trap-frame-recognizer branch from 587ea97 to 4dd2c33 Compare July 12, 2024 23:13

delcypher requested changes Jul 12, 2024

View reviewed changes

fixup! adjust SmallVector allocation size; remove redundant debug print

b1a955e

fixup! add FIXME

a919134

Michael137 merged commit 8a27ef6 into llvm:main Jul 16, 2024
4 of 5 checks passed

Michael137 deleted the lldb/verbose-trap-frame-recognizer branch July 16, 2024 03:28

luporl added a commit to luporl/llvm-project that referenced this pull request Jul 17, 2024

[lldb] Disable verbose_trap.test on Windows

f57dd94

verbose_trap.test, added in llvm#80368, fails on some Windows bots. See https://lab.llvm.org/buildbot/#/builders/141/builds/808.

luporl mentioned this pull request Jul 17, 2024

[lldb] Disable verbose_trap.test on Windows #99323

Merged

luporl added a commit that referenced this pull request Jul 17, 2024

[lldb] Disable verbose_trap.test on Windows (#99323)

ec9d62f

verbose_trap.test, added in #80368, fails on some Windows bots. See https://lab.llvm.org/buildbot/#/builders/141/builds/808.

sgundapa pushed a commit to sgundapa/upstream_effort that referenced this pull request Jul 23, 2024

[lldb] Disable verbose_trap.test on Windows (llvm#99323)

929337a

verbose_trap.test, added in llvm#80368, fails on some Windows bots. See https://lab.llvm.org/buildbot/#/builders/141/builds/808.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[lldb] Add frame recognizer for __builtin_verbose_trap #80368

[lldb] Add frame recognizer for __builtin_verbose_trap #80368

Michael137 commented Feb 2, 2024

llvmbot commented Feb 2, 2024

jimingham commented Feb 2, 2024 •

edited

Loading

clayborg commented Feb 2, 2024

jimingham commented Feb 2, 2024 via email

jimingham commented Feb 2, 2024 via email

Michael137 commented Feb 2, 2024 •

edited

Loading

jimingham commented Feb 2, 2024 via email

adrian-prantl commented Feb 2, 2024

adrian-prantl Feb 2, 2024

adrian-prantl Feb 2, 2024

jimingham commented Feb 3, 2024 via email

JDevlieghere left a comment

JDevlieghere Feb 7, 2024

Michael137 Mar 12, 2024

JDevlieghere Feb 7, 2024

JDevlieghere Feb 7, 2024

delcypher Feb 7, 2024

Michael137 Mar 12, 2024

delcypher Feb 7, 2024

Michael137 Mar 12, 2024

delcypher left a comment

delcypher Jul 12, 2024

Michael137 Jul 12, 2024

delcypher Jul 12, 2024

Michael137 Jul 13, 2024

Michael137 commented Jul 13, 2024

clayborg commented Jul 15, 2024

jimingham commented Jul 15, 2024 via email

Michael137 commented Jul 16, 2024

		@@ -0,0 +1,39 @@
		#ifndef LLDB_TARGET_VERBOSETRAPFRAMERECOGNIZER_H

[lldb] Add frame recognizer for __builtin_verbose_trap #80368

[lldb] Add frame recognizer for __builtin_verbose_trap #80368

Conversation

Michael137 commented Feb 2, 2024

llvmbot commented Feb 2, 2024

jimingham commented Feb 2, 2024 • edited Loading

clayborg commented Feb 2, 2024

jimingham commented Feb 2, 2024 via email

jimingham commented Feb 2, 2024 via email

Michael137 commented Feb 2, 2024 • edited Loading

jimingham commented Feb 2, 2024 via email

adrian-prantl commented Feb 2, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jimingham commented Feb 3, 2024 via email

JDevlieghere left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

delcypher left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Michael137 commented Jul 13, 2024

clayborg commented Jul 15, 2024

jimingham commented Jul 15, 2024 via email

Michael137 commented Jul 16, 2024

jimingham commented Feb 2, 2024 •

edited

Loading

Michael137 commented Feb 2, 2024 •

edited

Loading