[LLDB][Minidump] Add breakpoint stop reasons to the minidump.

[Jlalond]

Recently my coworker @jeffreytan81 pointed out that Minidumps don't show breakpoints when collected. This was prior blocked because Minidumps could only contain 1 exception, now that we support N signals/sections we can save all the threads stopped on breakpoints.

[llvmbot]

@llvm/pr-subscribers-llvm-binary-utilities

@llvm/pr-subscribers-lldb

Author: Jacob Lalonde (Jlalond)

Changes

Recently my coworker @jeffreytan81 pointed out that Minidumps don't show breakpoints when collected. This was prior blocked because Minidumps could only contain 1 exception, now that we support N signals/sections we can save all the threads stopped on breakpoints.

---

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

2 Files Affected:

• (modified) lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp (+10-10)
• (modified) lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h (+2-2)

diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index edc568a6b47e00..0b69f1b8205610 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -54,6 +54,13 @@ using namespace lldb;
 using namespace lldb_private;
 using namespace llvm::minidump;
 
+// Set of all the stop reasons minidumps will collect.
+const std::unordered_set<lldb::StopReason> MinidumpFileBuilder::thread_stop_reasons {
+  lldb::StopReason::eStopReasonException,
+  lldb::StopReason::eStopReasonSignal,
+  lldb::StopReason::eStopReasonBreakpoint,
+};
+
 Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories() {
   // First set the offset on the file, and on the bytes saved
   m_saved_data_size = HEADER_SIZE;
@@ -75,8 +82,7 @@ Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories() {
     StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
     if (stop_info_sp) {
       const StopReason &stop_reason = stop_info_sp->GetStopReason();
-      if (stop_reason == StopReason::eStopReasonException ||
-          stop_reason == StopReason::eStopReasonSignal)
+      if (thread_stop_reasons.count(stop_reason) > 0)
         m_expected_directories++;
     }
   }
@@ -686,16 +692,10 @@ Status MinidumpFileBuilder::AddExceptions() {
   for (const ThreadSP &thread_sp : thread_list) {
     StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
     bool add_exception = false;
-    if (stop_info_sp) {
-      switch (stop_info_sp->GetStopReason()) {
-      case eStopReasonSignal:
-      case eStopReasonException:
+    if (stop_info_sp && thread_stop_reasons.count(stop_info_sp->GetStopReason()) > 0) {
         add_exception = true;
-        break;
-      default:
-        break;
-      }
     }
+
     if (add_exception) {
       constexpr size_t minidump_exception_size =
           sizeof(llvm::minidump::ExceptionStream);
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index 71001e26c00e91..488eec7b9282bc 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -168,7 +168,7 @@ class MinidumpFileBuilder {
       m_tid_to_reg_ctx;
   std::unordered_set<lldb::addr_t> m_saved_stack_ranges;
   lldb::FileUP m_core_file;
-  lldb_private::SaveCoreOptions m_save_core_options;
+  lldb_private::SaveCoreOptions m_save_core_options; 
+  static const std::unordered_set<lldb::StopReason> thread_stop_reasons;
 };
-
 #endif // LLDB_SOURCE_PLUGINS_OBJECTFILE_MINIDUMP_MINIDUMPFILEBUILDER_H

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[jeffreytan81]

Can you add a test to check this (loaded minidump's current thread is the one hitting breakpoint in original live session, and stop reason is breakpoint)?

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[mstorsjo]

This causes lots of build time warnings when building with GCC, like this:

[271/4090] Building CXX object lib/Obj...akeFiles/LLVMObject.dir/Minidump.cpp.o
In file included from ../include/llvm/Object/Minidump.h:16,
                 from ../lib/Object/Minidump.cpp:9:
../include/llvm/BinaryFormat/Minidump.h:250:37: warning: multi-character character constant [-Wmultichar]
  250 |   static const uint32_t LLDB_FLAG = 'LLDB';
      |                                     ^~~~~~

Do we want to try to find a way around this, or do we want to silence this warning globally?

[labath]

Fixing definitely. Jacob, please change this to a hex constant and put the string in the comment. The name of the constants also seems out of place here. All of the other constants (even those that wrap existing windows ALL_CAPS) constants are using CamelCase). I'd suggest enum class ExceptionFlag : uint32 { LLDB = 0xdead };

[Jlalond]

c742a5d

Jinsong already authored a fix this to this build warning this morning