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 <jalalo...@fb.com> Date: Tue, 30 Jul 2024 11:04:45 -0700 Subject: [PATCH 01/11] 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 9669303252615..8054e81322a92 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 e45d4de0090de..6ca9eb2afe150 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 b0cee541cef20..2cccf7fba5853 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 6febff89ac519..92aa4f66a4b35 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 24b521a9925c7..f992c4e3bde76 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 fdbd2d8e6dcbc..aee17c520a010 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 <jalalo...@fb.com> Date: Tue, 30 Jul 2024 17:57:43 -0700 Subject: [PATCH 02/11] 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 d2ae6543141e8..157903f368a08 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 6ca9eb2afe150..eedebc10b3fe7 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 2cccf7fba5853..7f7924b92029f 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 92aa4f66a4b35..06bcbe9be195f 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 f992c4e3bde76..d2f6bcdc20838 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 aee17c520a010..b318a9e5cc2e5 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 dfabc132d8e71..9702f82dc5843 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 7b25b1869c6b6..53486d57d0932 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 03d7db5317acd..96256bc1307d6 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 0297f62d10699..79fb903bcfd10 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 <jalalo...@fb.com> Date: Tue, 30 Jul 2024 22:16:08 -0700 Subject: [PATCH 03/11] 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 eedebc10b3fe7..21446068de616 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 d2f6bcdc20838..fef890d1204d1 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 b318a9e5cc2e5..b6ae78f2c213e 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 79fb903bcfd10..377b57c0d7a9a 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 <jalalo...@fb.com> Date: Wed, 31 Jul 2024 11:43:58 -0700 Subject: [PATCH 04/11] 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 157903f368a08..d2ae6543141e8 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 06bcbe9be195f..2d0565f1afe9a 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 63c8cc55ee2d4..50afc1c56b5c5 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 377b57c0d7a9a..b431943594034 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 <jalalo...@fb.com> Date: Wed, 31 Jul 2024 14:12:38 -0700 Subject: [PATCH 05/11] 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 21446068de616..b489d0c56738c 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 2d0565f1afe9a..950aa328a56ec 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 b6ae78f2c213e..9a335ecf3f994 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 50afc1c56b5c5..63c8cc55ee2d4 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 b431943594034..ee72ee079f2fc 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 <jalalo...@fb.com> Date: Wed, 31 Jul 2024 14:35:00 -0700 Subject: [PATCH 06/11] 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 b489d0c56738c..766b0947809fc 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 7f7924b92029f..426e4027c7527 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 9a335ecf3f994..1801e6104108c 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 53486d57d0932..7b25b1869c6b6 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 96256bc1307d6..03d7db5317acd 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 <jalalo...@fb.com> Date: Wed, 31 Jul 2024 15:20:14 -0700 Subject: [PATCH 07/11] 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 766b0947809fc..9249ec6fea8a7 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 426e4027c7527..5c2ecd7b97314 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 950aa328a56ec..576d382dc6152 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 1801e6104108c..188efaf135aef 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 ee72ee079f2fc..f31c7d4842319 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 <jalalo...@fb.com> Date: Thu, 1 Aug 2024 10:03:08 -0700 Subject: [PATCH 08/11] 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 9249ec6fea8a7..9fa02920cd70e 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 5c2ecd7b97314..9c0a97ea92a0d 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 9702f82dc5843..ad563212885d3 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 <jalalo...@fb.com> Date: Tue, 6 Aug 2024 10:41:54 -0700 Subject: [PATCH 09/11] 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 9fa02920cd70e..9dc986379642c 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 576d382dc6152..57ad8e2c799af 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 fef890d1204d1..fa15e856740ac 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 188efaf135aef..eaa74c0974c46 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 f31c7d4842319..b622d00bb5e7a 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 <jalalo...@fb.com> Date: Tue, 6 Aug 2024 11:06:19 -0700 Subject: [PATCH 10/11] 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 9dc986379642c..d98ce3b244d4a 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 fa15e856740ac..462369b39d901 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 eaa74c0974c46..4117dfec3b50d 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 b622d00bb5e7a..38764421cdd41 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 <jalalo...@fb.com> Date: Tue, 6 Aug 2024 11:21:46 -0700 Subject: [PATCH 11/11] 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 d98ce3b244d4a..a1fcde9fb8944 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 9c0a97ea92a0d..5c2ecd7b97314 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 57ad8e2c799af..ba72081a1af0f 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 38764421cdd41..53cc8830cb5eb 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; _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits