https://github.com/Jlalond updated
https://github.com/llvm/llvm-project/pull/107319
>From 70f94e152ba2a6826a3c4f8071b5e0ee2bb81d2d Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Wed, 4 Sep 2024 14:06:04 -0700
Subject: [PATCH 1/7] Cherry-pick llvm only changes from minidump multiple
exceptions PR.
---
llvm/include/llvm/Object/Minidump.h | 39 +++++++++++++++++++++++-----
llvm/lib/Object/Minidump.cpp | 31 ++++++++++++++++++++++
llvm/lib/ObjectYAML/MinidumpYAML.cpp | 2 +-
3 files changed, 64 insertions(+), 8 deletions(-)
diff --git a/llvm/include/llvm/Object/Minidump.h
b/llvm/include/llvm/Object/Minidump.h
index 65ad6b171c2dc1..eb6f4a2714340a 100644
--- a/llvm/include/llvm/Object/Minidump.h
+++ b/llvm/include/llvm/Object/Minidump.h
@@ -83,15 +83,25 @@ 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<const 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<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
/// time the minidump was taken. An error is returned if the file does not
@@ -264,6 +274,12 @@ 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>
@@ -279,6 +295,15 @@ 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 93b2e2cecd1053..cd78f1b10ca17a 100644
--- a/llvm/lib/Object/Minidump.cpp
+++ b/llvm/lib/Object/Minidump.cpp
@@ -53,6 +53,27 @@ Expected<std::string> MinidumpFile::getString(size_t Offset)
const {
return Result;
}
+Expected<std::vector<const minidump::ExceptionStream *>>
+MinidumpFile::getExceptionStreams() const {
+
+ std::vector<const minidump::ExceptionStream *> exceptionStreamList;
+ for (const auto &directory : Streams) {
+ 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 (exceptionStreamList.empty())
+ return createError("No exception streams found");
+
+ return exceptionStreamList;
+}
+
Expected<iterator_range<MinidumpFile::MemoryInfoIterator>>
MinidumpFile::getMemoryInfoList() const {
std::optional<ArrayRef<uint8_t>> Stream =
@@ -128,6 +149,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;
@@ -143,6 +165,15 @@ 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 10b8676b5c4cfb..1818823180b7d4 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<const minidump::ExceptionStream &> ExpectedExceptionStream =
- File.getExceptionStream();
+ File.getExceptionStream(StreamDesc);
if (!ExpectedExceptionStream)
return ExpectedExceptionStream.takeError();
Expected<ArrayRef<uint8_t>> ExpectedThreadContext =
>From a33bb5902f99f3e0ec3ac01f9dca7c1781af6404 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Wed, 4 Sep 2024 14:45:58 -0700
Subject: [PATCH 2/7] Add minidump exception iterator
---
llvm/include/llvm/Object/Minidump.h | 76 +++++++++++++++++++++++++----
llvm/lib/Object/Minidump.cpp | 27 +++-------
2 files changed, 73 insertions(+), 30 deletions(-)
diff --git a/llvm/include/llvm/Object/Minidump.h
b/llvm/include/llvm/Object/Minidump.h
index eb6f4a2714340a..3df0ba6c0656ae 100644
--- a/llvm/include/llvm/Object/Minidump.h
+++ b/llvm/include/llvm/Object/Minidump.h
@@ -95,13 +95,6 @@ class MinidumpFile : public Binary {
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<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
/// time the minidump was taken. An error is returned if the file does not
@@ -226,8 +219,71 @@ class MinidumpFile : public Binary {
bool IsEnd;
};
+class ExceptionStreamsIterator {
+ public:
+
+ static ExceptionStreamsIterator begin(ArrayRef<minidump::Directory>
Streams, const MinidumpFile *File) {
+ return ExceptionStreamsIterator(Streams, File);
+ }
+
+ static ExceptionStreamsIterator end() {
+ return ExceptionStreamsIterator();
+ }
+
+ bool operator==(const ExceptionStreamsIterator &R) const {
+ return Streams.empty() && R.Streams.empty();
+ }
+
+ bool operator!=(const ExceptionStreamsIterator &R) const { return !(*this
== R); }
+
+ const Expected<const minidump::ExceptionStream &>
+ operator*() {
+ return ReadCurrent();
+ }
+
+ const Expected<const minidump::ExceptionStream &>
+ operator->() {
+ return ReadCurrent();
+ }
+
+ ExceptionStreamsIterator &
+ operator++ () {
+ if (!Streams.empty())
+ Streams = Streams.drop_front();
+
+
+ return *this;
+ }
+
+ private:
+ ExceptionStreamsIterator(ArrayRef<minidump::Directory> Streams, const
MinidumpFile *File)
+ : Streams(Streams), File(File) {}
+
+ ExceptionStreamsIterator() : Streams(ArrayRef<minidump::Directory>()),
File(nullptr) {}
+
+ ArrayRef<minidump::Directory> Streams;
+ const MinidumpFile *File;
+
+ Expected<const minidump::ExceptionStream&> ReadCurrent() {
+ assert(!Streams.empty());
+ Expected<const minidump::ExceptionStream &> ExceptionStream =
+ File->getExceptionStream(Streams.front());
+ if (!ExceptionStream)
+ return ExceptionStream.takeError();
+
+ return ExceptionStream;
+ }
+ };
+
using FallibleMemory64Iterator = llvm::fallible_iterator<Memory64Iterator>;
+ /// 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<ExceptionStreamsIterator>
+ 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
@@ -266,9 +322,10 @@ class MinidumpFile : public Binary {
MinidumpFile(MemoryBufferRef Source, const minidump::Header &Header,
ArrayRef<minidump::Directory> Streams,
- DenseMap<minidump::StreamType, std::size_t> StreamMap)
+ DenseMap<minidump::StreamType, std::size_t> StreamMap,
+ std::vector<minidump::Directory> ExceptionStreams)
: Binary(ID_Minidump, Source), Header(Header), Streams(Streams),
- StreamMap(std::move(StreamMap)) {}
+ StreamMap(std::move(StreamMap)),
ExceptionStreams(std::move(ExceptionStreams)) {}
ArrayRef<uint8_t> getData() const {
return arrayRefFromStringRef(Data.getBuffer());
@@ -293,6 +350,7 @@ class MinidumpFile : public Binary {
const minidump::Header &Header;
ArrayRef<minidump::Directory> Streams;
DenseMap<minidump::StreamType, std::size_t> StreamMap;
+ std::vector<minidump::Directory> ExceptionStreams;
};
template <typename T>
diff --git a/llvm/lib/Object/Minidump.cpp b/llvm/lib/Object/Minidump.cpp
index cd78f1b10ca17a..200aa5720ec2ca 100644
--- a/llvm/lib/Object/Minidump.cpp
+++ b/llvm/lib/Object/Minidump.cpp
@@ -53,25 +53,10 @@ Expected<std::string> MinidumpFile::getString(size_t
Offset) const {
return Result;
}
-Expected<std::vector<const minidump::ExceptionStream *>>
+iterator_range<llvm::object::MinidumpFile::ExceptionStreamsIterator>
MinidumpFile::getExceptionStreams() const {
-
- std::vector<const minidump::ExceptionStream *> exceptionStreamList;
- for (const auto &directory : Streams) {
- 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 (exceptionStreamList.empty())
- return createError("No exception streams found");
-
- return exceptionStreamList;
+ return make_range(ExceptionStreamsIterator::begin(ExceptionStreams, this),
+ ExceptionStreamsIterator::end());
}
Expected<iterator_range<MinidumpFile::MemoryInfoIterator>>
@@ -149,7 +134,7 @@ MinidumpFile::create(MemoryBufferRef Source) {
return ExpectedStreams.takeError();
DenseMap<StreamType, std::size_t> StreamMap;
- std::vector<std::size_t> ExceptionStreams;
+ std::vector<Directory> ExceptionStreams;
for (const auto &StreamDescriptor : llvm::enumerate(*ExpectedStreams)) {
StreamType Type = StreamDescriptor.value().Type;
const LocationDescriptor &Loc = StreamDescriptor.value().Location;
@@ -171,7 +156,7 @@ MinidumpFile::create(MemoryBufferRef Source) {
// 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;
+ ExceptionStreams.push_back(StreamDescriptor.value());
}
if (Type == DenseMapInfo<StreamType>::getEmptyKey() ||
@@ -184,7 +169,7 @@ MinidumpFile::create(MemoryBufferRef Source) {
}
return std::unique_ptr<MinidumpFile>(
- new MinidumpFile(Source, Hdr, *ExpectedStreams, std::move(StreamMap)));
+ new MinidumpFile(Source, Hdr, *ExpectedStreams, std::move(StreamMap),
ExceptionStreams));
}
iterator_range<MinidumpFile::FallibleMemory64Iterator>
>From c67ad4098a5ffe220eae82d53d8b0e391318cae2 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Wed, 4 Sep 2024 15:08:56 -0700
Subject: [PATCH 3/7] Migrate the unit tests, and add a basic test to ensure we
can read multiple exception streams
---
llvm/include/llvm/Object/Minidump.h | 4 +-
llvm/lib/Object/Minidump.cpp | 8 +--
llvm/unittests/Object/MinidumpTest.cpp | 6 +-
.../unittests/ObjectYAML/MinidumpYAMLTest.cpp | 66 ++++++++++++++++---
4 files changed, 67 insertions(+), 17 deletions(-)
diff --git a/llvm/include/llvm/Object/Minidump.h
b/llvm/include/llvm/Object/Minidump.h
index 3df0ba6c0656ae..cf3d88889e900e 100644
--- a/llvm/include/llvm/Object/Minidump.h
+++ b/llvm/include/llvm/Object/Minidump.h
@@ -236,12 +236,12 @@ class ExceptionStreamsIterator {
bool operator!=(const ExceptionStreamsIterator &R) const { return !(*this
== R); }
- const Expected<const minidump::ExceptionStream &>
+ Expected<const minidump::ExceptionStream &>
operator*() {
return ReadCurrent();
}
- const Expected<const minidump::ExceptionStream &>
+ Expected<const minidump::ExceptionStream &>
operator->() {
return ReadCurrent();
}
diff --git a/llvm/lib/Object/Minidump.cpp b/llvm/lib/Object/Minidump.cpp
index 200aa5720ec2ca..f334cd2a9a2e71 100644
--- a/llvm/lib/Object/Minidump.cpp
+++ b/llvm/lib/Object/Minidump.cpp
@@ -150,13 +150,11 @@ 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.
+ // 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<StreamType>::getEmptyKey() ||
diff --git a/llvm/unittests/Object/MinidumpTest.cpp
b/llvm/unittests/Object/MinidumpTest.cpp
index d2d9f115bb94a9..6dcdccc4dea81e 100644
--- a/llvm/unittests/Object/MinidumpTest.cpp
+++ b/llvm/unittests/Object/MinidumpTest.cpp
@@ -751,8 +751,10 @@ TEST(MinidumpFile, getExceptionStream) {
auto ExpectedFile = create(Data);
ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
const MinidumpFile &File = **ExpectedFile;
- Expected<const minidump::ExceptionStream &> ExpectedStream =
- File.getExceptionStream();
+ auto ExceptionIterator =
+ File.getExceptionStreams().begin();
+
+ Expected<const ExceptionStream &> ExpectedStream = *ExceptionIterator;
ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
EXPECT_EQ(0x04030201u, ExpectedStream->ThreadId);
const minidump::Exception &Exception = ExpectedStream->ExceptionRecord;
diff --git a/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
b/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
index a8b8da925d21d9..6b2963902a24b8 100644
--- a/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
+++ b/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
@@ -162,8 +162,10 @@ TEST(MinidumpYAML, ExceptionStream) {
ASSERT_EQ(1u, File.streams().size());
- Expected<const minidump::ExceptionStream &> ExpectedStream =
- File.getExceptionStream();
+ auto ExceptionIterator =
+ File.getExceptionStreams().begin();
+
+ Expected<const ExceptionStream &> ExpectedStream = *ExceptionIterator;
ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
@@ -205,9 +207,10 @@ TEST(MinidumpYAML, ExceptionStream_NoParameters) {
ASSERT_EQ(1u, File.streams().size());
- Expected<const minidump::ExceptionStream &> ExpectedStream =
- File.getExceptionStream();
+ auto ExceptionIterator =
+ File.getExceptionStreams().begin();
+ Expected<const ExceptionStream &> ExpectedStream = *ExceptionIterator;
ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
const minidump::ExceptionStream &Stream = *ExpectedStream;
@@ -261,8 +264,10 @@ TEST(MinidumpYAML, ExceptionStream_TooManyParameters) {
ASSERT_EQ(1u, File.streams().size());
- Expected<const minidump::ExceptionStream &> ExpectedStream =
- File.getExceptionStream();
+ auto ExceptionIterator =
+ File.getExceptionStreams().begin();
+
+ Expected<const ExceptionStream &> ExpectedStream = *ExceptionIterator;
ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
@@ -312,8 +317,10 @@ TEST(MinidumpYAML, ExceptionStream_ExtraParameter) {
ASSERT_EQ(1u, File.streams().size());
- Expected<const minidump::ExceptionStream &> ExpectedStream =
- File.getExceptionStream();
+ auto ExceptionIterator =
+ File.getExceptionStreams().begin();
+
+ Expected<const ExceptionStream &> ExpectedStream = *ExceptionIterator;
ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
@@ -398,4 +405,47 @@ TEST(MinidumpYAML, MemoryRegion_64bit) {
ASSERT_THAT(*DescTwoExpectedContentSlice, arrayRefFromStringRef("world"));
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);
}
>From 3a4e59824fcc1a966176f11e208c91460ad19f38 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Wed, 4 Sep 2024 15:28:20 -0700
Subject: [PATCH 4/7] Run GCF
---
llvm/include/llvm/Object/Minidump.h | 46 +++++++++----------
llvm/lib/Object/Minidump.cpp | 4 +-
llvm/unittests/Object/MinidumpTest.cpp | 3 +-
.../unittests/ObjectYAML/MinidumpYAMLTest.cpp | 15 ++----
4 files changed, 30 insertions(+), 38 deletions(-)
diff --git a/llvm/include/llvm/Object/Minidump.h
b/llvm/include/llvm/Object/Minidump.h
index cf3d88889e900e..1e9bc741d78725 100644
--- a/llvm/include/llvm/Object/Minidump.h
+++ b/llvm/include/llvm/Object/Minidump.h
@@ -219,70 +219,67 @@ class MinidumpFile : public Binary {
bool IsEnd;
};
-class ExceptionStreamsIterator {
+ class ExceptionStreamsIterator {
public:
-
- static ExceptionStreamsIterator begin(ArrayRef<minidump::Directory>
Streams, const MinidumpFile *File) {
+ static ExceptionStreamsIterator begin(ArrayRef<minidump::Directory>
Streams,
+ const MinidumpFile *File) {
return ExceptionStreamsIterator(Streams, File);
}
- static ExceptionStreamsIterator end() {
- return ExceptionStreamsIterator();
- }
+ static ExceptionStreamsIterator end() { return ExceptionStreamsIterator();
}
bool operator==(const ExceptionStreamsIterator &R) const {
return Streams.empty() && R.Streams.empty();
}
- bool operator!=(const ExceptionStreamsIterator &R) const { return !(*this
== R); }
+ bool operator!=(const ExceptionStreamsIterator &R) const {
+ return !(*this == R);
+ }
- Expected<const minidump::ExceptionStream &>
- operator*() {
+ Expected<const minidump::ExceptionStream &> operator*() {
return ReadCurrent();
}
- Expected<const minidump::ExceptionStream &>
- operator->() {
+ Expected<const minidump::ExceptionStream &> operator->() {
return ReadCurrent();
}
- ExceptionStreamsIterator &
- operator++ () {
+ ExceptionStreamsIterator &operator++() {
if (!Streams.empty())
Streams = Streams.drop_front();
-
return *this;
}
private:
- ExceptionStreamsIterator(ArrayRef<minidump::Directory> Streams, const
MinidumpFile *File)
- : Streams(Streams), File(File) {}
+ ExceptionStreamsIterator(ArrayRef<minidump::Directory> Streams,
+ const MinidumpFile *File)
+ : Streams(Streams), File(File) {}
- ExceptionStreamsIterator() : Streams(ArrayRef<minidump::Directory>()),
File(nullptr) {}
+ ExceptionStreamsIterator()
+ : Streams(ArrayRef<minidump::Directory>()), File(nullptr) {}
ArrayRef<minidump::Directory> Streams;
const MinidumpFile *File;
- Expected<const minidump::ExceptionStream&> ReadCurrent() {
+ Expected<const minidump::ExceptionStream &> ReadCurrent() {
assert(!Streams.empty());
Expected<const minidump::ExceptionStream &> ExceptionStream =
File->getExceptionStream(Streams.front());
if (!ExceptionStream)
return ExceptionStream.takeError();
-
+
return ExceptionStream;
}
};
using FallibleMemory64Iterator = llvm::fallible_iterator<Memory64Iterator>;
- /// Returns an iterator that reads each exception stream independently. The
- /// contents of the exception strema are not validated before being read, an
+ /// 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<ExceptionStreamsIterator>
- getExceptionStreams() const;
+ iterator_range<ExceptionStreamsIterator> 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
@@ -325,7 +322,8 @@ class ExceptionStreamsIterator {
DenseMap<minidump::StreamType, std::size_t> StreamMap,
std::vector<minidump::Directory> ExceptionStreams)
: Binary(ID_Minidump, Source), Header(Header), Streams(Streams),
- StreamMap(std::move(StreamMap)),
ExceptionStreams(std::move(ExceptionStreams)) {}
+ StreamMap(std::move(StreamMap)),
+ ExceptionStreams(std::move(ExceptionStreams)) {}
ArrayRef<uint8_t> getData() const {
return arrayRefFromStringRef(Data.getBuffer());
diff --git a/llvm/lib/Object/Minidump.cpp b/llvm/lib/Object/Minidump.cpp
index f334cd2a9a2e71..e53d11ddbe11b6 100644
--- a/llvm/lib/Object/Minidump.cpp
+++ b/llvm/lib/Object/Minidump.cpp
@@ -166,8 +166,8 @@ MinidumpFile::create(MemoryBufferRef Source) {
return createError("Duplicate stream type");
}
- return std::unique_ptr<MinidumpFile>(
- new MinidumpFile(Source, Hdr, *ExpectedStreams, std::move(StreamMap),
ExceptionStreams));
+ return std::unique_ptr<MinidumpFile>(new MinidumpFile(
+ Source, Hdr, *ExpectedStreams, std::move(StreamMap), ExceptionStreams));
}
iterator_range<MinidumpFile::FallibleMemory64Iterator>
diff --git a/llvm/unittests/Object/MinidumpTest.cpp
b/llvm/unittests/Object/MinidumpTest.cpp
index 6dcdccc4dea81e..7e5ec35c4111b1 100644
--- a/llvm/unittests/Object/MinidumpTest.cpp
+++ b/llvm/unittests/Object/MinidumpTest.cpp
@@ -751,8 +751,7 @@ TEST(MinidumpFile, getExceptionStream) {
auto ExpectedFile = create(Data);
ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
const MinidumpFile &File = **ExpectedFile;
- auto ExceptionIterator =
- File.getExceptionStreams().begin();
+ auto ExceptionIterator = File.getExceptionStreams().begin();
Expected<const ExceptionStream &> ExpectedStream = *ExceptionIterator;
ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
diff --git a/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
b/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
index 6b2963902a24b8..c805665e0b1ba4 100644
--- a/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
+++ b/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
@@ -162,8 +162,7 @@ TEST(MinidumpYAML, ExceptionStream) {
ASSERT_EQ(1u, File.streams().size());
- auto ExceptionIterator =
- File.getExceptionStreams().begin();
+ auto ExceptionIterator = File.getExceptionStreams().begin();
Expected<const ExceptionStream &> ExpectedStream = *ExceptionIterator;
@@ -207,8 +206,7 @@ TEST(MinidumpYAML, ExceptionStream_NoParameters) {
ASSERT_EQ(1u, File.streams().size());
- auto ExceptionIterator =
- File.getExceptionStreams().begin();
+ auto ExceptionIterator = File.getExceptionStreams().begin();
Expected<const ExceptionStream &> ExpectedStream = *ExceptionIterator;
ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
@@ -264,8 +262,7 @@ TEST(MinidumpYAML, ExceptionStream_TooManyParameters) {
ASSERT_EQ(1u, File.streams().size());
- auto ExceptionIterator =
- File.getExceptionStreams().begin();
+ auto ExceptionIterator = File.getExceptionStreams().begin();
Expected<const ExceptionStream &> ExpectedStream = *ExceptionIterator;
@@ -317,8 +314,7 @@ TEST(MinidumpYAML, ExceptionStream_ExtraParameter) {
ASSERT_EQ(1u, File.streams().size());
- auto ExceptionIterator =
- File.getExceptionStreams().begin();
+ auto ExceptionIterator = File.getExceptionStreams().begin();
Expected<const ExceptionStream &> ExpectedStream = *ExceptionIterator;
@@ -405,7 +401,6 @@ TEST(MinidumpYAML, MemoryRegion_64bit) {
ASSERT_THAT(*DescTwoExpectedContentSlice, arrayRefFromStringRef("world"));
ASSERT_EQ(Iterator, MemoryList.end());
-
}
// Test that we can parse multiple exception streams.
@@ -434,7 +429,7 @@ TEST(MinidumpYAML, ExceptionStream_MultipleExceptions) {
Exception Record: 0x0102030405060708
Exception Address: 0x0a0b0c0d0e0f1011
Thread Context: 3DeadBeefDefacedABadCafe)");
-
+
ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
object::MinidumpFile &File = **ExpectedFile;
>From c4d703e4bd26c13219468f7470de6a6eb708196c Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Thu, 5 Sep 2024 10:24:17 -0700
Subject: [PATCH 5/7] Implement code review feedback, with some minor tweaks
because initializer list isn't convertable, and the iterator doesn't have the
traits for std::next
---
llvm/include/llvm/Object/Minidump.h | 34 ++++----------------------
llvm/lib/Object/Minidump.cpp | 6 ++---
llvm/unittests/Object/MinidumpTest.cpp | 10 +++++---
3 files changed, 15 insertions(+), 35 deletions(-)
diff --git a/llvm/include/llvm/Object/Minidump.h
b/llvm/include/llvm/Object/Minidump.h
index 1e9bc741d78725..024feed26994ad 100644
--- a/llvm/include/llvm/Object/Minidump.h
+++ b/llvm/include/llvm/Object/Minidump.h
@@ -221,15 +221,12 @@ class MinidumpFile : public Binary {
class ExceptionStreamsIterator {
public:
- static ExceptionStreamsIterator begin(ArrayRef<minidump::Directory>
Streams,
- const MinidumpFile *File) {
- return ExceptionStreamsIterator(Streams, File);
- }
-
- static ExceptionStreamsIterator end() { return ExceptionStreamsIterator();
}
+ ExceptionStreamsIterator(ArrayRef<minidump::Directory> Streams,
+ const MinidumpFile *File)
+ : Streams(Streams), File(File) {}
bool operator==(const ExceptionStreamsIterator &R) const {
- return Streams.empty() && R.Streams.empty();
+ return Streams.size() == R.Streams.size();
}
bool operator!=(const ExceptionStreamsIterator &R) const {
@@ -237,11 +234,7 @@ class MinidumpFile : public Binary {
}
Expected<const minidump::ExceptionStream &> operator*() {
- return ReadCurrent();
- }
-
- Expected<const minidump::ExceptionStream &> operator->() {
- return ReadCurrent();
+ return File->getExceptionStream(Streams.front());
}
ExceptionStreamsIterator &operator++() {
@@ -252,25 +245,8 @@ class MinidumpFile : public Binary {
}
private:
- ExceptionStreamsIterator(ArrayRef<minidump::Directory> Streams,
- const MinidumpFile *File)
- : Streams(Streams), File(File) {}
-
- ExceptionStreamsIterator()
- : Streams(ArrayRef<minidump::Directory>()), File(nullptr) {}
-
ArrayRef<minidump::Directory> Streams;
const MinidumpFile *File;
-
- Expected<const minidump::ExceptionStream &> ReadCurrent() {
- assert(!Streams.empty());
- Expected<const minidump::ExceptionStream &> ExceptionStream =
- File->getExceptionStream(Streams.front());
- if (!ExceptionStream)
- return ExceptionStream.takeError();
-
- return ExceptionStream;
- }
};
using FallibleMemory64Iterator = llvm::fallible_iterator<Memory64Iterator>;
diff --git a/llvm/lib/Object/Minidump.cpp b/llvm/lib/Object/Minidump.cpp
index e53d11ddbe11b6..493b30476aeb41 100644
--- a/llvm/lib/Object/Minidump.cpp
+++ b/llvm/lib/Object/Minidump.cpp
@@ -55,8 +55,8 @@ Expected<std::string> MinidumpFile::getString(size_t Offset)
const {
iterator_range<llvm::object::MinidumpFile::ExceptionStreamsIterator>
MinidumpFile::getExceptionStreams() const {
- return make_range(ExceptionStreamsIterator::begin(ExceptionStreams, this),
- ExceptionStreamsIterator::end());
+ return make_range(ExceptionStreamsIterator(ExceptionStreams, this),
+ ExceptionStreamsIterator({}, this));
}
Expected<iterator_range<MinidumpFile::MemoryInfoIterator>>
@@ -167,7 +167,7 @@ MinidumpFile::create(MemoryBufferRef Source) {
}
return std::unique_ptr<MinidumpFile>(new MinidumpFile(
- Source, Hdr, *ExpectedStreams, std::move(StreamMap), ExceptionStreams));
+ Source, Hdr, *ExpectedStreams, std::move(StreamMap),
std::move(ExceptionStreams)));
}
iterator_range<MinidumpFile::FallibleMemory64Iterator>
diff --git a/llvm/unittests/Object/MinidumpTest.cpp
b/llvm/unittests/Object/MinidumpTest.cpp
index 7e5ec35c4111b1..11410b43261c94 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<uint8_t> Data{
// Header
'M', 'D', 'M', 'P', 0x93, 0xa7, 0, 0, // Signature, Version
@@ -751,8 +751,10 @@ TEST(MinidumpFile, getExceptionStream) {
auto ExpectedFile = create(Data);
ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
const MinidumpFile &File = **ExpectedFile;
- auto ExceptionIterator = File.getExceptionStreams().begin();
-
+
+ auto ExceptionStreams = File.getExceptionStreams();
+ ASSERT_NE(ExceptionStreams.begin(), ExceptionStreams.end());
+ auto ExceptionIterator = ExceptionStreams.begin();
Expected<const ExceptionStream &> ExpectedStream = *ExceptionIterator;
ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
EXPECT_EQ(0x04030201u, ExpectedStream->ThreadId);
@@ -768,4 +770,6 @@ TEST(MinidumpFile, getExceptionStream) {
}
EXPECT_EQ(0x84838281, ExpectedStream->ThreadContext.DataSize);
EXPECT_EQ(0x88878685, ExpectedStream->ThreadContext.RVA);
+ ++ExceptionIterator;
+ ASSERT_EQ(ExceptionIterator, ExceptionStreams.end());
}
>From a167f7a88d07fb7121611b8add0852844e754af8 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Thu, 5 Sep 2024 10:59:26 -0700
Subject: [PATCH 6/7] Readd the getExceptionStream API, so that we can modify
LLDB consumers of it in a different patch
---
llvm/include/llvm/Object/Minidump.h | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/llvm/include/llvm/Object/Minidump.h
b/llvm/include/llvm/Object/Minidump.h
index 024feed26994ad..779d7b8649ae98 100644
--- a/llvm/include/llvm/Object/Minidump.h
+++ b/llvm/include/llvm/Object/Minidump.h
@@ -95,6 +95,17 @@ class MinidumpFile : public Binary {
return getStreamFromDirectory<minidump::ExceptionStream>(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<const minidump::ExceptionStream &>
+ getExceptionStream() const {
+ 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
/// 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
>From df500b1a7d2097d60b5caf0a177cd9e664e19b03 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Thu, 5 Sep 2024 11:00:33 -0700
Subject: [PATCH 7/7] Run formatter
---
.../minidump-new/multiple-sigsev.yaml | 39 +++++++++++++++++++
llvm/include/llvm/Object/Minidump.h | 7 ++--
llvm/lib/Object/Minidump.cpp | 5 ++-
llvm/unittests/Object/MinidumpTest.cpp | 2 +-
4 files changed, 46 insertions(+), 7 deletions(-)
create mode 100644
lldb/test/API/functionalities/postmortem/minidump-new/multiple-sigsev.yaml
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/llvm/include/llvm/Object/Minidump.h
b/llvm/include/llvm/Object/Minidump.h
index 779d7b8649ae98..e6b21979ccaa1d 100644
--- a/llvm/include/llvm/Object/Minidump.h
+++ b/llvm/include/llvm/Object/Minidump.h
@@ -98,8 +98,7 @@ class MinidumpFile : public Binary {
/// 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<const minidump::ExceptionStream &>
- getExceptionStream() const {
+ Expected<const minidump::ExceptionStream &> getExceptionStream() const {
auto it = getExceptionStreams();
if (it.begin() == it.end())
return createError("No exception streams");
@@ -233,8 +232,8 @@ class MinidumpFile : public Binary {
class ExceptionStreamsIterator {
public:
ExceptionStreamsIterator(ArrayRef<minidump::Directory> Streams,
- const MinidumpFile *File)
- : Streams(Streams), File(File) {}
+ const MinidumpFile *File)
+ : Streams(Streams), File(File) {}
bool operator==(const ExceptionStreamsIterator &R) const {
return Streams.size() == R.Streams.size();
diff --git a/llvm/lib/Object/Minidump.cpp b/llvm/lib/Object/Minidump.cpp
index 493b30476aeb41..fe768c4c90711b 100644
--- a/llvm/lib/Object/Minidump.cpp
+++ b/llvm/lib/Object/Minidump.cpp
@@ -166,8 +166,9 @@ MinidumpFile::create(MemoryBufferRef Source) {
return createError("Duplicate stream type");
}
- return std::unique_ptr<MinidumpFile>(new MinidumpFile(
- Source, Hdr, *ExpectedStreams, std::move(StreamMap),
std::move(ExceptionStreams)));
+ return std::unique_ptr<MinidumpFile>(
+ new MinidumpFile(Source, Hdr, *ExpectedStreams, std::move(StreamMap),
+ std::move(ExceptionStreams)));
}
iterator_range<MinidumpFile::FallibleMemory64Iterator>
diff --git a/llvm/unittests/Object/MinidumpTest.cpp
b/llvm/unittests/Object/MinidumpTest.cpp
index 11410b43261c94..44a9661bd3f348 100644
--- a/llvm/unittests/Object/MinidumpTest.cpp
+++ b/llvm/unittests/Object/MinidumpTest.cpp
@@ -751,7 +751,7 @@ TEST(MinidumpFile, getExceptionStreams) {
auto ExpectedFile = create(Data);
ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
const MinidumpFile &File = **ExpectedFile;
-
+
auto ExceptionStreams = File.getExceptionStreams();
ASSERT_NE(ExceptionStreams.begin(), ExceptionStreams.end());
auto ExceptionIterator = ExceptionStreams.begin();
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
