From deba13409245aabf3fda8b82a873336ea5238d3a Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Fri, 6 Sep 2024 09:04:12 -0700 Subject: [Minidump] Support multiple exceptions in a minidump (#107319) A fork of #97470, splitting off the LLVM changes from the LLDB specific changes. This patch enables a minidump file to have multiple exceptions, exposed via an iterator of Expected streams. --- llvm/include/llvm/Object/Minidump.h | 83 +++++++++++++++++++++++--- llvm/lib/Object/Minidump.cpp | 17 +++++- llvm/lib/ObjectYAML/MinidumpYAML.cpp | 2 +- llvm/unittests/Object/MinidumpTest.cpp | 11 +++- llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp | 61 ++++++++++++++++--- 5 files changed, 153 insertions(+), 21 deletions(-) diff --git a/llvm/include/llvm/Object/Minidump.h b/llvm/include/llvm/Object/Minidump.h index 65ad6b1..e6b21979 100644 --- a/llvm/include/llvm/Object/Minidump.h +++ b/llvm/include/llvm/Object/Minidump.h @@ -83,13 +83,26 @@ public: return getListStream(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. + /// 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 + getExceptionStream(minidump::Directory Directory) const { + if (Directory.Type != minidump::StreamType::Exception) { + return createError("Not an exception stream"); + } + + return getStreamFromDirectory(Directory); + } + + /// Returns the first exception stream in the file. 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 getExceptionStream() const { - return getStream( - minidump::StreamType::Exception); + auto it = getExceptionStreams(); + if (it.begin() == it.end()) + return createError("No exception streams"); + return *it.begin(); } /// Returns the list of descriptors embedded in the MemoryList stream. The @@ -216,8 +229,44 @@ public: bool IsEnd; }; + class ExceptionStreamsIterator { + public: + ExceptionStreamsIterator(ArrayRef Streams, + const MinidumpFile *File) + : Streams(Streams), File(File) {} + + bool operator==(const ExceptionStreamsIterator &R) const { + return Streams.size() == R.Streams.size(); + } + + bool operator!=(const ExceptionStreamsIterator &R) const { + return !(*this == R); + } + + Expected operator*() { + return File->getExceptionStream(Streams.front()); + } + + ExceptionStreamsIterator &operator++() { + if (!Streams.empty()) + Streams = Streams.drop_front(); + + return *this; + } + + private: + ArrayRef Streams; + const MinidumpFile *File; + }; + using FallibleMemory64Iterator = llvm::fallible_iterator; + /// Returns an iterator that reads each exception stream independently. The + /// contents of the exception strema are not validated before being read, an + /// error will be returned if the stream is not large enough to contain an + /// exception stream, or if the stream points beyond the end of the file. + iterator_range getExceptionStreams() const; + /// Returns an iterator that pairs each descriptor with it's respective /// content from the Memory64List stream. An error is returned if the file /// does not contain a Memory64List stream, or if the descriptor data is @@ -256,9 +305,11 @@ private: MinidumpFile(MemoryBufferRef Source, const minidump::Header &Header, ArrayRef Streams, - DenseMap StreamMap) + DenseMap StreamMap, + std::vector ExceptionStreams) : Binary(ID_Minidump, Source), Header(Header), Streams(Streams), - StreamMap(std::move(StreamMap)) {} + StreamMap(std::move(StreamMap)), + ExceptionStreams(std::move(ExceptionStreams)) {} ArrayRef getData() const { return arrayRefFromStringRef(Data.getBuffer()); @@ -267,6 +318,12 @@ private: /// 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 + Expected + 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 Expected getStream(minidump::StreamType Stream) const; /// Return the contents of a stream which contains a list of fixed-size items, @@ -277,9 +334,19 @@ private: const minidump::Header &Header; ArrayRef Streams; DenseMap StreamMap; + std::vector ExceptionStreams; }; template +Expected +MinidumpFile::getStreamFromDirectory(minidump::Directory Directory) const { + ArrayRef Stream = getRawStream(Directory); + if (Stream.size() >= sizeof(T)) + return *reinterpret_cast(Stream.data()); + return createEOFError(); +} + +template Expected MinidumpFile::getStream(minidump::StreamType Type) const { if (std::optional> Stream = getRawStream(Type)) { if (Stream->size() >= sizeof(T)) diff --git a/llvm/lib/Object/Minidump.cpp b/llvm/lib/Object/Minidump.cpp index 93b2e2c..fe768c4 100644 --- a/llvm/lib/Object/Minidump.cpp +++ b/llvm/lib/Object/Minidump.cpp @@ -53,6 +53,12 @@ Expected MinidumpFile::getString(size_t Offset) const { return Result; } +iterator_range +MinidumpFile::getExceptionStreams() const { + return make_range(ExceptionStreamsIterator(ExceptionStreams, this), + ExceptionStreamsIterator({}, this)); +} + Expected> MinidumpFile::getMemoryInfoList() const { std::optional> Stream = @@ -128,6 +134,7 @@ MinidumpFile::create(MemoryBufferRef Source) { return ExpectedStreams.takeError(); DenseMap StreamMap; + std::vector ExceptionStreams; for (const auto &StreamDescriptor : llvm::enumerate(*ExpectedStreams)) { StreamType Type = StreamDescriptor.value().Type; const LocationDescriptor &Loc = StreamDescriptor.value().Location; @@ -143,6 +150,13 @@ MinidumpFile::create(MemoryBufferRef Source) { continue; } + // Exceptions can be treated as a special case of streams. Other streams + // represent a list of entities, but exceptions are unique per stream. + if (Type == StreamType::Exception) { + ExceptionStreams.push_back(StreamDescriptor.value()); + continue; + } + if (Type == DenseMapInfo::getEmptyKey() || Type == DenseMapInfo::getTombstoneKey()) return createError("Cannot handle one of the minidump streams"); @@ -153,7 +167,8 @@ MinidumpFile::create(MemoryBufferRef Source) { } return std::unique_ptr( - new MinidumpFile(Source, Hdr, *ExpectedStreams, std::move(StreamMap))); + new MinidumpFile(Source, Hdr, *ExpectedStreams, std::move(StreamMap), + std::move(ExceptionStreams))); } iterator_range diff --git a/llvm/lib/ObjectYAML/MinidumpYAML.cpp b/llvm/lib/ObjectYAML/MinidumpYAML.cpp index 10b8676..1818823 100644 --- a/llvm/lib/ObjectYAML/MinidumpYAML.cpp +++ b/llvm/lib/ObjectYAML/MinidumpYAML.cpp @@ -499,7 +499,7 @@ Stream::create(const Directory &StreamDesc, const object::MinidumpFile &File) { switch (Kind) { case StreamKind::Exception: { Expected ExpectedExceptionStream = - File.getExceptionStream(); + File.getExceptionStream(StreamDesc); if (!ExpectedExceptionStream) return ExpectedExceptionStream.takeError(); Expected> ExpectedThreadContext = diff --git a/llvm/unittests/Object/MinidumpTest.cpp b/llvm/unittests/Object/MinidumpTest.cpp index d2d9f11..44a9661 100644 --- a/llvm/unittests/Object/MinidumpTest.cpp +++ b/llvm/unittests/Object/MinidumpTest.cpp @@ -711,7 +711,7 @@ TEST(MinidumpFile, getMemoryInfoList) { 0x0001000908000000u)); } -TEST(MinidumpFile, getExceptionStream) { +TEST(MinidumpFile, getExceptionStreams) { std::vector Data{ // Header 'M', 'D', 'M', 'P', 0x93, 0xa7, 0, 0, // Signature, Version @@ -751,8 +751,11 @@ TEST(MinidumpFile, getExceptionStream) { auto ExpectedFile = create(Data); ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded()); const MinidumpFile &File = **ExpectedFile; - Expected ExpectedStream = - File.getExceptionStream(); + + auto ExceptionStreams = File.getExceptionStreams(); + ASSERT_NE(ExceptionStreams.begin(), ExceptionStreams.end()); + auto ExceptionIterator = ExceptionStreams.begin(); + Expected ExpectedStream = *ExceptionIterator; ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded()); EXPECT_EQ(0x04030201u, ExpectedStream->ThreadId); const minidump::Exception &Exception = ExpectedStream->ExceptionRecord; @@ -767,4 +770,6 @@ TEST(MinidumpFile, getExceptionStream) { } EXPECT_EQ(0x84838281, ExpectedStream->ThreadContext.DataSize); EXPECT_EQ(0x88878685, ExpectedStream->ThreadContext.RVA); + ++ExceptionIterator; + ASSERT_EQ(ExceptionIterator, ExceptionStreams.end()); } diff --git a/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp b/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp index a8b8da9..c805665 100644 --- a/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp +++ b/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp @@ -162,8 +162,9 @@ Streams: ASSERT_EQ(1u, File.streams().size()); - Expected ExpectedStream = - File.getExceptionStream(); + auto ExceptionIterator = File.getExceptionStreams().begin(); + + Expected ExpectedStream = *ExceptionIterator; ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded()); @@ -205,9 +206,9 @@ Streams: ASSERT_EQ(1u, File.streams().size()); - Expected ExpectedStream = - File.getExceptionStream(); + auto ExceptionIterator = File.getExceptionStreams().begin(); + Expected ExpectedStream = *ExceptionIterator; ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded()); const minidump::ExceptionStream &Stream = *ExpectedStream; @@ -261,8 +262,9 @@ Streams: ASSERT_EQ(1u, File.streams().size()); - Expected ExpectedStream = - File.getExceptionStream(); + auto ExceptionIterator = File.getExceptionStreams().begin(); + + Expected ExpectedStream = *ExceptionIterator; ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded()); @@ -312,8 +314,9 @@ Streams: ASSERT_EQ(1u, File.streams().size()); - Expected ExpectedStream = - File.getExceptionStream(); + auto ExceptionIterator = File.getExceptionStreams().begin(); + + Expected ExpectedStream = *ExceptionIterator; ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded()); @@ -399,3 +402,45 @@ Streams: ASSERT_EQ(Iterator, MemoryList.end()); } + +// Test that we can parse multiple exception streams. +TEST(MinidumpYAML, ExceptionStream_MultipleExceptions) { + SmallString<0> Storage; + auto ExpectedFile = toBinary(Storage, R"( +--- !minidump +Streams: + - Type: Exception + Thread ID: 0x7 + Exception Record: + Exception Code: 0x23 + Exception Flags: 0x5 + Exception Record: 0x0102030405060708 + Exception Address: 0x0a0b0c0d0e0f1011 + Number of Parameters: 2 + Parameter 0: 0x99 + Parameter 1: 0x23 + Parameter 2: 0x42 + Thread Context: 3DeadBeefDefacedABadCafe + - Type: Exception + Thread ID: 0x5 + Exception Record: + Exception Code: 0x23 + Exception Flags: 0x5 + Exception Record: 0x0102030405060708 + Exception Address: 0x0a0b0c0d0e0f1011 + Thread Context: 3DeadBeefDefacedABadCafe)"); + + ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded()); + object::MinidumpFile &File = **ExpectedFile; + + ASSERT_EQ(2u, File.streams().size()); + + size_t count = 0; + for (auto exception_stream : File.getExceptionStreams()) { + count++; + ASSERT_THAT_EXPECTED(exception_stream, Succeeded()); + ASSERT_THAT(0x23u, exception_stream->ExceptionRecord.ExceptionCode); + } + + ASSERT_THAT(2u, count); +} -- cgit v1.1