[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
Jacob Lalonde via lldb-commits
lldb-commits at lists.llvm.org
Mon Aug 12 09:26:01 PDT 2024
https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/101272
>From d28a238367aebb814be76749a3422a49963da8ac Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Tue, 30 Jul 2024 11:04:45 -0700
Subject: [PATCH 01/19] Squash 64b-memory-regions-minidump and take only
llvm-changes
---
llvm/include/llvm/BinaryFormat/Minidump.h | 12 ++++++++
llvm/include/llvm/Object/Minidump.h | 16 ++++++++--
llvm/include/llvm/ObjectYAML/MinidumpYAML.h | 28 +++++++++++++++++
llvm/lib/Object/Minidump.cpp | 19 ++++++++++--
llvm/lib/ObjectYAML/MinidumpEmitter.cpp | 11 +++++++
llvm/lib/ObjectYAML/MinidumpYAML.cpp | 34 +++++++++++++++++++++
6 files changed, 115 insertions(+), 5 deletions(-)
diff --git a/llvm/include/llvm/BinaryFormat/Minidump.h b/llvm/include/llvm/BinaryFormat/Minidump.h
index 96693032526159..8054e81322a92a 100644
--- a/llvm/include/llvm/BinaryFormat/Minidump.h
+++ b/llvm/include/llvm/BinaryFormat/Minidump.h
@@ -74,6 +74,18 @@ struct MemoryDescriptor_64 {
support::ulittle64_t StartOfMemoryRange;
support::ulittle64_t DataSize;
};
+static_assert(sizeof(MemoryDescriptor_64) == 16);
+
+struct MemoryListHeader {
+ support::ulittle32_t NumberOfMemoryRanges;
+};
+static_assert(sizeof(MemoryListHeader) == 4);
+
+struct Memory64ListHeader {
+ support::ulittle64_t NumberOfMemoryRanges;
+ support::ulittle64_t BaseRVA;
+};
+static_assert(sizeof(Memory64ListHeader) == 16);
struct MemoryInfoListHeader {
support::ulittle32_t SizeOfHeader;
diff --git a/llvm/include/llvm/Object/Minidump.h b/llvm/include/llvm/Object/Minidump.h
index e45d4de0090def..6ca9eb2afe1503 100644
--- a/llvm/include/llvm/Object/Minidump.h
+++ b/llvm/include/llvm/Object/Minidump.h
@@ -103,6 +103,15 @@ class MinidumpFile : public Binary {
minidump::StreamType::MemoryList);
}
+ /// Returns the header to the memory 64 list stream. An error is returned if
+ /// the file does not contain this stream.
+ Expected<minidump::Memory64ListHeader> getMemoryList64Header() const {
+ return getStream<minidump::Memory64ListHeader>(
+ minidump::StreamType::Memory64List);
+ }
+
+ Expected<ArrayRef<minidump::MemoryDescriptor_64>> getMemory64List() const;
+
class MemoryInfoIterator
: public iterator_facade_base<MemoryInfoIterator,
std::forward_iterator_tag,
@@ -152,15 +161,15 @@ class MinidumpFile : public Binary {
}
/// Return a slice of the given data array, with bounds checking.
- static Expected<ArrayRef<uint8_t>> getDataSlice(ArrayRef<uint8_t> Data,
- size_t Offset, size_t Size);
+ static Expected<ArrayRef<uint8_t>>
+ getDataSlice(ArrayRef<uint8_t> Data, uint64_t Offset, uint64_t Size);
/// Return the slice of the given data array as an array of objects of the
/// given type. The function checks that the input array is large enough to
/// contain the correct number of objects of the given type.
template <typename T>
static Expected<ArrayRef<T>> getDataSliceAs(ArrayRef<uint8_t> Data,
- size_t Offset, size_t Count);
+ uint64_t Offset, uint64_t Count);
MinidumpFile(MemoryBufferRef Source, const minidump::Header &Header,
ArrayRef<minidump::Directory> Streams,
@@ -208,6 +217,7 @@ Expected<ArrayRef<T>> MinidumpFile::getDataSliceAs(ArrayRef<uint8_t> Data,
getDataSlice(Data, Offset, sizeof(T) * Count);
if (!Slice)
return Slice.takeError();
+
return ArrayRef<T>(reinterpret_cast<const T *>(Slice->data()), Count);
}
diff --git a/llvm/include/llvm/ObjectYAML/MinidumpYAML.h b/llvm/include/llvm/ObjectYAML/MinidumpYAML.h
index b0cee541cef206..2cccf7fba58531 100644
--- a/llvm/include/llvm/ObjectYAML/MinidumpYAML.h
+++ b/llvm/include/llvm/ObjectYAML/MinidumpYAML.h
@@ -29,6 +29,7 @@ struct Stream {
Exception,
MemoryInfoList,
MemoryList,
+ Memory64List,
ModuleList,
RawContent,
SystemInfo,
@@ -104,6 +105,25 @@ using ModuleListStream = detail::ListStream<detail::ParsedModule>;
using ThreadListStream = detail::ListStream<detail::ParsedThread>;
using MemoryListStream = detail::ListStream<detail::ParsedMemoryDescriptor>;
+/// Memory64ListStream minidump stream.
+struct Memory64ListStream : public Stream {
+ minidump::Memory64ListHeader Header;
+ std::vector<minidump::MemoryDescriptor_64> Entries;
+ yaml::BinaryRef Content;
+
+ Memory64ListStream()
+ : Stream(StreamKind::Memory64List, minidump::StreamType::Memory64List) {}
+
+ explicit Memory64ListStream(
+ std::vector<minidump::MemoryDescriptor_64> Entries)
+ : Stream(StreamKind::Memory64List, minidump::StreamType::Memory64List),
+ Entries(Entries) {}
+
+ static bool classof(const Stream *S) {
+ return S->Kind == StreamKind::Memory64List;
+ }
+};
+
/// ExceptionStream minidump stream.
struct ExceptionStream : public Stream {
minidump::ExceptionStream MDExceptionStream;
@@ -244,6 +264,12 @@ template <> struct MappingContextTraits<minidump::MemoryDescriptor, BinaryRef> {
BinaryRef &Content);
};
+template <>
+struct MappingContextTraits<minidump::MemoryDescriptor_64, BinaryRef> {
+ static void mapping(IO &IO, minidump::MemoryDescriptor_64 &Memory,
+ BinaryRef &Content);
+};
+
} // namespace yaml
} // namespace llvm
@@ -262,6 +288,7 @@ LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::CPUInfo::X86Info)
LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::Exception)
LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::MemoryInfo)
LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::VSFixedFileInfo)
+LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::MemoryDescriptor_64)
LLVM_YAML_DECLARE_MAPPING_TRAITS(
llvm::MinidumpYAML::MemoryListStream::entry_type)
@@ -275,6 +302,7 @@ LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::MinidumpYAML::MemoryListStream::entry_type)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::MinidumpYAML::ModuleListStream::entry_type)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::MinidumpYAML::ThreadListStream::entry_type)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::minidump::MemoryInfo)
+LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::minidump::MemoryDescriptor_64)
LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::MinidumpYAML::Object)
diff --git a/llvm/lib/Object/Minidump.cpp b/llvm/lib/Object/Minidump.cpp
index 6febff89ac5195..92aa4f66a4b35c 100644
--- a/llvm/lib/Object/Minidump.cpp
+++ b/llvm/lib/Object/Minidump.cpp
@@ -99,8 +99,9 @@ template Expected<ArrayRef<Thread>>
template Expected<ArrayRef<MemoryDescriptor>>
MinidumpFile::getListStream(StreamType) const;
-Expected<ArrayRef<uint8_t>>
-MinidumpFile::getDataSlice(ArrayRef<uint8_t> Data, size_t Offset, size_t Size) {
+Expected<ArrayRef<uint8_t>> MinidumpFile::getDataSlice(ArrayRef<uint8_t> Data,
+ uint64_t Offset,
+ uint64_t Size) {
// Check for overflow.
if (Offset + Size < Offset || Offset + Size < Size ||
Offset + Size > Data.size())
@@ -154,3 +155,17 @@ MinidumpFile::create(MemoryBufferRef Source) {
return std::unique_ptr<MinidumpFile>(
new MinidumpFile(Source, Hdr, *ExpectedStreams, std::move(StreamMap)));
}
+
+Expected<ArrayRef<MemoryDescriptor_64>> MinidumpFile::getMemory64List() const {
+ Expected<minidump::Memory64ListHeader> MemoryList64 = getMemoryList64Header();
+ if (!MemoryList64)
+ return MemoryList64.takeError();
+
+ std::optional<ArrayRef<uint8_t>> Stream =
+ getRawStream(StreamType::Memory64List);
+ if (!Stream)
+ return createError("No such stream");
+
+ return getDataSliceAs<minidump::MemoryDescriptor_64>(
+ *Stream, sizeof(Memory64ListHeader), MemoryList64->NumberOfMemoryRanges);
+}
diff --git a/llvm/lib/ObjectYAML/MinidumpEmitter.cpp b/llvm/lib/ObjectYAML/MinidumpEmitter.cpp
index 24b521a9925c73..f992c4e3bde761 100644
--- a/llvm/lib/ObjectYAML/MinidumpEmitter.cpp
+++ b/llvm/lib/ObjectYAML/MinidumpEmitter.cpp
@@ -136,6 +136,14 @@ static size_t layout(BlobAllocator &File, MinidumpYAML::ExceptionStream &S) {
return DataEnd;
}
+static void layout(BlobAllocator &File, MinidumpYAML::Memory64ListStream &S) {
+ size_t BaseRVA = File.tell() + sizeof(minidump::Memory64ListHeader);
+ S.Header.BaseRVA = BaseRVA;
+ S.Header.NumberOfMemoryRanges = S.Entries.size();
+ File.allocateObject(S.Header);
+ File.allocateArray(ArrayRef(S.Entries));
+}
+
static void layout(BlobAllocator &File, MemoryListStream::entry_type &Range) {
Range.Entry.Memory = layout(File, Range.Content);
}
@@ -190,6 +198,9 @@ static Directory layout(BlobAllocator &File, Stream &S) {
case Stream::StreamKind::MemoryList:
DataEnd = layout(File, cast<MemoryListStream>(S));
break;
+ case Stream::StreamKind::Memory64List:
+ layout(File, cast<Memory64ListStream>(S));
+ break;
case Stream::StreamKind::ModuleList:
DataEnd = layout(File, cast<ModuleListStream>(S));
break;
diff --git a/llvm/lib/ObjectYAML/MinidumpYAML.cpp b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
index fdbd2d8e6dcbc7..aee17c520a0105 100644
--- a/llvm/lib/ObjectYAML/MinidumpYAML.cpp
+++ b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
@@ -75,6 +75,8 @@ Stream::StreamKind Stream::getKind(StreamType Type) {
return StreamKind::MemoryInfoList;
case StreamType::MemoryList:
return StreamKind::MemoryList;
+ case StreamType::Memory64List:
+ return StreamKind::Memory64List;
case StreamType::ModuleList:
return StreamKind::ModuleList;
case StreamType::SystemInfo:
@@ -103,6 +105,8 @@ std::unique_ptr<Stream> Stream::create(StreamType Type) {
return std::make_unique<MemoryInfoListStream>();
case StreamKind::MemoryList:
return std::make_unique<MemoryListStream>();
+ case StreamKind::Memory64List:
+ return std::make_unique<Memory64ListStream>();
case StreamKind::ModuleList:
return std::make_unique<ModuleListStream>();
case StreamKind::RawContent:
@@ -256,6 +260,12 @@ void yaml::MappingTraits<MemoryInfo>::mapping(IO &IO, MemoryInfo &Info) {
mapOptionalHex(IO, "Reserved1", Info.Reserved1, 0);
}
+void yaml::MappingTraits<MemoryDescriptor_64>::mapping(
+ IO &IO, MemoryDescriptor_64 &Mem) {
+ mapRequiredHex(IO, "Start of memory range", Mem.StartOfMemoryRange);
+ mapRequiredHex(IO, "Data Size", Mem.DataSize);
+}
+
void yaml::MappingTraits<VSFixedFileInfo>::mapping(IO &IO,
VSFixedFileInfo &Info) {
mapOptionalHex(IO, "Signature", Info.Signature, 0);
@@ -312,6 +322,10 @@ static void streamMapping(yaml::IO &IO, MemoryListStream &Stream) {
IO.mapRequired("Memory Ranges", Stream.Entries);
}
+static void streamMapping(yaml::IO &IO, Memory64ListStream &Stream) {
+ IO.mapRequired("Memory Ranges", Stream.Entries);
+}
+
static void streamMapping(yaml::IO &IO, ModuleListStream &Stream) {
IO.mapRequired("Modules", Stream.Entries);
}
@@ -356,6 +370,12 @@ void yaml::MappingContextTraits<MemoryDescriptor, yaml::BinaryRef>::mapping(
IO.mapRequired("Content", Content);
}
+void yaml::MappingContextTraits<MemoryDescriptor_64, yaml::BinaryRef>::mapping(
+ IO &IO, MemoryDescriptor_64 &Memory, BinaryRef &Content) {
+ mapRequiredHex(IO, "Start of Memory Range", Memory.StartOfMemoryRange);
+ IO.mapRequired("Content", Content);
+}
+
void yaml::MappingTraits<ThreadListStream::entry_type>::mapping(
IO &IO, ThreadListStream::entry_type &T) {
mapRequiredHex(IO, "Thread Id", T.Entry.ThreadId);
@@ -416,6 +436,9 @@ void yaml::MappingTraits<std::unique_ptr<Stream>>::mapping(
case MinidumpYAML::Stream::StreamKind::MemoryList:
streamMapping(IO, llvm::cast<MemoryListStream>(*S));
break;
+ case MinidumpYAML::Stream::StreamKind::Memory64List:
+ streamMapping(IO, llvm::cast<Memory64ListStream>(*S));
+ break;
case MinidumpYAML::Stream::StreamKind::ModuleList:
streamMapping(IO, llvm::cast<ModuleListStream>(*S));
break;
@@ -442,6 +465,7 @@ std::string yaml::MappingTraits<std::unique_ptr<Stream>>::validate(
case MinidumpYAML::Stream::StreamKind::Exception:
case MinidumpYAML::Stream::StreamKind::MemoryInfoList:
case MinidumpYAML::Stream::StreamKind::MemoryList:
+ case MinidumpYAML::Stream::StreamKind::Memory64List:
case MinidumpYAML::Stream::StreamKind::ModuleList:
case MinidumpYAML::Stream::StreamKind::SystemInfo:
case MinidumpYAML::Stream::StreamKind::TextContent:
@@ -494,6 +518,16 @@ Stream::create(const Directory &StreamDesc, const object::MinidumpFile &File) {
}
return std::make_unique<MemoryListStream>(std::move(Ranges));
}
+ case StreamKind::Memory64List: {
+ auto ExpectedList = File.getMemory64List();
+ if (!ExpectedList)
+ return ExpectedList.takeError();
+ std::vector<MemoryDescriptor_64> Ranges;
+ for (const MemoryDescriptor_64 &MD : *ExpectedList) {
+ Ranges.push_back(MD);
+ }
+ return std::make_unique<Memory64ListStream>(std::move(Ranges));
+ }
case StreamKind::ModuleList: {
auto ExpectedList = File.getModuleList();
if (!ExpectedList)
>From 101c006f4d61cf1b7e9af5521ca735d0c9b38289 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Tue, 30 Jul 2024 17:57:43 -0700
Subject: [PATCH 02/19] Make some further changes to enable content, remove
const modifier so we can cache the memory descriptors instead of doing N^2
lookups. Run Formatters and cleanup debugging code.
---
.../Shell/Minidump/Inputs/linux-x86_64.yaml | 6 +--
llvm/include/llvm/Object/Minidump.h | 10 +++-
llvm/include/llvm/ObjectYAML/MinidumpYAML.h | 37 +++++++-------
llvm/lib/Object/Minidump.cpp | 42 +++++++++++++---
llvm/lib/ObjectYAML/MinidumpEmitter.cpp | 14 ++++--
llvm/lib/ObjectYAML/MinidumpYAML.cpp | 22 +++++----
llvm/test/tools/obj2yaml/Minidump/basic.yaml | 12 ++++-
llvm/tools/obj2yaml/minidump2yaml.cpp | 2 +-
llvm/tools/obj2yaml/obj2yaml.h | 2 +-
.../unittests/ObjectYAML/MinidumpYAMLTest.cpp | 48 +++++++++++++++++++
10 files changed, 150 insertions(+), 45 deletions(-)
diff --git a/lldb/test/Shell/Minidump/Inputs/linux-x86_64.yaml b/lldb/test/Shell/Minidump/Inputs/linux-x86_64.yaml
index d2ae6543141e82..157903f368a08e 100644
--- a/lldb/test/Shell/Minidump/Inputs/linux-x86_64.yaml
+++ b/lldb/test/Shell/Minidump/Inputs/linux-x86_64.yaml
@@ -1,7 +1,7 @@
--- !minidump
-Streams:
+Streams:
- Type: ModuleList
- Modules:
+ Modules:
- Base of Image: 0x0000000000400000
Size of Image: 0x00001000
Module Name: '/tmp/test/linux-x86_64'
@@ -13,7 +13,7 @@ Streams:
Number of Processors: 40
Platform ID: Linux
CSD Version: 'Linux 3.13.0-91-generic #138-Ubuntu SMP Fri Jun 24 17:00:34 UTC 2016 x86_64'
- CPU:
+ CPU:
Vendor ID: GenuineIntel
Version Info: 0x00000000
Feature Info: 0x00000000
diff --git a/llvm/include/llvm/Object/Minidump.h b/llvm/include/llvm/Object/Minidump.h
index 6ca9eb2afe1503..eedebc10b3fe7b 100644
--- a/llvm/include/llvm/Object/Minidump.h
+++ b/llvm/include/llvm/Object/Minidump.h
@@ -15,6 +15,7 @@
#include "llvm/BinaryFormat/Minidump.h"
#include "llvm/Object/Binary.h"
#include "llvm/Support/Error.h"
+#include <map>
namespace llvm {
namespace object {
@@ -52,6 +53,9 @@ class MinidumpFile : public Binary {
return getDataSlice(getData(), Desc.RVA, Desc.DataSize);
}
+ Expected<ArrayRef<uint8_t>>
+ getRawData(minidump::MemoryDescriptor_64 Desc) const;
+
/// Returns the minidump string at the given offset. An error is returned if
/// we fail to parse the string, or the string is invalid UTF16.
Expected<std::string> getString(size_t Offset) const;
@@ -110,7 +114,7 @@ class MinidumpFile : public Binary {
minidump::StreamType::Memory64List);
}
- Expected<ArrayRef<minidump::MemoryDescriptor_64>> getMemory64List() const;
+ Expected<ArrayRef<minidump::MemoryDescriptor_64>> getMemory64List();
class MemoryInfoIterator
: public iterator_facade_base<MemoryInfoIterator,
@@ -191,9 +195,13 @@ class MinidumpFile : public Binary {
template <typename T>
Expected<ArrayRef<T>> getListStream(minidump::StreamType Stream) const;
+ void cacheMemory64RVAs(uint64_t BaseRVA,
+ ArrayRef<minidump::MemoryDescriptor_64> Descriptors);
+
const minidump::Header &Header;
ArrayRef<minidump::Directory> Streams;
DenseMap<minidump::StreamType, std::size_t> StreamMap;
+ std::unordered_map<uint64_t, uint64_t> Memory64DescriptorToRvaMap;
};
template <typename T>
diff --git a/llvm/include/llvm/ObjectYAML/MinidumpYAML.h b/llvm/include/llvm/ObjectYAML/MinidumpYAML.h
index 2cccf7fba58531..7f7924b92029fa 100644
--- a/llvm/include/llvm/ObjectYAML/MinidumpYAML.h
+++ b/llvm/include/llvm/ObjectYAML/MinidumpYAML.h
@@ -51,8 +51,7 @@ struct Stream {
/// Create a stream from the given stream directory entry.
static Expected<std::unique_ptr<Stream>>
- create(const minidump::Directory &StreamDesc,
- const object::MinidumpFile &File);
+ create(const minidump::Directory &StreamDesc, object::MinidumpFile &File);
};
namespace detail {
@@ -99,29 +98,28 @@ struct ParsedMemoryDescriptor {
minidump::MemoryDescriptor Entry;
yaml::BinaryRef Content;
};
+
+struct ParsedMemory64Descriptor {
+ static constexpr Stream::StreamKind Kind = Stream::StreamKind::Memory64List;
+ static constexpr minidump::StreamType Type =
+ minidump::StreamType::Memory64List;
+
+ minidump::MemoryDescriptor_64 Entry;
+ yaml::BinaryRef Content;
+};
} // namespace detail
using ModuleListStream = detail::ListStream<detail::ParsedModule>;
using ThreadListStream = detail::ListStream<detail::ParsedThread>;
using MemoryListStream = detail::ListStream<detail::ParsedMemoryDescriptor>;
-/// Memory64ListStream minidump stream.
-struct Memory64ListStream : public Stream {
+struct Memory64ListStream
+ : public detail::ListStream<detail::ParsedMemory64Descriptor> {
minidump::Memory64ListHeader Header;
- std::vector<minidump::MemoryDescriptor_64> Entries;
- yaml::BinaryRef Content;
-
- Memory64ListStream()
- : Stream(StreamKind::Memory64List, minidump::StreamType::Memory64List) {}
explicit Memory64ListStream(
- std::vector<minidump::MemoryDescriptor_64> Entries)
- : Stream(StreamKind::Memory64List, minidump::StreamType::Memory64List),
- Entries(Entries) {}
-
- static bool classof(const Stream *S) {
- return S->Kind == StreamKind::Memory64List;
- }
+ std::vector<detail::ParsedMemory64Descriptor> Entries = {})
+ : ListStream(Entries){};
};
/// ExceptionStream minidump stream.
@@ -235,7 +233,7 @@ struct Object {
/// The list of streams in this minidump object.
std::vector<std::unique_ptr<Stream>> Streams;
- static Expected<Object> create(const object::MinidumpFile &File);
+ static Expected<Object> create(object::MinidumpFile &File);
};
} // namespace MinidumpYAML
@@ -288,7 +286,6 @@ LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::CPUInfo::X86Info)
LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::Exception)
LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::MemoryInfo)
LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::VSFixedFileInfo)
-LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::MemoryDescriptor_64)
LLVM_YAML_DECLARE_MAPPING_TRAITS(
llvm::MinidumpYAML::MemoryListStream::entry_type)
@@ -296,13 +293,15 @@ LLVM_YAML_DECLARE_MAPPING_TRAITS(
llvm::MinidumpYAML::ModuleListStream::entry_type)
LLVM_YAML_DECLARE_MAPPING_TRAITS(
llvm::MinidumpYAML::ThreadListStream::entry_type)
+LLVM_YAML_DECLARE_MAPPING_TRAITS(
+ llvm::MinidumpYAML::Memory64ListStream::entry_type)
LLVM_YAML_IS_SEQUENCE_VECTOR(std::unique_ptr<llvm::MinidumpYAML::Stream>)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::MinidumpYAML::MemoryListStream::entry_type)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::MinidumpYAML::ModuleListStream::entry_type)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::MinidumpYAML::ThreadListStream::entry_type)
+LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::MinidumpYAML::Memory64ListStream::entry_type)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::minidump::MemoryInfo)
-LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::minidump::MemoryDescriptor_64)
LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::MinidumpYAML::Object)
diff --git a/llvm/lib/Object/Minidump.cpp b/llvm/lib/Object/Minidump.cpp
index 92aa4f66a4b35c..06bcbe9be195f4 100644
--- a/llvm/lib/Object/Minidump.cpp
+++ b/llvm/lib/Object/Minidump.cpp
@@ -156,16 +156,46 @@ MinidumpFile::create(MemoryBufferRef Source) {
new MinidumpFile(Source, Hdr, *ExpectedStreams, std::move(StreamMap)));
}
-Expected<ArrayRef<MemoryDescriptor_64>> MinidumpFile::getMemory64List() const {
- Expected<minidump::Memory64ListHeader> MemoryList64 = getMemoryList64Header();
- if (!MemoryList64)
- return MemoryList64.takeError();
+void MinidumpFile::cacheMemory64RVAs(
+ uint64_t BaseRVA, ArrayRef<minidump::MemoryDescriptor_64> Descriptors) {
+ // Check if we've already cached the RVAs for this list.
+ if (!Memory64DescriptorToRvaMap.empty())
+ return;
+
+ for (const auto &MD : Descriptors) {
+ Memory64DescriptorToRvaMap[MD.StartOfMemoryRange] = BaseRVA;
+ BaseRVA += MD.DataSize;
+ }
+}
+
+Expected<ArrayRef<MemoryDescriptor_64>> MinidumpFile::getMemory64List() {
+ Expected<minidump::Memory64ListHeader> ListHeader = getMemoryList64Header();
+ if (!ListHeader)
+ return ListHeader.takeError();
std::optional<ArrayRef<uint8_t>> Stream =
getRawStream(StreamType::Memory64List);
if (!Stream)
return createError("No such stream");
- return getDataSliceAs<minidump::MemoryDescriptor_64>(
- *Stream, sizeof(Memory64ListHeader), MemoryList64->NumberOfMemoryRanges);
+ Expected<ArrayRef<minidump::MemoryDescriptor_64>> Descriptors =
+ getDataSliceAs<minidump::MemoryDescriptor_64>(
+ *Stream, sizeof(Memory64ListHeader),
+ ListHeader->NumberOfMemoryRanges);
+
+ if (!Descriptors)
+ return Descriptors.takeError();
+
+ cacheMemory64RVAs(ListHeader->NumberOfMemoryRanges, *Descriptors);
+ return Descriptors;
+}
+
+Expected<ArrayRef<uint8_t>>
+MinidumpFile::getRawData(MemoryDescriptor_64 MD) const {
+ if (Memory64DescriptorToRvaMap.count(MD.StartOfMemoryRange) > 0) {
+ uint64_t RVA = Memory64DescriptorToRvaMap.at(MD.StartOfMemoryRange);
+ return getDataSlice(getData(), RVA, MD.DataSize);
+ }
+
+ return createError("No such Memory Descriptor64.");
}
diff --git a/llvm/lib/ObjectYAML/MinidumpEmitter.cpp b/llvm/lib/ObjectYAML/MinidumpEmitter.cpp
index f992c4e3bde761..d2f6bcdc20838c 100644
--- a/llvm/lib/ObjectYAML/MinidumpEmitter.cpp
+++ b/llvm/lib/ObjectYAML/MinidumpEmitter.cpp
@@ -136,12 +136,20 @@ static size_t layout(BlobAllocator &File, MinidumpYAML::ExceptionStream &S) {
return DataEnd;
}
-static void layout(BlobAllocator &File, MinidumpYAML::Memory64ListStream &S) {
+static size_t layout(BlobAllocator &File, MinidumpYAML::Memory64ListStream &S) {
size_t BaseRVA = File.tell() + sizeof(minidump::Memory64ListHeader);
S.Header.BaseRVA = BaseRVA;
S.Header.NumberOfMemoryRanges = S.Entries.size();
File.allocateObject(S.Header);
- File.allocateArray(ArrayRef(S.Entries));
+ for (auto &E : S.Entries)
+ File.allocateObject(E.Entry);
+
+ // Save the new offset for the stream size.
+ size_t DataEnd = File.tell();
+ for (auto &E : S.Entries)
+ File.allocateBytes(E.Content);
+
+ return DataEnd;
}
static void layout(BlobAllocator &File, MemoryListStream::entry_type &Range) {
@@ -199,7 +207,7 @@ static Directory layout(BlobAllocator &File, Stream &S) {
DataEnd = layout(File, cast<MemoryListStream>(S));
break;
case Stream::StreamKind::Memory64List:
- layout(File, cast<Memory64ListStream>(S));
+ DataEnd = layout(File, cast<Memory64ListStream>(S));
break;
case Stream::StreamKind::ModuleList:
DataEnd = layout(File, cast<ModuleListStream>(S));
diff --git a/llvm/lib/ObjectYAML/MinidumpYAML.cpp b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
index aee17c520a0105..b318a9e5cc2e5e 100644
--- a/llvm/lib/ObjectYAML/MinidumpYAML.cpp
+++ b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
@@ -260,10 +260,10 @@ void yaml::MappingTraits<MemoryInfo>::mapping(IO &IO, MemoryInfo &Info) {
mapOptionalHex(IO, "Reserved1", Info.Reserved1, 0);
}
-void yaml::MappingTraits<MemoryDescriptor_64>::mapping(
- IO &IO, MemoryDescriptor_64 &Mem) {
- mapRequiredHex(IO, "Start of memory range", Mem.StartOfMemoryRange);
- mapRequiredHex(IO, "Data Size", Mem.DataSize);
+void yaml::MappingTraits<Memory64ListStream::entry_type>::mapping(
+ IO &IO, Memory64ListStream::entry_type &Mem) {
+ MappingContextTraits<MemoryDescriptor_64, yaml::BinaryRef>::mapping(
+ IO, Mem.Entry, Mem.Content);
}
void yaml::MappingTraits<VSFixedFileInfo>::mapping(IO &IO,
@@ -373,6 +373,7 @@ void yaml::MappingContextTraits<MemoryDescriptor, yaml::BinaryRef>::mapping(
void yaml::MappingContextTraits<MemoryDescriptor_64, yaml::BinaryRef>::mapping(
IO &IO, MemoryDescriptor_64 &Memory, BinaryRef &Content) {
mapRequiredHex(IO, "Start of Memory Range", Memory.StartOfMemoryRange);
+ mapRequiredHex(IO, "Data Size", Memory.DataSize);
IO.mapRequired("Content", Content);
}
@@ -483,8 +484,8 @@ void yaml::MappingTraits<Object>::mapping(IO &IO, Object &O) {
IO.mapRequired("Streams", O.Streams);
}
-Expected<std::unique_ptr<Stream>>
-Stream::create(const Directory &StreamDesc, const object::MinidumpFile &File) {
+Expected<std::unique_ptr<Stream>> Stream::create(const Directory &StreamDesc,
+ object::MinidumpFile &File) {
StreamKind Kind = getKind(StreamDesc.Type);
switch (Kind) {
case StreamKind::Exception: {
@@ -522,9 +523,12 @@ Stream::create(const Directory &StreamDesc, const object::MinidumpFile &File) {
auto ExpectedList = File.getMemory64List();
if (!ExpectedList)
return ExpectedList.takeError();
- std::vector<MemoryDescriptor_64> Ranges;
+ std::vector<Memory64ListStream::entry_type> Ranges;
for (const MemoryDescriptor_64 &MD : *ExpectedList) {
- Ranges.push_back(MD);
+ auto ExpectedContent = File.getRawData(MD);
+ if (!ExpectedContent)
+ return ExpectedContent.takeError();
+ Ranges.push_back({MD, *ExpectedContent});
}
return std::make_unique<Memory64ListStream>(std::move(Ranges));
}
@@ -584,7 +588,7 @@ Stream::create(const Directory &StreamDesc, const object::MinidumpFile &File) {
llvm_unreachable("Unhandled stream kind!");
}
-Expected<Object> Object::create(const object::MinidumpFile &File) {
+Expected<Object> Object::create(object::MinidumpFile &File) {
std::vector<std::unique_ptr<Stream>> Streams;
Streams.reserve(File.streams().size());
for (const Directory &StreamDesc : File.streams()) {
diff --git a/llvm/test/tools/obj2yaml/Minidump/basic.yaml b/llvm/test/tools/obj2yaml/Minidump/basic.yaml
index dfabc132d8e71c..9702f82dc58437 100644
--- a/llvm/test/tools/obj2yaml/Minidump/basic.yaml
+++ b/llvm/test/tools/obj2yaml/Minidump/basic.yaml
@@ -67,9 +67,13 @@ Streams:
Parameter 1: 0x24
Thread Context: '8182838485868788'
- Type: MemoryList
- Memory Ranges:
+ Memory Ranges:
- Start of Memory Range: 0x7C7D7E7F80818283
Content: '8485868788'
+ - Type: Memory64List
+ Memory Ranges:
+ - Start of Memory Range: 0x7FFFFFFF0818283
+ Content: '104101108108111'
- Type: MemoryInfoList
Memory Ranges:
- Base Address: 0x0000000000000000
@@ -156,9 +160,13 @@ Streams:
# CHECK-NEXT: Parameter 1: 0x24
# CHECK-NEXT: Thread Context: '8182838485868788'
# CHECK-NEXT: - Type: MemoryList
-# CHECK-NEXT: Memory Ranges:
+# CHECK-NEXT: Memory Ranges:
# CHECK-NEXT: - Start of Memory Range: 0x7C7D7E7F80818283
# CHECK-NEXT: Content: '8485868788'
+# CHECK-NEXT: - Type: Memory64List
+# CHECK-NEXT: Memory Ranges:
+# CHECK-NEXT: - Start of Memory Range: 0x7FFFFFFF0818283
+# CHECK-NEXT: Content: '104101108108111'
# CHECK-NEXT: - Type: MemoryInfoList
# CHECK-NEXT: Memory Ranges:
# CHECK-NEXT: - Base Address: 0x0
diff --git a/llvm/tools/obj2yaml/minidump2yaml.cpp b/llvm/tools/obj2yaml/minidump2yaml.cpp
index 7b25b1869c6b6a..53486d57d0932d 100644
--- a/llvm/tools/obj2yaml/minidump2yaml.cpp
+++ b/llvm/tools/obj2yaml/minidump2yaml.cpp
@@ -13,7 +13,7 @@
using namespace llvm;
-Error minidump2yaml(raw_ostream &Out, const object::MinidumpFile &Obj) {
+Error minidump2yaml(raw_ostream &Out, object::MinidumpFile &Obj) {
auto ExpectedObject = MinidumpYAML::Object::create(Obj);
if (!ExpectedObject)
return ExpectedObject.takeError();
diff --git a/llvm/tools/obj2yaml/obj2yaml.h b/llvm/tools/obj2yaml/obj2yaml.h
index 03d7db5317acde..96256bc1307d65 100644
--- a/llvm/tools/obj2yaml/obj2yaml.h
+++ b/llvm/tools/obj2yaml/obj2yaml.h
@@ -28,7 +28,7 @@ llvm::Error elf2yaml(llvm::raw_ostream &Out,
llvm::Error macho2yaml(llvm::raw_ostream &Out, const llvm::object::Binary &Obj,
unsigned RawSegments);
llvm::Error minidump2yaml(llvm::raw_ostream &Out,
- const llvm::object::MinidumpFile &Obj);
+ llvm::object::MinidumpFile &Obj);
llvm::Error xcoff2yaml(llvm::raw_ostream &Out,
const llvm::object::XCOFFObjectFile &Obj);
std::error_code wasm2yaml(llvm::raw_ostream &Out,
diff --git a/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp b/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
index 0297f62d106998..79fb903bcfd10c 100644
--- a/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
+++ b/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
@@ -336,3 +336,51 @@ TEST(MinidumpYAML, ExceptionStream_ExtraParameter) {
0xab, 0xad, 0xca, 0xfe}),
*ExpectedContext);
}
+
+TEST(MinidumpYAML, MemoryRegion_64bit) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type: Memory64List
+ Memory Ranges:
+ - Start of Memory Range: 0x7FFFFFCF0818283
+ Data Size: 8
+ Content: '68656c6c6f'
+ - Start of Memory Range: 0x7FFFFFFF0818283
+ Data Size: 8
+ Content: '776f726c64'
+ )");
+
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+ object::MinidumpFile &File = **ExpectedFile;
+
+ ASSERT_EQ(1u, File.streams().size());
+
+ Expected<ArrayRef<minidump::MemoryDescriptor_64>> ExpectedMemoryList =
+ File.getMemory64List();
+
+ ASSERT_THAT_EXPECTED(ExpectedMemoryList, Succeeded());
+
+ ArrayRef<minidump::MemoryDescriptor_64> MemoryList = *ExpectedMemoryList;
+ ASSERT_EQ(2u, MemoryList.size());
+
+ const minidump::MemoryDescriptor_64 &DescOne = MemoryList[0];
+ ASSERT_EQ(0x7FFFFFCF0818283u, DescOne.StartOfMemoryRange);
+ ASSERT_EQ(8u, DescOne.DataSize);
+
+ const minidump::MemoryDescriptor_64 &DescTwo = MemoryList[1];
+ ASSERT_EQ(0x7FFFFFFF0818283u, DescTwo.StartOfMemoryRange);
+ ASSERT_EQ(8u, DescTwo.DataSize);
+
+ const std::optional<ArrayRef<uint8_t>> ExpectedContent =
+ File.getRawStream(StreamType::Memory64List);
+ ASSERT_TRUE(ExpectedContent);
+ const ArrayRef<uint8_t> Content = *ExpectedContent;
+ const size_t offset =
+ sizeof(Memory64ListHeader) + (sizeof(MemoryDescriptor_64) * 2);
+ const uint64_t *Array =
+ reinterpret_cast<const uint64_t *>(Content.slice(offset, 16).data(), 16);
+ ASSERT_EQ(0x7000000000002Au, Array[0]);
+ ASSERT_EQ(0x7000A00000000Au, Array[1]);
+}
>From fe45ccdfc3683abae6724a8c7ed348b41ad63442 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Tue, 30 Jul 2024 22:16:08 -0700
Subject: [PATCH 03/19] Quick cleanup as Greg pointed out, actually include
test code to test for hello and world
---
llvm/include/llvm/Object/Minidump.h | 6 ++---
llvm/lib/ObjectYAML/MinidumpEmitter.cpp | 5 +++-
llvm/lib/ObjectYAML/MinidumpYAML.cpp | 1 -
.../unittests/ObjectYAML/MinidumpYAMLTest.cpp | 23 ++++++++++---------
4 files changed, 19 insertions(+), 16 deletions(-)
diff --git a/llvm/include/llvm/Object/Minidump.h b/llvm/include/llvm/Object/Minidump.h
index eedebc10b3fe7b..21446068de616b 100644
--- a/llvm/include/llvm/Object/Minidump.h
+++ b/llvm/include/llvm/Object/Minidump.h
@@ -216,10 +216,10 @@ Expected<const T &> MinidumpFile::getStream(minidump::StreamType Type) const {
template <typename T>
Expected<ArrayRef<T>> MinidumpFile::getDataSliceAs(ArrayRef<uint8_t> Data,
- size_t Offset,
- size_t Count) {
+ uint64_t Offset,
+ uint64_t Count) {
// Check for overflow.
- if (Count > std::numeric_limits<size_t>::max() / sizeof(T))
+ if (Count > std::numeric_limits<uint64_t>::max() / sizeof(T))
return createEOFError();
Expected<ArrayRef<uint8_t>> Slice =
getDataSlice(Data, Offset, sizeof(T) * Count);
diff --git a/llvm/lib/ObjectYAML/MinidumpEmitter.cpp b/llvm/lib/ObjectYAML/MinidumpEmitter.cpp
index d2f6bcdc20838c..fef890d1204d18 100644
--- a/llvm/lib/ObjectYAML/MinidumpEmitter.cpp
+++ b/llvm/lib/ObjectYAML/MinidumpEmitter.cpp
@@ -138,11 +138,14 @@ static size_t layout(BlobAllocator &File, MinidumpYAML::ExceptionStream &S) {
static size_t layout(BlobAllocator &File, MinidumpYAML::Memory64ListStream &S) {
size_t BaseRVA = File.tell() + sizeof(minidump::Memory64ListHeader);
+ BaseRVA += S.Entries.size() * sizeof(minidump::MemoryDescriptor_64);
S.Header.BaseRVA = BaseRVA;
S.Header.NumberOfMemoryRanges = S.Entries.size();
File.allocateObject(S.Header);
- for (auto &E : S.Entries)
+ for (auto &E : S.Entries) {
+ E.Entry.DataSize = E.Content.binary_size();
File.allocateObject(E.Entry);
+ }
// Save the new offset for the stream size.
size_t DataEnd = File.tell();
diff --git a/llvm/lib/ObjectYAML/MinidumpYAML.cpp b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
index b318a9e5cc2e5e..b6ae78f2c213e7 100644
--- a/llvm/lib/ObjectYAML/MinidumpYAML.cpp
+++ b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
@@ -373,7 +373,6 @@ void yaml::MappingContextTraits<MemoryDescriptor, yaml::BinaryRef>::mapping(
void yaml::MappingContextTraits<MemoryDescriptor_64, yaml::BinaryRef>::mapping(
IO &IO, MemoryDescriptor_64 &Memory, BinaryRef &Content) {
mapRequiredHex(IO, "Start of Memory Range", Memory.StartOfMemoryRange);
- mapRequiredHex(IO, "Data Size", Memory.DataSize);
IO.mapRequired("Content", Content);
}
diff --git a/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp b/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
index 79fb903bcfd10c..377b57c0d7a9ac 100644
--- a/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
+++ b/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
@@ -345,10 +345,8 @@ TEST(MinidumpYAML, MemoryRegion_64bit) {
- Type: Memory64List
Memory Ranges:
- Start of Memory Range: 0x7FFFFFCF0818283
- Data Size: 8
Content: '68656c6c6f'
- Start of Memory Range: 0x7FFFFFFF0818283
- Data Size: 8
Content: '776f726c64'
)");
@@ -367,20 +365,23 @@ TEST(MinidumpYAML, MemoryRegion_64bit) {
const minidump::MemoryDescriptor_64 &DescOne = MemoryList[0];
ASSERT_EQ(0x7FFFFFCF0818283u, DescOne.StartOfMemoryRange);
- ASSERT_EQ(8u, DescOne.DataSize);
+ ASSERT_EQ(5u, DescOne.DataSize);
const minidump::MemoryDescriptor_64 &DescTwo = MemoryList[1];
ASSERT_EQ(0x7FFFFFFF0818283u, DescTwo.StartOfMemoryRange);
- ASSERT_EQ(8u, DescTwo.DataSize);
+ ASSERT_EQ(5u, DescTwo.DataSize);
const std::optional<ArrayRef<uint8_t>> ExpectedContent =
File.getRawStream(StreamType::Memory64List);
ASSERT_TRUE(ExpectedContent);
- const ArrayRef<uint8_t> Content = *ExpectedContent;
- const size_t offset =
- sizeof(Memory64ListHeader) + (sizeof(MemoryDescriptor_64) * 2);
- const uint64_t *Array =
- reinterpret_cast<const uint64_t *>(Content.slice(offset, 16).data(), 16);
- ASSERT_EQ(0x7000000000002Au, Array[0]);
- ASSERT_EQ(0x7000A00000000Au, Array[1]);
+ const size_t ExpectedStreamSize = sizeof(Memory64ListHeader) + (sizeof(MemoryDescriptor_64) * 2);
+ ASSERT_EQ(ExpectedStreamSize, ExpectedContent->size());
+
+ Expected<ArrayRef<uint8_t>> DescOneExpectedContentSlice = File.getRawData(DescOne);
+ ASSERT_THAT_EXPECTED(DescOneExpectedContentSlice, Succeeded());
+ ASSERT_EQ("hello", reinterpret_cast<const char *>(DescOneExpectedContentSlice->data()));
+
+ Expected<ArrayRef<uint8_t>> DescTwoExpectedContentSlice = File.getRawData(DescTwo);
+ ASSERT_THAT_EXPECTED(DescTwoExpectedContentSlice, Succeeded());
+ ASSERT_EQ("world", reinterpret_cast<const char *>(DescTwoExpectedContentSlice->data()));
}
>From b7352c9efdd7a8194514f858c98404c50b1c01f9 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Wed, 31 Jul 2024 11:43:58 -0700
Subject: [PATCH 04/19] Fix test with arrayRefFrom suggestion
---
lldb/test/Shell/Minidump/Inputs/linux-x86_64.yaml | 6 +++---
llvm/lib/Object/Minidump.cpp | 2 +-
llvm/tools/obj2yaml/obj2yaml.cpp | 6 ++++++
llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp | 11 ++++++++---
4 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/lldb/test/Shell/Minidump/Inputs/linux-x86_64.yaml b/lldb/test/Shell/Minidump/Inputs/linux-x86_64.yaml
index 157903f368a08e..d2ae6543141e82 100644
--- a/lldb/test/Shell/Minidump/Inputs/linux-x86_64.yaml
+++ b/lldb/test/Shell/Minidump/Inputs/linux-x86_64.yaml
@@ -1,7 +1,7 @@
--- !minidump
-Streams:
+Streams:
- Type: ModuleList
- Modules:
+ Modules:
- Base of Image: 0x0000000000400000
Size of Image: 0x00001000
Module Name: '/tmp/test/linux-x86_64'
@@ -13,7 +13,7 @@ Streams:
Number of Processors: 40
Platform ID: Linux
CSD Version: 'Linux 3.13.0-91-generic #138-Ubuntu SMP Fri Jun 24 17:00:34 UTC 2016 x86_64'
- CPU:
+ CPU:
Vendor ID: GenuineIntel
Version Info: 0x00000000
Feature Info: 0x00000000
diff --git a/llvm/lib/Object/Minidump.cpp b/llvm/lib/Object/Minidump.cpp
index 06bcbe9be195f4..2d0565f1afe9a3 100644
--- a/llvm/lib/Object/Minidump.cpp
+++ b/llvm/lib/Object/Minidump.cpp
@@ -186,7 +186,7 @@ Expected<ArrayRef<MemoryDescriptor_64>> MinidumpFile::getMemory64List() {
if (!Descriptors)
return Descriptors.takeError();
- cacheMemory64RVAs(ListHeader->NumberOfMemoryRanges, *Descriptors);
+ cacheMemory64RVAs(ListHeader->BaseRVA, *Descriptors);
return Descriptors;
}
diff --git a/llvm/tools/obj2yaml/obj2yaml.cpp b/llvm/tools/obj2yaml/obj2yaml.cpp
index 63c8cc55ee2d4a..50afc1c56b5c56 100644
--- a/llvm/tools/obj2yaml/obj2yaml.cpp
+++ b/llvm/tools/obj2yaml/obj2yaml.cpp
@@ -101,6 +101,12 @@ static void reportError(StringRef Input, Error Err) {
int main(int argc, char *argv[]) {
InitLLVM X(argc, argv);
+
+ long x = 1;
+ bool debug = true;
+ while (debug) {
+ x++;
+ }
cl::HideUnrelatedOptions(Cat);
cl::ParseCommandLineOptions(
argc, argv, "Dump a YAML description from an object file", nullptr,
diff --git a/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp b/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
index 377b57c0d7a9ac..b431943594034c 100644
--- a/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
+++ b/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
@@ -377,11 +377,16 @@ TEST(MinidumpYAML, MemoryRegion_64bit) {
const size_t ExpectedStreamSize = sizeof(Memory64ListHeader) + (sizeof(MemoryDescriptor_64) * 2);
ASSERT_EQ(ExpectedStreamSize, ExpectedContent->size());
+ Expected<minidump::Memory64ListHeader> ExpectedHeader = File.getMemoryList64Header();
+ ASSERT_THAT_EXPECTED(ExpectedHeader, Succeeded());
+ ASSERT_EQ(ExpectedHeader->BaseRVA, 92u);
+
Expected<ArrayRef<uint8_t>> DescOneExpectedContentSlice = File.getRawData(DescOne);
ASSERT_THAT_EXPECTED(DescOneExpectedContentSlice, Succeeded());
- ASSERT_EQ("hello", reinterpret_cast<const char *>(DescOneExpectedContentSlice->data()));
+ ASSERT_EQ(5u, DescOneExpectedContentSlice->size());
+ ASSERT_EQ(arrayRefFromStringRef("hello"), *DescOneExpectedContentSlice);
Expected<ArrayRef<uint8_t>> DescTwoExpectedContentSlice = File.getRawData(DescTwo);
- ASSERT_THAT_EXPECTED(DescTwoExpectedContentSlice, Succeeded());
- ASSERT_EQ("world", reinterpret_cast<const char *>(DescTwoExpectedContentSlice->data()));
+ ASSERT_THAT_EXPECTED(DescTwoExpectedContentSlice, Succeeded());
+ ASSERT_EQ(arrayRefFromStringRef("world"), *DescTwoExpectedContentSlice);
}
>From 06f263f2c4dc9d7abc3a20f51a3f54929e4970e6 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Wed, 31 Jul 2024 14:12:38 -0700
Subject: [PATCH 05/19] Convert to an iterator instead of cacheing
---
llvm/include/llvm/Object/Minidump.h | 72 ++++++++++++++++---
llvm/lib/Object/Minidump.cpp | 31 +++-----
llvm/lib/ObjectYAML/MinidumpYAML.cpp | 9 ++-
llvm/tools/obj2yaml/obj2yaml.cpp | 6 --
.../unittests/ObjectYAML/MinidumpYAMLTest.cpp | 18 +++--
5 files changed, 88 insertions(+), 48 deletions(-)
diff --git a/llvm/include/llvm/Object/Minidump.h b/llvm/include/llvm/Object/Minidump.h
index 21446068de616b..b489d0c56738cd 100644
--- a/llvm/include/llvm/Object/Minidump.h
+++ b/llvm/include/llvm/Object/Minidump.h
@@ -11,6 +11,7 @@
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/fallible_iterator.h"
#include "llvm/ADT/iterator.h"
#include "llvm/BinaryFormat/Minidump.h"
#include "llvm/Object/Binary.h"
@@ -53,9 +54,6 @@ class MinidumpFile : public Binary {
return getDataSlice(getData(), Desc.RVA, Desc.DataSize);
}
- Expected<ArrayRef<uint8_t>>
- getRawData(minidump::MemoryDescriptor_64 Desc) const;
-
/// Returns the minidump string at the given offset. An error is returned if
/// we fail to parse the string, or the string is invalid UTF16.
Expected<std::string> getString(size_t Offset) const;
@@ -114,8 +112,6 @@ class MinidumpFile : public Binary {
minidump::StreamType::Memory64List);
}
- Expected<ArrayRef<minidump::MemoryDescriptor_64>> getMemory64List();
-
class MemoryInfoIterator
: public iterator_facade_base<MemoryInfoIterator,
std::forward_iterator_tag,
@@ -145,6 +141,68 @@ class MinidumpFile : public Binary {
size_t Stride;
};
+ class Memory64ListFacade {
+ struct Memory64Iterator {
+ public:
+ Memory64Iterator(size_t Count, uint64_t BaseRVA, const Memory64ListFacade *Parent)
+ : Parent(Parent), BaseRVA(BaseRVA), Count(Count) {};
+
+ const std::pair<minidump::MemoryDescriptor_64, ArrayRef<uint8_t>> operator*() {
+ return Parent->Next(this);
+ }
+
+ bool operator==(const Memory64Iterator &R) const {
+ return Parent == R.Parent && Count == R.Count;
+ }
+
+ bool operator!=(const Memory64Iterator &R) const {
+ return !(*this == R);
+ }
+
+ private:
+ friend class Memory64ListFacade;
+ const Memory64ListFacade *Parent;
+ uint64_t BaseRVA;
+ size_t Count;
+ };
+
+ public:
+ Memory64ListFacade(ArrayRef<uint8_t> Storage, std::vector<minidump::MemoryDescriptor_64> Descriptors, uint64_t BaseRVA)
+ : BaseRVA(BaseRVA), Storage(Storage), Descriptors(std::move(Descriptors)) {
+ };
+
+ Memory64Iterator begin() const {
+ return Memory64Iterator(0, BaseRVA, this);
+ }
+
+ Memory64Iterator end() const {
+ return Memory64Iterator(Descriptors.size(), BaseRVA, this);
+ }
+
+ size_t size() const {
+ return Descriptors.size();
+ }
+
+ private:
+ uint64_t BaseRVA;
+ ArrayRef<uint8_t> Storage;
+ std::vector<minidump::MemoryDescriptor_64> Descriptors;
+
+ const std::pair<minidump::MemoryDescriptor_64, ArrayRef<uint8_t>> Next(Memory64Iterator *Iterator) const {
+ assert(Descriptors.size() > Iterator->Count);
+ minidump::MemoryDescriptor_64 Descriptor = Descriptors[Iterator->Count];
+ ArrayRef<uint8_t> Content = Storage.slice(Iterator->BaseRVA, Descriptor.DataSize);
+ Iterator->BaseRVA += Descriptor.DataSize;
+ Iterator->Count++;
+ return std::make_pair(Descriptor, Content);
+ }
+ };
+
+ /// 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 unreadable.
+ Expected<Memory64ListFacade> getMemory64List() const;
+
/// Returns the list of descriptors embedded in the MemoryInfoList stream. The
/// descriptors provide properties (e.g. permissions) of interesting regions
/// of memory at the time the minidump was taken. An error is returned if the
@@ -195,13 +253,9 @@ class MinidumpFile : public Binary {
template <typename T>
Expected<ArrayRef<T>> getListStream(minidump::StreamType Stream) const;
- void cacheMemory64RVAs(uint64_t BaseRVA,
- ArrayRef<minidump::MemoryDescriptor_64> Descriptors);
-
const minidump::Header &Header;
ArrayRef<minidump::Directory> Streams;
DenseMap<minidump::StreamType, std::size_t> StreamMap;
- std::unordered_map<uint64_t, uint64_t> Memory64DescriptorToRvaMap;
};
template <typename T>
diff --git a/llvm/lib/Object/Minidump.cpp b/llvm/lib/Object/Minidump.cpp
index 2d0565f1afe9a3..950aa328a56ecc 100644
--- a/llvm/lib/Object/Minidump.cpp
+++ b/llvm/lib/Object/Minidump.cpp
@@ -156,19 +156,8 @@ MinidumpFile::create(MemoryBufferRef Source) {
new MinidumpFile(Source, Hdr, *ExpectedStreams, std::move(StreamMap)));
}
-void MinidumpFile::cacheMemory64RVAs(
- uint64_t BaseRVA, ArrayRef<minidump::MemoryDescriptor_64> Descriptors) {
- // Check if we've already cached the RVAs for this list.
- if (!Memory64DescriptorToRvaMap.empty())
- return;
-
- for (const auto &MD : Descriptors) {
- Memory64DescriptorToRvaMap[MD.StartOfMemoryRange] = BaseRVA;
- BaseRVA += MD.DataSize;
- }
-}
-Expected<ArrayRef<MemoryDescriptor_64>> MinidumpFile::getMemory64List() {
+Expected<MinidumpFile::Memory64ListFacade> MinidumpFile::getMemory64List() const {
Expected<minidump::Memory64ListHeader> ListHeader = getMemoryList64Header();
if (!ListHeader)
return ListHeader.takeError();
@@ -186,16 +175,14 @@ Expected<ArrayRef<MemoryDescriptor_64>> MinidumpFile::getMemory64List() {
if (!Descriptors)
return Descriptors.takeError();
- cacheMemory64RVAs(ListHeader->BaseRVA, *Descriptors);
- return Descriptors;
-}
-
-Expected<ArrayRef<uint8_t>>
-MinidumpFile::getRawData(MemoryDescriptor_64 MD) const {
- if (Memory64DescriptorToRvaMap.count(MD.StartOfMemoryRange) > 0) {
- uint64_t RVA = Memory64DescriptorToRvaMap.at(MD.StartOfMemoryRange);
- return getDataSlice(getData(), RVA, MD.DataSize);
+ uint64_t FileSize = getData().size();
+ uint64_t BaseRVA = ListHeader->BaseRVA;
+ uint64_t Offset = BaseRVA;
+ for (const minidump::MemoryDescriptor_64 &Descriptor : *Descriptors) {
+ Offset += Descriptor.DataSize;
+ if (Offset > FileSize)
+ return createEOFError();
}
- return createError("No such Memory Descriptor64.");
+ return Memory64ListFacade(getData(), *Descriptors, BaseRVA);
}
diff --git a/llvm/lib/ObjectYAML/MinidumpYAML.cpp b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
index b6ae78f2c213e7..9a335ecf3f9942 100644
--- a/llvm/lib/ObjectYAML/MinidumpYAML.cpp
+++ b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
@@ -522,12 +522,11 @@ Expected<std::unique_ptr<Stream>> Stream::create(const Directory &StreamDesc,
auto ExpectedList = File.getMemory64List();
if (!ExpectedList)
return ExpectedList.takeError();
+ object::MinidumpFile::Memory64ListFacade &Memory64List = *ExpectedList;
std::vector<Memory64ListStream::entry_type> Ranges;
- for (const MemoryDescriptor_64 &MD : *ExpectedList) {
- auto ExpectedContent = File.getRawData(MD);
- if (!ExpectedContent)
- return ExpectedContent.takeError();
- Ranges.push_back({MD, *ExpectedContent});
+ for (auto It = Memory64List.begin(); It != Memory64List.end();) {
+ std::pair<MemoryDescriptor_64, ArrayRef<uint8_t>> Pair = *It;
+ Ranges.push_back({Pair.first, Pair.second});
}
return std::make_unique<Memory64ListStream>(std::move(Ranges));
}
diff --git a/llvm/tools/obj2yaml/obj2yaml.cpp b/llvm/tools/obj2yaml/obj2yaml.cpp
index 50afc1c56b5c56..63c8cc55ee2d4a 100644
--- a/llvm/tools/obj2yaml/obj2yaml.cpp
+++ b/llvm/tools/obj2yaml/obj2yaml.cpp
@@ -101,12 +101,6 @@ static void reportError(StringRef Input, Error Err) {
int main(int argc, char *argv[]) {
InitLLVM X(argc, argv);
-
- long x = 1;
- bool debug = true;
- while (debug) {
- x++;
- }
cl::HideUnrelatedOptions(Cat);
cl::ParseCommandLineOptions(
argc, argv, "Dump a YAML description from an object file", nullptr,
diff --git a/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp b/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
index b431943594034c..ee72ee079f2fc7 100644
--- a/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
+++ b/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
@@ -355,19 +355,23 @@ TEST(MinidumpYAML, MemoryRegion_64bit) {
ASSERT_EQ(1u, File.streams().size());
- Expected<ArrayRef<minidump::MemoryDescriptor_64>> ExpectedMemoryList =
+ Expected<object::MinidumpFile::Memory64ListFacade> ExpectedMemoryList =
File.getMemory64List();
ASSERT_THAT_EXPECTED(ExpectedMemoryList, Succeeded());
- ArrayRef<minidump::MemoryDescriptor_64> MemoryList = *ExpectedMemoryList;
+ object::MinidumpFile::Memory64ListFacade MemoryList = *ExpectedMemoryList;
ASSERT_EQ(2u, MemoryList.size());
- const minidump::MemoryDescriptor_64 &DescOne = MemoryList[0];
+ auto Iterator = MemoryList.begin();
+
+ auto DescOnePair = *Iterator;
+ const minidump::MemoryDescriptor_64 &DescOne = DescOnePair.first;
ASSERT_EQ(0x7FFFFFCF0818283u, DescOne.StartOfMemoryRange);
ASSERT_EQ(5u, DescOne.DataSize);
- const minidump::MemoryDescriptor_64 &DescTwo = MemoryList[1];
+ auto DescTwoPair = *Iterator;
+ const minidump::MemoryDescriptor_64 &DescTwo = DescTwoPair.first;
ASSERT_EQ(0x7FFFFFFF0818283u, DescTwo.StartOfMemoryRange);
ASSERT_EQ(5u, DescTwo.DataSize);
@@ -381,12 +385,14 @@ TEST(MinidumpYAML, MemoryRegion_64bit) {
ASSERT_THAT_EXPECTED(ExpectedHeader, Succeeded());
ASSERT_EQ(ExpectedHeader->BaseRVA, 92u);
- Expected<ArrayRef<uint8_t>> DescOneExpectedContentSlice = File.getRawData(DescOne);
+ Expected<ArrayRef<uint8_t>> DescOneExpectedContentSlice = DescOnePair.second;
ASSERT_THAT_EXPECTED(DescOneExpectedContentSlice, Succeeded());
ASSERT_EQ(5u, DescOneExpectedContentSlice->size());
ASSERT_EQ(arrayRefFromStringRef("hello"), *DescOneExpectedContentSlice);
- Expected<ArrayRef<uint8_t>> DescTwoExpectedContentSlice = File.getRawData(DescTwo);
+ Expected<ArrayRef<uint8_t>> DescTwoExpectedContentSlice = DescTwoPair.second;
ASSERT_THAT_EXPECTED(DescTwoExpectedContentSlice, Succeeded());
ASSERT_EQ(arrayRefFromStringRef("world"), *DescTwoExpectedContentSlice);
+
+ ASSERT_EQ(Iterator, MemoryList.end());
}
>From fe8417280aa41a6a5f94389b979243ea443613e4 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Wed, 31 Jul 2024 14:35:00 -0700
Subject: [PATCH 06/19] Readd const to the objectfile reference
---
llvm/include/llvm/Object/Minidump.h | 1 -
llvm/include/llvm/ObjectYAML/MinidumpYAML.h | 4 ++--
llvm/lib/ObjectYAML/MinidumpYAML.cpp | 4 ++--
llvm/tools/obj2yaml/minidump2yaml.cpp | 2 +-
llvm/tools/obj2yaml/obj2yaml.h | 2 +-
5 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/llvm/include/llvm/Object/Minidump.h b/llvm/include/llvm/Object/Minidump.h
index b489d0c56738cd..766b0947809fcf 100644
--- a/llvm/include/llvm/Object/Minidump.h
+++ b/llvm/include/llvm/Object/Minidump.h
@@ -16,7 +16,6 @@
#include "llvm/BinaryFormat/Minidump.h"
#include "llvm/Object/Binary.h"
#include "llvm/Support/Error.h"
-#include <map>
namespace llvm {
namespace object {
diff --git a/llvm/include/llvm/ObjectYAML/MinidumpYAML.h b/llvm/include/llvm/ObjectYAML/MinidumpYAML.h
index 7f7924b92029fa..426e4027c75270 100644
--- a/llvm/include/llvm/ObjectYAML/MinidumpYAML.h
+++ b/llvm/include/llvm/ObjectYAML/MinidumpYAML.h
@@ -51,7 +51,7 @@ struct Stream {
/// Create a stream from the given stream directory entry.
static Expected<std::unique_ptr<Stream>>
- create(const minidump::Directory &StreamDesc, object::MinidumpFile &File);
+ create(const minidump::Directory &StreamDesc, const object::MinidumpFile &File);
};
namespace detail {
@@ -233,7 +233,7 @@ struct Object {
/// The list of streams in this minidump object.
std::vector<std::unique_ptr<Stream>> Streams;
- static Expected<Object> create(object::MinidumpFile &File);
+ static Expected<Object> create(const object::MinidumpFile &File);
};
} // namespace MinidumpYAML
diff --git a/llvm/lib/ObjectYAML/MinidumpYAML.cpp b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
index 9a335ecf3f9942..1801e6104108cd 100644
--- a/llvm/lib/ObjectYAML/MinidumpYAML.cpp
+++ b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
@@ -484,7 +484,7 @@ void yaml::MappingTraits<Object>::mapping(IO &IO, Object &O) {
}
Expected<std::unique_ptr<Stream>> Stream::create(const Directory &StreamDesc,
- object::MinidumpFile &File) {
+ const object::MinidumpFile &File) {
StreamKind Kind = getKind(StreamDesc.Type);
switch (Kind) {
case StreamKind::Exception: {
@@ -586,7 +586,7 @@ Expected<std::unique_ptr<Stream>> Stream::create(const Directory &StreamDesc,
llvm_unreachable("Unhandled stream kind!");
}
-Expected<Object> Object::create(object::MinidumpFile &File) {
+Expected<Object> Object::create(const object::MinidumpFile &File) {
std::vector<std::unique_ptr<Stream>> Streams;
Streams.reserve(File.streams().size());
for (const Directory &StreamDesc : File.streams()) {
diff --git a/llvm/tools/obj2yaml/minidump2yaml.cpp b/llvm/tools/obj2yaml/minidump2yaml.cpp
index 53486d57d0932d..7b25b1869c6b6a 100644
--- a/llvm/tools/obj2yaml/minidump2yaml.cpp
+++ b/llvm/tools/obj2yaml/minidump2yaml.cpp
@@ -13,7 +13,7 @@
using namespace llvm;
-Error minidump2yaml(raw_ostream &Out, object::MinidumpFile &Obj) {
+Error minidump2yaml(raw_ostream &Out, const object::MinidumpFile &Obj) {
auto ExpectedObject = MinidumpYAML::Object::create(Obj);
if (!ExpectedObject)
return ExpectedObject.takeError();
diff --git a/llvm/tools/obj2yaml/obj2yaml.h b/llvm/tools/obj2yaml/obj2yaml.h
index 96256bc1307d65..03d7db5317acde 100644
--- a/llvm/tools/obj2yaml/obj2yaml.h
+++ b/llvm/tools/obj2yaml/obj2yaml.h
@@ -28,7 +28,7 @@ llvm::Error elf2yaml(llvm::raw_ostream &Out,
llvm::Error macho2yaml(llvm::raw_ostream &Out, const llvm::object::Binary &Obj,
unsigned RawSegments);
llvm::Error minidump2yaml(llvm::raw_ostream &Out,
- llvm::object::MinidumpFile &Obj);
+ const llvm::object::MinidumpFile &Obj);
llvm::Error xcoff2yaml(llvm::raw_ostream &Out,
const llvm::object::XCOFFObjectFile &Obj);
std::error_code wasm2yaml(llvm::raw_ostream &Out,
>From e4ad8756dcf349dd04548cb771165cdd5f104609 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Wed, 31 Jul 2024 15:20:14 -0700
Subject: [PATCH 07/19] Run git-clang-format
---
llvm/include/llvm/Object/Minidump.h | 63 ++++++++++---------
llvm/include/llvm/ObjectYAML/MinidumpYAML.h | 3 +-
llvm/lib/Object/Minidump.cpp | 4 +-
llvm/lib/ObjectYAML/MinidumpYAML.cpp | 4 +-
.../unittests/ObjectYAML/MinidumpYAMLTest.cpp | 8 ++-
5 files changed, 44 insertions(+), 38 deletions(-)
diff --git a/llvm/include/llvm/Object/Minidump.h b/llvm/include/llvm/Object/Minidump.h
index 766b0947809fcf..9249ec6fea8a75 100644
--- a/llvm/include/llvm/Object/Minidump.h
+++ b/llvm/include/llvm/Object/Minidump.h
@@ -142,11 +142,13 @@ class MinidumpFile : public Binary {
class Memory64ListFacade {
struct Memory64Iterator {
- public:
- Memory64Iterator(size_t Count, uint64_t BaseRVA, const Memory64ListFacade *Parent)
- : Parent(Parent), BaseRVA(BaseRVA), Count(Count) {};
+ public:
+ Memory64Iterator(size_t Count, uint64_t BaseRVA,
+ const Memory64ListFacade *Parent)
+ : Parent(Parent), BaseRVA(BaseRVA), Count(Count){};
- const std::pair<minidump::MemoryDescriptor_64, ArrayRef<uint8_t>> operator*() {
+ const std::pair<minidump::MemoryDescriptor_64, ArrayRef<uint8_t>>
+ operator*() {
return Parent->Next(this);
}
@@ -154,21 +156,21 @@ class MinidumpFile : public Binary {
return Parent == R.Parent && Count == R.Count;
}
- bool operator!=(const Memory64Iterator &R) const {
- return !(*this == R);
- }
+ bool operator!=(const Memory64Iterator &R) const { return !(*this == R); }
- private:
- friend class Memory64ListFacade;
- const Memory64ListFacade *Parent;
- uint64_t BaseRVA;
- size_t Count;
+ private:
+ friend class Memory64ListFacade;
+ const Memory64ListFacade *Parent;
+ uint64_t BaseRVA;
+ size_t Count;
};
- public:
- Memory64ListFacade(ArrayRef<uint8_t> Storage, std::vector<minidump::MemoryDescriptor_64> Descriptors, uint64_t BaseRVA)
- : BaseRVA(BaseRVA), Storage(Storage), Descriptors(std::move(Descriptors)) {
- };
+ public:
+ Memory64ListFacade(ArrayRef<uint8_t> Storage,
+ std::vector<minidump::MemoryDescriptor_64> Descriptors,
+ uint64_t BaseRVA)
+ : BaseRVA(BaseRVA), Storage(Storage),
+ Descriptors(std::move(Descriptors)) {};
Memory64Iterator begin() const {
return Memory64Iterator(0, BaseRVA, this);
@@ -178,28 +180,29 @@ class MinidumpFile : public Binary {
return Memory64Iterator(Descriptors.size(), BaseRVA, this);
}
- size_t size() const {
- return Descriptors.size();
- }
-
- private:
- uint64_t BaseRVA;
- ArrayRef<uint8_t> Storage;
- std::vector<minidump::MemoryDescriptor_64> Descriptors;
+ size_t size() const { return Descriptors.size(); }
+
+ private:
+ uint64_t BaseRVA;
+ ArrayRef<uint8_t> Storage;
+ std::vector<minidump::MemoryDescriptor_64> Descriptors;
- const std::pair<minidump::MemoryDescriptor_64, ArrayRef<uint8_t>> Next(Memory64Iterator *Iterator) const {
+ const std::pair<minidump::MemoryDescriptor_64, ArrayRef<uint8_t>>
+ Next(Memory64Iterator *Iterator) const {
assert(Descriptors.size() > Iterator->Count);
minidump::MemoryDescriptor_64 Descriptor = Descriptors[Iterator->Count];
- ArrayRef<uint8_t> Content = Storage.slice(Iterator->BaseRVA, Descriptor.DataSize);
+ ArrayRef<uint8_t> Content =
+ Storage.slice(Iterator->BaseRVA, Descriptor.DataSize);
Iterator->BaseRVA += Descriptor.DataSize;
Iterator->Count++;
return std::make_pair(Descriptor, Content);
}
};
- /// 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 unreadable.
+ /// 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
+ /// unreadable.
Expected<Memory64ListFacade> getMemory64List() const;
/// Returns the list of descriptors embedded in the MemoryInfoList stream. The
@@ -236,7 +239,7 @@ class MinidumpFile : public Binary {
ArrayRef<minidump::Directory> Streams,
DenseMap<minidump::StreamType, std::size_t> StreamMap)
: Binary(ID_Minidump, Source), Header(Header), Streams(Streams),
- StreamMap(std::move(StreamMap)) {}
+ StreamMap(std::move(StreamMap)) {};
ArrayRef<uint8_t> getData() const {
return arrayRefFromStringRef(Data.getBuffer());
diff --git a/llvm/include/llvm/ObjectYAML/MinidumpYAML.h b/llvm/include/llvm/ObjectYAML/MinidumpYAML.h
index 426e4027c75270..5c2ecd7b97314c 100644
--- a/llvm/include/llvm/ObjectYAML/MinidumpYAML.h
+++ b/llvm/include/llvm/ObjectYAML/MinidumpYAML.h
@@ -51,7 +51,8 @@ struct Stream {
/// Create a stream from the given stream directory entry.
static Expected<std::unique_ptr<Stream>>
- create(const minidump::Directory &StreamDesc, const object::MinidumpFile &File);
+ create(const minidump::Directory &StreamDesc,
+ const object::MinidumpFile &File);
};
namespace detail {
diff --git a/llvm/lib/Object/Minidump.cpp b/llvm/lib/Object/Minidump.cpp
index 950aa328a56ecc..576d382dc61526 100644
--- a/llvm/lib/Object/Minidump.cpp
+++ b/llvm/lib/Object/Minidump.cpp
@@ -156,8 +156,8 @@ MinidumpFile::create(MemoryBufferRef Source) {
new MinidumpFile(Source, Hdr, *ExpectedStreams, std::move(StreamMap)));
}
-
-Expected<MinidumpFile::Memory64ListFacade> MinidumpFile::getMemory64List() const {
+Expected<MinidumpFile::Memory64ListFacade>
+MinidumpFile::getMemory64List() const {
Expected<minidump::Memory64ListHeader> ListHeader = getMemoryList64Header();
if (!ListHeader)
return ListHeader.takeError();
diff --git a/llvm/lib/ObjectYAML/MinidumpYAML.cpp b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
index 1801e6104108cd..188efaf135aef1 100644
--- a/llvm/lib/ObjectYAML/MinidumpYAML.cpp
+++ b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
@@ -483,8 +483,8 @@ void yaml::MappingTraits<Object>::mapping(IO &IO, Object &O) {
IO.mapRequired("Streams", O.Streams);
}
-Expected<std::unique_ptr<Stream>> Stream::create(const Directory &StreamDesc,
- const object::MinidumpFile &File) {
+Expected<std::unique_ptr<Stream>>
+Stream::create(const Directory &StreamDesc, const object::MinidumpFile &File) {
StreamKind Kind = getKind(StreamDesc.Type);
switch (Kind) {
case StreamKind::Exception: {
diff --git a/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp b/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
index ee72ee079f2fc7..f31c7d48423196 100644
--- a/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
+++ b/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
@@ -378,10 +378,12 @@ TEST(MinidumpYAML, MemoryRegion_64bit) {
const std::optional<ArrayRef<uint8_t>> ExpectedContent =
File.getRawStream(StreamType::Memory64List);
ASSERT_TRUE(ExpectedContent);
- const size_t ExpectedStreamSize = sizeof(Memory64ListHeader) + (sizeof(MemoryDescriptor_64) * 2);
+ const size_t ExpectedStreamSize =
+ sizeof(Memory64ListHeader) + (sizeof(MemoryDescriptor_64) * 2);
ASSERT_EQ(ExpectedStreamSize, ExpectedContent->size());
- Expected<minidump::Memory64ListHeader> ExpectedHeader = File.getMemoryList64Header();
+ Expected<minidump::Memory64ListHeader> ExpectedHeader =
+ File.getMemoryList64Header();
ASSERT_THAT_EXPECTED(ExpectedHeader, Succeeded());
ASSERT_EQ(ExpectedHeader->BaseRVA, 92u);
@@ -391,7 +393,7 @@ TEST(MinidumpYAML, MemoryRegion_64bit) {
ASSERT_EQ(arrayRefFromStringRef("hello"), *DescOneExpectedContentSlice);
Expected<ArrayRef<uint8_t>> DescTwoExpectedContentSlice = DescTwoPair.second;
- ASSERT_THAT_EXPECTED(DescTwoExpectedContentSlice, Succeeded());
+ ASSERT_THAT_EXPECTED(DescTwoExpectedContentSlice, Succeeded());
ASSERT_EQ(arrayRefFromStringRef("world"), *DescTwoExpectedContentSlice);
ASSERT_EQ(Iterator, MemoryList.end());
>From 6052339a819a2d28433ad2420ceecb23e7a1be26 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Thu, 1 Aug 2024 10:03:08 -0700
Subject: [PATCH 08/19] Manual fix formatting for {} and fix the basic.yaml
test
---
llvm/include/llvm/Object/Minidump.h | 2 +-
llvm/include/llvm/ObjectYAML/MinidumpYAML.h | 2 +-
llvm/test/tools/obj2yaml/Minidump/basic.yaml | 12 ++++++------
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/llvm/include/llvm/Object/Minidump.h b/llvm/include/llvm/Object/Minidump.h
index 9249ec6fea8a75..9fa02920cd70e4 100644
--- a/llvm/include/llvm/Object/Minidump.h
+++ b/llvm/include/llvm/Object/Minidump.h
@@ -145,7 +145,7 @@ class MinidumpFile : public Binary {
public:
Memory64Iterator(size_t Count, uint64_t BaseRVA,
const Memory64ListFacade *Parent)
- : Parent(Parent), BaseRVA(BaseRVA), Count(Count){};
+ : Parent(Parent), BaseRVA(BaseRVA), Count(Count) {};
const std::pair<minidump::MemoryDescriptor_64, ArrayRef<uint8_t>>
operator*() {
diff --git a/llvm/include/llvm/ObjectYAML/MinidumpYAML.h b/llvm/include/llvm/ObjectYAML/MinidumpYAML.h
index 5c2ecd7b97314c..9c0a97ea92a0d6 100644
--- a/llvm/include/llvm/ObjectYAML/MinidumpYAML.h
+++ b/llvm/include/llvm/ObjectYAML/MinidumpYAML.h
@@ -120,7 +120,7 @@ struct Memory64ListStream
explicit Memory64ListStream(
std::vector<detail::ParsedMemory64Descriptor> Entries = {})
- : ListStream(Entries){};
+ : ListStream(Entries) {};
};
/// ExceptionStream minidump stream.
diff --git a/llvm/test/tools/obj2yaml/Minidump/basic.yaml b/llvm/test/tools/obj2yaml/Minidump/basic.yaml
index 9702f82dc58437..ad563212885d31 100644
--- a/llvm/test/tools/obj2yaml/Minidump/basic.yaml
+++ b/llvm/test/tools/obj2yaml/Minidump/basic.yaml
@@ -72,8 +72,8 @@ Streams:
Content: '8485868788'
- Type: Memory64List
Memory Ranges:
- - Start of Memory Range: 0x7FFFFFFF0818283
- Content: '104101108108111'
+ - Start of Memory Range: 0x7FFFFFCF08180283
+ Content: '68656c6c6f776f726c64'
- Type: MemoryInfoList
Memory Ranges:
- Base Address: 0x0000000000000000
@@ -163,10 +163,10 @@ Streams:
# CHECK-NEXT: Memory Ranges:
# CHECK-NEXT: - Start of Memory Range: 0x7C7D7E7F80818283
# CHECK-NEXT: Content: '8485868788'
-# CHECK-NEXT: - Type: Memory64List
-# CHECK-NEXT: Memory Ranges:
-# CHECK-NEXT: - Start of Memory Range: 0x7FFFFFFF0818283
-# CHECK-NEXT: Content: '104101108108111'
+# CHECK-NEXT: - Type: Memory64List
+# CHECK-NEXT: Memory Ranges:
+# CHECK-NEXT: - Start of Memory Range: 0x7FFFFFCF08180283
+# CHECK-NEXT: Content: 68656C6C6F776F726C64
# CHECK-NEXT: - Type: MemoryInfoList
# CHECK-NEXT: Memory Ranges:
# CHECK-NEXT: - Base Address: 0x0
>From 4fe66e1b9135dbe58255c1545ba3fa218053de20 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Tue, 6 Aug 2024 10:41:54 -0700
Subject: [PATCH 09/19] Get fallible iterator happy path complete, need to
remove debug code and add unhappy testcase
---
llvm/include/llvm/Object/Minidump.h | 102 ++++++++++--------
llvm/lib/Object/Minidump.cpp | 17 +--
llvm/lib/ObjectYAML/MinidumpEmitter.cpp | 6 +-
llvm/lib/ObjectYAML/MinidumpYAML.cpp | 39 +++++--
.../unittests/ObjectYAML/MinidumpYAMLTest.cpp | 20 ++--
5 files changed, 108 insertions(+), 76 deletions(-)
diff --git a/llvm/include/llvm/Object/Minidump.h b/llvm/include/llvm/Object/Minidump.h
index 9fa02920cd70e4..9dc986379642c8 100644
--- a/llvm/include/llvm/Object/Minidump.h
+++ b/llvm/include/llvm/Object/Minidump.h
@@ -17,6 +17,8 @@
#include "llvm/Object/Binary.h"
#include "llvm/Support/Error.h"
+#include <iostream>
+
namespace llvm {
namespace object {
@@ -140,70 +142,78 @@ class MinidumpFile : public Binary {
size_t Stride;
};
- class Memory64ListFacade {
- struct Memory64Iterator {
- public:
- Memory64Iterator(size_t Count, uint64_t BaseRVA,
- const Memory64ListFacade *Parent)
- : Parent(Parent), BaseRVA(BaseRVA), Count(Count) {};
+class Memory64Iterator {
+ public:
+ static Memory64Iterator begin(ArrayRef<uint8_t> Storage, ArrayRef<minidump::MemoryDescriptor_64> Descriptors, uint64_t BaseRVA) {
+ return Memory64Iterator(Storage, Descriptors, BaseRVA);
+ }
- const std::pair<minidump::MemoryDescriptor_64, ArrayRef<uint8_t>>
- operator*() {
- return Parent->Next(this);
- }
+ static Memory64Iterator end() {
+ return Memory64Iterator();
+ }
- bool operator==(const Memory64Iterator &R) const {
- return Parent == R.Parent && Count == R.Count;
- }
+ std::pair<minidump::MemoryDescriptor_64, ArrayRef<uint8_t>> Current;
- bool operator!=(const Memory64Iterator &R) const { return !(*this == R); }
+ bool operator==(const Memory64Iterator &R) const {
+ return isEnd == R.isEnd;
+ }
- private:
- friend class Memory64ListFacade;
- const Memory64ListFacade *Parent;
- uint64_t BaseRVA;
- size_t Count;
- };
+ bool operator!=(const Memory64Iterator &R) const { return !(*this == R); }
- public:
- Memory64ListFacade(ArrayRef<uint8_t> Storage,
- std::vector<minidump::MemoryDescriptor_64> Descriptors,
- uint64_t BaseRVA)
- : BaseRVA(BaseRVA), Storage(Storage),
- Descriptors(std::move(Descriptors)) {};
-
- Memory64Iterator begin() const {
- return Memory64Iterator(0, BaseRVA, this);
+ const std::pair<minidump::MemoryDescriptor_64, ArrayRef<uint8_t>> &operator*() const {
+ return Current;
}
- Memory64Iterator end() const {
- return Memory64Iterator(Descriptors.size(), BaseRVA, this);
+ const std::pair<minidump::MemoryDescriptor_64, ArrayRef<uint8_t>> *operator->() const {
+ return &Current;
}
- size_t size() const { return Descriptors.size(); }
+ Error inc() {
+ if (Storage.size() == 0 || Descriptors.size() == 0)
+ return make_error<GenericBinaryError>("No Memory64List Stream", object_error::parse_failed);
- private:
- uint64_t BaseRVA;
- ArrayRef<uint8_t> Storage;
- std::vector<minidump::MemoryDescriptor_64> Descriptors;
+ if (Index >= Descriptors.size())
+ return createError("Can't read past of Memory64List Stream");
+
+ const minidump::MemoryDescriptor_64 &Descriptor = Descriptors[Index];
+ if (RVA + Descriptor.DataSize > Storage.size())
+ return make_error<GenericBinaryError>("Memory64 Descriptor exceeds end of file.",
+ object_error::unexpected_eof);
- const std::pair<minidump::MemoryDescriptor_64, ArrayRef<uint8_t>>
- Next(Memory64Iterator *Iterator) const {
- assert(Descriptors.size() > Iterator->Count);
- minidump::MemoryDescriptor_64 Descriptor = Descriptors[Iterator->Count];
ArrayRef<uint8_t> Content =
- Storage.slice(Iterator->BaseRVA, Descriptor.DataSize);
- Iterator->BaseRVA += Descriptor.DataSize;
- Iterator->Count++;
- return std::make_pair(Descriptor, Content);
+ Storage.slice(RVA, Descriptor.DataSize);
+ Current = std::make_pair(Descriptor, Content);
+ RVA += Descriptor.DataSize;
+ Index++;
+ if (Index >= Descriptors.size())
+ isEnd = true;
+ return Error::success();
}
+
+ private:
+ Memory64Iterator(ArrayRef<uint8_t> Storage,
+ ArrayRef<minidump::MemoryDescriptor_64> Descriptors,
+ uint64_t BaseRVA)
+ : RVA(BaseRVA), Storage(Storage),
+ Descriptors(Descriptors), isEnd(false) {}
+
+ Memory64Iterator() : RVA(0),
+ Storage(ArrayRef<uint8_t>()), Descriptors(ArrayRef<minidump::MemoryDescriptor_64>()), isEnd(true) {}
+
+ size_t Index = 0;
+ uint64_t RVA;
+ ArrayRef<uint8_t> Storage;
+ ArrayRef<minidump::MemoryDescriptor_64> Descriptors;
+ bool isEnd;
};
+ using FallibleMemory64Iterator = llvm::fallible_iterator<Memory64Iterator>;
+
/// 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
/// unreadable.
- Expected<Memory64ListFacade> getMemory64List() const;
+ Expected<iterator_range<FallibleMemory64Iterator>> getMemory64List(Error &Err) const;
/// Returns the list of descriptors embedded in the MemoryInfoList stream. The
/// descriptors provide properties (e.g. permissions) of interesting regions
@@ -239,7 +249,7 @@ class MinidumpFile : public Binary {
ArrayRef<minidump::Directory> Streams,
DenseMap<minidump::StreamType, std::size_t> StreamMap)
: Binary(ID_Minidump, Source), Header(Header), Streams(Streams),
- StreamMap(std::move(StreamMap)) {};
+ StreamMap(std::move(StreamMap)) {}
ArrayRef<uint8_t> getData() const {
return arrayRefFromStringRef(Data.getBuffer());
diff --git a/llvm/lib/Object/Minidump.cpp b/llvm/lib/Object/Minidump.cpp
index 576d382dc61526..57ad8e2c799afe 100644
--- a/llvm/lib/Object/Minidump.cpp
+++ b/llvm/lib/Object/Minidump.cpp
@@ -156,8 +156,8 @@ MinidumpFile::create(MemoryBufferRef Source) {
new MinidumpFile(Source, Hdr, *ExpectedStreams, std::move(StreamMap)));
}
-Expected<MinidumpFile::Memory64ListFacade>
-MinidumpFile::getMemory64List() const {
+Expected<iterator_range<MinidumpFile::FallibleMemory64Iterator>>
+MinidumpFile::getMemory64List(Error &Err) const {
Expected<minidump::Memory64ListHeader> ListHeader = getMemoryList64Header();
if (!ListHeader)
return ListHeader.takeError();
@@ -175,14 +175,7 @@ MinidumpFile::getMemory64List() const {
if (!Descriptors)
return Descriptors.takeError();
- uint64_t FileSize = getData().size();
- uint64_t BaseRVA = ListHeader->BaseRVA;
- uint64_t Offset = BaseRVA;
- for (const minidump::MemoryDescriptor_64 &Descriptor : *Descriptors) {
- Offset += Descriptor.DataSize;
- if (Offset > FileSize)
- return createEOFError();
- }
-
- return Memory64ListFacade(getData(), *Descriptors, BaseRVA);
+ return make_range(
+ FallibleMemory64Iterator::itr(Memory64Iterator::begin(getData(), *Descriptors, ListHeader->BaseRVA), Err),
+ FallibleMemory64Iterator::end(Memory64Iterator::end()));
}
diff --git a/llvm/lib/ObjectYAML/MinidumpEmitter.cpp b/llvm/lib/ObjectYAML/MinidumpEmitter.cpp
index fef890d1204d18..fa15e856740ac3 100644
--- a/llvm/lib/ObjectYAML/MinidumpEmitter.cpp
+++ b/llvm/lib/ObjectYAML/MinidumpEmitter.cpp
@@ -141,11 +141,9 @@ static size_t layout(BlobAllocator &File, MinidumpYAML::Memory64ListStream &S) {
BaseRVA += S.Entries.size() * sizeof(minidump::MemoryDescriptor_64);
S.Header.BaseRVA = BaseRVA;
S.Header.NumberOfMemoryRanges = S.Entries.size();
- File.allocateObject(S.Header);
- for (auto &E : S.Entries) {
- E.Entry.DataSize = E.Content.binary_size();
+ File.allocateObject(S.Header);\
+ for (auto &E : S.Entries)
File.allocateObject(E.Entry);
- }
// Save the new offset for the stream size.
size_t DataEnd = File.tell();
diff --git a/llvm/lib/ObjectYAML/MinidumpYAML.cpp b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
index 188efaf135aef1..eaa74c0974c468 100644
--- a/llvm/lib/ObjectYAML/MinidumpYAML.cpp
+++ b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
@@ -9,6 +9,8 @@
#include "llvm/ObjectYAML/MinidumpYAML.h"
#include "llvm/Support/Allocator.h"
+#include <iostream>
+
using namespace llvm;
using namespace llvm::MinidumpYAML;
using namespace llvm::minidump;
@@ -326,6 +328,14 @@ static void streamMapping(yaml::IO &IO, Memory64ListStream &Stream) {
IO.mapRequired("Memory Ranges", Stream.Entries);
}
+static std::string streamValidate(Memory64ListStream &Stream) {
+ for (auto &Entry : Stream.Entries) {
+ if (Entry.Entry.DataSize < Entry.Content.binary_size())
+ return "Memory region size must be greater or equal to the content size";
+ }
+ return "";
+}
+
static void streamMapping(yaml::IO &IO, ModuleListStream &Stream) {
IO.mapRequired("Modules", Stream.Entries);
}
@@ -374,6 +384,7 @@ void yaml::MappingContextTraits<MemoryDescriptor_64, yaml::BinaryRef>::mapping(
IO &IO, MemoryDescriptor_64 &Memory, BinaryRef &Content) {
mapRequiredHex(IO, "Start of Memory Range", Memory.StartOfMemoryRange);
IO.mapRequired("Content", Content);
+ mapOptional(IO, "Data Size", Memory.DataSize, Content.binary_size());
}
void yaml::MappingTraits<ThreadListStream::entry_type>::mapping(
@@ -462,10 +473,11 @@ std::string yaml::MappingTraits<std::unique_ptr<Stream>>::validate(
switch (S->Kind) {
case MinidumpYAML::Stream::StreamKind::RawContent:
return streamValidate(cast<RawContentStream>(*S));
+ case MinidumpYAML::Stream::StreamKind::Memory64List:
+ return streamValidate(cast<Memory64ListStream>(*S));
case MinidumpYAML::Stream::StreamKind::Exception:
case MinidumpYAML::Stream::StreamKind::MemoryInfoList:
case MinidumpYAML::Stream::StreamKind::MemoryList:
- case MinidumpYAML::Stream::StreamKind::Memory64List:
case MinidumpYAML::Stream::StreamKind::ModuleList:
case MinidumpYAML::Stream::StreamKind::SystemInfo:
case MinidumpYAML::Stream::StreamKind::TextContent:
@@ -519,16 +531,29 @@ Stream::create(const Directory &StreamDesc, const object::MinidumpFile &File) {
return std::make_unique<MemoryListStream>(std::move(Ranges));
}
case StreamKind::Memory64List: {
- auto ExpectedList = File.getMemory64List();
+ // Error, unlike expected is true in failure state
+ Error Err = Error::success();
+ // Explicit check on Err so that if we return due to getmemory64list
+ // getting an error, it's not destructed when unchecked.
+ if (Err)
+ return Err;
+ auto ExpectedList = File.getMemory64List(Err);
if (!ExpectedList)
return ExpectedList.takeError();
- object::MinidumpFile::Memory64ListFacade &Memory64List = *ExpectedList;
std::vector<Memory64ListStream::entry_type> Ranges;
- for (auto It = Memory64List.begin(); It != Memory64List.end();) {
- std::pair<MemoryDescriptor_64, ArrayRef<uint8_t>> Pair = *It;
- Ranges.push_back({Pair.first, Pair.second});
+ for (auto It = ExpectedList->begin(); It != ExpectedList->end(); ++It) {
+ const auto Pair = *It;
+ if (Err.success()) {
+ Ranges.push_back({Pair.first, Pair.second});
+ }
}
- return std::make_unique<Memory64ListStream>(std::move(Ranges));
+
+ // If we don't have an error, or if any of the reads succeed, return ranges
+ // this would also work if we have no descriptors.
+ if (!Err || Ranges.size() > 0)
+ return std::make_unique<Memory64ListStream>(std::move(Ranges));
+
+ return Err;
}
case StreamKind::ModuleList: {
auto ExpectedList = File.getModuleList();
diff --git a/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp b/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
index f31c7d48423196..b622d00bb5e7ab 100644
--- a/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
+++ b/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
@@ -355,26 +355,32 @@ TEST(MinidumpYAML, MemoryRegion_64bit) {
ASSERT_EQ(1u, File.streams().size());
- Expected<object::MinidumpFile::Memory64ListFacade> ExpectedMemoryList =
- File.getMemory64List();
+ Error Err = Error::success();
+ // Explicit Err check
+ ASSERT_FALSE(Err);
+ Expected<iterator_range<object::MinidumpFile::FallibleMemory64Iterator>> ExpectedMemoryList =
+ File.getMemory64List(Err);
ASSERT_THAT_EXPECTED(ExpectedMemoryList, Succeeded());
- object::MinidumpFile::Memory64ListFacade MemoryList = *ExpectedMemoryList;
- ASSERT_EQ(2u, MemoryList.size());
-
+ iterator_range<object::MinidumpFile::FallibleMemory64Iterator> MemoryList = *ExpectedMemoryList;
auto Iterator = MemoryList.begin();
+ ++Iterator;
+ ASSERT_FALSE(Err);
+
auto DescOnePair = *Iterator;
const minidump::MemoryDescriptor_64 &DescOne = DescOnePair.first;
ASSERT_EQ(0x7FFFFFCF0818283u, DescOne.StartOfMemoryRange);
ASSERT_EQ(5u, DescOne.DataSize);
+ ++Iterator;
+ ASSERT_FALSE(Err);
+
auto DescTwoPair = *Iterator;
const minidump::MemoryDescriptor_64 &DescTwo = DescTwoPair.first;
ASSERT_EQ(0x7FFFFFFF0818283u, DescTwo.StartOfMemoryRange);
ASSERT_EQ(5u, DescTwo.DataSize);
-
const std::optional<ArrayRef<uint8_t>> ExpectedContent =
File.getRawStream(StreamType::Memory64List);
ASSERT_TRUE(ExpectedContent);
@@ -396,5 +402,5 @@ TEST(MinidumpYAML, MemoryRegion_64bit) {
ASSERT_THAT_EXPECTED(DescTwoExpectedContentSlice, Succeeded());
ASSERT_EQ(arrayRefFromStringRef("world"), *DescTwoExpectedContentSlice);
- ASSERT_EQ(Iterator, MemoryList.end());
+ ASSERT_TRUE(Iterator == MemoryList.end());
}
>From 1c6e5137489c2245107bcd176099488ed7251a69 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Tue, 6 Aug 2024 11:06:19 -0700
Subject: [PATCH 10/19] Add test case for DataSize < Content
---
llvm/include/llvm/Object/Minidump.h | 2 -
llvm/lib/ObjectYAML/MinidumpEmitter.cpp | 2 +-
llvm/lib/ObjectYAML/MinidumpYAML.cpp | 2 -
.../unittests/ObjectYAML/MinidumpYAMLTest.cpp | 37 ++++++++++++++-----
4 files changed, 28 insertions(+), 15 deletions(-)
diff --git a/llvm/include/llvm/Object/Minidump.h b/llvm/include/llvm/Object/Minidump.h
index 9dc986379642c8..d98ce3b244d4aa 100644
--- a/llvm/include/llvm/Object/Minidump.h
+++ b/llvm/include/llvm/Object/Minidump.h
@@ -17,8 +17,6 @@
#include "llvm/Object/Binary.h"
#include "llvm/Support/Error.h"
-#include <iostream>
-
namespace llvm {
namespace object {
diff --git a/llvm/lib/ObjectYAML/MinidumpEmitter.cpp b/llvm/lib/ObjectYAML/MinidumpEmitter.cpp
index fa15e856740ac3..462369b39d901b 100644
--- a/llvm/lib/ObjectYAML/MinidumpEmitter.cpp
+++ b/llvm/lib/ObjectYAML/MinidumpEmitter.cpp
@@ -141,7 +141,7 @@ static size_t layout(BlobAllocator &File, MinidumpYAML::Memory64ListStream &S) {
BaseRVA += S.Entries.size() * sizeof(minidump::MemoryDescriptor_64);
S.Header.BaseRVA = BaseRVA;
S.Header.NumberOfMemoryRanges = S.Entries.size();
- File.allocateObject(S.Header);\
+ File.allocateObject(S.Header);
for (auto &E : S.Entries)
File.allocateObject(E.Entry);
diff --git a/llvm/lib/ObjectYAML/MinidumpYAML.cpp b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
index eaa74c0974c468..4117dfec3b50df 100644
--- a/llvm/lib/ObjectYAML/MinidumpYAML.cpp
+++ b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
@@ -9,8 +9,6 @@
#include "llvm/ObjectYAML/MinidumpYAML.h"
#include "llvm/Support/Allocator.h"
-#include <iostream>
-
using namespace llvm;
using namespace llvm::MinidumpYAML;
using namespace llvm::minidump;
diff --git a/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp b/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
index b622d00bb5e7ab..38764421cdd41e 100644
--- a/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
+++ b/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
@@ -353,7 +353,7 @@ TEST(MinidumpYAML, MemoryRegion_64bit) {
ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
object::MinidumpFile &File = **ExpectedFile;
- ASSERT_EQ(1u, File.streams().size());
+ ASSERT_THAT(1u, File.streams().size());
Error Err = Error::success();
// Explicit Err check
@@ -371,36 +371,53 @@ TEST(MinidumpYAML, MemoryRegion_64bit) {
auto DescOnePair = *Iterator;
const minidump::MemoryDescriptor_64 &DescOne = DescOnePair.first;
- ASSERT_EQ(0x7FFFFFCF0818283u, DescOne.StartOfMemoryRange);
- ASSERT_EQ(5u, DescOne.DataSize);
+ ASSERT_THAT(0x7FFFFFCF0818283u, DescOne.StartOfMemoryRange);
+ ASSERT_THAT(5u, DescOne.DataSize);
++Iterator;
ASSERT_FALSE(Err);
auto DescTwoPair = *Iterator;
const minidump::MemoryDescriptor_64 &DescTwo = DescTwoPair.first;
- ASSERT_EQ(0x7FFFFFFF0818283u, DescTwo.StartOfMemoryRange);
- ASSERT_EQ(5u, DescTwo.DataSize);
+ ASSERT_THAT(0x7FFFFFFF0818283u, DescTwo.StartOfMemoryRange);
+ ASSERT_THAT(5u, DescTwo.DataSize);
const std::optional<ArrayRef<uint8_t>> ExpectedContent =
File.getRawStream(StreamType::Memory64List);
ASSERT_TRUE(ExpectedContent);
const size_t ExpectedStreamSize =
sizeof(Memory64ListHeader) + (sizeof(MemoryDescriptor_64) * 2);
- ASSERT_EQ(ExpectedStreamSize, ExpectedContent->size());
+ ASSERT_THAT(ExpectedStreamSize, ExpectedContent->size());
Expected<minidump::Memory64ListHeader> ExpectedHeader =
File.getMemoryList64Header();
ASSERT_THAT_EXPECTED(ExpectedHeader, Succeeded());
- ASSERT_EQ(ExpectedHeader->BaseRVA, 92u);
+ ASSERT_THAT(ExpectedHeader->BaseRVA, 92u);
Expected<ArrayRef<uint8_t>> DescOneExpectedContentSlice = DescOnePair.second;
ASSERT_THAT_EXPECTED(DescOneExpectedContentSlice, Succeeded());
- ASSERT_EQ(5u, DescOneExpectedContentSlice->size());
- ASSERT_EQ(arrayRefFromStringRef("hello"), *DescOneExpectedContentSlice);
+ ASSERT_THAT(5u, DescOneExpectedContentSlice->size());
+ ASSERT_THAT(arrayRefFromStringRef("hello"), *DescOneExpectedContentSlice);
Expected<ArrayRef<uint8_t>> DescTwoExpectedContentSlice = DescTwoPair.second;
ASSERT_THAT_EXPECTED(DescTwoExpectedContentSlice, Succeeded());
- ASSERT_EQ(arrayRefFromStringRef("world"), *DescTwoExpectedContentSlice);
+ ASSERT_THAT(arrayRefFromStringRef("world"), *DescTwoExpectedContentSlice);
ASSERT_TRUE(Iterator == MemoryList.end());
}
+
+TEST(MinidumpYAML, MemoryRegion_DataSize_TooSmall) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type: Memory64List
+ Memory Ranges:
+ - Start of Memory Range: 0x7FFFFFCF0818283
+ Data Size: 4 1
+ Content: '68656c6c6f'
+ - Start of Memory Range: 0x7FFFFFFF0818283
+ Content: '776f726c64'
+ )");
+
+ ASSERT_THAT_EXPECTED(ExpectedFile, Failed());
+}
>From 2bf73715457247773bf43a4117520e561075c068 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Tue, 6 Aug 2024 11:21:46 -0700
Subject: [PATCH 11/19] Rebase and run clang format
---
llvm/include/llvm/Object/Minidump.h | 46 +++++++++++--------
llvm/include/llvm/ObjectYAML/MinidumpYAML.h | 2 +-
llvm/lib/Object/Minidump.cpp | 6 ++-
.../unittests/ObjectYAML/MinidumpYAMLTest.cpp | 7 +--
4 files changed, 35 insertions(+), 26 deletions(-)
diff --git a/llvm/include/llvm/Object/Minidump.h b/llvm/include/llvm/Object/Minidump.h
index d98ce3b244d4aa..a1fcde9fb89446 100644
--- a/llvm/include/llvm/Object/Minidump.h
+++ b/llvm/include/llvm/Object/Minidump.h
@@ -140,15 +140,16 @@ class MinidumpFile : public Binary {
size_t Stride;
};
-class Memory64Iterator {
+ class Memory64Iterator {
public:
- static Memory64Iterator begin(ArrayRef<uint8_t> Storage, ArrayRef<minidump::MemoryDescriptor_64> Descriptors, uint64_t BaseRVA) {
+ static Memory64Iterator
+ begin(ArrayRef<uint8_t> Storage,
+ ArrayRef<minidump::MemoryDescriptor_64> Descriptors,
+ uint64_t BaseRVA) {
return Memory64Iterator(Storage, Descriptors, BaseRVA);
}
- static Memory64Iterator end() {
- return Memory64Iterator();
- }
+ static Memory64Iterator end() { return Memory64Iterator(); }
std::pair<minidump::MemoryDescriptor_64, ArrayRef<uint8_t>> Current;
@@ -158,28 +159,31 @@ class Memory64Iterator {
bool operator!=(const Memory64Iterator &R) const { return !(*this == R); }
- const std::pair<minidump::MemoryDescriptor_64, ArrayRef<uint8_t>> &operator*() const {
+ const std::pair<minidump::MemoryDescriptor_64, ArrayRef<uint8_t>> &
+ operator*() const {
return Current;
}
- const std::pair<minidump::MemoryDescriptor_64, ArrayRef<uint8_t>> *operator->() const {
+ const std::pair<minidump::MemoryDescriptor_64, ArrayRef<uint8_t>> *
+ operator->() const {
return &Current;
}
Error inc() {
if (Storage.size() == 0 || Descriptors.size() == 0)
- return make_error<GenericBinaryError>("No Memory64List Stream", object_error::parse_failed);
+ return make_error<GenericBinaryError>("No Memory64List Stream",
+ object_error::parse_failed);
if (Index >= Descriptors.size())
return createError("Can't read past of Memory64List Stream");
const minidump::MemoryDescriptor_64 &Descriptor = Descriptors[Index];
if (RVA + Descriptor.DataSize > Storage.size())
- return make_error<GenericBinaryError>("Memory64 Descriptor exceeds end of file.",
- object_error::unexpected_eof);
+ return make_error<GenericBinaryError>(
+ "Memory64 Descriptor exceeds end of file.",
+ object_error::unexpected_eof);
- ArrayRef<uint8_t> Content =
- Storage.slice(RVA, Descriptor.DataSize);
+ ArrayRef<uint8_t> Content = Storage.slice(RVA, Descriptor.DataSize);
Current = std::make_pair(Descriptor, Content);
RVA += Descriptor.DataSize;
Index++;
@@ -190,13 +194,14 @@ class Memory64Iterator {
private:
Memory64Iterator(ArrayRef<uint8_t> Storage,
- ArrayRef<minidump::MemoryDescriptor_64> Descriptors,
- uint64_t BaseRVA)
- : RVA(BaseRVA), Storage(Storage),
- Descriptors(Descriptors), isEnd(false) {}
-
- Memory64Iterator() : RVA(0),
- Storage(ArrayRef<uint8_t>()), Descriptors(ArrayRef<minidump::MemoryDescriptor_64>()), isEnd(true) {}
+ ArrayRef<minidump::MemoryDescriptor_64> Descriptors,
+ uint64_t BaseRVA)
+ : RVA(BaseRVA), Storage(Storage), Descriptors(Descriptors),
+ isEnd(false) {}
+
+ Memory64Iterator()
+ : RVA(0), Storage(ArrayRef<uint8_t>()),
+ Descriptors(ArrayRef<minidump::MemoryDescriptor_64>()), isEnd(true) {}
size_t Index = 0;
uint64_t RVA;
@@ -211,7 +216,8 @@ class Memory64Iterator {
/// content from the Memory64List stream. An error is returned if the file
/// does not contain a Memory64List stream, or if the descriptor data is
/// unreadable.
- Expected<iterator_range<FallibleMemory64Iterator>> getMemory64List(Error &Err) const;
+ Expected<iterator_range<FallibleMemory64Iterator>>
+ getMemory64List(Error &Err) const;
/// Returns the list of descriptors embedded in the MemoryInfoList stream. The
/// descriptors provide properties (e.g. permissions) of interesting regions
diff --git a/llvm/include/llvm/ObjectYAML/MinidumpYAML.h b/llvm/include/llvm/ObjectYAML/MinidumpYAML.h
index 9c0a97ea92a0d6..5c2ecd7b97314c 100644
--- a/llvm/include/llvm/ObjectYAML/MinidumpYAML.h
+++ b/llvm/include/llvm/ObjectYAML/MinidumpYAML.h
@@ -120,7 +120,7 @@ struct Memory64ListStream
explicit Memory64ListStream(
std::vector<detail::ParsedMemory64Descriptor> Entries = {})
- : ListStream(Entries) {};
+ : ListStream(Entries){};
};
/// ExceptionStream minidump stream.
diff --git a/llvm/lib/Object/Minidump.cpp b/llvm/lib/Object/Minidump.cpp
index 57ad8e2c799afe..ba72081a1af0f9 100644
--- a/llvm/lib/Object/Minidump.cpp
+++ b/llvm/lib/Object/Minidump.cpp
@@ -176,6 +176,8 @@ MinidumpFile::getMemory64List(Error &Err) const {
return Descriptors.takeError();
return make_range(
- FallibleMemory64Iterator::itr(Memory64Iterator::begin(getData(), *Descriptors, ListHeader->BaseRVA), Err),
- FallibleMemory64Iterator::end(Memory64Iterator::end()));
+ FallibleMemory64Iterator::itr(
+ Memory64Iterator::begin(getData(), *Descriptors, ListHeader->BaseRVA),
+ Err),
+ FallibleMemory64Iterator::end(Memory64Iterator::end()));
}
diff --git a/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp b/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
index 38764421cdd41e..53cc8830cb5ebc 100644
--- a/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
+++ b/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
@@ -358,12 +358,13 @@ TEST(MinidumpYAML, MemoryRegion_64bit) {
Error Err = Error::success();
// Explicit Err check
ASSERT_FALSE(Err);
- Expected<iterator_range<object::MinidumpFile::FallibleMemory64Iterator>> ExpectedMemoryList =
- File.getMemory64List(Err);
+ Expected<iterator_range<object::MinidumpFile::FallibleMemory64Iterator>>
+ ExpectedMemoryList = File.getMemory64List(Err);
ASSERT_THAT_EXPECTED(ExpectedMemoryList, Succeeded());
- iterator_range<object::MinidumpFile::FallibleMemory64Iterator> MemoryList = *ExpectedMemoryList;
+ iterator_range<object::MinidumpFile::FallibleMemory64Iterator> MemoryList =
+ *ExpectedMemoryList;
auto Iterator = MemoryList.begin();
++Iterator;
>From 807dfd392cccccd41f242d80888e54424f48111d Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Tue, 6 Aug 2024 11:33:03 -0700
Subject: [PATCH 12/19] Remove last extraneous ; on constructor curly braces
---
llvm/include/llvm/ObjectYAML/MinidumpYAML.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/include/llvm/ObjectYAML/MinidumpYAML.h b/llvm/include/llvm/ObjectYAML/MinidumpYAML.h
index 5c2ecd7b97314c..02371e6d867206 100644
--- a/llvm/include/llvm/ObjectYAML/MinidumpYAML.h
+++ b/llvm/include/llvm/ObjectYAML/MinidumpYAML.h
@@ -120,7 +120,7 @@ struct Memory64ListStream
explicit Memory64ListStream(
std::vector<detail::ParsedMemory64Descriptor> Entries = {})
- : ListStream(Entries){};
+ : ListStream(Entries) {}
};
/// ExceptionStream minidump stream.
>From 123f6eddc5b060d43f9a3e4a7ab6fb534e6a5f96 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Tue, 6 Aug 2024 15:49:15 -0700
Subject: [PATCH 13/19] Fix Obj2yaml bug and add comment where the iterator
starts uninitialized!
---
llvm/include/llvm/Object/Minidump.h | 5 +++++
llvm/lib/ObjectYAML/MinidumpYAML.cpp | 6 ++++--
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/llvm/include/llvm/Object/Minidump.h b/llvm/include/llvm/Object/Minidump.h
index a1fcde9fb89446..e5126f0b0a4548 100644
--- a/llvm/include/llvm/Object/Minidump.h
+++ b/llvm/include/llvm/Object/Minidump.h
@@ -140,6 +140,11 @@ class MinidumpFile : public Binary {
size_t Stride;
};
+
+ /// Class the provides an iterator over the memory64 memory ranges. Ranges
+ /// are not validated before hand, and so any increment operation could fail.
+ /// For this reason, the iterator in it's initial state points to a default
+ /// initialized std::pair and should be advanced before dereferencing.
class Memory64Iterator {
public:
static Memory64Iterator
diff --git a/llvm/lib/ObjectYAML/MinidumpYAML.cpp b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
index 4117dfec3b50df..fee4463c53ba28 100644
--- a/llvm/lib/ObjectYAML/MinidumpYAML.cpp
+++ b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
@@ -539,9 +539,11 @@ Stream::create(const Directory &StreamDesc, const object::MinidumpFile &File) {
if (!ExpectedList)
return ExpectedList.takeError();
std::vector<Memory64ListStream::entry_type> Ranges;
- for (auto It = ExpectedList->begin(); It != ExpectedList->end(); ++It) {
+ for (auto It = ExpectedList->begin(); It != ExpectedList->end();) {
+ // Explicilty advance the iterator before dereference.
+ ++It;
const auto Pair = *It;
- if (Err.success()) {
+ if (!Err) {
Ranges.push_back({Pair.first, Pair.second});
}
}
>From e5794ae007bcc63f183ddd309550778aadb549fd Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Tue, 6 Aug 2024 17:11:19 -0700
Subject: [PATCH 14/19] Create out of bounds check for first element of
iterator, allowing normal iterator behavior, and fixes filecheck
---
llvm/include/llvm/Object/Minidump.h | 31 ++++++++++++++++------------
llvm/lib/Object/Minidump.cpp | 4 ++++
llvm/lib/ObjectYAML/MinidumpYAML.cpp | 9 ++++----
3 files changed, 26 insertions(+), 18 deletions(-)
diff --git a/llvm/include/llvm/Object/Minidump.h b/llvm/include/llvm/Object/Minidump.h
index e5126f0b0a4548..4b71fb1e4dc183 100644
--- a/llvm/include/llvm/Object/Minidump.h
+++ b/llvm/include/llvm/Object/Minidump.h
@@ -140,11 +140,8 @@ class MinidumpFile : public Binary {
size_t Stride;
};
-
- /// Class the provides an iterator over the memory64 memory ranges. Ranges
- /// are not validated before hand, and so any increment operation could fail.
- /// For this reason, the iterator in it's initial state points to a default
- /// initialized std::pair and should be advanced before dereferencing.
+ /// Class the provides an iterator over the memory64 memory ranges. Only the
+ /// the first descriptor is validated as readable beforehand.
class Memory64Iterator {
public:
static Memory64Iterator
@@ -159,18 +156,18 @@ class MinidumpFile : public Binary {
std::pair<minidump::MemoryDescriptor_64, ArrayRef<uint8_t>> Current;
bool operator==(const Memory64Iterator &R) const {
- return isEnd == R.isEnd;
+ return IsEnd == R.IsEnd;
}
bool operator!=(const Memory64Iterator &R) const { return !(*this == R); }
const std::pair<minidump::MemoryDescriptor_64, ArrayRef<uint8_t>> &
- operator*() const {
+ operator*() {
return Current;
}
const std::pair<minidump::MemoryDescriptor_64, ArrayRef<uint8_t>> *
- operator->() const {
+ operator&() {
return &Current;
}
@@ -190,29 +187,37 @@ class MinidumpFile : public Binary {
ArrayRef<uint8_t> Content = Storage.slice(RVA, Descriptor.DataSize);
Current = std::make_pair(Descriptor, Content);
- RVA += Descriptor.DataSize;
Index++;
+ RVA += Descriptor.DataSize;
if (Index >= Descriptors.size())
- isEnd = true;
+ IsEnd = true;
return Error::success();
}
private:
+ // This constructor will only happen after a validation check to see
+ // if the first descriptor is readable.
Memory64Iterator(ArrayRef<uint8_t> Storage,
ArrayRef<minidump::MemoryDescriptor_64> Descriptors,
uint64_t BaseRVA)
: RVA(BaseRVA), Storage(Storage), Descriptors(Descriptors),
- isEnd(false) {}
+ IsEnd(false) {
+ assert(Descriptors.size() > 0);
+ assert(Storage.size() >= BaseRVA + Descriptors.front().DataSize);
+ Current =
+ std::make_pair(Descriptors.front(),
+ Storage.slice(BaseRVA, Descriptors.front().DataSize));
+ }
Memory64Iterator()
: RVA(0), Storage(ArrayRef<uint8_t>()),
- Descriptors(ArrayRef<minidump::MemoryDescriptor_64>()), isEnd(true) {}
+ Descriptors(ArrayRef<minidump::MemoryDescriptor_64>()), IsEnd(true) {}
size_t Index = 0;
uint64_t RVA;
ArrayRef<uint8_t> Storage;
ArrayRef<minidump::MemoryDescriptor_64> Descriptors;
- bool isEnd;
+ bool IsEnd;
};
using FallibleMemory64Iterator = llvm::fallible_iterator<Memory64Iterator>;
diff --git a/llvm/lib/Object/Minidump.cpp b/llvm/lib/Object/Minidump.cpp
index ba72081a1af0f9..9356d94ce9414c 100644
--- a/llvm/lib/Object/Minidump.cpp
+++ b/llvm/lib/Object/Minidump.cpp
@@ -175,6 +175,10 @@ MinidumpFile::getMemory64List(Error &Err) const {
if (!Descriptors)
return Descriptors.takeError();
+ if (!Descriptors->empty() &&
+ ListHeader->BaseRVA + Descriptors->front().DataSize > getData().size())
+ return createEOFError();
+
return make_range(
FallibleMemory64Iterator::itr(
Memory64Iterator::begin(getData(), *Descriptors, ListHeader->BaseRVA),
diff --git a/llvm/lib/ObjectYAML/MinidumpYAML.cpp b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
index fee4463c53ba28..4c4677c9ec3e1a 100644
--- a/llvm/lib/ObjectYAML/MinidumpYAML.cpp
+++ b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
@@ -539,13 +539,12 @@ Stream::create(const Directory &StreamDesc, const object::MinidumpFile &File) {
if (!ExpectedList)
return ExpectedList.takeError();
std::vector<Memory64ListStream::entry_type> Ranges;
- for (auto It = ExpectedList->begin(); It != ExpectedList->end();) {
- // Explicilty advance the iterator before dereference.
- ++It;
+ for (auto It = ExpectedList->begin(); It != ExpectedList->end(); ++It) {
const auto Pair = *It;
- if (!Err) {
+ if (!Err)
Ranges.push_back({Pair.first, Pair.second});
- }
+ else
+ break;
}
// If we don't have an error, or if any of the reads succeed, return ranges
>From bb7bf1f822106fb7e811ab2c6ad908411ca5e064 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Wed, 7 Aug 2024 11:03:32 -0700
Subject: [PATCH 15/19] Drop RVA property and index and instead pop off the
front of the arrayrefs, remove expected method and use range loop
---
llvm/include/llvm/Object/Minidump.h | 60 +++++++++----------
llvm/lib/Object/Minidump.cpp | 33 ++++++----
llvm/lib/ObjectYAML/MinidumpYAML.cpp | 13 +---
.../unittests/ObjectYAML/MinidumpYAMLTest.cpp | 27 +++++----
4 files changed, 69 insertions(+), 64 deletions(-)
diff --git a/llvm/include/llvm/Object/Minidump.h b/llvm/include/llvm/Object/Minidump.h
index 4b71fb1e4dc183..eda394408db31f 100644
--- a/llvm/include/llvm/Object/Minidump.h
+++ b/llvm/include/llvm/Object/Minidump.h
@@ -146,15 +146,12 @@ class MinidumpFile : public Binary {
public:
static Memory64Iterator
begin(ArrayRef<uint8_t> Storage,
- ArrayRef<minidump::MemoryDescriptor_64> Descriptors,
- uint64_t BaseRVA) {
- return Memory64Iterator(Storage, Descriptors, BaseRVA);
+ ArrayRef<minidump::MemoryDescriptor_64> Descriptors) {
+ return Memory64Iterator(Storage, Descriptors);
}
static Memory64Iterator end() { return Memory64Iterator(); }
- std::pair<minidump::MemoryDescriptor_64, ArrayRef<uint8_t>> Current;
-
bool operator==(const Memory64Iterator &R) const {
return IsEnd == R.IsEnd;
}
@@ -167,29 +164,33 @@ class MinidumpFile : public Binary {
}
const std::pair<minidump::MemoryDescriptor_64, ArrayRef<uint8_t>> *
- operator&() {
+ operator->() {
return &Current;
}
Error inc() {
- if (Storage.size() == 0 || Descriptors.size() == 0)
- return make_error<GenericBinaryError>("No Memory64List Stream",
- object_error::parse_failed);
-
- if (Index >= Descriptors.size())
- return createError("Can't read past of Memory64List Stream");
+ if (Storage.size() == 0 || Descriptors.size() == 0) {
+ IsEnd = true;
+ return Error::success();
+ }
- const minidump::MemoryDescriptor_64 &Descriptor = Descriptors[Index];
- if (RVA + Descriptor.DataSize > Storage.size())
+ // Drop front gives us an array ref, so we need to call .front() as well.
+ const minidump::MemoryDescriptor_64 &Descriptor = Descriptors.front();
+ if (Descriptor.DataSize > Storage.size()) {
+ IsEnd = true;
return make_error<GenericBinaryError>(
"Memory64 Descriptor exceeds end of file.",
object_error::unexpected_eof);
+ }
- ArrayRef<uint8_t> Content = Storage.slice(RVA, Descriptor.DataSize);
+
+ ArrayRef<uint8_t> Content = Storage.take_front(Descriptor.DataSize);
Current = std::make_pair(Descriptor, Content);
- Index++;
- RVA += Descriptor.DataSize;
- if (Index >= Descriptors.size())
+
+ Storage = Storage.drop_front(Descriptor.DataSize);
+ Descriptors = Descriptors.drop_front();
+
+ if (Descriptors.empty())
IsEnd = true;
return Error::success();
}
@@ -198,23 +199,22 @@ class MinidumpFile : public Binary {
// This constructor will only happen after a validation check to see
// if the first descriptor is readable.
Memory64Iterator(ArrayRef<uint8_t> Storage,
- ArrayRef<minidump::MemoryDescriptor_64> Descriptors,
- uint64_t BaseRVA)
- : RVA(BaseRVA), Storage(Storage), Descriptors(Descriptors),
+ ArrayRef<minidump::MemoryDescriptor_64> Descriptors)
+ : Storage(Storage), Descriptors(Descriptors),
IsEnd(false) {
- assert(Descriptors.size() > 0);
- assert(Storage.size() >= BaseRVA + Descriptors.front().DataSize);
- Current =
- std::make_pair(Descriptors.front(),
- Storage.slice(BaseRVA, Descriptors.front().DataSize));
+ assert(Descriptors.size() > 0 && Storage.size() >= Descriptors.front().DataSize);
+ minidump::MemoryDescriptor_64 Descriptor = Descriptors.front();
+ ArrayRef<uint8_t> Content = Storage.take_front(Descriptor.DataSize);
+ Current = std::make_pair(Descriptor, Content);
+ this->Descriptors = Descriptors.drop_front();
+ this->Storage = Storage.drop_front(Descriptor.DataSize);
}
Memory64Iterator()
- : RVA(0), Storage(ArrayRef<uint8_t>()),
+ : Storage(ArrayRef<uint8_t>()),
Descriptors(ArrayRef<minidump::MemoryDescriptor_64>()), IsEnd(true) {}
- size_t Index = 0;
- uint64_t RVA;
+ std::pair<minidump::MemoryDescriptor_64, ArrayRef<uint8_t>> Current;
ArrayRef<uint8_t> Storage;
ArrayRef<minidump::MemoryDescriptor_64> Descriptors;
bool IsEnd;
@@ -226,7 +226,7 @@ class MinidumpFile : public Binary {
/// content from the Memory64List stream. An error is returned if the file
/// does not contain a Memory64List stream, or if the descriptor data is
/// unreadable.
- Expected<iterator_range<FallibleMemory64Iterator>>
+ iterator_range<FallibleMemory64Iterator>
getMemory64List(Error &Err) const;
/// Returns the list of descriptors embedded in the MemoryInfoList stream. The
diff --git a/llvm/lib/Object/Minidump.cpp b/llvm/lib/Object/Minidump.cpp
index 9356d94ce9414c..34abcf6149a71a 100644
--- a/llvm/lib/Object/Minidump.cpp
+++ b/llvm/lib/Object/Minidump.cpp
@@ -156,32 +156,45 @@ MinidumpFile::create(MemoryBufferRef Source) {
new MinidumpFile(Source, Hdr, *ExpectedStreams, std::move(StreamMap)));
}
-Expected<iterator_range<MinidumpFile::FallibleMemory64Iterator>>
+static iterator_range<MinidumpFile::FallibleMemory64Iterator> makeEmptyRange(Error &Err) {
+ return make_range(llvm::object::MinidumpFile::FallibleMemory64Iterator::itr(llvm::object::MinidumpFile::Memory64Iterator::end(), Err),
+ llvm::object::MinidumpFile::FallibleMemory64Iterator::end(llvm::object::MinidumpFile::Memory64Iterator::end()));
+}
+
+iterator_range<MinidumpFile::FallibleMemory64Iterator>
MinidumpFile::getMemory64List(Error &Err) const {
Expected<minidump::Memory64ListHeader> ListHeader = getMemoryList64Header();
- if (!ListHeader)
- return ListHeader.takeError();
+ if (!ListHeader) {
+ Err = ListHeader.takeError();
+ return makeEmptyRange(Err);
+ }
std::optional<ArrayRef<uint8_t>> Stream =
getRawStream(StreamType::Memory64List);
- if (!Stream)
- return createError("No such stream");
+ if (!Stream) {
+ Err = createError("No such stream");
+ return makeEmptyRange(Err);
+ }
Expected<ArrayRef<minidump::MemoryDescriptor_64>> Descriptors =
getDataSliceAs<minidump::MemoryDescriptor_64>(
*Stream, sizeof(Memory64ListHeader),
ListHeader->NumberOfMemoryRanges);
- if (!Descriptors)
- return Descriptors.takeError();
+ if (!Descriptors) {
+ Err = Descriptors.takeError();
+ return makeEmptyRange(Err);
+ }
if (!Descriptors->empty() &&
- ListHeader->BaseRVA + Descriptors->front().DataSize > getData().size())
- return createEOFError();
+ ListHeader->BaseRVA + Descriptors->front().DataSize > getData().size()) {
+ Err = createError("Memory64List header RVA out of range");
+ return makeEmptyRange(Err);
+ }
return make_range(
FallibleMemory64Iterator::itr(
- Memory64Iterator::begin(getData(), *Descriptors, ListHeader->BaseRVA),
+ Memory64Iterator::begin(getData().slice(ListHeader->BaseRVA), *Descriptors),
Err),
FallibleMemory64Iterator::end(Memory64Iterator::end()));
}
diff --git a/llvm/lib/ObjectYAML/MinidumpYAML.cpp b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
index 4c4677c9ec3e1a..d2ef137e53a1ef 100644
--- a/llvm/lib/ObjectYAML/MinidumpYAML.cpp
+++ b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
@@ -529,22 +529,13 @@ Stream::create(const Directory &StreamDesc, const object::MinidumpFile &File) {
return std::make_unique<MemoryListStream>(std::move(Ranges));
}
case StreamKind::Memory64List: {
- // Error, unlike expected is true in failure state
Error Err = Error::success();
- // Explicit check on Err so that if we return due to getmemory64list
- // getting an error, it's not destructed when unchecked.
+ auto Memory64List = File.getMemory64List(Err);
if (Err)
return Err;
- auto ExpectedList = File.getMemory64List(Err);
- if (!ExpectedList)
- return ExpectedList.takeError();
std::vector<Memory64ListStream::entry_type> Ranges;
- for (auto It = ExpectedList->begin(); It != ExpectedList->end(); ++It) {
- const auto Pair = *It;
- if (!Err)
+ for (const auto &Pair : Memory64List) {
Ranges.push_back({Pair.first, Pair.second});
- else
- break;
}
// If we don't have an error, or if any of the reads succeed, return ranges
diff --git a/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp b/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
index 53cc8830cb5ebc..c787827be4c333 100644
--- a/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
+++ b/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
@@ -353,7 +353,7 @@ TEST(MinidumpYAML, MemoryRegion_64bit) {
ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
object::MinidumpFile &File = **ExpectedFile;
- ASSERT_THAT(1u, File.streams().size());
+ ASSERT_THAT(File.streams().size(), 1u);
Error Err = Error::success();
// Explicit Err check
@@ -367,21 +367,22 @@ TEST(MinidumpYAML, MemoryRegion_64bit) {
*ExpectedMemoryList;
auto Iterator = MemoryList.begin();
- ++Iterator;
- ASSERT_FALSE(Err);
+
auto DescOnePair = *Iterator;
const minidump::MemoryDescriptor_64 &DescOne = DescOnePair.first;
- ASSERT_THAT(0x7FFFFFCF0818283u, DescOne.StartOfMemoryRange);
- ASSERT_THAT(5u, DescOne.DataSize);
-
+ ASSERT_THAT(DescOne.StartOfMemoryRange, 0x7FFFFFCF0818283u);
+ ASSERT_THAT(DescOne.DataSize, 5u);
++Iterator;
ASSERT_FALSE(Err);
auto DescTwoPair = *Iterator;
const minidump::MemoryDescriptor_64 &DescTwo = DescTwoPair.first;
- ASSERT_THAT(0x7FFFFFFF0818283u, DescTwo.StartOfMemoryRange);
- ASSERT_THAT(5u, DescTwo.DataSize);
+ ASSERT_THAT(DescTwo.StartOfMemoryRange, 0x7FFFFFFF0818283u);
+ ASSERT_THAT(DescTwo.DataSize, 5u);
+ ++Iterator;
+ ASSERT_FALSE(Err);
+
const std::optional<ArrayRef<uint8_t>> ExpectedContent =
File.getRawStream(StreamType::Memory64List);
ASSERT_TRUE(ExpectedContent);
@@ -396,14 +397,14 @@ TEST(MinidumpYAML, MemoryRegion_64bit) {
Expected<ArrayRef<uint8_t>> DescOneExpectedContentSlice = DescOnePair.second;
ASSERT_THAT_EXPECTED(DescOneExpectedContentSlice, Succeeded());
- ASSERT_THAT(5u, DescOneExpectedContentSlice->size());
- ASSERT_THAT(arrayRefFromStringRef("hello"), *DescOneExpectedContentSlice);
+ ASSERT_THAT(DescOneExpectedContentSlice->size(), 5u);
+ ASSERT_THAT(*DescOneExpectedContentSlice, arrayRefFromStringRef("hello"));
Expected<ArrayRef<uint8_t>> DescTwoExpectedContentSlice = DescTwoPair.second;
ASSERT_THAT_EXPECTED(DescTwoExpectedContentSlice, Succeeded());
- ASSERT_THAT(arrayRefFromStringRef("world"), *DescTwoExpectedContentSlice);
+ ASSERT_THAT(*DescTwoExpectedContentSlice, arrayRefFromStringRef("world"));
- ASSERT_TRUE(Iterator == MemoryList.end());
+ ASSERT_EQ(Iterator, MemoryList.end());
}
TEST(MinidumpYAML, MemoryRegion_DataSize_TooSmall) {
@@ -414,7 +415,7 @@ TEST(MinidumpYAML, MemoryRegion_DataSize_TooSmall) {
- Type: Memory64List
Memory Ranges:
- Start of Memory Range: 0x7FFFFFCF0818283
- Data Size: 4 1
+ Data Size: 1
Content: '68656c6c6f'
- Start of Memory Range: 0x7FFFFFFF0818283
Content: '776f726c64'
>From 06732f0ca23c859d986ab62344cc598d90252d67 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Wed, 7 Aug 2024 11:12:00 -0700
Subject: [PATCH 16/19] Run GCF
---
llvm/include/llvm/Object/Minidump.h | 10 ++++------
llvm/lib/Object/Minidump.cpp | 20 +++++++++++--------
llvm/lib/ObjectYAML/MinidumpYAML.cpp | 2 +-
.../unittests/ObjectYAML/MinidumpYAMLTest.cpp | 2 --
4 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/llvm/include/llvm/Object/Minidump.h b/llvm/include/llvm/Object/Minidump.h
index eda394408db31f..4c55e841a48c89 100644
--- a/llvm/include/llvm/Object/Minidump.h
+++ b/llvm/include/llvm/Object/Minidump.h
@@ -183,7 +183,6 @@ class MinidumpFile : public Binary {
object_error::unexpected_eof);
}
-
ArrayRef<uint8_t> Content = Storage.take_front(Descriptor.DataSize);
Current = std::make_pair(Descriptor, Content);
@@ -200,9 +199,9 @@ class MinidumpFile : public Binary {
// if the first descriptor is readable.
Memory64Iterator(ArrayRef<uint8_t> Storage,
ArrayRef<minidump::MemoryDescriptor_64> Descriptors)
- : Storage(Storage), Descriptors(Descriptors),
- IsEnd(false) {
- assert(Descriptors.size() > 0 && Storage.size() >= Descriptors.front().DataSize);
+ : Storage(Storage), Descriptors(Descriptors), IsEnd(false) {
+ assert(Descriptors.size() > 0 &&
+ Storage.size() >= Descriptors.front().DataSize);
minidump::MemoryDescriptor_64 Descriptor = Descriptors.front();
ArrayRef<uint8_t> Content = Storage.take_front(Descriptor.DataSize);
Current = std::make_pair(Descriptor, Content);
@@ -226,8 +225,7 @@ class MinidumpFile : public Binary {
/// content from the Memory64List stream. An error is returned if the file
/// does not contain a Memory64List stream, or if the descriptor data is
/// unreadable.
- iterator_range<FallibleMemory64Iterator>
- getMemory64List(Error &Err) const;
+ iterator_range<FallibleMemory64Iterator> getMemory64List(Error &Err) const;
/// Returns the list of descriptors embedded in the MemoryInfoList stream. The
/// descriptors provide properties (e.g. permissions) of interesting regions
diff --git a/llvm/lib/Object/Minidump.cpp b/llvm/lib/Object/Minidump.cpp
index 34abcf6149a71a..95e99c57b057a7 100644
--- a/llvm/lib/Object/Minidump.cpp
+++ b/llvm/lib/Object/Minidump.cpp
@@ -156,9 +156,13 @@ MinidumpFile::create(MemoryBufferRef Source) {
new MinidumpFile(Source, Hdr, *ExpectedStreams, std::move(StreamMap)));
}
-static iterator_range<MinidumpFile::FallibleMemory64Iterator> makeEmptyRange(Error &Err) {
- return make_range(llvm::object::MinidumpFile::FallibleMemory64Iterator::itr(llvm::object::MinidumpFile::Memory64Iterator::end(), Err),
- llvm::object::MinidumpFile::FallibleMemory64Iterator::end(llvm::object::MinidumpFile::Memory64Iterator::end()));
+static iterator_range<MinidumpFile::FallibleMemory64Iterator>
+makeEmptyRange(Error &Err) {
+ return make_range(
+ llvm::object::MinidumpFile::FallibleMemory64Iterator::itr(
+ llvm::object::MinidumpFile::Memory64Iterator::end(), Err),
+ llvm::object::MinidumpFile::FallibleMemory64Iterator::end(
+ llvm::object::MinidumpFile::Memory64Iterator::end()));
}
iterator_range<MinidumpFile::FallibleMemory64Iterator>
@@ -192,9 +196,9 @@ MinidumpFile::getMemory64List(Error &Err) const {
return makeEmptyRange(Err);
}
- return make_range(
- FallibleMemory64Iterator::itr(
- Memory64Iterator::begin(getData().slice(ListHeader->BaseRVA), *Descriptors),
- Err),
- FallibleMemory64Iterator::end(Memory64Iterator::end()));
+ return make_range(FallibleMemory64Iterator::itr(
+ Memory64Iterator::begin(
+ getData().slice(ListHeader->BaseRVA), *Descriptors),
+ Err),
+ FallibleMemory64Iterator::end(Memory64Iterator::end()));
}
diff --git a/llvm/lib/ObjectYAML/MinidumpYAML.cpp b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
index d2ef137e53a1ef..d15ff64f83bb7b 100644
--- a/llvm/lib/ObjectYAML/MinidumpYAML.cpp
+++ b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
@@ -535,7 +535,7 @@ Stream::create(const Directory &StreamDesc, const object::MinidumpFile &File) {
return Err;
std::vector<Memory64ListStream::entry_type> Ranges;
for (const auto &Pair : Memory64List) {
- Ranges.push_back({Pair.first, Pair.second});
+ Ranges.push_back({Pair.first, Pair.second});
}
// If we don't have an error, or if any of the reads succeed, return ranges
diff --git a/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp b/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
index c787827be4c333..fdeb7a7a18f75b 100644
--- a/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
+++ b/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
@@ -367,8 +367,6 @@ TEST(MinidumpYAML, MemoryRegion_64bit) {
*ExpectedMemoryList;
auto Iterator = MemoryList.begin();
-
-
auto DescOnePair = *Iterator;
const minidump::MemoryDescriptor_64 &DescOne = DescOnePair.first;
ASSERT_THAT(DescOne.StartOfMemoryRange, 0x7FFFFFCF0818283u);
>From 63ca9e10957332f413edc8ebd4651001022ffc15 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Wed, 7 Aug 2024 11:20:41 -0700
Subject: [PATCH 17/19] Modify comment
---
llvm/include/llvm/Object/Minidump.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/llvm/include/llvm/Object/Minidump.h b/llvm/include/llvm/Object/Minidump.h
index 4c55e841a48c89..fd2fa43cdf01f8 100644
--- a/llvm/include/llvm/Object/Minidump.h
+++ b/llvm/include/llvm/Object/Minidump.h
@@ -195,8 +195,7 @@ class MinidumpFile : public Binary {
}
private:
- // This constructor will only happen after a validation check to see
- // if the first descriptor is readable.
+ // This constructor expects that the first descriptor is readable.
Memory64Iterator(ArrayRef<uint8_t> Storage,
ArrayRef<minidump::MemoryDescriptor_64> Descriptors)
: Storage(Storage), Descriptors(Descriptors), IsEnd(false) {
>From 72d6a1666ddbc630553f022831f7c6432fcf6164 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Thu, 8 Aug 2024 09:42:36 -0700
Subject: [PATCH 18/19] Impliment good feedback on making code more concise,
and fix callback to pad 0's if datasize exceeds content
---
llvm/include/llvm/Object/Minidump.h | 2 --
llvm/lib/Object/Minidump.cpp | 19 ++++-------
llvm/lib/ObjectYAML/MinidumpEmitter.cpp | 9 ++++-
llvm/lib/ObjectYAML/MinidumpYAML.cpp | 11 ++----
llvm/test/tools/obj2yaml/Minidump/basic.yaml | 5 +++
.../unittests/ObjectYAML/MinidumpYAMLTest.cpp | 34 ++++---------------
6 files changed, 28 insertions(+), 52 deletions(-)
diff --git a/llvm/include/llvm/Object/Minidump.h b/llvm/include/llvm/Object/Minidump.h
index fd2fa43cdf01f8..95b36d95eee621 100644
--- a/llvm/include/llvm/Object/Minidump.h
+++ b/llvm/include/llvm/Object/Minidump.h
@@ -189,8 +189,6 @@ class MinidumpFile : public Binary {
Storage = Storage.drop_front(Descriptor.DataSize);
Descriptors = Descriptors.drop_front();
- if (Descriptors.empty())
- IsEnd = true;
return Error::success();
}
diff --git a/llvm/lib/Object/Minidump.cpp b/llvm/lib/Object/Minidump.cpp
index 95e99c57b057a7..93b2e2cecd1053 100644
--- a/llvm/lib/Object/Minidump.cpp
+++ b/llvm/lib/Object/Minidump.cpp
@@ -156,28 +156,21 @@ MinidumpFile::create(MemoryBufferRef Source) {
new MinidumpFile(Source, Hdr, *ExpectedStreams, std::move(StreamMap)));
}
-static iterator_range<MinidumpFile::FallibleMemory64Iterator>
-makeEmptyRange(Error &Err) {
- return make_range(
- llvm::object::MinidumpFile::FallibleMemory64Iterator::itr(
- llvm::object::MinidumpFile::Memory64Iterator::end(), Err),
- llvm::object::MinidumpFile::FallibleMemory64Iterator::end(
- llvm::object::MinidumpFile::Memory64Iterator::end()));
-}
-
iterator_range<MinidumpFile::FallibleMemory64Iterator>
MinidumpFile::getMemory64List(Error &Err) const {
+ ErrorAsOutParameter ErrAsOutParam(&Err);
+ auto end = FallibleMemory64Iterator::end(Memory64Iterator::end());
Expected<minidump::Memory64ListHeader> ListHeader = getMemoryList64Header();
if (!ListHeader) {
Err = ListHeader.takeError();
- return makeEmptyRange(Err);
+ return make_range(end, end);
}
std::optional<ArrayRef<uint8_t>> Stream =
getRawStream(StreamType::Memory64List);
if (!Stream) {
Err = createError("No such stream");
- return makeEmptyRange(Err);
+ return make_range(end, end);
}
Expected<ArrayRef<minidump::MemoryDescriptor_64>> Descriptors =
@@ -187,13 +180,13 @@ MinidumpFile::getMemory64List(Error &Err) const {
if (!Descriptors) {
Err = Descriptors.takeError();
- return makeEmptyRange(Err);
+ return make_range(end, end);
}
if (!Descriptors->empty() &&
ListHeader->BaseRVA + Descriptors->front().DataSize > getData().size()) {
Err = createError("Memory64List header RVA out of range");
- return makeEmptyRange(Err);
+ return make_range(end, end);
}
return make_range(FallibleMemory64Iterator::itr(
diff --git a/llvm/lib/ObjectYAML/MinidumpEmitter.cpp b/llvm/lib/ObjectYAML/MinidumpEmitter.cpp
index 462369b39d901b..44cdfbdd80ea53 100644
--- a/llvm/lib/ObjectYAML/MinidumpEmitter.cpp
+++ b/llvm/lib/ObjectYAML/MinidumpEmitter.cpp
@@ -147,8 +147,15 @@ static size_t layout(BlobAllocator &File, MinidumpYAML::Memory64ListStream &S) {
// Save the new offset for the stream size.
size_t DataEnd = File.tell();
- for (auto &E : S.Entries)
+ for (auto &E : S.Entries) {
File.allocateBytes(E.Content);
+ if (E.Entry.DataSize > E.Content.binary_size()) {
+ size_t Padding = E.Entry.DataSize - E.Content.binary_size();
+ File.allocateCallback(Padding, [Padding](raw_ostream &OS) {
+ OS << std::string(Padding, '\0');
+ });
+ }
+ }
return DataEnd;
}
diff --git a/llvm/lib/ObjectYAML/MinidumpYAML.cpp b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
index d15ff64f83bb7b..10b8676b5c4cfb 100644
--- a/llvm/lib/ObjectYAML/MinidumpYAML.cpp
+++ b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
@@ -531,19 +531,14 @@ Stream::create(const Directory &StreamDesc, const object::MinidumpFile &File) {
case StreamKind::Memory64List: {
Error Err = Error::success();
auto Memory64List = File.getMemory64List(Err);
- if (Err)
- return Err;
std::vector<Memory64ListStream::entry_type> Ranges;
for (const auto &Pair : Memory64List) {
Ranges.push_back({Pair.first, Pair.second});
}
- // If we don't have an error, or if any of the reads succeed, return ranges
- // this would also work if we have no descriptors.
- if (!Err || Ranges.size() > 0)
- return std::make_unique<Memory64ListStream>(std::move(Ranges));
-
- return Err;
+ if (Err)
+ return Err;
+ return std::make_unique<Memory64ListStream>(std::move(Ranges));
}
case StreamKind::ModuleList: {
auto ExpectedList = File.getModuleList();
diff --git a/llvm/test/tools/obj2yaml/Minidump/basic.yaml b/llvm/test/tools/obj2yaml/Minidump/basic.yaml
index ad563212885d31..3df4689d6761ea 100644
--- a/llvm/test/tools/obj2yaml/Minidump/basic.yaml
+++ b/llvm/test/tools/obj2yaml/Minidump/basic.yaml
@@ -74,6 +74,9 @@ Streams:
Memory Ranges:
- Start of Memory Range: 0x7FFFFFCF08180283
Content: '68656c6c6f776f726c64'
+ - Start of Memory Range: 0x7FFAFFCF08180283
+ Data Size: 8
+ Content: '8008'
- Type: MemoryInfoList
Memory Ranges:
- Base Address: 0x0000000000000000
@@ -167,6 +170,8 @@ Streams:
# CHECK-NEXT: Memory Ranges:
# CHECK-NEXT: - Start of Memory Range: 0x7FFFFFCF08180283
# CHECK-NEXT: Content: 68656C6C6F776F726C64
+# CHECK-NEXT: - Start of Memory Range: 0x7FFAFFCF08180283
+# CHECK-NEXT: Content: '8008000000000000'
# CHECK-NEXT: - Type: MemoryInfoList
# CHECK-NEXT: Memory Ranges:
# CHECK-NEXT: - Base Address: 0x0
diff --git a/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp b/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
index fdeb7a7a18f75b..a8b8da925d21d9 100644
--- a/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
+++ b/llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
@@ -356,15 +356,10 @@ TEST(MinidumpYAML, MemoryRegion_64bit) {
ASSERT_THAT(File.streams().size(), 1u);
Error Err = Error::success();
- // Explicit Err check
- ASSERT_FALSE(Err);
- Expected<iterator_range<object::MinidumpFile::FallibleMemory64Iterator>>
- ExpectedMemoryList = File.getMemory64List(Err);
-
- ASSERT_THAT_EXPECTED(ExpectedMemoryList, Succeeded());
-
iterator_range<object::MinidumpFile::FallibleMemory64Iterator> MemoryList =
- *ExpectedMemoryList;
+ File.getMemory64List(Err);
+
+ ASSERT_THAT_ERROR(std::move(Err), Succeeded());
auto Iterator = MemoryList.begin();
auto DescOnePair = *Iterator;
@@ -372,21 +367,21 @@ TEST(MinidumpYAML, MemoryRegion_64bit) {
ASSERT_THAT(DescOne.StartOfMemoryRange, 0x7FFFFFCF0818283u);
ASSERT_THAT(DescOne.DataSize, 5u);
++Iterator;
- ASSERT_FALSE(Err);
+ ASSERT_THAT_ERROR(std::move(Err), Succeeded());
auto DescTwoPair = *Iterator;
const minidump::MemoryDescriptor_64 &DescTwo = DescTwoPair.first;
ASSERT_THAT(DescTwo.StartOfMemoryRange, 0x7FFFFFFF0818283u);
ASSERT_THAT(DescTwo.DataSize, 5u);
++Iterator;
- ASSERT_FALSE(Err);
+ ASSERT_THAT_ERROR(std::move(Err), Succeeded());
const std::optional<ArrayRef<uint8_t>> ExpectedContent =
File.getRawStream(StreamType::Memory64List);
ASSERT_TRUE(ExpectedContent);
const size_t ExpectedStreamSize =
sizeof(Memory64ListHeader) + (sizeof(MemoryDescriptor_64) * 2);
- ASSERT_THAT(ExpectedStreamSize, ExpectedContent->size());
+ ASSERT_THAT(ExpectedContent->size(), ExpectedStreamSize);
Expected<minidump::Memory64ListHeader> ExpectedHeader =
File.getMemoryList64Header();
@@ -404,20 +399,3 @@ TEST(MinidumpYAML, MemoryRegion_64bit) {
ASSERT_EQ(Iterator, MemoryList.end());
}
-
-TEST(MinidumpYAML, MemoryRegion_DataSize_TooSmall) {
- SmallString<0> Storage;
- auto ExpectedFile = toBinary(Storage, R"(
---- !minidump
-Streams:
- - Type: Memory64List
- Memory Ranges:
- - Start of Memory Range: 0x7FFFFFCF0818283
- Data Size: 1
- Content: '68656c6c6f'
- - Start of Memory Range: 0x7FFFFFFF0818283
- Content: '776f726c64'
- )");
-
- ASSERT_THAT_EXPECTED(ExpectedFile, Failed());
-}
>From 388e0aa82eba074a55ab50524358b19609ca9e66 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Mon, 12 Aug 2024 09:25:31 -0700
Subject: [PATCH 19/19] Add .empty() calls that got missed when I reorganized
the code
---
llvm/include/llvm/Object/Minidump.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/llvm/include/llvm/Object/Minidump.h b/llvm/include/llvm/Object/Minidump.h
index 95b36d95eee621..65ad6b171c2dc1 100644
--- a/llvm/include/llvm/Object/Minidump.h
+++ b/llvm/include/llvm/Object/Minidump.h
@@ -169,7 +169,7 @@ class MinidumpFile : public Binary {
}
Error inc() {
- if (Storage.size() == 0 || Descriptors.size() == 0) {
+ if (Descriptors.empty()) {
IsEnd = true;
return Error::success();
}
@@ -197,7 +197,7 @@ class MinidumpFile : public Binary {
Memory64Iterator(ArrayRef<uint8_t> Storage,
ArrayRef<minidump::MemoryDescriptor_64> Descriptors)
: Storage(Storage), Descriptors(Descriptors), IsEnd(false) {
- assert(Descriptors.size() > 0 &&
+ assert(!Descriptors.empty() &&
Storage.size() >= Descriptors.front().DataSize);
minidump::MemoryDescriptor_64 Descriptor = Descriptors.front();
ArrayRef<uint8_t> Content = Storage.take_front(Descriptor.DataSize);
More information about the lldb-commits
mailing list