[Lldb-commits] [lldb] [LLDB][Minidump] Add breakpoint stop reasons to the minidump. (PR #108448)

Jacob Lalonde via lldb-commits lldb-commits at lists.llvm.org
Thu Sep 12 14:28:44 PDT 2024


https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/108448

>From adcc5e8065d2a1b7edf877bd74de9cebd2f84cc9 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Thu, 12 Sep 2024 13:10:58 -0700
Subject: [PATCH 1/3] Create set for the stop reasons we collect, and add
 breakpoint to it.

---
 .../ObjectFile/Minidump/MinidumpFileBuilder.cpp     | 13 +++----------
 .../ObjectFile/Minidump/MinidumpFileBuilder.h       |  4 +++-
 2 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index edc568a6b47e00..38e71e0e485d55 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -75,8 +75,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 (m_thread_stop_reasons.count(stop_reason) > 0)
         m_expected_directories++;
     }
   }
@@ -686,16 +685,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 && m_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..569ba7fb07d871 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -168,7 +168,9 @@ 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; 
+  const std::unordered_set<lldb::StopReason> m_thread_stop_reasons = {lldb::StopReason::eStopReasonException, lldb::StopReason::eStopReasonSignal,
+  lldb::StopReason::eStopReasonBreakpoint };
 };
 
 #endif // LLDB_SOURCE_PLUGINS_OBJECTFILE_MINIDUMP_MINIDUMPFILEBUILDER_H

>From 030b74c9e763c86d217da39487478e7cf2dff074 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Thu, 12 Sep 2024 13:30:04 -0700
Subject: [PATCH 2/3] Make const set with the stop reasons

---
 .../ObjectFile/Minidump/MinidumpFileBuilder.cpp       | 11 +++++++++--
 .../Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h |  4 +---
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 38e71e0e485d55..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,7 +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 (m_thread_stop_reasons.count(stop_reason) > 0)
+      if (thread_stop_reasons.count(stop_reason) > 0)
         m_expected_directories++;
     }
   }
@@ -685,7 +692,7 @@ 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 && m_thread_stop_reasons.count(stop_info_sp->GetStopReason()) > 0) {
+    if (stop_info_sp && thread_stop_reasons.count(stop_info_sp->GetStopReason()) > 0) {
         add_exception = true;
     }
 
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index 569ba7fb07d871..488eec7b9282bc 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -169,8 +169,6 @@ class MinidumpFileBuilder {
   std::unordered_set<lldb::addr_t> m_saved_stack_ranges;
   lldb::FileUP m_core_file;
   lldb_private::SaveCoreOptions m_save_core_options; 
-  const std::unordered_set<lldb::StopReason> m_thread_stop_reasons = {lldb::StopReason::eStopReasonException, lldb::StopReason::eStopReasonSignal,
-  lldb::StopReason::eStopReasonBreakpoint };
+  static const std::unordered_set<lldb::StopReason> thread_stop_reasons;
 };
-
 #endif // LLDB_SOURCE_PLUGINS_OBJECTFILE_MINIDUMP_MINIDUMPFILEBUILDER_H

>From 099f017208ddadc07b3743ea7b04612f0f79617f Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Thu, 12 Sep 2024 14:28:31 -0700
Subject: [PATCH 3/3] Remove the static set and add test

---
 .../Minidump/MinidumpFileBuilder.cpp          | 11 +-----
 .../ObjectFile/Minidump/MinidumpFileBuilder.h |  9 ++++-
 .../TestProcessSaveCoreMinidump.py            | 39 ++++++++++++++++++-
 3 files changed, 48 insertions(+), 11 deletions(-)

diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 0b69f1b8205610..38e71e0e485d55 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -54,13 +54,6 @@ 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;
@@ -82,7 +75,7 @@ Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories() {
     StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
     if (stop_info_sp) {
       const StopReason &stop_reason = stop_info_sp->GetStopReason();
-      if (thread_stop_reasons.count(stop_reason) > 0)
+      if (m_thread_stop_reasons.count(stop_reason) > 0)
         m_expected_directories++;
     }
   }
@@ -692,7 +685,7 @@ 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 && thread_stop_reasons.count(stop_info_sp->GetStopReason()) > 0) {
+    if (stop_info_sp && m_thread_stop_reasons.count(stop_info_sp->GetStopReason()) > 0) {
         add_exception = true;
     }
 
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index 488eec7b9282bc..851d62d16920c1 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -169,6 +169,13 @@ class MinidumpFileBuilder {
   std::unordered_set<lldb::addr_t> m_saved_stack_ranges;
   lldb::FileUP m_core_file;
   lldb_private::SaveCoreOptions m_save_core_options; 
-  static const std::unordered_set<lldb::StopReason> thread_stop_reasons;
+  // Non const so we don't introduce an issue with the default copy assignment 
+  // operator, in the future we also want to be able to define these in the 
+  // save core options.
+  std::unordered_set<lldb::StopReason> m_thread_stop_reasons {
+    lldb::StopReason::eStopReasonException,
+    lldb::StopReason::eStopReasonSignal,
+    lldb::StopReason::eStopReasonBreakpoint
+  };
 };
 #endif // LLDB_SOURCE_PLUGINS_OBJECTFILE_MINIDUMP_MINIDUMPFILEBUILDER_H
diff --git a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
index 2cbe20ee10b1af..50a23eeb6496e2 100644
--- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -8,7 +8,6 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
-
 class ProcessSaveCoreMinidumpTestCase(TestBase):
     def verify_core_file(
         self,
@@ -493,3 +492,41 @@ def test_save_minidump_custom_save_style_duplicated_regions(self):
 
         finally:
             self.assertTrue(self.dbg.DeleteTarget(target))
+
+    @skipUnlessPlatform(["linux"])
+    def test_save_minidump_breakpoint(self):
+        """Test that verifies a custom and unspecified save style fails for
+        containing no data to save"""
+
+        self.build()
+        exe = self.getBuildArtifact("a.out")
+        custom_file = self.getBuildArtifact("core.custom.dmp")
+        try:
+            target = self.dbg.CreateTarget(exe)
+            process = target.LaunchSimple(
+                None, None, self.get_process_working_directory()
+            )
+            breakpoint = target.BreakpointCreateByName("main")
+            self.assertState(process.GetState(), lldb.eStateStopped)
+
+            options = lldb.SBSaveCoreOptions()
+            options.SetOutputFile(lldb.SBFileSpec(custom_file))
+            options.SetPluginName("minidump")
+            options.SetStyle(lldb.eSaveCoreStackOnly)
+
+            error = process.SaveCore(options)
+            self.assertTrue(error.Success())
+            foundSigInt = False
+            for thread_idx in range(process.GetNumThreads()):
+                thread = process.GetThreadAtIndex(thread_idx)
+                stop = thread.stop_reason
+                if stop == 1:
+                    foundSigInt = True
+                    break
+            
+            self.assertTrue(foundSigInt, "Breakpoint not included in minidump.")
+
+        finally:
+            self.assertTrue(self.dbg.DeleteTarget(target))
+            if os.path.isfile(custom_file):
+                os.unlink(custom_file)



More information about the lldb-commits mailing list