Add support to read multiple exception streams in minidumps

Jlalond · Jlalond · commit e8d1f3a7f978 · 2024-07-02T14:59:16.000-07:00
diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -408,7 +408,7 @@ std::vector<const minidump::Module *> MinidumpParser::GetFilteredModuleList() {
         continue;
       }
       // This module has been seen. Modules are sometimes mentioned multiple
-      // times when they are mapped discontiguously, so find the module with
+      // times when they are mapped discontiguously, so find the module with 
       // the lowest "base_of_image" and use that as the filtered module.
       if (module.BaseOfImage < dup_module->BaseOfImage)
         filtered_modules[iter->second] = &module;
@@ -417,14 +417,15 @@ 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
@@ -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
@@ -39,6 +39,7 @@
 
 #include <memory>
 #include <optional>
+#include <iostream>
 
 using namespace lldb;
 using namespace lldb_private;
@@ -157,7 +158,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_core_data(std::move(core_data)),
       m_is_wow64(false) {}
 
 ProcessMinidump::~ProcessMinidump() {
@@ -209,7 +210,19 @@ 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.count(exception_stream.ThreadId) > 0) {
+      // 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;
+    } else 
+      m_exceptions_by_tid[exception_stream.ThreadId] = exception_stream;
+
+
+    std::cout << "Adding Exception Stream # " << (uint32_t)exception_stream.ThreadId << std::endl;
+    std::cout << "Added index " << (uint32_t)m_exceptions_by_tid[exception_stream.ThreadId].ExceptionRecord.ExceptionCode << std::endl;
+  }
 
   SetUnixSignals(UnixSignals::Create(GetArchitecture()));
 
@@ -232,60 +245,59 @@ 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;
+      std::cout << "Thread Id : " << exception_stream.ThreadId << " has signal " << signo << std::endl;
+      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,19 +398,21 @@ 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;
     if (!m_is_wow64)
       context = m_minidump_parser->GetThreadContext(context_location);
     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
@@ -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
@@ -34,9 +34,9 @@ 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 +115,12 @@ ThreadMinidump::CreateRegisterContextForFrame(StackFrame *frame) {
   return reg_ctx_sp;
 }
 
-bool ThreadMinidump::CalculateStopInfo() { return false; }
+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
@@ -21,7 +21,7 @@ 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 +35,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/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::minidump::ExceptionStream *exception_stream =
-      parser->GetExceptionStream();
-  ASSERT_NE(nullptr, exception_stream);
-  ASSERT_EQ(11UL, exception_stream->ExceptionRecord.ExceptionCode);
+  llvm::Expected<std::vector<ExceptionStream>> exception_stream =
+      parser->GetExceptionStreams();
+  // LLVM::Expected has an explicit bool operator that determines if 
+  // the expected value is an error or not.
+  ASSERT_TRUE((bool)exception_stream);
+  ASSERT_EQ(1UL, exception_stream->size());
+  ASSERT_EQ(11UL, exception_stream.get()[0].ExceptionRecord.ExceptionCode);
 }
 
 void check_mem_range_exists(MinidumpParser &parser, const uint64_t range_start,
diff --git a/llvm/include/llvm/Object/Minidump.h b/llvm/include/llvm/Object/Minidump.h
@@ -82,15 +82,22 @@ class MinidumpFile : public Binary {
     return getListStream<minidump::Thread>(minidump::StreamType::ThreadList);
   }
 
-  /// Returns the contents of the Exception stream.  An error is returned if the
-  /// file does not contain this stream, or the stream is smaller than the size
-  /// of the ExceptionStream structure.  The internal consistency of the stream
-  /// is not checked in any way.
-  Expected<const minidump::ExceptionStream &> getExceptionStream() const {
-    return getStream<minidump::ExceptionStream>(
-        minidump::StreamType::Exception);
+  /// Returns the contents of the Exception stream. An error is returned if the 
+  /// associated stream is smaller than the size of the ExceptionStream structure.  
+  /// Or the directory supplied is not of kind exception stream.
+  Expected<minidump::ExceptionStream> getExceptionStream(minidump::Directory Directory) const {
+    if (Directory.Type != minidump::StreamType::Exception) {
+      return createError("Not an exception stream");
+    }
+
+    return getStreamFromDirectory<minidump::ExceptionStream>(Directory);
   }
 
+  /// Returns the contents of the Exception streams.  An error is returned if
+  /// any of the streams are smaller than the size of the ExceptionStream structure.  
+  /// The internal consistency of the stream is not checked in any way.
+  Expected<std::vector<minidump::ExceptionStream>> getExceptionStreams() const;
+
   /// Returns the list of descriptors embedded in the MemoryList stream. The
   /// descriptors provide the content of interesting regions of memory at the
   /// time the minidump was taken. An error is returned if the file does not
@@ -172,6 +179,11 @@ class MinidumpFile : public Binary {
     return arrayRefFromStringRef(Data.getBuffer());
   }
 
+  /// Return the stream of the given type, cast to the appropriate type. Checks
+  /// that the stream is large enough to hold an object of this type.
+  template <typename T>
+  Expected<const T &> getStreamFromDirectory(minidump::Directory Directory) const;
+
   /// Return the stream of the given type, cast to the appropriate type. Checks
   /// that the stream is large enough to hold an object of this type.
   template <typename T>
@@ -187,6 +199,14 @@ class MinidumpFile : public Binary {
   DenseMap<minidump::StreamType, std::size_t> StreamMap;
 };
 
+template <typename T>
+Expected<const T &> MinidumpFile::getStreamFromDirectory(minidump::Directory Directory) const {
+  ArrayRef<uint8_t> Stream = getRawStream(Directory);
+  if (Stream.size() >= sizeof(T))
+    return *reinterpret_cast<const T *>(Stream.data());
+  return createEOFError();
+}
+
 template <typename T>
 Expected<const T &> MinidumpFile::getStream(minidump::StreamType Type) const {
   if (std::optional<ArrayRef<uint8_t>> Stream = getRawStream(Type)) {
diff --git a/llvm/lib/Object/Minidump.cpp b/llvm/lib/Object/Minidump.cpp
diff --git a/llvm/lib/ObjectYAML/MinidumpYAML.cpp b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
