llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-llvm-binary-utilities Author: Jacob Lalonde (Jlalond) <details> <summary>Changes</summary> Currently, LLDB assumes all minidumps will have unique sections. This is intuitive because almost all of the minidump sections are themselves lists. Exceptions including Signals are unique in that they are all individual sections with their own directory. This means LLDB fails to load minidumps with multiple exceptions due to them not being unique. This behavior is erroneous and this PR introduces support for an arbitrary number of exception streams. Additionally, stop info was calculated on for a single thread before, and now we properly support mapping exceptions to threads. This PR is starting in DRAFT because implementing testing is still required. --- Patch is 26.96 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/97470.diff 12 Files Affected: - (modified) lldb/source/Plugins/Process/minidump/MinidumpParser.cpp (+6-4) - (modified) lldb/source/Plugins/Process/minidump/MinidumpParser.h (+1-1) - (modified) lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp (+66-55) - (modified) lldb/source/Plugins/Process/minidump/ProcessMinidump.h (+1-1) - (modified) lldb/source/Plugins/Process/minidump/ThreadMinidump.cpp (+15-3) - (modified) lldb/source/Plugins/Process/minidump/ThreadMinidump.h (+3-1) - (modified) lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py (+14) - (added) lldb/test/API/functionalities/postmortem/minidump-new/multiple-sigsev.yaml (+39) - (modified) lldb/unittests/Process/minidump/MinidumpParserTest.cpp (+7-4) - (modified) llvm/include/llvm/Object/Minidump.h (+31-7) - (modified) llvm/lib/Object/Minidump.cpp (+36) - (modified) llvm/lib/ObjectYAML/MinidumpYAML.cpp (+2-2) ``````````diff diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp index be9fae938e227..c51fed8d91b07 100644 --- a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp +++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp @@ -417,14 +417,16 @@ std::vector<const minidump::Module *> MinidumpParser::GetFilteredModuleList() { return filtered_modules; } -const minidump::ExceptionStream *MinidumpParser::GetExceptionStream() { - auto ExpectedStream = GetMinidumpFile().getExceptionStream(); +const std::vector<minidump::ExceptionStream> +MinidumpParser::GetExceptionStreams() { + auto ExpectedStream = GetMinidumpFile().getExceptionStreams(); if (ExpectedStream) - return &*ExpectedStream; + return ExpectedStream.get(); LLDB_LOG_ERROR(GetLog(LLDBLog::Process), ExpectedStream.takeError(), "Failed to read minidump exception stream: {0}"); - return nullptr; + // return empty on failure. + return std::vector<minidump::ExceptionStream>(); } std::optional<minidump::Range> diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.h b/lldb/source/Plugins/Process/minidump/MinidumpParser.h index 050ba086f46f5..e552c7210e330 100644 --- a/lldb/source/Plugins/Process/minidump/MinidumpParser.h +++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.h @@ -82,7 +82,7 @@ class MinidumpParser { // have the same name, it keeps the copy with the lowest load address. std::vector<const minidump::Module *> GetFilteredModuleList(); - const llvm::minidump::ExceptionStream *GetExceptionStream(); + const std::vector<llvm::minidump::ExceptionStream> GetExceptionStreams(); std::optional<Range> FindMemoryRange(lldb::addr_t addr); diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp index 13599f4a1553f..9a7e481b92796 100644 --- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp +++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp @@ -157,8 +157,7 @@ ProcessMinidump::ProcessMinidump(lldb::TargetSP target_sp, const FileSpec &core_file, DataBufferSP core_data) : PostMortemProcess(target_sp, listener_sp, core_file), - m_core_data(std::move(core_data)), m_active_exception(nullptr), - m_is_wow64(false) {} + m_core_data(std::move(core_data)), m_is_wow64(false) {} ProcessMinidump::~ProcessMinidump() { Clear(); @@ -209,7 +208,20 @@ Status ProcessMinidump::DoLoadCore() { GetTarget().SetArchitecture(arch, true /*set_platform*/); m_thread_list = m_minidump_parser->GetThreads(); - m_active_exception = m_minidump_parser->GetExceptionStream(); + std::vector<minidump::ExceptionStream> exception_streams = + m_minidump_parser->GetExceptionStreams(); + for (const auto &exception_stream : exception_streams) { + if (!m_exceptions_by_tid + .try_emplace(exception_stream.ThreadId, exception_stream) + .second) { + // We only cast to avoid the warning around converting little endian in + // printf. + error.SetErrorStringWithFormat( + "duplicate exception stream for tid %" PRIu32, + (uint32_t)exception_stream.ThreadId); + return error; + } + } SetUnixSignals(UnixSignals::Create(GetArchitecture())); @@ -232,60 +244,57 @@ Status ProcessMinidump::DoDestroy() { return Status(); } void ProcessMinidump::RefreshStateAfterStop() { - if (!m_active_exception) - return; - - constexpr uint32_t BreakpadDumpRequested = 0xFFFFFFFF; - if (m_active_exception->ExceptionRecord.ExceptionCode == - BreakpadDumpRequested) { - // This "ExceptionCode" value is a sentinel that is sometimes used - // when generating a dump for a process that hasn't crashed. - - // TODO: The definition and use of this "dump requested" constant - // in Breakpad are actually Linux-specific, and for similar use - // cases on Mac/Windows it defines different constants, referring - // to them as "simulated" exceptions; consider moving this check - // down to the OS-specific paths and checking each OS for its own - // constant. - return; - } + for (auto &[_, exception_stream] : m_exceptions_by_tid) { + constexpr uint32_t BreakpadDumpRequested = 0xFFFFFFFF; + if (exception_stream.ExceptionRecord.ExceptionCode == + BreakpadDumpRequested) { + // This "ExceptionCode" value is a sentinel that is sometimes used + // when generating a dump for a process that hasn't crashed. + + // TODO: The definition and use of this "dump requested" constant + // in Breakpad are actually Linux-specific, and for similar use + // cases on Mac/Windows it defines different constants, referring + // to them as "simulated" exceptions; consider moving this check + // down to the OS-specific paths and checking each OS for its own + // constant. + return; + } - lldb::StopInfoSP stop_info; - lldb::ThreadSP stop_thread; + lldb::StopInfoSP stop_info; + lldb::ThreadSP stop_thread; - Process::m_thread_list.SetSelectedThreadByID(m_active_exception->ThreadId); - stop_thread = Process::m_thread_list.GetSelectedThread(); - ArchSpec arch = GetArchitecture(); + Process::m_thread_list.SetSelectedThreadByID(exception_stream.ThreadId); + stop_thread = Process::m_thread_list.GetSelectedThread(); + ArchSpec arch = GetArchitecture(); - if (arch.GetTriple().getOS() == llvm::Triple::Linux) { - uint32_t signo = m_active_exception->ExceptionRecord.ExceptionCode; + if (arch.GetTriple().getOS() == llvm::Triple::Linux) { + uint32_t signo = exception_stream.ExceptionRecord.ExceptionCode; + if (signo == 0) { + // No stop. + return; + } - if (signo == 0) { - // No stop. - return; + stop_info = StopInfo::CreateStopReasonWithSignal(*stop_thread, signo); + } else if (arch.GetTriple().getVendor() == llvm::Triple::Apple) { + stop_info = StopInfoMachException::CreateStopReasonWithMachException( + *stop_thread, exception_stream.ExceptionRecord.ExceptionCode, 2, + exception_stream.ExceptionRecord.ExceptionFlags, + exception_stream.ExceptionRecord.ExceptionAddress, 0); + } else { + std::string desc; + llvm::raw_string_ostream desc_stream(desc); + desc_stream << "Exception " + << llvm::format_hex( + exception_stream.ExceptionRecord.ExceptionCode, 8) + << " encountered at address " + << llvm::format_hex( + exception_stream.ExceptionRecord.ExceptionAddress, 8); + stop_info = StopInfo::CreateStopReasonWithException( + *stop_thread, desc_stream.str().c_str()); } - stop_info = StopInfo::CreateStopReasonWithSignal( - *stop_thread, signo); - } else if (arch.GetTriple().getVendor() == llvm::Triple::Apple) { - stop_info = StopInfoMachException::CreateStopReasonWithMachException( - *stop_thread, m_active_exception->ExceptionRecord.ExceptionCode, 2, - m_active_exception->ExceptionRecord.ExceptionFlags, - m_active_exception->ExceptionRecord.ExceptionAddress, 0); - } else { - std::string desc; - llvm::raw_string_ostream desc_stream(desc); - desc_stream << "Exception " - << llvm::format_hex( - m_active_exception->ExceptionRecord.ExceptionCode, 8) - << " encountered at address " - << llvm::format_hex( - m_active_exception->ExceptionRecord.ExceptionAddress, 8); - stop_info = StopInfo::CreateStopReasonWithException( - *stop_thread, desc_stream.str().c_str()); - } - - stop_thread->SetStopInfo(stop_info); + stop_thread->SetStopInfo(stop_info); + } } bool ProcessMinidump::IsAlive() { return true; } @@ -386,10 +395,11 @@ bool ProcessMinidump::DoUpdateThreadList(ThreadList &old_thread_list, for (const minidump::Thread &thread : m_thread_list) { LocationDescriptor context_location = thread.Context; + std::optional<minidump::Exception> exception; // If the minidump contains an exception context, use it - if (m_active_exception != nullptr && - m_active_exception->ThreadId == thread.ThreadId) { - context_location = m_active_exception->ThreadContext; + if (m_exceptions_by_tid.count(thread.ThreadId) > 0) { + context_location = m_exceptions_by_tid[thread.ThreadId].ThreadContext; + exception = m_exceptions_by_tid[thread.ThreadId].ExceptionRecord; } llvm::ArrayRef<uint8_t> context; @@ -398,7 +408,8 @@ bool ProcessMinidump::DoUpdateThreadList(ThreadList &old_thread_list, else context = m_minidump_parser->GetThreadContextWow64(thread); - lldb::ThreadSP thread_sp(new ThreadMinidump(*this, thread, context)); + lldb::ThreadSP thread_sp( + new ThreadMinidump(*this, thread, context, exception)); new_thread_list.AddThread(thread_sp); } return new_thread_list.GetSize(false) > 0; diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.h b/lldb/source/Plugins/Process/minidump/ProcessMinidump.h index 3f3123a0a8b5d..0379de7bee5e6 100644 --- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.h +++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.h @@ -109,7 +109,7 @@ class ProcessMinidump : public PostMortemProcess { private: lldb::DataBufferSP m_core_data; llvm::ArrayRef<minidump::Thread> m_thread_list; - const minidump::ExceptionStream *m_active_exception; + std::unordered_map<uint32_t, minidump::ExceptionStream> m_exceptions_by_tid; lldb::CommandObjectSP m_command_sp; bool m_is_wow64; std::optional<MemoryRegionInfos> m_memory_regions; diff --git a/lldb/source/Plugins/Process/minidump/ThreadMinidump.cpp b/lldb/source/Plugins/Process/minidump/ThreadMinidump.cpp index 1fbc52815238b..a1d7da376b02b 100644 --- a/lldb/source/Plugins/Process/minidump/ThreadMinidump.cpp +++ b/lldb/source/Plugins/Process/minidump/ThreadMinidump.cpp @@ -34,9 +34,10 @@ using namespace lldb_private; using namespace minidump; ThreadMinidump::ThreadMinidump(Process &process, const minidump::Thread &td, - llvm::ArrayRef<uint8_t> gpregset_data) + llvm::ArrayRef<uint8_t> gpregset_data, + std::optional<minidump::Exception> exception) : Thread(process, td.ThreadId), m_thread_reg_ctx_sp(), - m_gpregset_data(gpregset_data) {} + m_gpregset_data(gpregset_data), m_exception(exception) {} ThreadMinidump::~ThreadMinidump() = default; @@ -115,4 +116,15 @@ ThreadMinidump::CreateRegisterContextForFrame(StackFrame *frame) { return reg_ctx_sp; } -bool ThreadMinidump::CalculateStopInfo() { return false; } +// This method doesn't end up getting called for minidump +// because the stopinfo is set in `ProcessMinidump::RefreshStateAfterStop` +bool ThreadMinidump::CalculateStopInfo() { + if (!m_exception) + return false; + + minidump::Exception thread_exception = m_exception.value(); + SetStopInfo(StopInfo::CreateStopReasonWithSignal( + *this, thread_exception.ExceptionCode, /*description=*/nullptr, + thread_exception.ExceptionCode)); + return true; +} diff --git a/lldb/source/Plugins/Process/minidump/ThreadMinidump.h b/lldb/source/Plugins/Process/minidump/ThreadMinidump.h index aed7cfbc1b161..abde13ccb3f2a 100644 --- a/lldb/source/Plugins/Process/minidump/ThreadMinidump.h +++ b/lldb/source/Plugins/Process/minidump/ThreadMinidump.h @@ -21,7 +21,8 @@ namespace minidump { class ThreadMinidump : public Thread { public: ThreadMinidump(Process &process, const minidump::Thread &td, - llvm::ArrayRef<uint8_t> gpregset_data); + llvm::ArrayRef<uint8_t> gpregset_data, + std::optional<minidump::Exception> exception); ~ThreadMinidump() override; @@ -35,6 +36,7 @@ class ThreadMinidump : public Thread { protected: lldb::RegisterContextSP m_thread_reg_ctx_sp; llvm::ArrayRef<uint8_t> m_gpregset_data; + std::optional<minidump::Exception> m_exception; bool CalculateStopInfo() override; }; diff --git a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py index 91fd2439492b5..ba93eff6f1f82 100644 --- a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py +++ b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py @@ -491,3 +491,17 @@ def test_minidump_sysroot(self): spec_dir_norm = os.path.normcase(module.GetFileSpec().GetDirectory()) exe_dir_norm = os.path.normcase(exe_dir) self.assertEqual(spec_dir_norm, exe_dir_norm) + + def test_multiple_exceptions_or_signals(self): + """Test that lldb can read the exception information from the Minidump.""" + print("Starting to read multiple-sigsev.yaml") + self.process_from_yaml("multiple-sigsev.yaml") + print("Done reading multiple-sigsev.yaml") + self.check_state() + # This process crashed due to a segmentation fault in both it's threads. + self.assertEqual(self.process.GetNumThreads(), 2) + for i in range(2): + thread = self.process.GetThreadAtIndex(i) + self.assertStopReason(thread.GetStopReason(), lldb.eStopReasonSignal) + stop_description = thread.GetStopDescription(256) + self.assertIn("SIGSEGV", stop_description) diff --git a/lldb/test/API/functionalities/postmortem/minidump-new/multiple-sigsev.yaml b/lldb/test/API/functionalities/postmortem/minidump-new/multiple-sigsev.yaml new file mode 100644 index 0000000000000..f6fcfdbf5c0eb --- /dev/null +++ b/lldb/test/API/functionalities/postmortem/minidump-new/multiple-sigsev.yaml @@ -0,0 +1,39 @@ +--- !minidump +Streams: + - Type: ThreadList + Threads: + - Thread Id: 0x1B4F23 + Context: 0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000B0010000000000033000000000000000000000006020100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000010A234EBFC7F000010A234EBFC7F00000000000000000000F09C34EBFC7F0000C0A91ABCE97F00000000000000000000A0163FBCE97F00004602000000000000921C40000000000030A434EBFC7F000000000000000000000000000000000000C61D4000000000007F0300000000000000000000000000000000000000000000801F0000FFFF0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000FFFF00FFFFFFFFFFFFFF00FFFFFFFF25252525252525252525252525252525000000000000000000000000000000000000000000000000000000000000000000FFFF00FFFFFFFFFFFFFF00FFFFFFFF0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000FF00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 + Stack: + Start of Memory Range: 0x7FFFFFFFD348 + Content: '' + - Thread Id: 0x1B6D22 + Context: 0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000B0010000000000033000000000000000000000006020100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000010A234EBFC7F000010A234EBFC7F00000000000000000000F09C34EBFC7F0000C0A91ABCE97F00000000000000000000A0163FBCE97F00004602000000000000921C40000000000030A434EBFC7F000000000000000000000000000000000000C61D4000000000007F0300000000000000000000000000000000000000000000801F0000FFFF0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000FFFF00FFFFFFFFFFFFFF00FFFFFFFF25252525252525252525252525252525000000000000000000000000000000000000000000000000000000000000000000FFFF00FFFFFFFFFFFFFF00FFFFFFFF0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000FF00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 + Stack: + Start of Memory Range: 0x7FFFF75FDE28 + Content: '' + - Type: ModuleList + Modules: + - Base of Image: 0x0000000000400000 + Size of Image: 0x00017000 + Module Name: 'a.out' + CodeView Record: '' + - Type: SystemInfo + Processor Arch: AMD64 + Platform ID: Linux + CSD Version: 'Linux 3.13' + CPU: + Vendor ID: GenuineIntel + Version Info: 0x00000000 + Feature Info: 0x00000000 + - Type: Exception + Thread ID: 0x1B4F23 + Exception Record: + Exception Code: 0xB + Thread Context: 00000000 + - Type: Exception + Thread ID: 0x1B6D22 + Exception Record: + Exception Code: 0xB + Thread Context: 00000000 +... diff --git a/lldb/unittests/Process/minidump/MinidumpParserTest.cpp b/lldb/unittests/Process/minidump/MinidumpParserTest.cpp index 632a7fd4e4f8f..aa74e690d45a5 100644 --- a/lldb/unittests/Process/minidump/MinidumpParserTest.cpp +++ b/lldb/unittests/Process/minidump/MinidumpParserTest.cpp @@ -251,10 +251,13 @@ TEST_F(MinidumpParserTest, GetFilteredModuleList) { TEST_F(MinidumpParserTest, GetExceptionStream) { SetUpData("linux-x86_64.dmp"); - const llvm::... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/97470 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
