[Lldb-commits] [PATCH] D61885: Minidump: Add support for the MemoryList stream
This revision was automatically updated to reflect the committed changes.
Closed by commit rL360908: Minidump: Add support for the MemoryList stream
(authored by labath, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D61885?vs=199570&id=199837#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61885/new/
https://reviews.llvm.org/D61885
Files:
llvm/trunk/include/llvm/Object/Minidump.h
llvm/trunk/include/llvm/ObjectYAML/MinidumpYAML.h
llvm/trunk/lib/Object/Minidump.cpp
llvm/trunk/lib/ObjectYAML/MinidumpYAML.cpp
llvm/trunk/test/tools/obj2yaml/basic-minidump.yaml
llvm/trunk/unittests/Object/MinidumpTest.cpp
Index: llvm/trunk/lib/Object/Minidump.cpp
===
--- llvm/trunk/lib/Object/Minidump.cpp
+++ llvm/trunk/lib/Object/Minidump.cpp
@@ -78,6 +78,8 @@
MinidumpFile::getListStream(StreamType) const;
template Expected>
MinidumpFile::getListStream(StreamType) const;
+template Expected>
+MinidumpFile::getListStream(StreamType) const;
Expected>
MinidumpFile::getDataSlice(ArrayRef Data, size_t Offset, size_t Size) {
Index: llvm/trunk/lib/ObjectYAML/MinidumpYAML.cpp
===
--- llvm/trunk/lib/ObjectYAML/MinidumpYAML.cpp
+++ llvm/trunk/lib/ObjectYAML/MinidumpYAML.cpp
@@ -168,6 +168,8 @@
Stream::StreamKind Stream::getKind(StreamType Type) {
switch (Type) {
+ case StreamType::MemoryList:
+return StreamKind::MemoryList;
case StreamType::ModuleList:
return StreamKind::ModuleList;
case StreamType::SystemInfo:
@@ -190,6 +192,8 @@
std::unique_ptr Stream::create(StreamType Type) {
StreamKind Kind = getKind(Type);
switch (Kind) {
+ case StreamKind::MemoryList:
+return llvm::make_unique();
case StreamKind::ModuleList:
return llvm::make_unique();
case StreamKind::RawContent:
@@ -353,6 +357,16 @@
return "";
}
+void yaml::MappingTraits::mapping(
+IO &IO, MemoryListStream::entry_type &Range) {
+ MappingContextTraits::mapping(
+ IO, Range.Entry, Range.Content);
+}
+
+static void streamMapping(yaml::IO &IO, MemoryListStream &Stream) {
+ IO.mapRequired("Memory Ranges", Stream.Entries);
+}
+
static void streamMapping(yaml::IO &IO, ModuleListStream &Stream) {
IO.mapRequired("Modules", Stream.Entries);
}
@@ -421,6 +435,9 @@
if (!IO.outputting())
S = MinidumpYAML::Stream::create(Type);
switch (S->Kind) {
+ case MinidumpYAML::Stream::StreamKind::MemoryList:
+streamMapping(IO, llvm::cast(*S));
+break;
case MinidumpYAML::Stream::StreamKind::ModuleList:
streamMapping(IO, llvm::cast(*S));
break;
@@ -444,6 +461,7 @@
switch (S->Kind) {
case MinidumpYAML::Stream::StreamKind::RawContent:
return streamValidate(cast(*S));
+ case MinidumpYAML::Stream::StreamKind::MemoryList:
case MinidumpYAML::Stream::StreamKind::ModuleList:
case MinidumpYAML::Stream::StreamKind::SystemInfo:
case MinidumpYAML::Stream::StreamKind::TextContent:
@@ -466,6 +484,10 @@
support::ulittle32_t(File.allocateBytes(Data))};
}
+static void layout(BlobAllocator &File, MemoryListStream::entry_type &Range) {
+ Range.Entry.Memory = layout(File, Range.Content);
+}
+
static void layout(BlobAllocator &File, ModuleListStream::entry_type &M) {
M.Entry.ModuleNameRVA = File.allocateString(M.Name);
@@ -502,6 +524,9 @@
Result.Location.RVA = File.tell();
Optional DataEnd;
switch (S.Kind) {
+ case Stream::StreamKind::MemoryList:
+DataEnd = layout(File, cast(S));
+break;
case Stream::StreamKind::ModuleList:
DataEnd = layout(File, cast(S));
break;
@@ -566,6 +591,19 @@
Stream::create(const Directory &StreamDesc, const object::MinidumpFile &File) {
StreamKind Kind = getKind(StreamDesc.Type);
switch (Kind) {
+ case StreamKind::MemoryList: {
+auto ExpectedList = File.getMemoryList();
+if (!ExpectedList)
+ return ExpectedList.takeError();
+std::vector Ranges;
+for (const MemoryDescriptor &MD : *ExpectedList) {
+ auto ExpectedContent = File.getRawData(MD.Memory);
+ if (!ExpectedContent)
+return ExpectedContent.takeError();
+ Ranges.push_back({MD, *ExpectedContent});
+}
+return llvm::make_unique(std::move(Ranges));
+ }
case StreamKind::ModuleList: {
auto ExpectedList = File.getModuleList();
if (!ExpectedList)
Index: llvm/trunk/unittests/Object/MinidumpTest.cpp
===
--- llvm/trunk/unittests/Object/MinidumpTest.cpp
+++ llvm/trunk/unittests/Object/MinidumpTest.cpp
@@ -463,3 +463,51 @@
EXPECT_EQ(0x08070605u, T.Context.RVA);
}
}
+
+TEST(MinidumpFile, getMemoryList) {
+ std::vector OneRange{
+ // Header
+ 'M', 'D', 'M', 'P', 0x93, 0xa7, 0, 0, // Signature, Version
+ 1, 0, 0, 0, // NumberOfStreams,
+ 32, 0, 0, 0, // StreamDir
[Lldb-commits] [PATCH] D61885: Minidump: Add support for the MemoryList stream
labath added a comment. Thanks. This is very straight-forward, so I don't think another set of eyes is needed (though I expect Greg is keeping at least one eye on this). Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61885/new/ https://reviews.llvm.org/D61885 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D61885: Minidump: Add support for the MemoryList stream
jhenderson accepted this revision. jhenderson added a comment. This revision is now accepted and ready to land. LGTM, though I haven't bothered to inspect the detail of the minidump format. I'll leave it up to you whether you think it's worth getting a separate pair of eyes. Comment at: test/tools/obj2yaml/basic-minidump.yaml:54-57 + - Type:MemoryList +Memory Ranges: + - Start of Memory Range: 0x7C7D7E7F80818283 +Content: '8485868788' labath wrote: > jhenderson wrote: > > I'd probably find this neater if the Indentation of values for each entry > > were more consistent, but I'm not too fussed. > > > > Also, in the ThreadList above, the Content is not quoted, but here it is. > > Please standardise it on one or the other. > Done. The different quoting of is actually a relict of how obj2yaml prints > BinaryRef values (they omit quotes if the data happens to be contain hex > (A-F) characters). Do you think it would be worth making this output more > consistent too? Probably, but that sounds like an unrelated changes. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61885/new/ https://reviews.llvm.org/D61885 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D61885: Minidump: Add support for the MemoryList stream
labath updated this revision to Diff 199570.
labath marked 6 inline comments as done.
labath added a comment.
Address review comments.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61885/new/
https://reviews.llvm.org/D61885
Files:
include/llvm/Object/Minidump.h
include/llvm/ObjectYAML/MinidumpYAML.h
lib/Object/Minidump.cpp
lib/ObjectYAML/MinidumpYAML.cpp
test/tools/obj2yaml/basic-minidump.yaml
unittests/Object/MinidumpTest.cpp
Index: unittests/Object/MinidumpTest.cpp
===
--- unittests/Object/MinidumpTest.cpp
+++ unittests/Object/MinidumpTest.cpp
@@ -463,3 +463,51 @@
EXPECT_EQ(0x08070605u, T.Context.RVA);
}
}
+
+TEST(MinidumpFile, getMemoryList) {
+ std::vector OneRange{
+ // Header
+ 'M', 'D', 'M', 'P', 0x93, 0xa7, 0, 0, // Signature, Version
+ 1, 0, 0, 0, // NumberOfStreams,
+ 32, 0, 0, 0, // StreamDirectoryRVA
+ 0, 1, 2, 3, 4, 5, 6, 7, // Checksum, TimeDateStamp
+ 0, 0, 0, 0, 0, 0, 0, 0, // Flags
+// Stream Directory
+ 5, 0, 0, 0, 20, 0, 0, 0, // Type, DataSize,
+ 44, 0, 0, 0, // RVA
+ // MemoryDescriptor
+ 1, 0, 0, 0, // NumberOfMemoryRanges
+ 5, 6, 7, 8, 9, 0, 1, 2, // StartOfMemoryRange
+ 3, 4, 5, 6, 7, 8, 9, 0, // DataSize, RVA
+ };
+ // Same as before, but with a padded memory list.
+ std::vector PaddedRange{
+ // Header
+ 'M', 'D', 'M', 'P', 0x93, 0xa7, 0, 0, // Signature, Version
+ 1, 0, 0, 0, // NumberOfStreams,
+ 32, 0, 0, 0, // StreamDirectoryRVA
+ 0, 1, 2, 3, 4, 5, 6, 7, // Checksum, TimeDateStamp
+ 0, 0, 0, 0, 0, 0, 0, 0, // Flags
+// Stream Directory
+ 5, 0, 0, 0, 24, 0, 0, 0, // Type, DataSize,
+ 44, 0, 0, 0, // RVA
+ // MemoryDescriptor
+ 1, 0, 0, 0, // NumberOfMemoryRanges
+ 0, 0, 0, 0, // Padding
+ 5, 6, 7, 8, 9, 0, 1, 2, // StartOfMemoryRange
+ 3, 4, 5, 6, 7, 8, 9, 0, // DataSize, RVA
+ };
+
+ for (ArrayRef Data : {OneRange, PaddedRange}) {
+auto ExpectedFile = create(Data);
+ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+const MinidumpFile &File = **ExpectedFile;
+Expected> ExpectedRanges = File.getMemoryList();
+ASSERT_THAT_EXPECTED(ExpectedRanges, Succeeded());
+ASSERT_EQ(1u, ExpectedRanges->size());
+const MemoryDescriptor &MD = ExpectedRanges.get()[0];
+EXPECT_EQ(0x0201000908070605u, MD.StartOfMemoryRange);
+EXPECT_EQ(0x06050403u, MD.Memory.DataSize);
+EXPECT_EQ(0x00090807u, MD.Memory.RVA);
+ }
+}
Index: test/tools/obj2yaml/basic-minidump.yaml
===
--- test/tools/obj2yaml/basic-minidump.yaml
+++ test/tools/obj2yaml/basic-minidump.yaml
@@ -37,20 +37,24 @@
File Date High: 0x3C3D3E3F
File Date Low:0x40414243
CodeView Record: '44454647'
-Misc Record: 48494A4B
+Misc Record: '48494A4B'
- Base of Image: 0x4C4D4E4F50515253
Size of Image: 0x54555657
Module Name: libb.so
-CodeView Record: 58595A5B
+CodeView Record: '58595A5B'
- Type:ThreadList
Threads:
- Thread Id: 0x5C5D5E5F
Priority Class:0x60616263
Environment Block: 0x6465666768696A6B
-Context: 7C7D7E7F80818283
+Context: '7C7D7E7F80818283'
Stack:
Start of Memory Range: 0x6C6D6E6F70717273
- Content: 7475767778797A7B
+ Content: '7475767778797A7B'
+ - Type:MemoryList
+Memory Ranges:
+ - Start of Memory Range: 0x7C7D7E7F80818283
+Content: '8485868788'
...
# CHECK: --- !minidump
@@ -97,11 +101,15 @@
# CHECK-NEXT: CodeView Record: 58595A5B
# CHECK-NEXT: - Type:ThreadList
# CHECK-NEXT: Threads:
-# CHECK-NEXT: - Thread Id: 0x5C5D5E5F
-# CHECK-NEXT: Priority Class: 0x60616263
+# CHECK-NEXT: - Thread Id: 0x5C5D5E5F
+# CHECK-NEXT: Priority Class:0x60616263
# CHECK-NEXT: Environment Block: 0x6465666768696A6B
-# CHECK-NEXT: Context: 7C7D7E7F80818283
+# CHECK-NEXT: Context: 7C7D7E7F80818283
# CHECK-NEXT: Stack:
# CHECK-NEXT: Start of Memory Range: 0x6C6D6E6F70717273
-# CHECK-NEXT: Content: 7475767778797A7B
+# CHECK-NEXT: Content: 7475767778797A7B
+# CHECK-NEXT: - Type:MemoryList
+# CHECK-NEXT: Memory Ranges:
+# CHECK-NEXT: -
[Lldb-commits] [PATCH] D61885: Minidump: Add support for the MemoryList stream
labath added inline comments.
Comment at: include/llvm/Object/Minidump.h:85-86
+ /// error is returned if the file does not contain this stream, or if the
+ /// stream is not large enough to contain the number of threads declared in
+ /// the stream header. The consistency of the Thread entries themselves is
not
+ /// checked in any way.
jhenderson wrote:
> These two lines talk about threads. Is that a copy/paste error?
Yep, sorry. This is the downside of "straight-forward applications of
established
patterns" :/
Comment at: include/llvm/ObjectYAML/MinidumpYAML.h:92
+/// A structure containing all data describing a single memory region.
+struct ParsedMemoryRange {
+ static constexpr Stream::StreamKind Kind = Stream::StreamKind::MemoryList;
jhenderson wrote:
> Would it make more sense to call this ParsedMermoryList, to match the
> StreamType?
Not really, because this represents only one entry in the MemoryList stream,
and not list as a whole. However, we could call it ParsedMemoryDescriptor, as
that's the type of the list entries.
Comment at: test/tools/obj2yaml/basic-minidump.yaml:54-57
+ - Type:MemoryList
+Memory Ranges:
+ - Start of Memory Range: 0x7C7D7E7F80818283
+Content: '8485868788'
jhenderson wrote:
> I'd probably find this neater if the Indentation of values for each entry
> were more consistent, but I'm not too fussed.
>
> Also, in the ThreadList above, the Content is not quoted, but here it is.
> Please standardise it on one or the other.
Done. The different quoting of is actually a relict of how obj2yaml prints
BinaryRef values (they omit quotes if the data happens to be contain hex (A-F)
characters). Do you think it would be worth making this output more consistent
too?
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61885/new/
https://reviews.llvm.org/D61885
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D61885: Minidump: Add support for the MemoryList stream
jhenderson added inline comments.
Comment at: include/llvm/Object/Minidump.h:85-86
+ /// error is returned if the file does not contain this stream, or if the
+ /// stream is not large enough to contain the number of threads declared in
+ /// the stream header. The consistency of the Thread entries themselves is
not
+ /// checked in any way.
These two lines talk about threads. Is that a copy/paste error?
Comment at: include/llvm/ObjectYAML/MinidumpYAML.h:92
+/// A structure containing all data describing a single memory region.
+struct ParsedMemoryRange {
+ static constexpr Stream::StreamKind Kind = Stream::StreamKind::MemoryList;
Would it make more sense to call this ParsedMermoryList, to match the
StreamType?
Comment at: test/tools/obj2yaml/basic-minidump.yaml:54-57
+ - Type:MemoryList
+Memory Ranges:
+ - Start of Memory Range: 0x7C7D7E7F80818283
+Content: '8485868788'
I'd probably find this neater if the Indentation of values for each entry were
more consistent, but I'm not too fussed.
Also, in the ThreadList above, the Content is not quoted, but here it is.
Please standardise it on one or the other.
Comment at: unittests/Object/MinidumpTest.cpp:483
+ };
+ // Same as before, but with a padded thread list.
+ std::vector PaddedRange{
thread? Copy/paste error?
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61885/new/
https://reviews.llvm.org/D61885
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D61885: Minidump: Add support for the MemoryList stream
labath created this revision.
labath added reviewers: amccarth, jhenderson, clayborg.
Herald added a project: LLVM.
the stream format is exactly the same as for ThreadList and ModuleList
streams, only the entry types are slightly different, so the changes in
this patch are just straight-forward applications of established
patterns.
Repository:
rL LLVM
https://reviews.llvm.org/D61885
Files:
include/llvm/Object/Minidump.h
include/llvm/ObjectYAML/MinidumpYAML.h
lib/Object/Minidump.cpp
lib/ObjectYAML/MinidumpYAML.cpp
test/tools/obj2yaml/basic-minidump.yaml
unittests/Object/MinidumpTest.cpp
Index: unittests/Object/MinidumpTest.cpp
===
--- unittests/Object/MinidumpTest.cpp
+++ unittests/Object/MinidumpTest.cpp
@@ -463,3 +463,51 @@
EXPECT_EQ(0x08070605u, T.Context.RVA);
}
}
+
+TEST(MinidumpFile, getMemoryList) {
+ std::vector OneRange{
+ // Header
+ 'M', 'D', 'M', 'P', 0x93, 0xa7, 0, 0, // Signature, Version
+ 1, 0, 0, 0, // NumberOfStreams,
+ 32, 0, 0, 0, // StreamDirectoryRVA
+ 0, 1, 2, 3, 4, 5, 6, 7, // Checksum, TimeDateStamp
+ 0, 0, 0, 0, 0, 0, 0, 0, // Flags
+// Stream Directory
+ 5, 0, 0, 0, 20, 0, 0, 0, // Type, DataSize,
+ 44, 0, 0, 0, // RVA
+ // MemoryDescriptor
+ 1, 0, 0, 0, // NumberOfMemoryRanges
+ 5, 6, 7, 8, 9, 0, 1, 2, // StartOfMemoryRange
+ 3, 4, 5, 6, 7, 8, 9, 0, // DataSize, RVA
+ };
+ // Same as before, but with a padded thread list.
+ std::vector PaddedRange{
+ // Header
+ 'M', 'D', 'M', 'P', 0x93, 0xa7, 0, 0, // Signature, Version
+ 1, 0, 0, 0, // NumberOfStreams,
+ 32, 0, 0, 0, // StreamDirectoryRVA
+ 0, 1, 2, 3, 4, 5, 6, 7, // Checksum, TimeDateStamp
+ 0, 0, 0, 0, 0, 0, 0, 0, // Flags
+// Stream Directory
+ 5, 0, 0, 0, 24, 0, 0, 0, // Type, DataSize,
+ 44, 0, 0, 0, // RVA
+ // MemoryDescriptor
+ 1, 0, 0, 0, // NumberOfMemoryRanges
+ 0, 0, 0, 0, // Padding
+ 5, 6, 7, 8, 9, 0, 1, 2, // StartOfMemoryRange
+ 3, 4, 5, 6, 7, 8, 9, 0, // DataSize, RVA
+ };
+
+ for (ArrayRef Data : {OneRange, PaddedRange}) {
+auto ExpectedFile = create(Data);
+ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+const MinidumpFile &File = **ExpectedFile;
+Expected> ExpectedRanges = File.getMemoryList();
+ASSERT_THAT_EXPECTED(ExpectedRanges, Succeeded());
+ASSERT_EQ(1u, ExpectedRanges->size());
+const MemoryDescriptor &MD = ExpectedRanges.get()[0];
+EXPECT_EQ(0x0201000908070605u, MD.StartOfMemoryRange);
+EXPECT_EQ(0x06050403u, MD.Memory.DataSize);
+EXPECT_EQ(0x00090807u, MD.Memory.RVA);
+ }
+}
Index: test/tools/obj2yaml/basic-minidump.yaml
===
--- test/tools/obj2yaml/basic-minidump.yaml
+++ test/tools/obj2yaml/basic-minidump.yaml
@@ -51,6 +51,10 @@
Stack:
Start of Memory Range: 0x6C6D6E6F70717273
Content: 7475767778797A7B
+ - Type:MemoryList
+Memory Ranges:
+ - Start of Memory Range: 0x7C7D7E7F80818283
+Content: '8485868788'
...
# CHECK: --- !minidump
@@ -104,4 +108,8 @@
# CHECK-NEXT: Stack:
# CHECK-NEXT: Start of Memory Range: 0x6C6D6E6F70717273
# CHECK-NEXT: Content: 7475767778797A7B
+# CHECK-NEXT: - Type:MemoryList
+# CHECK-NEXT: Memory Ranges:
+# CHECK-NEXT: - Start of Memory Range: 0x7C7D7E7F80818283
+# CHECK-NEXT: Content: '8485868788'
# CHECK-NEXT: ...
Index: lib/ObjectYAML/MinidumpYAML.cpp
===
--- lib/ObjectYAML/MinidumpYAML.cpp
+++ lib/ObjectYAML/MinidumpYAML.cpp
@@ -168,6 +168,8 @@
Stream::StreamKind Stream::getKind(StreamType Type) {
switch (Type) {
+ case StreamType::MemoryList:
+return StreamKind::MemoryList;
case StreamType::ModuleList:
return StreamKind::ModuleList;
case StreamType::SystemInfo:
@@ -190,6 +192,8 @@
std::unique_ptr Stream::create(StreamType Type) {
StreamKind Kind = getKind(Type);
switch (Kind) {
+ case StreamKind::MemoryList:
+return llvm::make_unique();
case StreamKind::ModuleList:
return llvm::make_unique();
case StreamKind::RawContent:
@@ -353,6 +357,16 @@
return "";
}
+void yaml::MappingTraits::mapping(
+IO &IO, MemoryListStream::entry_type &Range) {
+ MappingContextTraits::mapping(
+ IO, Range.Entry, Range.Content);
+}
+
+static void streamMapping(yaml::IO &IO, MemoryListStream &Stream) {
+ IO.mapRequired("Memory R
