From 139e9f247ab376734dcda9f2197d5fd2aea8c46f Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Wed, 10 Apr 2019 11:07:28 +0000 Subject: [PATCH] Minidump: Use llvm parser for reading the ModuleList stream In this patch, I just remove the structure definitions for the ModuleList stream and the associated parsing code. The rest of the code is converted to work with the definitions in llvm. NFC. llvm-svn: 358070 --- .../Plugins/Process/minidump/MinidumpParser.cpp | 57 ++++++++++------------ .../Plugins/Process/minidump/MinidumpParser.h | 6 +-- .../Plugins/Process/minidump/MinidumpTypes.cpp | 27 ---------- .../Plugins/Process/minidump/MinidumpTypes.h | 40 --------------- .../Plugins/Process/minidump/ProcessMinidump.cpp | 13 +++-- .../Process/minidump/MinidumpParserTest.cpp | 48 +++++++++--------- 6 files changed, 58 insertions(+), 133 deletions(-) diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp index d4d29f2..11203f2 100644 --- a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp +++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp @@ -24,21 +24,6 @@ using namespace lldb_private; using namespace minidump; -const Header *ParseHeader(llvm::ArrayRef &data) { - const Header *header = nullptr; - Status error = consumeObject(data, header); - - uint32_t signature = header->Signature; - uint32_t version = header->Version & 0x0000ffff; - // the high 16 bits of the version field are implementation specific - - if (error.Fail() || signature != Header::MagicSignature || - version != Header::MagicVersion) - return nullptr; - - return header; -} - llvm::Expected MinidumpParser::Create(const lldb::DataBufferSP &data_sp) { auto ExpectedFile = llvm::object::MinidumpFile::create( @@ -63,9 +48,9 @@ llvm::ArrayRef MinidumpParser::GetStream(StreamType stream_type) { .getValueOr(llvm::ArrayRef()); } -UUID MinidumpParser::GetModuleUUID(const MinidumpModule *module) { +UUID MinidumpParser::GetModuleUUID(const minidump::Module *module) { auto cv_record = - GetData().slice(module->CV_record.RVA, module->CV_record.DataSize); + GetData().slice(module->CvRecord.RVA, module->CvRecord.DataSize); // Read the CV record signature const llvm::support::ulittle32_t *signature = nullptr; @@ -284,28 +269,36 @@ llvm::Optional MinidumpParser::GetPid() { return llvm::None; } -llvm::ArrayRef MinidumpParser::GetModuleList() { - llvm::ArrayRef data = GetStream(StreamType::ModuleList); - - if (data.size() == 0) - return {}; +llvm::ArrayRef MinidumpParser::GetModuleList() { + auto ExpectedModules = GetMinidumpFile().getModuleList(); + if (ExpectedModules) + return *ExpectedModules; - return MinidumpModule::ParseModuleList(data); + LLDB_LOG_ERROR(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_MODULES), + ExpectedModules.takeError(), + "Failed to read module list: {0}"); + return {}; } -std::vector MinidumpParser::GetFilteredModuleList() { - llvm::ArrayRef modules = GetModuleList(); +std::vector MinidumpParser::GetFilteredModuleList() { + Log *log = GetLogIfAnyCategoriesSet(LIBLLDB_LOG_MODULES); + auto ExpectedModules = GetMinidumpFile().getModuleList(); + if (!ExpectedModules) { + LLDB_LOG_ERROR(log, ExpectedModules.takeError(), + "Failed to read module list: {0}"); + return {}; + } + // map module_name -> filtered_modules index typedef llvm::StringMap MapType; MapType module_name_to_filtered_index; - std::vector filtered_modules; - - for (const auto &module : modules) { - auto ExpectedName = m_file->getString(module.module_name_rva); + std::vector filtered_modules; + + for (const auto &module : *ExpectedModules) { + auto ExpectedName = m_file->getString(module.ModuleNameRVA); if (!ExpectedName) { - LLDB_LOG_ERROR(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_MODULES), - ExpectedName.takeError(), + LLDB_LOG_ERROR(log, ExpectedName.takeError(), "Failed to module name: {0}"); continue; } @@ -328,7 +321,7 @@ std::vector MinidumpParser::GetFilteredModuleList() { // times when they are mapped discontiguously, so find the module with // the lowest "base_of_image" and use that as the filtered module. auto dup_module = filtered_modules[iter->second]; - if (module.base_of_image < dup_module->base_of_image) + if (module.BaseOfImage < dup_module->BaseOfImage) filtered_modules[iter->second] = &module; } } diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.h b/lldb/source/Plugins/Process/minidump/MinidumpParser.h index 5df2d74..3488bcd 100644 --- a/lldb/source/Plugins/Process/minidump/MinidumpParser.h +++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.h @@ -52,7 +52,7 @@ public: llvm::ArrayRef GetStream(StreamType stream_type); - UUID GetModuleUUID(const MinidumpModule* module); + UUID GetModuleUUID(const minidump::Module *module); llvm::ArrayRef GetThreads(); @@ -70,13 +70,13 @@ public: llvm::Optional GetPid(); - llvm::ArrayRef GetModuleList(); + llvm::ArrayRef GetModuleList(); // There are cases in which there is more than one record in the ModuleList // for the same module name.(e.g. when the binary has non contiguous segments) // So this function returns a filtered module list - if it finds records that // have the same name, it keeps the copy with the lowest load address. - std::vector GetFilteredModuleList(); + std::vector GetFilteredModuleList(); const MinidumpExceptionStream *GetExceptionStream(); diff --git a/lldb/source/Plugins/Process/minidump/MinidumpTypes.cpp b/lldb/source/Plugins/Process/minidump/MinidumpTypes.cpp index ba1f102..13caa58 100644 --- a/lldb/source/Plugins/Process/minidump/MinidumpTypes.cpp +++ b/lldb/source/Plugins/Process/minidump/MinidumpTypes.cpp @@ -84,33 +84,6 @@ LinuxProcStatus::Parse(llvm::ArrayRef &data) { lldb::pid_t LinuxProcStatus::GetPid() const { return pid; } -// Module stuff -const MinidumpModule *MinidumpModule::Parse(llvm::ArrayRef &data) { - const MinidumpModule *module = nullptr; - Status error = consumeObject(data, module); - if (error.Fail()) - return nullptr; - - return module; -} - -llvm::ArrayRef -MinidumpModule::ParseModuleList(llvm::ArrayRef &data) { - const auto orig_size = data.size(); - const llvm::support::ulittle32_t *modules_count; - Status error = consumeObject(data, modules_count); - if (error.Fail() || *modules_count * sizeof(MinidumpModule) > data.size()) - return {}; - - // Compilers might end up padding an extra 4 bytes depending on how the - // structure is padded by the compiler and the #pragma pack settings. - if (4 + *modules_count * sizeof(MinidumpModule) < orig_size) - data = data.drop_front(4); - - return llvm::ArrayRef( - reinterpret_cast(data.data()), *modules_count); -} - // Exception stuff const MinidumpExceptionStream * MinidumpExceptionStream::Parse(llvm::ArrayRef &data) { diff --git a/lldb/source/Plugins/Process/minidump/MinidumpTypes.h b/lldb/source/Plugins/Process/minidump/MinidumpTypes.h index ac69066..711636b 100644 --- a/lldb/source/Plugins/Process/minidump/MinidumpTypes.h +++ b/lldb/source/Plugins/Process/minidump/MinidumpTypes.h @@ -228,46 +228,6 @@ private: LinuxProcStatus() = default; }; -// MinidumpModule stuff -struct MinidumpVSFixedFileInfo { - llvm::support::ulittle32_t signature; - llvm::support::ulittle32_t struct_version; - llvm::support::ulittle32_t file_version_hi; - llvm::support::ulittle32_t file_version_lo; - llvm::support::ulittle32_t product_version_hi; - llvm::support::ulittle32_t product_version_lo; - // file_flags_mask - identifies valid bits in fileFlags - llvm::support::ulittle32_t file_flags_mask; - llvm::support::ulittle32_t file_flags; - llvm::support::ulittle32_t file_os; - llvm::support::ulittle32_t file_type; - llvm::support::ulittle32_t file_subtype; - llvm::support::ulittle32_t file_date_hi; - llvm::support::ulittle32_t file_date_lo; -}; -static_assert(sizeof(MinidumpVSFixedFileInfo) == 52, - "sizeof MinidumpVSFixedFileInfo is not correct!"); - -struct MinidumpModule { - llvm::support::ulittle64_t base_of_image; - llvm::support::ulittle32_t size_of_image; - llvm::support::ulittle32_t checksum; - llvm::support::ulittle32_t time_date_stamp; - llvm::support::ulittle32_t module_name_rva; - MinidumpVSFixedFileInfo version_info; - LocationDescriptor CV_record; - LocationDescriptor misc_record; - llvm::support::ulittle32_t reserved0[2]; - llvm::support::ulittle32_t reserved1[2]; - - static const MinidumpModule *Parse(llvm::ArrayRef &data); - - static llvm::ArrayRef - ParseModuleList(llvm::ArrayRef &data); -}; -static_assert(sizeof(MinidumpModule) == 108, - "sizeof MinidumpVSFixedFileInfo is not correct!"); - // Exception stuff struct MinidumpException { enum : unsigned { diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp index 79de4aa..5d32ba6 100644 --- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp +++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp @@ -348,18 +348,17 @@ bool ProcessMinidump::UpdateThreadList(ThreadList &old_thread_list, } void ProcessMinidump::ReadModuleList() { - std::vector filtered_modules = + std::vector filtered_modules = m_minidump_parser->GetFilteredModuleList(); Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_MODULES)); for (auto module : filtered_modules) { std::string name = cantFail(m_minidump_parser->GetMinidumpFile().getString( - module->module_name_rva)); + module->ModuleNameRVA)); LLDB_LOG(log, "found module: name: {0} {1:x10}-{2:x10} size: {3}", name, - module->base_of_image, - module->base_of_image + module->size_of_image, - module->size_of_image); + module->BaseOfImage, module->BaseOfImage + module->SizeOfImage, + module->SizeOfImage); // check if the process is wow64 - a 32 bit windows process running on a // 64 bit windows @@ -417,12 +416,12 @@ void ProcessMinidump::ReadModuleList() { name); module_sp = Module::CreateModuleFromObjectFile( - module_spec, module->base_of_image, module->size_of_image); + module_spec, module->BaseOfImage, module->SizeOfImage); GetTarget().GetImages().Append(module_sp, true /* notify */); } bool load_addr_changed = false; - module_sp->SetLoadAddress(GetTarget(), module->base_of_image, false, + module_sp->SetLoadAddress(GetTarget(), module->BaseOfImage, false, load_addr_changed); } } diff --git a/lldb/unittests/Process/minidump/MinidumpParserTest.cpp b/lldb/unittests/Process/minidump/MinidumpParserTest.cpp index 8e2005d..f6c85f9 100644 --- a/lldb/unittests/Process/minidump/MinidumpParserTest.cpp +++ b/lldb/unittests/Process/minidump/MinidumpParserTest.cpp @@ -132,10 +132,10 @@ TEST_F(MinidumpParserTest, GetModuleListNotPadded) { SetUpData("module-list-not-padded.dmp"); auto module_list = parser->GetModuleList(); ASSERT_EQ(2UL, module_list.size()); - EXPECT_EQ(0x1000UL, module_list[0].base_of_image); - EXPECT_EQ(0x2000UL, module_list[0].size_of_image); - EXPECT_EQ(0x5000UL, module_list[1].base_of_image); - EXPECT_EQ(0x3000UL, module_list[1].size_of_image); + EXPECT_EQ(0x1000UL, module_list[0].BaseOfImage); + EXPECT_EQ(0x2000UL, module_list[0].SizeOfImage); + EXPECT_EQ(0x5000UL, module_list[1].BaseOfImage); + EXPECT_EQ(0x3000UL, module_list[1].SizeOfImage); } TEST_F(MinidumpParserTest, GetModuleListPadded) { @@ -144,10 +144,10 @@ TEST_F(MinidumpParserTest, GetModuleListPadded) { SetUpData("module-list-padded.dmp"); auto module_list = parser->GetModuleList(); ASSERT_EQ(2UL, module_list.size()); - EXPECT_EQ(0x1000UL, module_list[0].base_of_image); - EXPECT_EQ(0x2000UL, module_list[0].size_of_image); - EXPECT_EQ(0x5000UL, module_list[1].base_of_image); - EXPECT_EQ(0x3000UL, module_list[1].size_of_image); + EXPECT_EQ(0x1000UL, module_list[0].BaseOfImage); + EXPECT_EQ(0x2000UL, module_list[0].SizeOfImage); + EXPECT_EQ(0x5000UL, module_list[1].BaseOfImage); + EXPECT_EQ(0x3000UL, module_list[1].SizeOfImage); } TEST_F(MinidumpParserTest, GetMemoryListNotPadded) { @@ -220,10 +220,10 @@ TEST_F(MinidumpParserTest, GetPid) { TEST_F(MinidumpParserTest, GetModuleList) { SetUpData("linux-x86_64.dmp"); - llvm::ArrayRef modules = parser->GetModuleList(); + llvm::ArrayRef modules = parser->GetModuleList(); ASSERT_EQ(8UL, modules.size()); const auto &getName = [&](size_t i) { - return parser->GetMinidumpFile().getString(modules[i].module_name_rva); + return parser->GetMinidumpFile().getString(modules[i].ModuleNameRVA); }; EXPECT_THAT_EXPECTED( @@ -248,15 +248,15 @@ TEST_F(MinidumpParserTest, GetModuleList) { TEST_F(MinidumpParserTest, GetFilteredModuleList) { SetUpData("linux-x86_64_not_crashed.dmp"); - llvm::ArrayRef modules = parser->GetModuleList(); - std::vector filtered_modules = + llvm::ArrayRef modules = parser->GetModuleList(); + std::vector filtered_modules = parser->GetFilteredModuleList(); EXPECT_EQ(10UL, modules.size()); EXPECT_EQ(9UL, filtered_modules.size()); std::vector names; - for (const MinidumpModule *m : filtered_modules) + for (const minidump::Module *m : filtered_modules) names.push_back( - cantFail(parser->GetMinidumpFile().getString(m->module_name_rva))); + cantFail(parser->GetMinidumpFile().getString(m->ModuleNameRVA))); EXPECT_EQ(1u, llvm::count(names, "/tmp/test/linux-x86_64_not_crashed")); } @@ -496,10 +496,10 @@ TEST_F(MinidumpParserTest, GetPidWow64) { TEST_F(MinidumpParserTest, GetModuleListWow64) { SetUpData("fizzbuzz_wow64.dmp"); - llvm::ArrayRef modules = parser->GetModuleList(); + llvm::ArrayRef modules = parser->GetModuleList(); ASSERT_EQ(16UL, modules.size()); const auto &getName = [&](size_t i) { - return parser->GetMinidumpFile().getString(modules[i].module_name_rva); + return parser->GetMinidumpFile().getString(modules[i].ModuleNameRVA); }; EXPECT_THAT_EXPECTED( @@ -654,11 +654,11 @@ TEST_F(MinidumpParserTest, MinidumpDuplicateModuleMinAddress) { // That we end up with one module in the filtered list with the // range [0x1000-0x2000). MinidumpParser::GetFilteredModuleList() is // trying to ensure that if we have the same module mentioned more than - // one time, we pick the one with the lowest base_of_image. - std::vector filtered_modules = + // one time, we pick the one with the lowest BaseOfImage. + std::vector filtered_modules = parser->GetFilteredModuleList(); EXPECT_EQ(1u, filtered_modules.size()); - EXPECT_EQ(0x0000000000001000u, filtered_modules[0]->base_of_image); + EXPECT_EQ(0x0000000000001000u, filtered_modules[0]->BaseOfImage); } TEST_F(MinidumpParserTest, MinidumpModuleOrder) { @@ -670,17 +670,17 @@ TEST_F(MinidumpParserTest, MinidumpModuleOrder) { // and in the same order. Previous versions of the // MinidumpParser::GetFilteredModuleList() function would sort all images // by address and modify the order of the modules. - std::vector filtered_modules = + std::vector filtered_modules = parser->GetFilteredModuleList(); llvm::Optional name; EXPECT_EQ(2u, filtered_modules.size()); - EXPECT_EQ(0x0000000000002000u, filtered_modules[0]->base_of_image); + EXPECT_EQ(0x0000000000002000u, filtered_modules[0]->BaseOfImage); EXPECT_THAT_EXPECTED( - parser->GetMinidumpFile().getString(filtered_modules[0]->module_name_rva), + parser->GetMinidumpFile().getString(filtered_modules[0]->ModuleNameRVA), llvm::HasValue("/tmp/a")); - EXPECT_EQ(0x0000000000001000u, filtered_modules[1]->base_of_image); + EXPECT_EQ(0x0000000000001000u, filtered_modules[1]->BaseOfImage); EXPECT_THAT_EXPECTED( - parser->GetMinidumpFile().getString(filtered_modules[1]->module_name_rva), + parser->GetMinidumpFile().getString(filtered_modules[1]->ModuleNameRVA), llvm::HasValue("/tmp/b")); } -- 2.7.4