From ff12913b638e58b7a64d7ad819bac4367f49bc3f Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Tue, 9 Apr 2019 08:28:27 +0000 Subject: [PATCH] Minidump: use string parsing functionality from llvm llvm-svn: 357977 --- .../Plugins/Process/minidump/MinidumpParser.cpp | 42 +++---- .../Plugins/Process/minidump/MinidumpParser.h | 2 - .../Plugins/Process/minidump/MinidumpTypes.cpp | 37 ------ .../Plugins/Process/minidump/MinidumpTypes.h | 5 - .../Plugins/Process/minidump/ProcessMinidump.cpp | 38 ++---- .../Process/minidump/MinidumpParserTest.cpp | 131 ++++++++++++--------- 6 files changed, 104 insertions(+), 151 deletions(-) diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp index 605e56e0..d4d29f2 100644 --- a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp +++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp @@ -63,14 +63,6 @@ llvm::ArrayRef MinidumpParser::GetStream(StreamType stream_type) { .getValueOr(llvm::ArrayRef()); } -llvm::Optional MinidumpParser::GetMinidumpString(uint32_t rva) { - auto arr_ref = m_data_sp->GetData(); - if (rva > arr_ref.size()) - return llvm::None; - arr_ref = arr_ref.drop_front(rva); - return parseMinidumpString(arr_ref); -} - UUID MinidumpParser::GetModuleUUID(const MinidumpModule *module) { auto cv_record = GetData().slice(module->CV_record.RVA, module->CV_record.DataSize); @@ -244,13 +236,17 @@ ArchSpec MinidumpParser::GetArchitecture() { break; default: { triple.setOS(llvm::Triple::OSType::UnknownOS); - std::string csd_version; - if (auto s = GetMinidumpString(system_info->CSDVersionRVA)) - csd_version = *s; - if (csd_version.find("Linux") != std::string::npos) - triple.setOS(llvm::Triple::OSType::Linux); - break; + auto ExpectedCSD = m_file->getString(system_info->CSDVersionRVA); + if (!ExpectedCSD) { + LLDB_LOG_ERROR(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS), + ExpectedCSD.takeError(), + "Failed to CSD Version string: {0}"); + } else { + if (ExpectedCSD->find("Linux") != std::string::npos) + triple.setOS(llvm::Triple::OSType::Linux); } + break; + } } m_arch.SetTriple(triple); return m_arch; @@ -305,24 +301,22 @@ std::vector MinidumpParser::GetFilteredModuleList() { std::vector filtered_modules; - llvm::Optional name; - std::string module_name; - for (const auto &module : modules) { - name = GetMinidumpString(module.module_name_rva); - - if (!name) + auto ExpectedName = m_file->getString(module.module_name_rva); + if (!ExpectedName) { + LLDB_LOG_ERROR(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_MODULES), + ExpectedName.takeError(), + "Failed to module name: {0}"); continue; - - module_name = name.getValue(); - + } + MapType::iterator iter; bool inserted; // See if we have inserted this module aready into filtered_modules. If we // haven't insert an entry into module_name_to_filtered_index with the // index where we will insert it if it isn't in the vector already. std::tie(iter, inserted) = module_name_to_filtered_index.try_emplace( - module_name, filtered_modules.size()); + *ExpectedName, filtered_modules.size()); if (inserted) { // This module has not been seen yet, insert it into filtered_modules at diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.h b/lldb/source/Plugins/Process/minidump/MinidumpParser.h index 5cff646..5df2d74 100644 --- a/lldb/source/Plugins/Process/minidump/MinidumpParser.h +++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.h @@ -52,8 +52,6 @@ public: llvm::ArrayRef GetStream(StreamType stream_type); - llvm::Optional GetMinidumpString(uint32_t rva); - UUID GetModuleUUID(const MinidumpModule* module); llvm::ArrayRef GetThreads(); diff --git a/lldb/source/Plugins/Process/minidump/MinidumpTypes.cpp b/lldb/source/Plugins/Process/minidump/MinidumpTypes.cpp index 221f715..ba1f102 100644 --- a/lldb/source/Plugins/Process/minidump/MinidumpTypes.cpp +++ b/lldb/source/Plugins/Process/minidump/MinidumpTypes.cpp @@ -14,43 +14,6 @@ using namespace lldb_private; using namespace minidump; -// Minidump string -llvm::Optional -lldb_private::minidump::parseMinidumpString(llvm::ArrayRef &data) { - std::string result; - - const llvm::support::ulittle32_t *source_length; - Status error = consumeObject(data, source_length); - - if (error.Fail() || *source_length > data.size() || *source_length % 2 != 0) - return llvm::None; - - auto *source_start = - reinterpret_cast(data.data()); - // source_length is the length of the string in bytes we need the length of - // the string in UTF-16 characters/code points (16 bits per char) that's why - // it's divided by 2 - uint32_t utf16_length = *source_length / 2; - - // Correct the endianness and alignment of the string. - llvm::SmallVector utf16(utf16_length, 0); - std::copy_n(source_start, utf16_length, utf16.begin()); - - const llvm::UTF16 *utf16_start = utf16.begin(); - - // resize to worst case length - result.resize(UNI_MAX_UTF8_BYTES_PER_CODE_POINT * utf16_length); - auto result_start = reinterpret_cast(&result[0]); - const auto result_end = result_start + result.size(); - llvm::ConvertUTF16toUTF8(&utf16_start, utf16.end(), &result_start, result_end, - llvm::strictConversion); - const auto result_size = - std::distance(reinterpret_cast(&result[0]), result_start); - result.resize(result_size); // shrink to actual length - - return result; -} - // MinidumpThread const MinidumpThread *MinidumpThread::Parse(llvm::ArrayRef &data) { const MinidumpThread *thread = nullptr; diff --git a/lldb/source/Plugins/Process/minidump/MinidumpTypes.h b/lldb/source/Plugins/Process/minidump/MinidumpTypes.h index 67cda5d..ac69066 100644 --- a/lldb/source/Plugins/Process/minidump/MinidumpTypes.h +++ b/lldb/source/Plugins/Process/minidump/MinidumpTypes.h @@ -70,11 +70,6 @@ Status consumeObject(llvm::ArrayRef &Buffer, const T *&Object) { return error; } -// parse a MinidumpString which is with UTF-16 -// Reference: -// https://msdn.microsoft.com/en-us/library/windows/desktop/ms680395(v=vs.85).aspx -llvm::Optional parseMinidumpString(llvm::ArrayRef &data); - // Reference: // https://msdn.microsoft.com/en-us/library/windows/desktop/ms680384(v=vs.85).aspx struct MinidumpMemoryDescriptor { diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp index 3da103b..79de4aa 100644 --- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp +++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp @@ -354,34 +354,21 @@ void ProcessMinidump::ReadModuleList() { Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_MODULES)); for (auto module : filtered_modules) { - llvm::Optional name = - m_minidump_parser->GetMinidumpString(module->module_name_rva); - - if (!name) - continue; - - if (log) { - log->Printf("ProcessMinidump::%s found module: name: %s %#010" PRIx64 - "-%#010" PRIx64 " size: %" PRIu32, - __FUNCTION__, name.getValue().c_str(), - uint64_t(module->base_of_image), - module->base_of_image + module->size_of_image, - uint32_t(module->size_of_image)); - } + std::string name = cantFail(m_minidump_parser->GetMinidumpFile().getString( + module->module_name_rva)); + 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); // check if the process is wow64 - a 32 bit windows process running on a // 64 bit windows - if (llvm::StringRef(name.getValue()).endswith_lower("wow64.dll")) { + if (llvm::StringRef(name).endswith_lower("wow64.dll")) { m_is_wow64 = true; } - if (log) { - log->Printf("ProcessMinidump::%s load module: name: %s", __FUNCTION__, - name.getValue().c_str()); - } - const auto uuid = m_minidump_parser->GetModuleUUID(module); - auto file_spec = FileSpec(name.getValue(), GetArchitecture().GetTriple()); + auto file_spec = FileSpec(name, GetArchitecture().GetTriple()); FileSystem::Instance().Resolve(file_spec); ModuleSpec module_spec(file_spec, uuid); module_spec.GetArchitecture() = GetArchitecture(); @@ -424,11 +411,10 @@ void ProcessMinidump::ReadModuleList() { // This enables most LLDB functionality involving address-to-module // translations (ex. identifing the module for a stack frame PC) and // modules/sections commands (ex. target modules list, ...) - if (log) { - log->Printf("Unable to locate the matching object file, creating a " - "placeholder module for: %s", - name.getValue().c_str()); - } + LLDB_LOG(log, + "Unable to locate the matching object file, creating a " + "placeholder module for: {0}", + name); module_sp = Module::CreateModuleFromObjectFile( module_spec, module->base_of_image, module->size_of_image); diff --git a/lldb/unittests/Process/minidump/MinidumpParserTest.cpp b/lldb/unittests/Process/minidump/MinidumpParserTest.cpp index b036091..8e2005d 100644 --- a/lldb/unittests/Process/minidump/MinidumpParserTest.cpp +++ b/lldb/unittests/Process/minidump/MinidumpParserTest.cpp @@ -222,23 +222,28 @@ TEST_F(MinidumpParserTest, GetModuleList) { SetUpData("linux-x86_64.dmp"); llvm::ArrayRef modules = parser->GetModuleList(); ASSERT_EQ(8UL, modules.size()); - std::string module_names[8] = { - "/usr/local/google/home/dvlahovski/projects/test_breakpad/a.out", - "/lib/x86_64-linux-gnu/libm-2.19.so", - "/lib/x86_64-linux-gnu/libc-2.19.so", - "/lib/x86_64-linux-gnu/libgcc_s.so.1", - "/usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.19", - "/lib/x86_64-linux-gnu/libpthread-2.19.so", - "/lib/x86_64-linux-gnu/ld-2.19.so", - "linux-gate.so", + const auto &getName = [&](size_t i) { + return parser->GetMinidumpFile().getString(modules[i].module_name_rva); }; - for (int i = 0; i < 8; ++i) { - llvm::Optional name = - parser->GetMinidumpString(modules[i].module_name_rva); - ASSERT_TRUE(name.hasValue()); - EXPECT_EQ(module_names[i], name.getValue()); - } + EXPECT_THAT_EXPECTED( + getName(0), + llvm::HasValue( + "/usr/local/google/home/dvlahovski/projects/test_breakpad/a.out")); + EXPECT_THAT_EXPECTED(getName(1), + llvm::HasValue("/lib/x86_64-linux-gnu/libm-2.19.so")); + EXPECT_THAT_EXPECTED(getName(2), + llvm::HasValue("/lib/x86_64-linux-gnu/libc-2.19.so")); + EXPECT_THAT_EXPECTED(getName(3), + llvm::HasValue("/lib/x86_64-linux-gnu/libgcc_s.so.1")); + EXPECT_THAT_EXPECTED( + getName(4), + llvm::HasValue("/usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.19")); + EXPECT_THAT_EXPECTED( + getName(5), llvm::HasValue("/lib/x86_64-linux-gnu/libpthread-2.19.so")); + EXPECT_THAT_EXPECTED(getName(6), + llvm::HasValue("/lib/x86_64-linux-gnu/ld-2.19.so")); + EXPECT_THAT_EXPECTED(getName(7), llvm::HasValue("linux-gate.so")); } TEST_F(MinidumpParserTest, GetFilteredModuleList) { @@ -248,19 +253,12 @@ TEST_F(MinidumpParserTest, GetFilteredModuleList) { parser->GetFilteredModuleList(); EXPECT_EQ(10UL, modules.size()); EXPECT_EQ(9UL, filtered_modules.size()); - // EXPECT_GT(modules.size(), filtered_modules.size()); - bool found = false; - for (size_t i = 0; i < filtered_modules.size(); ++i) { - llvm::Optional name = - parser->GetMinidumpString(filtered_modules[i]->module_name_rva); - ASSERT_TRUE(name.hasValue()); - if (name.getValue() == "/tmp/test/linux-x86_64_not_crashed") { - ASSERT_FALSE(found) << "There should be only one module with this name " - "in the filtered module list"; - found = true; - ASSERT_EQ(0x400000UL, filtered_modules[i]->base_of_image); - } - } + std::vector names; + for (const MinidumpModule *m : filtered_modules) + names.push_back( + cantFail(parser->GetMinidumpFile().getString(m->module_name_rva))); + + EXPECT_EQ(1u, llvm::count(names, "/tmp/test/linux-x86_64_not_crashed")); } TEST_F(MinidumpParserTest, GetExceptionStream) { @@ -500,31 +498,50 @@ TEST_F(MinidumpParserTest, GetModuleListWow64) { SetUpData("fizzbuzz_wow64.dmp"); llvm::ArrayRef modules = parser->GetModuleList(); ASSERT_EQ(16UL, modules.size()); - std::string module_names[16] = { - R"(D:\src\llvm\llvm\tools\lldb\packages\Python\lldbsuite\test\functionalities\postmortem\wow64_minidump\fizzbuzz.exe)", - R"(C:\Windows\System32\ntdll.dll)", - R"(C:\Windows\System32\wow64.dll)", - R"(C:\Windows\System32\wow64win.dll)", - R"(C:\Windows\System32\wow64cpu.dll)", - R"(D:\src\llvm\llvm\tools\lldb\packages\Python\lldbsuite\test\functionalities\postmortem\wow64_minidump\fizzbuzz.exe)", - R"(C:\Windows\SysWOW64\ntdll.dll)", - R"(C:\Windows\SysWOW64\kernel32.dll)", - R"(C:\Windows\SysWOW64\KERNELBASE.dll)", - R"(C:\Windows\SysWOW64\advapi32.dll)", - R"(C:\Windows\SysWOW64\msvcrt.dll)", - R"(C:\Windows\SysWOW64\sechost.dll)", - R"(C:\Windows\SysWOW64\rpcrt4.dll)", - R"(C:\Windows\SysWOW64\sspicli.dll)", - R"(C:\Windows\SysWOW64\CRYPTBASE.dll)", - R"(C:\Windows\System32\api-ms-win-core-synch-l1-2-0.DLL)", + const auto &getName = [&](size_t i) { + return parser->GetMinidumpFile().getString(modules[i].module_name_rva); }; - for (int i = 0; i < 16; ++i) { - llvm::Optional name = - parser->GetMinidumpString(modules[i].module_name_rva); - ASSERT_TRUE(name.hasValue()); - EXPECT_EQ(module_names[i], name.getValue()); - } + EXPECT_THAT_EXPECTED( + getName(0), + llvm::HasValue( + R"(D:\src\llvm\llvm\tools\lldb\packages\Python\lldbsuite\test\functionalities\postmortem\wow64_minidump\fizzbuzz.exe)")); + EXPECT_THAT_EXPECTED(getName(1), llvm::HasValue( + R"(C:\Windows\System32\ntdll.dll)")); + EXPECT_THAT_EXPECTED(getName(2), llvm::HasValue( + R"(C:\Windows\System32\wow64.dll)")); + EXPECT_THAT_EXPECTED(getName(3), llvm::HasValue( + R"(C:\Windows\System32\wow64win.dll)")); + EXPECT_THAT_EXPECTED(getName(4), llvm::HasValue( + R"(C:\Windows\System32\wow64cpu.dll)")); + EXPECT_THAT_EXPECTED( + getName(5), + llvm::HasValue( + R"(D:\src\llvm\llvm\tools\lldb\packages\Python\lldbsuite\test\functionalities\postmortem\wow64_minidump\fizzbuzz.exe)")); + EXPECT_THAT_EXPECTED(getName(6), llvm::HasValue( + R"(C:\Windows\SysWOW64\ntdll.dll)")); + EXPECT_THAT_EXPECTED(getName(7), llvm::HasValue( + R"(C:\Windows\SysWOW64\kernel32.dll)")); + EXPECT_THAT_EXPECTED(getName(8), + llvm::HasValue( + R"(C:\Windows\SysWOW64\KERNELBASE.dll)")); + EXPECT_THAT_EXPECTED(getName(9), llvm::HasValue( + R"(C:\Windows\SysWOW64\advapi32.dll)")); + EXPECT_THAT_EXPECTED(getName(10), llvm::HasValue( + R"(C:\Windows\SysWOW64\msvcrt.dll)")); + EXPECT_THAT_EXPECTED(getName(11), llvm::HasValue( + R"(C:\Windows\SysWOW64\sechost.dll)")); + EXPECT_THAT_EXPECTED(getName(12), llvm::HasValue( + R"(C:\Windows\SysWOW64\rpcrt4.dll)")); + EXPECT_THAT_EXPECTED(getName(13), llvm::HasValue( + R"(C:\Windows\SysWOW64\sspicli.dll)")); + EXPECT_THAT_EXPECTED(getName(14), + llvm::HasValue( + R"(C:\Windows\SysWOW64\CRYPTBASE.dll)")); + EXPECT_THAT_EXPECTED( + getName(15), + llvm::HasValue( + R"(C:\Windows\System32\api-ms-win-core-synch-l1-2-0.DLL)")); } // Register tests @@ -658,12 +675,12 @@ TEST_F(MinidumpParserTest, MinidumpModuleOrder) { llvm::Optional name; EXPECT_EQ(2u, filtered_modules.size()); EXPECT_EQ(0x0000000000002000u, filtered_modules[0]->base_of_image); - name = parser->GetMinidumpString(filtered_modules[0]->module_name_rva); - ASSERT_TRUE((bool)name); - EXPECT_EQ(std::string("/tmp/a"), *name); + EXPECT_THAT_EXPECTED( + parser->GetMinidumpFile().getString(filtered_modules[0]->module_name_rva), + llvm::HasValue("/tmp/a")); EXPECT_EQ(0x0000000000001000u, filtered_modules[1]->base_of_image); - name = parser->GetMinidumpString(filtered_modules[1]->module_name_rva); - ASSERT_TRUE((bool)name); - EXPECT_EQ(std::string("/tmp/b"), *name); + EXPECT_THAT_EXPECTED( + parser->GetMinidumpFile().getString(filtered_modules[1]->module_name_rva), + llvm::HasValue("/tmp/b")); } -- 2.7.4