[LLDB][Minidump] Support minidumps where there are multiple exception streams by Jlalond · Pull Request #97470 · llvm/llvm-project (original) (raw)

@llvm/pr-subscribers-objectyaml

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

Author: Jacob Lalonde (Jlalond)

Changes

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:

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::vectorminidump::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::vectorminidump::ExceptionStream(); } std::optionalminidump::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::vectorllvm::minidump::ExceptionStream GetExceptionStreams(); std::optional 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::vectorminidump::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;

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;

@@ -398,7 +408,8 @@ bool ProcessMinidump::DoUpdateThreadList(ThreadList &old_thread_list, else context = m_minidump_parser->GetThreadContextWow64(thread);

ThreadMinidump::ThreadMinidump(Process &process, const minidump::Thread &td,

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() {

@@ -35,6 +36,7 @@ class ThreadMinidump : public Thread { protected: lldb::RegisterContextSP m_thread_reg_ctx_sp; llvm::ArrayRef m_gpregset_data;

}; 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) +

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:

+... 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");