https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/108448
>From 7bdbce7ef2f60e819dea296ff8da832c1acbaef6 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Thu, 12 Sep 2024 13:10:58 -0700 Subject: [PATCH 01/10] 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 f6c16b6e3d96ae..8f43c06ec14b79 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 a4240f871c8a2f..f555a0fc9fbff1 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h @@ -173,7 +173,9 @@ class MinidumpFileBuilder { std::unordered_map<lldb::tid_t, llvm::minidump::LocationDescriptor> m_tid_to_reg_ctx; 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 6306b5d8d9c4eb976e37c1953540d649209b829a Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Thu, 12 Sep 2024 13:30:04 -0700 Subject: [PATCH 02/10] 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 8f43c06ec14b79..58a59c4ee6ddef 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 f555a0fc9fbff1..a95265191e1ece 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h @@ -174,8 +174,6 @@ class MinidumpFileBuilder { m_tid_to_reg_ctx; 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 82bdc501bb0be62ea03a90e55f50e53e1dd01984 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Thu, 12 Sep 2024 14:28:31 -0700 Subject: [PATCH 03/10] Remove the static set and add test --- .../Minidump/MinidumpFileBuilder.cpp | 11 +----- .../ObjectFile/Minidump/MinidumpFileBuilder.h | 9 ++++- .../TestProcessSaveCoreMinidump.py | 38 ++++++++++++++++++- 3 files changed, 47 insertions(+), 11 deletions(-) diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp index 58a59c4ee6ddef..8f43c06ec14b79 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 a95265191e1ece..b6fe5b681aefd8 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h @@ -174,6 +174,13 @@ class MinidumpFileBuilder { m_tid_to_reg_ctx; 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 03cc415924e0bb..21b23988f0c237 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, @@ -495,16 +494,27 @@ def test_save_minidump_custom_save_style_duplicated_regions(self): self.assertTrue(self.dbg.DeleteTarget(target)) @skipUnlessPlatform(["linux"]) +<<<<<<< HEAD def minidump_deleted_on_save_failure(self): """Test that verifies the minidump file is deleted after an error""" self.build() exe = self.getBuildArtifact("a.out") +======= + 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") +>>>>>>> 099f017208dd (Remove the static set and add test) try: target = self.dbg.CreateTarget(exe) process = target.LaunchSimple( None, None, self.get_process_working_directory() ) +<<<<<<< HEAD self.assertState(process.GetState(), lldb.eStateStopped) custom_file = self.getBuildArtifact("core.should.be.deleted.custom.dmp") @@ -565,3 +575,29 @@ def minidump_deterministic_difference(self): finally: self.assertTrue(self.dbg.DeleteTarget(target)) +======= + 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) +>>>>>>> 099f017208dd (Remove the static set and add test) >From 197e87faa0f7e19fdfd13bc573c57ba6072a2d2c Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Fri, 27 Sep 2024 11:23:35 -0700 Subject: [PATCH 04/10] Add description to stream and add yaml test --- .../Minidump/MinidumpFileBuilder.cpp | 70 +++++++++---------- .../ObjectFile/Minidump/MinidumpFileBuilder.h | 8 --- .../Process/minidump/ProcessMinidump.cpp | 5 +- .../minidump-new/TestMiniDumpNew.py | 21 ++++++ .../linux-x86_64-exceptiondescription.yaml | 37 ++++++++++ llvm/include/llvm/BinaryFormat/Minidump.h | 2 + 6 files changed, 98 insertions(+), 45 deletions(-) create mode 100644 lldb/test/API/functionalities/postmortem/minidump-new/linux-x86_64-exceptiondescription.yaml diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp index 8f43c06ec14b79..c78acbf17e7a7d 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp @@ -75,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 (m_thread_stop_reasons.count(stop_reason) > 0) + if (stop_reason) m_expected_directories++; } } @@ -684,44 +684,42 @@ Status MinidumpFileBuilder::AddExceptions() { Status error; 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) { - add_exception = true; - } + if (!stop_info_sp) + continue; - if (add_exception) { - constexpr size_t minidump_exception_size = - sizeof(llvm::minidump::ExceptionStream); - error = AddDirectory(StreamType::Exception, minidump_exception_size); - if (error.Fail()) - return error; + constexpr size_t minidump_exception_size = + sizeof(llvm::minidump::ExceptionStream); + error = AddDirectory(StreamType::Exception, minidump_exception_size); + if (error.Fail()) + return error; - StopInfoSP stop_info_sp = thread_sp->GetStopInfo(); - RegisterContextSP reg_ctx_sp(thread_sp->GetRegisterContext()); - Exception exp_record = {}; - exp_record.ExceptionCode = - static_cast<llvm::support::ulittle32_t>(stop_info_sp->GetValue()); - exp_record.ExceptionFlags = static_cast<llvm::support::ulittle32_t>(0); - exp_record.ExceptionRecord = static_cast<llvm::support::ulittle64_t>(0); - exp_record.ExceptionAddress = reg_ctx_sp->GetPC(); - exp_record.NumberParameters = static_cast<llvm::support::ulittle32_t>(0); - exp_record.UnusedAlignment = static_cast<llvm::support::ulittle32_t>(0); - // exp_record.ExceptionInformation; - - ExceptionStream exp_stream; - exp_stream.ThreadId = - static_cast<llvm::support::ulittle32_t>(thread_sp->GetID()); - exp_stream.UnusedAlignment = static_cast<llvm::support::ulittle32_t>(0); - exp_stream.ExceptionRecord = exp_record; - auto Iter = m_tid_to_reg_ctx.find(thread_sp->GetID()); - if (Iter != m_tid_to_reg_ctx.end()) { - exp_stream.ThreadContext = Iter->second; - } else { - exp_stream.ThreadContext.DataSize = 0; - exp_stream.ThreadContext.RVA = 0; - } - m_data.AppendData(&exp_stream, minidump_exception_size); + RegisterContextSP reg_ctx_sp(thread_sp->GetRegisterContext()); + Exception exp_record = {}; + exp_record.ExceptionCode = static_cast<llvm::support::ulittle32_t>(stop_info_sp->GetValue()); + exp_record.ExceptionCode = + static_cast<llvm::support::ulittle32_t>(stop_info_sp->GetValue()); + exp_record.ExceptionFlags = static_cast<llvm::support::ulittle32_t>(Exception::LLDB_FLAG); + exp_record.ExceptionRecord = static_cast<llvm::support::ulittle64_t>(0); + exp_record.ExceptionAddress = reg_ctx_sp->GetPC(); + exp_record.NumberParameters = static_cast<llvm::support::ulittle32_t>(1); + std::string description = stop_info_sp->GetDescription(); + // We have 120 bytes to work with and it's unlikely description will + // overflow, but we gotta check. + memcpy(&exp_record.ExceptionInformation, description.c_str(), std::max(description.size(), Exception::MaxParameterBytes)); + exp_record.UnusedAlignment = static_cast<llvm::support::ulittle32_t>(0); + ExceptionStream exp_stream; + exp_stream.ThreadId = + static_cast<llvm::support::ulittle32_t>(thread_sp->GetID()); + exp_stream.UnusedAlignment = static_cast<llvm::support::ulittle32_t>(0); + exp_stream.ExceptionRecord = exp_record; + auto Iter = m_tid_to_reg_ctx.find(thread_sp->GetID()); + if (Iter != m_tid_to_reg_ctx.end()) { + exp_stream.ThreadContext = Iter->second; + } else { + exp_stream.ThreadContext.DataSize = 0; + exp_stream.ThreadContext.RVA = 0; } + m_data.AppendData(&exp_stream, minidump_exception_size); } return error; diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h index b6fe5b681aefd8..e338f947e09573 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h @@ -174,13 +174,5 @@ class MinidumpFileBuilder { m_tid_to_reg_ctx; lldb::FileUP m_core_file; lldb_private::SaveCoreOptions m_save_core_options; - // 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/source/Plugins/Process/minidump/ProcessMinidump.cpp b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp index 32ffba763c08e3..25eea4a71be87e 100644 --- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp +++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp @@ -273,8 +273,11 @@ void ProcessMinidump::RefreshStateAfterStop() { // No stop. return; } + const char * description = nullptr; + if ((exception_stream.ExceptionRecord.ExceptionFlags & llvm::minidump::Exception::LLDB_FLAG) == llvm::minidump::Exception::LLDB_FLAG) + description = reinterpret_cast<const char *>(exception_stream.ExceptionRecord.ExceptionInformation); - stop_info = StopInfo::CreateStopReasonWithSignal(*stop_thread, signo); + stop_info = StopInfo::CreateStopReasonWithSignal(*stop_thread, signo, description); } else if (arch.GetTriple().getVendor() == llvm::Triple::Apple) { stop_info = StopInfoMachException::CreateStopReasonWithMachException( *stop_thread, exception_stream.ExceptionRecord.ExceptionCode, 2, diff --git a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py index 5a0b6e790a424c..6f6e19bcbb15d9 100644 --- a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py +++ b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py @@ -524,3 +524,24 @@ def test_multiple_exceptions_or_signals(self): self.assertStopReason(thread.GetStopReason(), lldb.eStopReasonSignal) stop_description = thread.GetStopDescription(256) self.assertIn("SIGSEGV", stop_description) + + def test_breakpoint_on_minidump(self): + """ + Test that LLDB breakpoints are recorded in Minidumps + """ + yaml = "linux-x86_64-exceptiondescription.yaml" + core = self.getBuildArtifact("breakpoint.core.dmp") + self.yaml2obj(yaml, core) + try: + # Create a target with the object file we just created from YAML + target = self.dbg.CreateTarget(None) + self.assertTrue(target, VALID_TARGET) + process = target.LoadCore(core) + self.assertTrue(process, VALID_PROCESS) + thread = process.GetThreadAtIndex(0) + stop_reason = thread.GetStopDescription(256) + self.assertIn("breakpoint 1.1", stop_reason) + finally: + if os.path.isfile(core): + os.unlink(core) + \ No newline at end of file diff --git a/lldb/test/API/functionalities/postmortem/minidump-new/linux-x86_64-exceptiondescription.yaml b/lldb/test/API/functionalities/postmortem/minidump-new/linux-x86_64-exceptiondescription.yaml new file mode 100644 index 00000000000000..ac121c630a0a48 --- /dev/null +++ b/lldb/test/API/functionalities/postmortem/minidump-new/linux-x86_64-exceptiondescription.yaml @@ -0,0 +1,37 @@ +--- !minidump +Streams: + - Type: SystemInfo + Processor Arch: AMD64 + Processor Level: 6 + Processor Revision: 15876 + Number of Processors: 40 + Platform ID: Linux + CSD Version: 'Linux 3.13.0-91-generic' + CPU: + Vendor ID: GenuineIntel + Version Info: 0x00000000 + Feature Info: 0x00000000 + - Type: ThreadList + Threads: + - Thread Id: 0x31F222 + Context: 00000000000000 + Stack: + Start of Memory Range: 0x7FFFFFFFD660 + Content: '' + - Type: Exception + Thread ID: 0x31F222 + Exception Record: + Exception Code: 0x2 + Exception Flags: 0x80000000 + Exception Address: 0x555555556671 + Number of Parameters: 1 + Parameter 0: 0x696F706B61657262 + Parameter 1: 0x312E3120746E + Parameter 2: 0x1 + Parameter 3: 0x8000000000000000 + Parameter 4: 0x200000002 + Parameter 5: 0x8000000000000002 + Parameter 7: 0x555555556671 + Parameter 8: 0x1 + Thread Context: '' +... diff --git a/llvm/include/llvm/BinaryFormat/Minidump.h b/llvm/include/llvm/BinaryFormat/Minidump.h index 8054e81322a92a..c9df330cbe89bd 100644 --- a/llvm/include/llvm/BinaryFormat/Minidump.h +++ b/llvm/include/llvm/BinaryFormat/Minidump.h @@ -246,6 +246,8 @@ static_assert(sizeof(Thread) == 48); struct Exception { static constexpr size_t MaxParameters = 15; + static constexpr size_t MaxParameterBytes = MaxParameters * sizeof(uint64_t); + static const uint32_t LLDB_FLAG = 0x80000000; support::ulittle32_t ExceptionCode; support::ulittle32_t ExceptionFlags; >From 504c7192c0b07282998749a0cdb1868d7eb9889a Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Fri, 27 Sep 2024 11:25:19 -0700 Subject: [PATCH 05/10] Drop test in process save core minidump --- .../TestProcessSaveCoreMinidump.py | 38 +------------------ 1 file changed, 1 insertion(+), 37 deletions(-) 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 21b23988f0c237..03cc415924e0bb 100644 --- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py +++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py @@ -8,6 +8,7 @@ from lldbsuite.test.lldbtest import * from lldbsuite.test import lldbutil + class ProcessSaveCoreMinidumpTestCase(TestBase): def verify_core_file( self, @@ -494,27 +495,16 @@ def test_save_minidump_custom_save_style_duplicated_regions(self): self.assertTrue(self.dbg.DeleteTarget(target)) @skipUnlessPlatform(["linux"]) -<<<<<<< HEAD def minidump_deleted_on_save_failure(self): """Test that verifies the minidump file is deleted after an error""" self.build() exe = self.getBuildArtifact("a.out") -======= - 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") ->>>>>>> 099f017208dd (Remove the static set and add test) try: target = self.dbg.CreateTarget(exe) process = target.LaunchSimple( None, None, self.get_process_working_directory() ) -<<<<<<< HEAD self.assertState(process.GetState(), lldb.eStateStopped) custom_file = self.getBuildArtifact("core.should.be.deleted.custom.dmp") @@ -575,29 +565,3 @@ def minidump_deterministic_difference(self): finally: self.assertTrue(self.dbg.DeleteTarget(target)) -======= - 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) ->>>>>>> 099f017208dd (Remove the static set and add test) >From 71dcafbf6eec467444e4c1856aa9aa4d18f91d9a Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Mon, 30 Sep 2024 11:09:41 -0700 Subject: [PATCH 06/10] Use ASCII flag --- lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp | 1 - lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp | 2 +- llvm/include/llvm/BinaryFormat/Minidump.h | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp index c78acbf17e7a7d..ff1e0b89827746 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp @@ -695,7 +695,6 @@ Status MinidumpFileBuilder::AddExceptions() { RegisterContextSP reg_ctx_sp(thread_sp->GetRegisterContext()); Exception exp_record = {}; - exp_record.ExceptionCode = static_cast<llvm::support::ulittle32_t>(stop_info_sp->GetValue()); exp_record.ExceptionCode = static_cast<llvm::support::ulittle32_t>(stop_info_sp->GetValue()); exp_record.ExceptionFlags = static_cast<llvm::support::ulittle32_t>(Exception::LLDB_FLAG); diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp index 25eea4a71be87e..24d1db7b0103d6 100644 --- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp +++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp @@ -274,7 +274,7 @@ void ProcessMinidump::RefreshStateAfterStop() { return; } const char * description = nullptr; - if ((exception_stream.ExceptionRecord.ExceptionFlags & llvm::minidump::Exception::LLDB_FLAG) == llvm::minidump::Exception::LLDB_FLAG) + if (exception_stream.ExceptionRecord.ExceptionFlags == llvm::minidump::Exception::LLDB_FLAG) description = reinterpret_cast<const char *>(exception_stream.ExceptionRecord.ExceptionInformation); stop_info = StopInfo::CreateStopReasonWithSignal(*stop_thread, signo, description); diff --git a/llvm/include/llvm/BinaryFormat/Minidump.h b/llvm/include/llvm/BinaryFormat/Minidump.h index c9df330cbe89bd..addff42982352f 100644 --- a/llvm/include/llvm/BinaryFormat/Minidump.h +++ b/llvm/include/llvm/BinaryFormat/Minidump.h @@ -247,7 +247,7 @@ static_assert(sizeof(Thread) == 48); struct Exception { static constexpr size_t MaxParameters = 15; static constexpr size_t MaxParameterBytes = MaxParameters * sizeof(uint64_t); - static const uint32_t LLDB_FLAG = 0x80000000; + static const uint32_t LLDB_FLAG = 'LLDB'; support::ulittle32_t ExceptionCode; support::ulittle32_t ExceptionFlags; >From f30ad62f9aa8366b7fbcfa01d7492596be8078b9 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Mon, 30 Sep 2024 12:43:21 -0700 Subject: [PATCH 07/10] Update yaml to new flag --- .../minidump-new/linux-x86_64-exceptiondescription.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/test/API/functionalities/postmortem/minidump-new/linux-x86_64-exceptiondescription.yaml b/lldb/test/API/functionalities/postmortem/minidump-new/linux-x86_64-exceptiondescription.yaml index ac121c630a0a48..bf26e05cd775ae 100644 --- a/lldb/test/API/functionalities/postmortem/minidump-new/linux-x86_64-exceptiondescription.yaml +++ b/lldb/test/API/functionalities/postmortem/minidump-new/linux-x86_64-exceptiondescription.yaml @@ -22,7 +22,7 @@ Streams: Thread ID: 0x31F222 Exception Record: Exception Code: 0x2 - Exception Flags: 0x80000000 + Exception Flags: 0x4C4C4442 Exception Address: 0x555555556671 Number of Parameters: 1 Parameter 0: 0x696F706B61657262 >From 260f39db0b9f896af54189d9996e8da9e6cb5d2a Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Tue, 1 Oct 2024 16:37:33 -0700 Subject: [PATCH 08/10] Run python formatter --- .../functionalities/postmortem/minidump-new/TestMiniDumpNew.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py index 6f6e19bcbb15d9..8776d72ecbc027 100644 --- a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py +++ b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py @@ -544,4 +544,3 @@ def test_breakpoint_on_minidump(self): finally: if os.path.isfile(core): os.unlink(core) - \ No newline at end of file >From d942f206016ea02467e01d27d9d0566485964537 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Mon, 7 Oct 2024 12:40:24 -0700 Subject: [PATCH 09/10] Formatter --- .../ObjectFile/Minidump/MinidumpFileBuilder.cpp | 8 +++++--- .../ObjectFile/Minidump/MinidumpFileBuilder.h | 2 +- .../Plugins/Process/minidump/ProcessMinidump.cpp | 13 ++++++++----- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp index ff1e0b89827746..b1976ac8d4d66e 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp @@ -697,14 +697,16 @@ Status MinidumpFileBuilder::AddExceptions() { Exception exp_record = {}; exp_record.ExceptionCode = static_cast<llvm::support::ulittle32_t>(stop_info_sp->GetValue()); - exp_record.ExceptionFlags = static_cast<llvm::support::ulittle32_t>(Exception::LLDB_FLAG); + exp_record.ExceptionFlags = + static_cast<llvm::support::ulittle32_t>(Exception::LLDB_FLAG); exp_record.ExceptionRecord = static_cast<llvm::support::ulittle64_t>(0); exp_record.ExceptionAddress = reg_ctx_sp->GetPC(); exp_record.NumberParameters = static_cast<llvm::support::ulittle32_t>(1); std::string description = stop_info_sp->GetDescription(); - // We have 120 bytes to work with and it's unlikely description will + // We have 120 bytes to work with and it's unlikely description will // overflow, but we gotta check. - memcpy(&exp_record.ExceptionInformation, description.c_str(), std::max(description.size(), Exception::MaxParameterBytes)); + memcpy(&exp_record.ExceptionInformation, description.c_str(), + std::max(description.size(), Exception::MaxParameterBytes)); exp_record.UnusedAlignment = static_cast<llvm::support::ulittle32_t>(0); ExceptionStream exp_stream; exp_stream.ThreadId = diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h index e338f947e09573..58b284608bd535 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h @@ -173,6 +173,6 @@ class MinidumpFileBuilder { std::unordered_map<lldb::tid_t, llvm::minidump::LocationDescriptor> m_tid_to_reg_ctx; lldb::FileUP m_core_file; - lldb_private::SaveCoreOptions m_save_core_options; + lldb_private::SaveCoreOptions m_save_core_options; }; #endif // LLDB_SOURCE_PLUGINS_OBJECTFILE_MINIDUMP_MINIDUMPFILEBUILDER_H diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp index 24d1db7b0103d6..bab761f1d9ff55 100644 --- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp +++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp @@ -273,11 +273,14 @@ void ProcessMinidump::RefreshStateAfterStop() { // No stop. return; } - const char * description = nullptr; - if (exception_stream.ExceptionRecord.ExceptionFlags == llvm::minidump::Exception::LLDB_FLAG) - description = reinterpret_cast<const char *>(exception_stream.ExceptionRecord.ExceptionInformation); - - stop_info = StopInfo::CreateStopReasonWithSignal(*stop_thread, signo, description); + const char *description = nullptr; + if (exception_stream.ExceptionRecord.ExceptionFlags == + llvm::minidump::Exception::LLDB_FLAG) + description = reinterpret_cast<const char *>( + exception_stream.ExceptionRecord.ExceptionInformation); + + stop_info = StopInfo::CreateStopReasonWithSignal(*stop_thread, signo, + description); } else if (arch.GetTriple().getVendor() == llvm::Triple::Apple) { stop_info = StopInfoMachException::CreateStopReasonWithMachException( *stop_thread, exception_stream.ExceptionRecord.ExceptionCode, 2, >From 282eea4f0c2d13f5c63efcc654d7980e4343ff9a Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Tue, 8 Oct 2024 13:40:54 -0700 Subject: [PATCH 10/10] Add Greg recommended invalid check, and string ref with max value --- .../Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp | 5 +++-- lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp index b1976ac8d4d66e..6fffd826c577e0 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp @@ -75,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 (stop_reason) + if (stop_reason != lldb::eStopReasonInvalid) m_expected_directories++; } } @@ -684,7 +684,8 @@ Status MinidumpFileBuilder::AddExceptions() { Status error; for (const ThreadSP &thread_sp : thread_list) { StopInfoSP stop_info_sp = thread_sp->GetStopInfo(); - if (!stop_info_sp) + // If we don't have a stop info, or if it's invalid, skip. + if (!stop_info_sp || stop_info_sp->GetStopReason() == lldb::eStopReasonInvalid) continue; constexpr size_t minidump_exception_size = diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp index bab761f1d9ff55..bbf787053c23f1 100644 --- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp +++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp @@ -279,8 +279,9 @@ void ProcessMinidump::RefreshStateAfterStop() { description = reinterpret_cast<const char *>( exception_stream.ExceptionRecord.ExceptionInformation); + llvm::StringRef description_str(description, Exception::MaxParameterBytes); stop_info = StopInfo::CreateStopReasonWithSignal(*stop_thread, signo, - description); + description_str.str().c_str()); } else if (arch.GetTriple().getVendor() == llvm::Triple::Apple) { stop_info = StopInfoMachException::CreateStopReasonWithMachException( *stop_thread, exception_stream.ExceptionRecord.ExceptionCode, 2, _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits