Skip to content

Commit 98edcd9

Browse files
labathlabath
committedApr 5, 2019
MinidumpParser: use minidump parser in llvm/Object
This patch removes the lower layers of the minidump parsing code from the MinidumpParser class, and replaces it with the minidump parser in llvm. Not all functionality is already avaiable in the llvm class, but it is enough for us to be able to stop enumerating streams manually, and rely on the minidump directory parsing code from the llvm class. This also removes some checked-in binaries which were used to test error handling in the parser, as the error handling is now done (and tested) in llvm. Instead I just add one test that ensures we correctly propagate the errors reported by the llvm parser. The input for this test can be written in yaml instead of a checked-in binary. llvm-svn: 357748
·
llvmorg-21-initllvmorg-9.0.0-rc1
1 parent 546bccf commit 98edcd9

File tree

10 files changed

+59
-154
lines changed

10 files changed

+59
-154
lines changed
 

‎lldb/lit/Minidump/dump-all.test

+7-7
Original file line numberDiff line numberDiff line change
@@ -40,17 +40,17 @@
4040
# CHECKDIR: RVA SIZE TYPE StreamType
4141
# CHECKDIR-NEXT: ---------- ---------- ---------- --------------------------
4242
# CHECKDIR-NEXT: 0x000000b0 0x00000038 0x00000007 SystemInfo
43-
# CHECKDIR-NEXT: 0x0000015d 0x0000001b 0x47670007 LinuxEnviron
44-
# CHECKDIR-NEXT: 0x00000190 0x000000bc 0x47670009 LinuxMaps
45-
# CHECKDIR-NEXT: 0x00000110 0x0000001a 0x47670004 LinuxProcStatus
46-
# CHECKDIR-NEXT: 0x0000024c 0x00000018 0x4767000b LinuxProcStat
47-
# CHECKDIR-NEXT: 0x00000142 0x0000001b 0x47670006 LinuxCMDLine
48-
# CHECKDIR-NEXT: 0x00000272 0x00000016 0x4767000d LinuxProcFD
49-
# CHECKDIR-NEXT: 0x00000178 0x00000018 0x47670008 LinuxAuxv
5043
# CHECKDIR-NEXT: 0x000000e8 0x00000018 0x0000000f MiscInfo
5144
# CHECKDIR-NEXT: 0x00000100 0x00000010 0x47670003 LinuxCPUInfo
45+
# CHECKDIR-NEXT: 0x00000110 0x0000001a 0x47670004 LinuxProcStatus
5246
# CHECKDIR-NEXT: 0x0000012a 0x00000018 0x47670005 LinuxLSBRelease
47+
# CHECKDIR-NEXT: 0x00000142 0x0000001b 0x47670006 LinuxCMDLine
48+
# CHECKDIR-NEXT: 0x0000015d 0x0000001b 0x47670007 LinuxEnviron
49+
# CHECKDIR-NEXT: 0x00000178 0x00000018 0x47670008 LinuxAuxv
50+
# CHECKDIR-NEXT: 0x00000190 0x000000bc 0x47670009 LinuxMaps
51+
# CHECKDIR-NEXT: 0x0000024c 0x00000018 0x4767000b LinuxProcStat
5352
# CHECKDIR-NEXT: 0x00000264 0x0000000e 0x4767000c LinuxProcUptime
53+
# CHECKDIR-NEXT: 0x00000272 0x00000016 0x4767000d LinuxProcFD
5454

5555
# CHECKCPU: /proc/cpuinfo:
5656
# CHECKCPU-NEXT: cpu info output

‎lldb/lit/Minidump/fb-dump.test

+7-7
Original file line numberDiff line numberDiff line change
@@ -57,19 +57,19 @@
5757
# RUN: FileCheck --check-prefix=CHECK-LOGCAT %s
5858
# CHECK-DIR: RVA SIZE TYPE StreamType
5959
# CHECK-DIR-NEXT: ---------- ---------- ---------- --------------------------
60+
# CHECK-DIR-NEXT: 0x000000bc 0x00000038 0x00000007 SystemInfo
61+
# CHECK-DIR-NEXT: 0x000000f4 0x00000018 0x0000000f MiscInfo
6062
# CHECK-DIR-NEXT: 0x0000010c 0x00000013 0xfacecb00 FacebookDumpErrorLog
6163
# CHECK-DIR-NEXT: 0x0000011f 0x00000015 0xfacee000 FacebookThreadName
64+
# CHECK-DIR-NEXT: 0x00000134 0x00000010 0xface1ca7 FacebookLogcat
65+
# CHECK-DIR-NEXT: 0x00000144 0x00000017 0xfacecccc FacebookAppStateLog
6266
# CHECK-DIR-NEXT: 0x0000015b 0x00000016 0xfacedead FacebookAbortReason
63-
# CHECK-DIR-NEXT: 0x000000bc 0x00000038 0x00000007 SystemInfo
64-
# CHECK-DIR-NEXT: 0x000001aa 0x00000005 0xfacecafb FacebookBuildID
65-
# CHECK-DIR-NEXT: 0x000001bc 0x00000019 0xfacecafd FacebookJavaStack
66-
# CHECK-DIR-NEXT: 0x000001ea 0x00000005 0xfacecaff FacebookUnwindSymbols
6767
# CHECK-DIR-NEXT: 0x00000171 0x00000039 0xfacecafa FacebookAppCustomData
68-
# CHECK-DIR-NEXT: 0x00000134 0x00000010 0xface1ca7 FacebookLogcat
69-
# CHECK-DIR-NEXT: 0x000000f4 0x00000018 0x0000000f MiscInfo
68+
# CHECK-DIR-NEXT: 0x000001aa 0x00000005 0xfacecafb FacebookBuildID
7069
# CHECK-DIR-NEXT: 0x000001af 0x0000000d 0xfacecafc FacebookAppVersionName
70+
# CHECK-DIR-NEXT: 0x000001bc 0x00000019 0xfacecafd FacebookJavaStack
7171
# CHECK-DIR-NEXT: 0x000001d5 0x00000015 0xfacecafe FacebookDalvikInfo
72-
# CHECK-DIR-NEXT: 0x00000144 0x00000017 0xfacecccc FacebookAppStateLog
72+
# CHECK-DIR-NEXT: 0x000001ea 0x00000005 0xfacecaff FacebookUnwindSymbols
7373

7474
# CHECK-APPDATA: Facebook App Data:
7575
# CHECK-APPDATA-NEXT: {"global": {"Fingerprint":"invalid device fingerprint"}}

‎lldb/source/Plugins/Process/minidump/CMakeLists.txt

+1
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,6 @@ add_lldb_library(lldbPluginProcessMinidump PLUGIN
1616
lldbPluginProcessElfCore
1717
LINK_COMPONENTS
1818
BinaryFormat
19+
Object
1920
Support
2021
)

‎lldb/source/Plugins/Process/minidump/MinidumpParser.cpp

+12-108
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@
1010
#include "NtStructures.h"
1111
#include "RegisterContextMinidump_x86_32.h"
1212

13-
#include "lldb/Utility/LLDBAssert.h"
1413
#include "Plugins/Process/Utility/LinuxProcMaps.h"
14+
#include "lldb/Utility/LLDBAssert.h"
1515

1616
// C includes
1717
// C++ includes
@@ -23,11 +23,6 @@
2323
using namespace lldb_private;
2424
using namespace minidump;
2525

26-
static llvm::Error stringError(llvm::StringRef Err) {
27-
return llvm::make_error<llvm::StringError>(Err,
28-
llvm::inconvertibleErrorCode());
29-
}
30-
3126
const Header *ParseHeader(llvm::ArrayRef<uint8_t> &data) {
3227
const Header *header = nullptr;
3328
Status error = consumeObject(data, header);
@@ -45,117 +40,26 @@ const Header *ParseHeader(llvm::ArrayRef<uint8_t> &data) {
4540

4641
llvm::Expected<MinidumpParser>
4742
MinidumpParser::Create(const lldb::DataBufferSP &data_sp) {
48-
if (data_sp->GetByteSize() < sizeof(Header))
49-
return stringError("Buffer too small.");
50-
51-
llvm::ArrayRef<uint8_t> header_data(data_sp->GetBytes(),
52-
sizeof(Header));
53-
const Header *header = ParseHeader(header_data);
54-
if (!header)
55-
return stringError("invalid minidump: can't parse the header");
56-
57-
// A minidump without at least one stream is clearly ill-formed
58-
if (header->NumberOfStreams == 0)
59-
return stringError("invalid minidump: no streams present");
60-
61-
struct FileRange {
62-
uint32_t offset = 0;
63-
uint32_t size = 0;
64-
65-
FileRange(uint32_t offset, uint32_t size) : offset(offset), size(size) {}
66-
uint32_t end() const { return offset + size; }
67-
};
68-
69-
const uint32_t file_size = data_sp->GetByteSize();
70-
71-
// Build a global minidump file map, checking for:
72-
// - overlapping streams/data structures
73-
// - truncation (streams pointing past the end of file)
74-
std::vector<FileRange> minidump_map;
75-
76-
minidump_map.emplace_back(0, sizeof(Header));
77-
78-
// Add the directory entries to the file map
79-
FileRange directory_range(header->StreamDirectoryRVA,
80-
header->NumberOfStreams * sizeof(Directory));
81-
if (directory_range.end() > file_size)
82-
return stringError("invalid minidump: truncated streams directory");
83-
minidump_map.push_back(directory_range);
84-
85-
llvm::DenseMap<StreamType, LocationDescriptor> directory_map;
86-
87-
// Parse stream directory entries
88-
llvm::ArrayRef<uint8_t> directory_data(
89-
data_sp->GetBytes() + directory_range.offset, directory_range.size);
90-
for (uint32_t i = 0; i < header->NumberOfStreams; ++i) {
91-
const Directory *directory_entry = nullptr;
92-
Status error = consumeObject(directory_data, directory_entry);
93-
if (error.Fail())
94-
return error.ToError();
95-
if (directory_entry->Type == StreamType::Unused) {
96-
// Ignore dummy streams (technically ill-formed, but a number of
97-
// existing minidumps seem to contain such streams)
98-
if (directory_entry->Location.DataSize == 0)
99-
continue;
100-
return stringError("invalid minidump: bad stream type");
101-
}
102-
// Update the streams map, checking for duplicate stream types
103-
if (!directory_map
104-
.insert({directory_entry->Type, directory_entry->Location})
105-
.second)
106-
return stringError("invalid minidump: duplicate stream type");
107-
108-
// Ignore the zero-length streams for layout checks
109-
if (directory_entry->Location.DataSize != 0) {
110-
minidump_map.emplace_back(directory_entry->Location.RVA,
111-
directory_entry->Location.DataSize);
112-
}
113-
}
43+
auto ExpectedFile = llvm::object::MinidumpFile::create(
44+
llvm::MemoryBufferRef(toStringRef(data_sp->GetData()), "minidump"));
45+
if (!ExpectedFile)
46+
return ExpectedFile.takeError();
11447

115-
// Sort the file map ranges by start offset
116-
llvm::sort(minidump_map.begin(), minidump_map.end(),
117-
[](const FileRange &a, const FileRange &b) {
118-
return a.offset < b.offset;
119-
});
120-
121-
// Check for overlapping streams/data structures
122-
for (size_t i = 1; i < minidump_map.size(); ++i) {
123-
const auto &prev_range = minidump_map[i - 1];
124-
if (prev_range.end() > minidump_map[i].offset)
125-
return stringError("invalid minidump: overlapping streams");
126-
}
127-
128-
// Check for streams past the end of file
129-
const auto &last_range = minidump_map.back();
130-
if (last_range.end() > file_size)
131-
return stringError("invalid minidump: truncated stream");
132-
133-
return MinidumpParser(std::move(data_sp), std::move(directory_map));
48+
return MinidumpParser(data_sp, std::move(*ExpectedFile));
13449
}
13550

136-
MinidumpParser::MinidumpParser(
137-
lldb::DataBufferSP data_sp,
138-
llvm::DenseMap<StreamType, LocationDescriptor> directory_map)
139-
: m_data_sp(std::move(data_sp)), m_directory_map(std::move(directory_map)) {
140-
}
51+
MinidumpParser::MinidumpParser(lldb::DataBufferSP data_sp,
52+
std::unique_ptr<llvm::object::MinidumpFile> file)
53+
: m_data_sp(std::move(data_sp)), m_file(std::move(file)) {}
14154

14255
llvm::ArrayRef<uint8_t> MinidumpParser::GetData() {
14356
return llvm::ArrayRef<uint8_t>(m_data_sp->GetBytes(),
14457
m_data_sp->GetByteSize());
14558
}
14659

147-
llvm::ArrayRef<uint8_t>
148-
MinidumpParser::GetStream(StreamType stream_type) {
149-
auto iter = m_directory_map.find(stream_type);
150-
if (iter == m_directory_map.end())
151-
return {};
152-
153-
// check if there is enough data
154-
if (iter->second.RVA + iter->second.DataSize > m_data_sp->GetByteSize())
155-
return {};
156-
157-
return llvm::ArrayRef<uint8_t>(m_data_sp->GetBytes() + iter->second.RVA,
158-
iter->second.DataSize);
60+
llvm::ArrayRef<uint8_t> MinidumpParser::GetStream(StreamType stream_type) {
61+
return m_file->getRawStream(stream_type)
62+
.getValueOr(llvm::ArrayRef<uint8_t>());
15963
}
16064

16165
llvm::Optional<std::string> MinidumpParser::GetMinidumpString(uint32_t rva) {

‎lldb/source/Plugins/Process/minidump/MinidumpParser.h

+4-6
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "llvm/ADT/DenseMap.h"
2222
#include "llvm/ADT/Optional.h"
2323
#include "llvm/ADT/StringRef.h"
24+
#include "llvm/Object/Minidump.h"
2425

2526
// C includes
2627

@@ -93,20 +94,17 @@ class MinidumpParser {
9394

9495
static llvm::StringRef GetStreamTypeAsString(StreamType stream_type);
9596

96-
const llvm::DenseMap<StreamType, LocationDescriptor> &
97-
GetDirectoryMap() const {
98-
return m_directory_map;
99-
}
97+
llvm::object::MinidumpFile &GetMinidumpFile() { return *m_file; }
10098

10199
private:
102100
MinidumpParser(lldb::DataBufferSP data_sp,
103-
llvm::DenseMap<StreamType, LocationDescriptor> directory_map);
101+
std::unique_ptr<llvm::object::MinidumpFile> file);
104102

105103
MemoryRegionInfo FindMemoryRegion(lldb::addr_t load_addr) const;
106104

107105
private:
108106
lldb::DataBufferSP m_data_sp;
109-
llvm::DenseMap<StreamType, LocationDescriptor> m_directory_map;
107+
std::unique_ptr<llvm::object::MinidumpFile> m_file;
110108
ArchSpec m_arch;
111109
MemoryRegionInfos m_regions;
112110
bool m_parsed_regions = false;

‎lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp

+6-4
Original file line numberDiff line numberDiff line change
@@ -718,10 +718,12 @@ class CommandObjectProcessMinidumpDump : public CommandObjectParsed {
718718
if (DumpDirectory()) {
719719
s.Printf("RVA SIZE TYPE StreamType\n");
720720
s.Printf("---------- ---------- ---------- --------------------------\n");
721-
for (const auto &pair: minidump.GetDirectoryMap())
722-
s.Printf("0x%8.8x 0x%8.8x 0x%8.8x %s\n", (uint32_t)pair.second.RVA,
723-
(uint32_t)pair.second.DataSize, (unsigned)pair.first,
724-
MinidumpParser::GetStreamTypeAsString(pair.first).data());
721+
for (const auto &stream_desc : minidump.GetMinidumpFile().streams())
722+
s.Printf(
723+
"0x%8.8x 0x%8.8x 0x%8.8x %s\n", (uint32_t)stream_desc.Location.RVA,
724+
(uint32_t)stream_desc.Location.DataSize,
725+
(unsigned)(StreamType)stream_desc.Type,
726+
MinidumpParser::GetStreamTypeAsString(stream_desc.Type).data());
725727
s.Printf("\n");
726728
}
727729
auto DumpTextStream = [&](StreamType stream_type,

‎lldb/unittests/Process/minidump/CMakeLists.txt

+1-2
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,11 @@ add_lldb_unittest(LLDBMinidumpTests
1111
lldbUtilityHelpers
1212
LLVMTestingSupport
1313
LINK_COMPONENTS
14+
ObjectYAML
1415
Support
1516
)
1617

1718
set(test_inputs
18-
bad_duplicate_streams.dmp
19-
bad_overlapping_streams.dmp
2019
fizzbuzz_no_heap.dmp
2120
fizzbuzz_wow64.dmp
2221
linux-i386.dmp

‎lldb/unittests/Process/minidump/Inputs/bad_duplicate_streams.dmp

-32.2 KB
Binary file not shown.

‎lldb/unittests/Process/minidump/Inputs/bad_overlapping_streams.dmp

-32.2 KB
Binary file not shown.

‎lldb/unittests/Process/minidump/MinidumpParserTest.cpp

+21-20
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,12 @@
1414
#include "lldb/Host/FileSystem.h"
1515
#include "lldb/Target/MemoryRegionInfo.h"
1616
#include "lldb/Utility/ArchSpec.h"
17+
#include "lldb/Utility/DataBufferHeap.h"
1718
#include "lldb/Utility/DataExtractor.h"
1819
#include "lldb/Utility/FileSpec.h"
1920
#include "llvm/ADT/ArrayRef.h"
2021
#include "llvm/ADT/Optional.h"
22+
#include "llvm/ObjectYAML/MinidumpYAML.h"
2123
#include "llvm/Support/FileSystem.h"
2224
#include "llvm/Support/MemoryBuffer.h"
2325
#include "llvm/Support/Path.h"
@@ -49,18 +51,28 @@ class MinidumpParserTest : public testing::Test {
4951
ASSERT_GT(parser->GetData().size(), 0UL);
5052
}
5153

52-
void InvalidMinidump(const char *minidump_filename, uint64_t load_size) {
53-
std::string filename = GetInputFilePath(minidump_filename);
54-
auto BufferPtr =
55-
FileSystem::Instance().CreateDataBuffer(filename, load_size, 0);
56-
ASSERT_NE(BufferPtr, nullptr);
57-
58-
EXPECT_THAT_EXPECTED(MinidumpParser::Create(BufferPtr), llvm::Failed());
59-
}
60-
6154
llvm::Optional<MinidumpParser> parser;
6255
};
6356

57+
TEST_F(MinidumpParserTest, InvalidMinidump) {
58+
std::string duplicate_streams;
59+
llvm::raw_string_ostream os(duplicate_streams);
60+
ASSERT_THAT_ERROR(llvm::MinidumpYAML::writeAsBinary(R"(
61+
--- !minidump
62+
Streams:
63+
- Type: LinuxAuxv
64+
Content: DEADBEEFBAADF00D
65+
- Type: LinuxAuxv
66+
Content: DEADBEEFBAADF00D
67+
)",
68+
os),
69+
llvm::Succeeded());
70+
os.flush();
71+
auto data_buffer_sp = std::make_shared<DataBufferHeap>(
72+
duplicate_streams.data(), duplicate_streams.size());
73+
ASSERT_THAT_EXPECTED(MinidumpParser::Create(data_buffer_sp), llvm::Failed());
74+
}
75+
6476
TEST_F(MinidumpParserTest, GetThreadsAndGetThreadContext) {
6577
SetUpData("linux-x86_64.dmp");
6678
llvm::ArrayRef<MinidumpThread> thread_list;
@@ -146,17 +158,6 @@ TEST_F(MinidumpParserTest, GetMemoryListPadded) {
146158
EXPECT_EQ((lldb::addr_t)0x8010, mem->start);
147159
}
148160

149-
TEST_F(MinidumpParserTest, TruncatedMinidumps) {
150-
InvalidMinidump("linux-x86_64.dmp", 32);
151-
InvalidMinidump("linux-x86_64.dmp", 100);
152-
InvalidMinidump("linux-x86_64.dmp", 20 * 1024);
153-
}
154-
155-
TEST_F(MinidumpParserTest, IllFormedMinidumps) {
156-
InvalidMinidump("bad_duplicate_streams.dmp", -1);
157-
InvalidMinidump("bad_overlapping_streams.dmp", -1);
158-
}
159-
160161
TEST_F(MinidumpParserTest, GetArchitecture) {
161162
SetUpData("linux-x86_64.dmp");
162163
ASSERT_EQ(llvm::Triple::ArchType::x86_64,

0 commit comments

Comments
 (0)
Please sign in to comment.