https://github.com/Jlalond updated
https://github.com/llvm/llvm-project/pull/97470
>From 8647eccd35085ab80f978fabb78b016915c5524f Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Tue, 2 Jul 2024 12:41:02 -0700
Subject: [PATCH 1/8] Add support to read multiple exception streams in
minidumps
---
.../Process/minidump/MinidumpParser.cpp | 11 +-
.../Plugins/Process/minidump/MinidumpParser.h | 2 +-
.../Process/minidump/ProcessMinidump.cpp | 122 ++++++++++--------
.../Process/minidump/ProcessMinidump.h | 2 +-
.../Process/minidump/ThreadMinidump.cpp | 14 +-
.../Plugins/Process/minidump/ThreadMinidump.h | 3 +-
.../Process/minidump/MinidumpParserTest.cpp | 11 +-
llvm/include/llvm/Object/Minidump.h | 34 ++++-
llvm/lib/Object/Minidump.cpp | 37 ++++++
llvm/lib/ObjectYAML/MinidumpYAML.cpp | 4 +-
10 files changed, 162 insertions(+), 78 deletions(-)
diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
index be9fae938e227..ac487a5ed0c0a 100644
--- 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
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..9f707c0d8a7a7 100644
--- 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
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..3cc154f1eac47 100644
--- 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
index aed7cfbc1b161..cc336ad2d7b3d 100644
--- 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
index 632a7fd4e4f8f..e5355e07e41fc 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::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
index e45d4de0090de..87fdc555fb1ac 100644
--- 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
index 6febff89ac519..d366b4d3d579e 100644
--- a/llvm/lib/Object/Minidump.cpp
+++ b/llvm/lib/Object/Minidump.cpp
@@ -9,6 +9,7 @@
#include "llvm/Object/Minidump.h"
#include "llvm/Object/Error.h"
#include "llvm/Support/ConvertUTF.h"
+#include <iostream>
using namespace llvm;
using namespace llvm::object;
@@ -53,6 +54,33 @@ Expected<std::string> MinidumpFile::getString(size_t Offset)
const {
return Result;
}
+Expected<std::vector<minidump::ExceptionStream>>
MinidumpFile::getExceptionStreams() const {
+ // Scan the directories for exceptions first
+ std::vector<Directory> exceptionStreams;
+ for (const auto &directory : Streams) {
+ if (directory.Type == StreamType::Exception)
+ exceptionStreams.push_back(directory);
+ }
+
+ if (exceptionStreams.empty())
+ return createError("No exception streams found");
+
+ std::vector<minidump::ExceptionStream> exceptionStreamList;
+ for (const auto &exceptionStream : exceptionStreams) {
+ llvm::Expected<minidump::ExceptionStream> ExpectedStream =
getStreamFromDirectory<minidump::ExceptionStream>(exceptionStream);
+ if (!ExpectedStream)
+ return ExpectedStream.takeError();
+
+ std::cout << "Adding Exception Stream # " << exceptionStreamList.size() <<
std::endl;
+ std::cout << "Thread Id : " << ExpectedStream->ThreadId << std::endl;
+ std::cout << "Exception Code : " <<
ExpectedStream.get().ExceptionRecord.ExceptionCode << std::endl;
+ exceptionStreamList.push_back(ExpectedStream.get());
+ assert(exceptionStreamList.back().ThreadId == ExpectedStream->ThreadId);
+ }
+
+ return exceptionStreamList;
+}
+
Expected<iterator_range<MinidumpFile::MemoryInfoIterator>>
MinidumpFile::getMemoryInfoList() const {
std::optional<ArrayRef<uint8_t>> Stream =
@@ -127,6 +155,7 @@ MinidumpFile::create(MemoryBufferRef Source) {
return ExpectedStreams.takeError();
DenseMap<StreamType, std::size_t> StreamMap;
+ std::vector<std::size_t> ExceptionStreams;
for (const auto &StreamDescriptor : llvm::enumerate(*ExpectedStreams)) {
StreamType Type = StreamDescriptor.value().Type;
const LocationDescriptor &Loc = StreamDescriptor.value().Location;
@@ -142,6 +171,14 @@ MinidumpFile::create(MemoryBufferRef Source) {
continue;
}
+ // We treat exceptions differently here because the LLDB minidump
+ // makes some assumptions about uniqueness, all the streams other than
exceptions
+ // are lists. But exceptions are not a list, they are single streams that
point back to their thread
+ // So we will omit them here, and will find them when needed in the
MinidumpFile.
+ if (Type == StreamType::Exception) {
+ continue;
+ }
+
if (Type == DenseMapInfo<StreamType>::getEmptyKey() ||
Type == DenseMapInfo<StreamType>::getTombstoneKey())
return createError("Cannot handle one of the minidump streams");
diff --git a/llvm/lib/ObjectYAML/MinidumpYAML.cpp
b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
index fdbd2d8e6dcbc..e00feceea57af 100644
--- a/llvm/lib/ObjectYAML/MinidumpYAML.cpp
+++ b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
@@ -464,8 +464,8 @@ Stream::create(const Directory &StreamDesc, const
object::MinidumpFile &File) {
StreamKind Kind = getKind(StreamDesc.Type);
switch (Kind) {
case StreamKind::Exception: {
- Expected<const minidump::ExceptionStream &> ExpectedExceptionStream =
- File.getExceptionStream();
+ Expected<minidump::ExceptionStream> ExpectedExceptionStream =
+ File.getExceptionStream(StreamDesc);
if (!ExpectedExceptionStream)
return ExpectedExceptionStream.takeError();
Expected<ArrayRef<uint8_t>> ExpectedThreadContext =
>From b990ed49098884d7f1ae0d8ca0cc6ab85dbd34e2 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Tue, 2 Jul 2024 12:44:59 -0700
Subject: [PATCH 2/8] Remove debug std::cout code. Run git-clang-format
---
.../Process/minidump/MinidumpParser.cpp | 5 ++-
.../Process/minidump/ProcessMinidump.cpp | 37 +++++++++----------
.../Process/minidump/ThreadMinidump.cpp | 8 +++-
.../Plugins/Process/minidump/ThreadMinidump.h | 3 +-
.../Process/minidump/MinidumpParserTest.cpp | 2 +-
llvm/include/llvm/Object/Minidump.h | 20 ++++++----
llvm/lib/Object/Minidump.cpp | 17 ++++-----
7 files changed, 49 insertions(+), 43 deletions(-)
diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
index ac487a5ed0c0a..c51fed8d91b07 100644
--- 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,7 +417,8 @@ std::vector<const minidump::Module *>
MinidumpParser::GetFilteredModuleList() {
return filtered_modules;
}
-const std::vector<minidump::ExceptionStream>
MinidumpParser::GetExceptionStreams() {
+const std::vector<minidump::ExceptionStream>
+MinidumpParser::GetExceptionStreams() {
auto ExpectedStream = GetMinidumpFile().getExceptionStreams();
if (ExpectedStream)
return ExpectedStream.get();
diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
index 9f707c0d8a7a7..9a7e481b92796 100644
--- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -39,7 +39,6 @@
#include <memory>
#include <optional>
-#include <iostream>
using namespace lldb;
using namespace lldb_private;
@@ -158,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_is_wow64(false) {}
+ m_core_data(std::move(core_data)), m_is_wow64(false) {}
ProcessMinidump::~ProcessMinidump() {
Clear();
@@ -210,18 +208,19 @@ Status ProcessMinidump::DoLoadCore() {
GetTarget().SetArchitecture(arch, true /*set_platform*/);
m_thread_list = m_minidump_parser->GetThreads();
- std::vector<minidump::ExceptionStream> exception_streams =
m_minidump_parser->GetExceptionStreams();
+ 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);
+ 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;
- } 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()));
@@ -270,14 +269,12 @@ void ProcessMinidump::RefreshStateAfterStop() {
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;
}
- stop_info = StopInfo::CreateStopReasonWithSignal(
- *stop_thread, signo);
+ 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,
@@ -288,10 +285,10 @@ void ProcessMinidump::RefreshStateAfterStop() {
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,14 +402,14 @@ bool ProcessMinidump::DoUpdateThreadList(ThreadList
&old_thread_list,
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,
exception));
+ 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/ThreadMinidump.cpp
b/lldb/source/Plugins/Process/minidump/ThreadMinidump.cpp
index 3cc154f1eac47..a1d7da376b02b 100644
--- a/lldb/source/Plugins/Process/minidump/ThreadMinidump.cpp
+++ b/lldb/source/Plugins/Process/minidump/ThreadMinidump.cpp
@@ -34,7 +34,8 @@ using namespace lldb_private;
using namespace minidump;
ThreadMinidump::ThreadMinidump(Process &process, const minidump::Thread &td,
- llvm::ArrayRef<uint8_t> gpregset_data,
std::optional<minidump::Exception> exception)
+ 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_exception(exception) {}
@@ -115,12 +116,15 @@ ThreadMinidump::CreateRegisterContextForFrame(StackFrame
*frame) {
return reg_ctx_sp;
}
+// 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));
+ *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 cc336ad2d7b3d..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,
std::optional<minidump::Exception> exception);
+ llvm::ArrayRef<uint8_t> gpregset_data,
+ std::optional<minidump::Exception> exception);
~ThreadMinidump() override;
diff --git a/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
b/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
index e5355e07e41fc..aa74e690d45a5 100644
--- a/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
+++ b/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
@@ -253,7 +253,7 @@ TEST_F(MinidumpParserTest, GetExceptionStream) {
SetUpData("linux-x86_64.dmp");
llvm::Expected<std::vector<ExceptionStream>> exception_stream =
parser->GetExceptionStreams();
- // LLVM::Expected has an explicit bool operator that determines if
+ // 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());
diff --git a/llvm/include/llvm/Object/Minidump.h
b/llvm/include/llvm/Object/Minidump.h
index 87fdc555fb1ac..14fad92d1d2b6 100644
--- a/llvm/include/llvm/Object/Minidump.h
+++ b/llvm/include/llvm/Object/Minidump.h
@@ -82,10 +82,11 @@ class MinidumpFile : public Binary {
return getListStream<minidump::Thread>(minidump::StreamType::ThreadList);
}
- /// 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 {
+ /// 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");
}
@@ -94,8 +95,9 @@ class MinidumpFile : public Binary {
}
/// 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.
+ /// 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
@@ -182,7 +184,8 @@ class MinidumpFile : public Binary {
/// 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;
+ 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.
@@ -200,7 +203,8 @@ class MinidumpFile : public Binary {
};
template <typename T>
-Expected<const T &> MinidumpFile::getStreamFromDirectory(minidump::Directory
Directory) const {
+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());
diff --git a/llvm/lib/Object/Minidump.cpp b/llvm/lib/Object/Minidump.cpp
index d366b4d3d579e..43c2a4a949214 100644
--- a/llvm/lib/Object/Minidump.cpp
+++ b/llvm/lib/Object/Minidump.cpp
@@ -54,7 +54,8 @@ Expected<std::string> MinidumpFile::getString(size_t Offset)
const {
return Result;
}
-Expected<std::vector<minidump::ExceptionStream>>
MinidumpFile::getExceptionStreams() const {
+Expected<std::vector<minidump::ExceptionStream>>
+MinidumpFile::getExceptionStreams() const {
// Scan the directories for exceptions first
std::vector<Directory> exceptionStreams;
for (const auto &directory : Streams) {
@@ -67,15 +68,12 @@ Expected<std::vector<minidump::ExceptionStream>>
MinidumpFile::getExceptionStrea
std::vector<minidump::ExceptionStream> exceptionStreamList;
for (const auto &exceptionStream : exceptionStreams) {
- llvm::Expected<minidump::ExceptionStream> ExpectedStream =
getStreamFromDirectory<minidump::ExceptionStream>(exceptionStream);
+ llvm::Expected<minidump::ExceptionStream> ExpectedStream =
+ getStreamFromDirectory<minidump::ExceptionStream>(exceptionStream);
if (!ExpectedStream)
return ExpectedStream.takeError();
- std::cout << "Adding Exception Stream # " << exceptionStreamList.size() <<
std::endl;
- std::cout << "Thread Id : " << ExpectedStream->ThreadId << std::endl;
- std::cout << "Exception Code : " <<
ExpectedStream.get().ExceptionRecord.ExceptionCode << std::endl;
exceptionStreamList.push_back(ExpectedStream.get());
- assert(exceptionStreamList.back().ThreadId == ExpectedStream->ThreadId);
}
return exceptionStreamList;
@@ -172,9 +170,10 @@ MinidumpFile::create(MemoryBufferRef Source) {
}
// We treat exceptions differently here because the LLDB minidump
- // makes some assumptions about uniqueness, all the streams other than
exceptions
- // are lists. But exceptions are not a list, they are single streams that
point back to their thread
- // So we will omit them here, and will find them when needed in the
MinidumpFile.
+ // makes some assumptions about uniqueness, all the streams other than
+ // exceptions are lists. But exceptions are not a list, they are single
+ // streams that point back to their thread So we will omit them here, and
+ // will find them when needed in the MinidumpFile.
if (Type == StreamType::Exception) {
continue;
}
>From 149dbebfcc7ea7bfa08622fa349d99ca7ad8cfd1 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Tue, 2 Jul 2024 14:54:03 -0700
Subject: [PATCH 3/8] Add test case and obj yaml
---
.../minidump-new/TestMiniDumpNew.py | 14 +++++++
.../minidump-new/multiple-sigsev.yaml | 39 +++++++++++++++++++
2 files changed, 53 insertions(+)
create mode 100644
lldb/test/API/functionalities/postmortem/minidump-new/multiple-sigsev.yaml
diff --git
a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
index 91fd2439492b5..038ca819e6a0b 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
+...
>From 21b8ad1abd738acbcd2bdc365ba19b077cfd8d20 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Tue, 2 Jul 2024 15:00:00 -0700
Subject: [PATCH 4/8] Rebase and run python formatter
---
.../postmortem/minidump-new/TestMiniDumpNew.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git
a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
index 038ca819e6a0b..ba93eff6f1f82 100644
--- a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
+++ b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
@@ -494,9 +494,9 @@ def test_minidump_sysroot(self):
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")
+ print("Starting to read multiple-sigsev.yaml")
self.process_from_yaml("multiple-sigsev.yaml")
- print ("Done reading 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)
>From 4af38387217a771fcfe0b3bdeab46b4c6a16ae8a Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Wed, 3 Jul 2024 11:31:37 -0700
Subject: [PATCH 5/8] Remove ThreadMinidump calculate stop info which was
causing a hang in test, reset them to main. Run git clang format and run
tests.
---
.../Process/minidump/ProcessMinidump.cpp | 8 ++------
.../Process/minidump/ThreadMinidump.cpp | 18 +++---------------
.../Plugins/Process/minidump/ThreadMinidump.h | 4 +---
3 files changed, 6 insertions(+), 24 deletions(-)
diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
index 9a7e481b92796..5afaa01d91fb3 100644
--- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -395,12 +395,9 @@ 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_exceptions_by_tid.count(thread.ThreadId) > 0) {
+ 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)
@@ -408,8 +405,7 @@ bool ProcessMinidump::DoUpdateThreadList(ThreadList
&old_thread_list,
else
context = m_minidump_parser->GetThreadContextWow64(thread);
- lldb::ThreadSP thread_sp(
- new ThreadMinidump(*this, thread, context, exception));
+ lldb::ThreadSP thread_sp(new ThreadMinidump(*this, thread, context));
new_thread_list.AddThread(thread_sp);
}
return new_thread_list.GetSize(false) > 0;
diff --git a/lldb/source/Plugins/Process/minidump/ThreadMinidump.cpp
b/lldb/source/Plugins/Process/minidump/ThreadMinidump.cpp
index a1d7da376b02b..1fbc52815238b 100644
--- a/lldb/source/Plugins/Process/minidump/ThreadMinidump.cpp
+++ b/lldb/source/Plugins/Process/minidump/ThreadMinidump.cpp
@@ -34,10 +34,9 @@ using namespace lldb_private;
using namespace minidump;
ThreadMinidump::ThreadMinidump(Process &process, const minidump::Thread &td,
- llvm::ArrayRef<uint8_t> gpregset_data,
- std::optional<minidump::Exception> exception)
+ llvm::ArrayRef<uint8_t> gpregset_data)
: Thread(process, td.ThreadId), m_thread_reg_ctx_sp(),
- m_gpregset_data(gpregset_data), m_exception(exception) {}
+ m_gpregset_data(gpregset_data) {}
ThreadMinidump::~ThreadMinidump() = default;
@@ -116,15 +115,4 @@ ThreadMinidump::CreateRegisterContextForFrame(StackFrame
*frame) {
return reg_ctx_sp;
}
-// 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;
-}
+bool ThreadMinidump::CalculateStopInfo() { return false; }
diff --git a/lldb/source/Plugins/Process/minidump/ThreadMinidump.h
b/lldb/source/Plugins/Process/minidump/ThreadMinidump.h
index abde13ccb3f2a..aed7cfbc1b161 100644
--- a/lldb/source/Plugins/Process/minidump/ThreadMinidump.h
+++ b/lldb/source/Plugins/Process/minidump/ThreadMinidump.h
@@ -21,8 +21,7 @@ namespace minidump {
class ThreadMinidump : public Thread {
public:
ThreadMinidump(Process &process, const minidump::Thread &td,
- llvm::ArrayRef<uint8_t> gpregset_data,
- std::optional<minidump::Exception> exception);
+ llvm::ArrayRef<uint8_t> gpregset_data);
~ThreadMinidump() override;
@@ -36,7 +35,6 @@ 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;
};
>From d70784ce01cb66356fff6553ce5f6495488fc8b4 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Mon, 8 Jul 2024 11:00:24 -0700
Subject: [PATCH 6/8] Implement feedback, return const * vector instead of list
of copies
---
.../Process/minidump/MinidumpParser.cpp | 4 +-
.../Plugins/Process/minidump/MinidumpParser.h | 3 +-
.../Process/minidump/ProcessMinidump.cpp | 52 +++++++++++--------
.../Process/minidump/ProcessMinidump.h | 3 +-
.../Process/minidump/MinidumpParserTest.cpp | 6 +--
llvm/include/llvm/Object/Minidump.h | 5 +-
llvm/lib/Object/Minidump.cpp | 28 +++++-----
7 files changed, 54 insertions(+), 47 deletions(-)
diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
index c51fed8d91b07..49a3c275b7271 100644
--- a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
+++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -417,7 +417,7 @@ std::vector<const minidump::Module *>
MinidumpParser::GetFilteredModuleList() {
return filtered_modules;
}
-const std::vector<minidump::ExceptionStream>
+std::optional<std::vector<const minidump::ExceptionStream *>>
MinidumpParser::GetExceptionStreams() {
auto ExpectedStream = GetMinidumpFile().getExceptionStreams();
if (ExpectedStream)
@@ -426,7 +426,7 @@ MinidumpParser::GetExceptionStreams() {
LLDB_LOG_ERROR(GetLog(LLDBLog::Process), ExpectedStream.takeError(),
"Failed to read minidump exception stream: {0}");
// return empty on failure.
- return std::vector<minidump::ExceptionStream>();
+ 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 e552c7210e330..9215f942e26e5 100644
--- a/lldb/source/Plugins/Process/minidump/MinidumpParser.h
+++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.h
@@ -82,7 +82,8 @@ class MinidumpParser {
// have the same name, it keeps the copy with the lowest load address.
std::vector<const minidump::Module *> GetFilteredModuleList();
- const std::vector<llvm::minidump::ExceptionStream> GetExceptionStreams();
+ 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 5afaa01d91fb3..baa4dde71cd82 100644
--- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -208,18 +208,26 @@ Status ProcessMinidump::DoLoadCore() {
GetTarget().SetArchitecture(arch, true /*set_platform*/);
m_thread_list = m_minidump_parser->GetThreads();
- 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;
+ 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;
+ }
}
}
@@ -244,9 +252,9 @@ Status ProcessMinidump::DoDestroy() { return Status(); }
void ProcessMinidump::RefreshStateAfterStop() {
- for (auto &[_, exception_stream] : m_exceptions_by_tid) {
+ 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.
@@ -263,12 +271,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;
@@ -277,18 +285,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());
}
@@ -397,7 +405,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 0379de7bee5e6..720af51de5d46 100644
--- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.h
+++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.h
@@ -109,7 +109,8 @@ class ProcessMinidump : public PostMortemProcess {
private:
lldb::DataBufferSP m_core_data;
llvm::ArrayRef<minidump::Thread> m_thread_list;
- std::unordered_map<uint32_t, minidump::ExceptionStream> m_exceptions_by_tid;
+ 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/unittests/Process/minidump/MinidumpParserTest.cpp
b/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
index aa74e690d45a5..1b496c18091fc 100644
--- a/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
+++ b/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
@@ -251,13 +251,13 @@ TEST_F(MinidumpParserTest, GetFilteredModuleList) {
TEST_F(MinidumpParserTest, GetExceptionStream) {
SetUpData("linux-x86_64.dmp");
- llvm::Expected<std::vector<ExceptionStream>> exception_stream =
- parser->GetExceptionStreams();
+ std::optional<std::vector<const minidump::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);
+ ASSERT_EQ(11UL, exception_stream->at(0)->ThreadId);
}
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
index 14fad92d1d2b6..a589b8b307396 100644
--- a/llvm/include/llvm/Object/Minidump.h
+++ b/llvm/include/llvm/Object/Minidump.h
@@ -85,7 +85,7 @@ class MinidumpFile : public Binary {
/// 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>
+ Expected<const minidump::ExceptionStream &>
getExceptionStream(minidump::Directory Directory) const {
if (Directory.Type != minidump::StreamType::Exception) {
return createError("Not an exception stream");
@@ -98,7 +98,8 @@ class MinidumpFile : public Binary {
/// 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;
+ Expected<std::vector<const 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
diff --git a/llvm/lib/Object/Minidump.cpp b/llvm/lib/Object/Minidump.cpp
index 43c2a4a949214..ede5b53389070 100644
--- a/llvm/lib/Object/Minidump.cpp
+++ b/llvm/lib/Object/Minidump.cpp
@@ -54,28 +54,24 @@ Expected<std::string> MinidumpFile::getString(size_t
Offset) const {
return Result;
}
-Expected<std::vector<minidump::ExceptionStream>>
+Expected<std::vector<const minidump::ExceptionStream *>>
MinidumpFile::getExceptionStreams() const {
- // Scan the directories for exceptions first
- std::vector<Directory> exceptionStreams;
+
+ std::vector<const minidump::ExceptionStream *> exceptionStreamList;
for (const auto &directory : Streams) {
- if (directory.Type == StreamType::Exception)
- exceptionStreams.push_back(directory);
+ if (directory.Type == StreamType::Exception) {
+ llvm::Expected<minidump::ExceptionStream *> ExpectedStream =
+ getStreamFromDirectory<minidump::ExceptionStream *>(directory);
+ if (!ExpectedStream)
+ return ExpectedStream.takeError();
+
+ exceptionStreamList.push_back(ExpectedStream.get());
+ }
}
- if (exceptionStreams.empty())
+ if (exceptionStreamList.empty())
return createError("No exception streams found");
- std::vector<minidump::ExceptionStream> exceptionStreamList;
- for (const auto &exceptionStream : exceptionStreams) {
- llvm::Expected<minidump::ExceptionStream> ExpectedStream =
- getStreamFromDirectory<minidump::ExceptionStream>(exceptionStream);
- if (!ExpectedStream)
- return ExpectedStream.takeError();
-
- exceptionStreamList.push_back(ExpectedStream.get());
- }
-
return exceptionStreamList;
}
>From 5b6da6c0be92bab938cdb777ba3097b66debf561 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Tue, 9 Jul 2024 10:05:46 -0700
Subject: [PATCH 7/8] Cleanup unsured references, and remove incorrect comment
---
lldb/source/Plugins/Process/minidump/MinidumpParser.cpp | 2 +-
lldb/unittests/Process/minidump/MinidumpParserTest.cpp | 4 +---
llvm/lib/Object/Minidump.cpp | 1 -
3 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
index 49a3c275b7271..3e4f6b1831b4b 100644
--- a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
+++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -425,7 +425,7 @@ MinidumpParser::GetExceptionStreams() {
LLDB_LOG_ERROR(GetLog(LLDBLog::Process), ExpectedStream.takeError(),
"Failed to read minidump exception stream: {0}");
- // return empty on failure.
+
return std::nullopt;
}
diff --git a/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
b/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
index 1b496c18091fc..c27fce80d67a1 100644
--- a/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
+++ b/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
@@ -253,9 +253,7 @@ TEST_F(MinidumpParserTest, GetExceptionStream) {
SetUpData("linux-x86_64.dmp");
std::optional<std::vector<const minidump::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_TRUE(exception_stream);
ASSERT_EQ(1UL, exception_stream->size());
ASSERT_EQ(11UL, exception_stream->at(0)->ThreadId);
}
diff --git a/llvm/lib/Object/Minidump.cpp b/llvm/lib/Object/Minidump.cpp
index ede5b53389070..bed44494645a5 100644
--- a/llvm/lib/Object/Minidump.cpp
+++ b/llvm/lib/Object/Minidump.cpp
@@ -9,7 +9,6 @@
#include "llvm/Object/Minidump.h"
#include "llvm/Object/Error.h"
#include "llvm/Support/ConvertUTF.h"
-#include <iostream>
using namespace llvm;
using namespace llvm::object;
>From a8c1683a3d64845a2380f1858c36cbbc570ce2e0 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Tue, 9 Jul 2024 10:23:22 -0700
Subject: [PATCH 8/8] Make MinidumpYAML const ref not a copy
---
llvm/lib/ObjectYAML/MinidumpYAML.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/ObjectYAML/MinidumpYAML.cpp
b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
index e00feceea57af..47a5a27d5798f 100644
--- a/llvm/lib/ObjectYAML/MinidumpYAML.cpp
+++ b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
@@ -464,7 +464,7 @@ Stream::create(const Directory &StreamDesc, const
object::MinidumpFile &File) {
StreamKind Kind = getKind(StreamDesc.Type);
switch (Kind) {
case StreamKind::Exception: {
- Expected<minidump::ExceptionStream> ExpectedExceptionStream =
+ Expected<const minidump::ExceptionStream &> ExpectedExceptionStream =
File.getExceptionStream(StreamDesc);
if (!ExpectedExceptionStream)
return ExpectedExceptionStream.takeError();
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
