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 <jalalo...@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 <jalalo...@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 <jalalo...@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) _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits