[Lldb-commits] [lldb] [LLDB][Minidump] Support minidumps where there are multiple exception streams (PR #97470)

Jacob Lalonde via lldb-commits lldb-commits at lists.llvm.org
Fri Sep 6 16:48:29 PDT 2024


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

>From 5d7f2a6fb8dc06b838bf072aca1d8829eb61b316 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Fri, 6 Sep 2024 13:09:27 -0700
Subject: [PATCH 1/4] MSquash merge lldb changes and drop llvm changes

---
 .../Process/minidump/MinidumpParser.cpp       |  10 +-
 .../Plugins/Process/minidump/MinidumpParser.h |   3 +-
 .../Process/minidump/ProcessMinidump.cpp      | 125 ++++++++++--------
 .../Process/minidump/ProcessMinidump.h        |   3 +-
 .../minidump-new/TestMiniDumpNew.py           |  14 ++
 .../minidump-new/multiple-sigsev.yaml         |  39 ++++++
 .../Process/minidump/MinidumpParserTest.cpp   |   9 +-
 7 files changed, 138 insertions(+), 65 deletions(-)
 create mode 100644 lldb/test/API/functionalities/postmortem/minidump-new/multiple-sigsev.yaml

diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
index c099c28a620ecf..b0a6ed6e460daa 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();
+std::optional<std::vector<const 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 std::nullopt;
 }
 
 std::optional<minidump::Range>
diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.h b/lldb/source/Plugins/Process/minidump/MinidumpParser.h
index 222c0ef47fb853..be6364ca6ecdc7 100644
--- a/lldb/source/Plugins/Process/minidump/MinidumpParser.h
+++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.h
@@ -84,7 +84,8 @@ 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();
+  std::optional<std::vector<const 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 ac1ecbfc0e2e70..f876d666031c9a 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,28 @@ Status ProcessMinidump::DoLoadCore() {
   GetTarget().SetArchitecture(arch, true /*set_platform*/);
 
   m_thread_list = m_minidump_parser->GetThreads();
-  m_active_exception = m_minidump_parser->GetExceptionStream();
+  std::optional<std::vector<const minidump::ExceptionStream *>>
+      exception_streams = m_minidump_parser->GetExceptionStreams();
+
+  if (exception_streams) {
+    for (const auto *exception_stream : *exception_streams) {
+      if (!exception_stream) {
+        error.SetErrorString(
+            "Minidump returned null pointer for exception stream");
+        return error;
+      }
+      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 +252,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 (const 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; }
@@ -387,10 +404,8 @@ bool ProcessMinidump::DoUpdateThreadList(ThreadList &old_thread_list,
     LocationDescriptor context_location = thread.Context;
 
     // 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;
 
     llvm::ArrayRef<uint8_t> context;
     if (!m_is_wow64)
diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.h b/lldb/source/Plugins/Process/minidump/ProcessMinidump.h
index e39ae3913e8782..7b7c970a38d4ec 100644
--- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.h
+++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.h
@@ -107,7 +107,8 @@ 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, const 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/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
index 2de3e36b507341..61a21309fa1835 100644
--- a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
+++ b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
@@ -510,3 +510,17 @@ def test_minidump_memory64list(self):
         self.assertTrue(region_info_list.GetMemoryRegionAtIndex(2, region))
         self.assertEqual(region.GetRegionBase(), 0x00007fff12a87018)
         self.assertTrue(region.GetRegionEnd(), 0x00007fff12a87018 + 0x00000400)
+
+    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 00000000000000..f6fcfdbf5c0eb0
--- /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 632a7fd4e4f8fa..c27fce80d67a13 100644
--- a/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
+++ b/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
@@ -251,10 +251,11 @@ TEST_F(MinidumpParserTest, GetFilteredModuleList) {
 
 TEST_F(MinidumpParserTest, GetExceptionStream) {
   SetUpData("linux-x86_64.dmp");
-  const llvm::minidump::ExceptionStream *exception_stream =
-      parser->GetExceptionStream();
-  ASSERT_NE(nullptr, exception_stream);
-  ASSERT_EQ(11UL, exception_stream->ExceptionRecord.ExceptionCode);
+  std::optional<std::vector<const minidump::ExceptionStream *>>
+      exception_stream = parser->GetExceptionStreams();
+  ASSERT_TRUE(exception_stream);
+  ASSERT_EQ(1UL, exception_stream->size());
+  ASSERT_EQ(11UL, exception_stream->at(0)->ThreadId);
 }
 
 void check_mem_range_exists(MinidumpParser &parser, const uint64_t range_start,

>From 9c90aa6466f050729637aed8bed3ee6c93374c4a Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Fri, 6 Sep 2024 13:33:36 -0700
Subject: [PATCH 2/4] Migrate code over to the new iterator api

---
 .../Process/minidump/MinidumpParser.cpp       | 11 +---
 .../Plugins/Process/minidump/MinidumpParser.h |  3 +-
 .../Process/minidump/ProcessMinidump.cpp      | 53 +++++++++----------
 .../Process/minidump/ProcessMinidump.h        |  2 +-
 4 files changed, 29 insertions(+), 40 deletions(-)

diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
index b0a6ed6e460daa..afc095ddbb2f91 100644
--- a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
+++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -417,16 +417,9 @@ std::vector<const minidump::Module *> MinidumpParser::GetFilteredModuleList() {
   return filtered_modules;
 }
 
-std::optional<std::vector<const minidump::ExceptionStream *>>
+llvm::iterator_range<ExceptionStreamsIterator>
 MinidumpParser::GetExceptionStreams() {
-  auto ExpectedStream = GetMinidumpFile().getExceptionStreams();
-  if (ExpectedStream)
-    return ExpectedStream.get();
-
-  LLDB_LOG_ERROR(GetLog(LLDBLog::Process), ExpectedStream.takeError(),
-                 "Failed to read minidump exception stream: {0}");
-
-  return std::nullopt;
+  return GetMinidumpFile().getExceptionStreams();
 }
 
 std::optional<minidump::Range>
diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.h b/lldb/source/Plugins/Process/minidump/MinidumpParser.h
index be6364ca6ecdc7..20f6d2a18989a1 100644
--- a/lldb/source/Plugins/Process/minidump/MinidumpParser.h
+++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.h
@@ -48,6 +48,7 @@ struct Range {
 };
 
 using FallibleMemory64Iterator = llvm::object::MinidumpFile::FallibleMemory64Iterator;
+using ExceptionStreamsIterator = llvm::object::MinidumpFile::ExceptionStreamsIterator;
 
 class MinidumpParser {
 public:
@@ -84,7 +85,7 @@ class MinidumpParser {
   // have the same name, it keeps the copy with the lowest load address.
   std::vector<const minidump::Module *> GetFilteredModuleList();
 
-  std::optional<std::vector<const llvm::minidump::ExceptionStream *>>
+  llvm::iterator_range<ExceptionStreamsIterator>
   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 f876d666031c9a..67f3cfda9c9ecd 100644
--- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -208,26 +208,21 @@ Status ProcessMinidump::DoLoadCore() {
   GetTarget().SetArchitecture(arch, true /*set_platform*/);
 
   m_thread_list = m_minidump_parser->GetThreads();
-  std::optional<std::vector<const minidump::ExceptionStream *>>
-      exception_streams = m_minidump_parser->GetExceptionStreams();
-
-  if (exception_streams) {
-    for (const auto *exception_stream : *exception_streams) {
-      if (!exception_stream) {
-        error.SetErrorString(
-            "Minidump returned null pointer for exception stream");
-        return error;
-      }
-      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;
-      }
+  auto exception_stream_it = m_minidump_parser->GetExceptionStreams();
+  for (auto exception_stream : exception_stream_it) {
+    // If we can't read an exception stream skip it
+    // We should probably serve a warning
+    if (!exception_stream)
+      continue;
+
+    if (!m_exceptions_by_tid
+              .try_emplace(exception_stream->ThreadId, exception_stream.get())
+              .second) {
+      // We only cast to avoid the warning around converting little endian in
+      // printf.
+      return Status::FromErrorStringWithFormat(
+          "Duplicate exception stream for tid %" PRIu32,
+          (uint32_t)exception_stream->ThreadId);
     }
   }
 
@@ -254,7 +249,7 @@ void ProcessMinidump::RefreshStateAfterStop() {
 
   for (const auto &[_, exception_stream] : m_exceptions_by_tid) {
     constexpr uint32_t BreakpadDumpRequested = 0xFFFFFFFF;
-    if (exception_stream->ExceptionRecord.ExceptionCode ==
+    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.
@@ -271,12 +266,12 @@ void ProcessMinidump::RefreshStateAfterStop() {
     lldb::StopInfoSP stop_info;
     lldb::ThreadSP stop_thread;
 
-    Process::m_thread_list.SetSelectedThreadByID(exception_stream->ThreadId);
+    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 = exception_stream->ExceptionRecord.ExceptionCode;
+      uint32_t signo = exception_stream.ExceptionRecord.ExceptionCode;
       if (signo == 0) {
         // No stop.
         return;
@@ -285,18 +280,18 @@ void ProcessMinidump::RefreshStateAfterStop() {
       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);
+          *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)
+                         exception_stream.ExceptionRecord.ExceptionCode, 8)
                   << " encountered at address "
                   << llvm::format_hex(
-                         exception_stream->ExceptionRecord.ExceptionAddress, 8);
+                         exception_stream.ExceptionRecord.ExceptionAddress, 8);
       stop_info = StopInfo::CreateStopReasonWithException(
           *stop_thread, desc_stream.str().c_str());
     }
@@ -405,7 +400,7 @@ bool ProcessMinidump::DoUpdateThreadList(ThreadList &old_thread_list,
 
     // If the minidump contains an exception context, use it
     if (m_exceptions_by_tid.count(thread.ThreadId) > 0)
-      context_location = m_exceptions_by_tid[thread.ThreadId]->ThreadContext;
+      context_location = m_exceptions_by_tid[thread.ThreadId].ThreadContext;
 
     llvm::ArrayRef<uint8_t> context;
     if (!m_is_wow64)
diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.h b/lldb/source/Plugins/Process/minidump/ProcessMinidump.h
index 7b7c970a38d4ec..f2ea0a2b61d14e 100644
--- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.h
+++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.h
@@ -107,7 +107,7 @@ class ProcessMinidump : public PostMortemProcess {
 private:
   lldb::DataBufferSP m_core_data;
   llvm::ArrayRef<minidump::Thread> m_thread_list;
-  std::unordered_map<uint32_t, const minidump::ExceptionStream *>
+  std::unordered_map<uint32_t, const minidump::ExceptionStream>
       m_exceptions_by_tid;
   lldb::CommandObjectSP m_command_sp;
   bool m_is_wow64;

>From bfb8f4f2f9becfd3156aefbcf1d34673abbcd8bd Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Fri, 6 Sep 2024 15:28:27 -0700
Subject: [PATCH 3/4] Edit tests

---
 .../Process/minidump/MinidumpParserTest.cpp        | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/lldb/unittests/Process/minidump/MinidumpParserTest.cpp b/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
index c27fce80d67a13..4e13a48cf11126 100644
--- a/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
+++ b/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
@@ -251,11 +251,15 @@ TEST_F(MinidumpParserTest, GetFilteredModuleList) {
 
 TEST_F(MinidumpParserTest, GetExceptionStream) {
   SetUpData("linux-x86_64.dmp");
-  std::optional<std::vector<const minidump::ExceptionStream *>>
-      exception_stream = parser->GetExceptionStreams();
-  ASSERT_TRUE(exception_stream);
-  ASSERT_EQ(1UL, exception_stream->size());
-  ASSERT_EQ(11UL, exception_stream->at(0)->ThreadId);
+  auto exception_streams = parser->GetExceptionStreams();
+  size_t count = 0;
+  for (auto exception_stream : exception_streams) {
+    ASSERT_THAT_EXPECTED(exception_stream, llvm::Succeeded());
+    ASSERT_EQ(16001UL, exception_stream->ThreadId);
+    count++;
+  }
+  
+  ASSERT_THAT(1UL, count);
 }
 
 void check_mem_range_exists(MinidumpParser &parser, const uint64_t range_start,

>From 06e493fbd57cae1f2eb1240174d50a60d945a3de Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Fri, 6 Sep 2024 16:48:18 -0700
Subject: [PATCH 4/4] Run formatters

---
 lldb/source/Plugins/Process/minidump/MinidumpParser.h     | 6 +++---
 lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp  | 4 ++--
 .../postmortem/minidump-new/TestMiniDumpNew.py            | 8 ++++----
 lldb/unittests/Process/minidump/MinidumpParserTest.cpp    | 2 +-
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.h b/lldb/source/Plugins/Process/minidump/MinidumpParser.h
index 20f6d2a18989a1..f0b6e6027c52f0 100644
--- a/lldb/source/Plugins/Process/minidump/MinidumpParser.h
+++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.h
@@ -48,7 +48,8 @@ struct Range {
 };
 
 using FallibleMemory64Iterator = llvm::object::MinidumpFile::FallibleMemory64Iterator;
-using ExceptionStreamsIterator = llvm::object::MinidumpFile::ExceptionStreamsIterator;
+using ExceptionStreamsIterator =
+    llvm::object::MinidumpFile::ExceptionStreamsIterator;
 
 class MinidumpParser {
 public:
@@ -85,8 +86,7 @@ class MinidumpParser {
   // have the same name, it keeps the copy with the lowest load address.
   std::vector<const minidump::Module *> GetFilteredModuleList();
 
-  llvm::iterator_range<ExceptionStreamsIterator>
-  GetExceptionStreams();
+  llvm::iterator_range<ExceptionStreamsIterator> 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 67f3cfda9c9ecd..218012e656f07a 100644
--- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -216,8 +216,8 @@ Status ProcessMinidump::DoLoadCore() {
       continue;
 
     if (!m_exceptions_by_tid
-              .try_emplace(exception_stream->ThreadId, exception_stream.get())
-              .second) {
+             .try_emplace(exception_stream->ThreadId, exception_stream.get())
+             .second) {
       // We only cast to avoid the warning around converting little endian in
       // printf.
       return Status::FromErrorStringWithFormat(
diff --git a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
index 61a21309fa1835..5a0b6e790a424c 100644
--- a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
+++ b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
@@ -505,11 +505,11 @@ def test_minidump_memory64list(self):
         self.assertEqual(region.GetRegionBase(), 0x7FFF12A84030)
         self.assertTrue(region.GetRegionEnd(), 0x7FFF12A84030 + 0x2FD0)
         self.assertTrue(region_info_list.GetMemoryRegionAtIndex(1, region))
-        self.assertEqual(region.GetRegionBase(), 0x00007fff12a87000)
-        self.assertTrue(region.GetRegionEnd(), 0x00007fff12a87000 + 0x00000018)
+        self.assertEqual(region.GetRegionBase(), 0x00007FFF12A87000)
+        self.assertTrue(region.GetRegionEnd(), 0x00007FFF12A87000 + 0x00000018)
         self.assertTrue(region_info_list.GetMemoryRegionAtIndex(2, region))
-        self.assertEqual(region.GetRegionBase(), 0x00007fff12a87018)
-        self.assertTrue(region.GetRegionEnd(), 0x00007fff12a87018 + 0x00000400)
+        self.assertEqual(region.GetRegionBase(), 0x00007FFF12A87018)
+        self.assertTrue(region.GetRegionEnd(), 0x00007FFF12A87018 + 0x00000400)
 
     def test_multiple_exceptions_or_signals(self):
         """Test that lldb can read the exception information from the Minidump."""
diff --git a/lldb/unittests/Process/minidump/MinidumpParserTest.cpp b/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
index 4e13a48cf11126..c7547ba261c7f7 100644
--- a/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
+++ b/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
@@ -258,7 +258,7 @@ TEST_F(MinidumpParserTest, GetExceptionStream) {
     ASSERT_EQ(16001UL, exception_stream->ThreadId);
     count++;
   }
-  
+
   ASSERT_THAT(1UL, count);
 }
 



More information about the lldb-commits mailing list