diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py b/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py --- a/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py +++ b/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py @@ -42,7 +42,7 @@ def test_zero_uuid_modules(self): """ Test multiple modules having a MINIDUMP_MODULE.CvRecord that is valid, - but contains a PDB70 value whose age is zero and whose UUID values are + but contains a PDB70 value whose age is zero and whose UUID values are all zero. Prior to a fix all such modules would be duplicated to the first one since the UUIDs claimed to be valid and all zeroes. Now we ensure that the UUID is not valid for each module and that we have @@ -56,7 +56,7 @@ def test_uuid_modules_no_age(self): """ Test multiple modules having a MINIDUMP_MODULE.CvRecord that is valid, - and contains a PDB70 value whose age is zero and whose UUID values are + and contains a PDB70 value whose age is zero and whose UUID values are valid. Ensure we decode the UUID and don't include the age field in the UUID. """ modules = self.get_minidump_modules("linux-arm-uuids-no-age.yaml") @@ -68,7 +68,7 @@ def test_uuid_modules_no_age_apple(self): """ Test multiple modules having a MINIDUMP_MODULE.CvRecord that is valid, - and contains a PDB70 value whose age is zero and whose UUID values are + and contains a PDB70 value whose age is zero and whose UUID values are valid. Ensure we decode the UUID and don't include the age field in the UUID. Also ensure that the first uint32_t is byte swapped, along with the next two uint16_t values. Breakpad incorrectly byte swaps these values when it @@ -83,7 +83,7 @@ def test_uuid_modules_with_age(self): """ Test multiple modules having a MINIDUMP_MODULE.CvRecord that is valid, - and contains a PDB70 value whose age is valid and whose UUID values are + and contains a PDB70 value whose age is valid and whose UUID values are valid. Ensure we decode the UUID and include the age field in the UUID. """ modules = self.get_minidump_modules("linux-arm-uuids-with-age.yaml") @@ -121,13 +121,31 @@ self.verify_module(modules[0], "/not/exist/a", None) self.verify_module(modules[1], "/not/exist/b", None) + def test_uuid_modules_elf_build_id_same(self): + """ + Test multiple modules having a MINIDUMP_MODULE.CvRecord that is + valid, and contains a ELF build ID whose value is the same. There + is an assert in the PlaceholderObjectFile that was firing when we + encountered this which was crashing the process that was checking + if PlaceholderObjectFile.m_base was the same as the address this + fake module was being loaded at. We need to ensure we don't crash + in such cases and that we add both modules even though they have + the same UUID. + """ + modules = self.get_minidump_modules("linux-arm-same-uuids.yaml") + self.assertEqual(2, len(modules)) + self.verify_module(modules[0], "/file/does/not/exist/a", + '11223344-1122-3344-1122-334411223344-11223344') + self.verify_module(modules[1], "/file/does/not/exist/b", + '11223344-1122-3344-1122-334411223344-11223344') + @expectedFailureAll(oslist=["windows"]) def test_partial_uuid_match(self): """ Breakpad has been known to create minidump files using CvRecord in each module whose signature is set to PDB70 where the UUID only contains the - first 16 bytes of a 20 byte ELF build ID. Code was added to - ProcessMinidump.cpp to deal with this and allows partial UUID matching. + first 16 bytes of a 20 byte ELF build ID. Code was added to + ProcessMinidump.cpp to deal with this and allows partial UUID matching. This test verifies that if we have a minidump with a 16 byte UUID, that we are able to associate a symbol file with a 20 byte UUID only if the @@ -141,16 +159,16 @@ self.dbg.HandleCommand(cmd) modules = self.get_minidump_modules("linux-arm-partial-uuids-match.yaml") self.assertEqual(1, len(modules)) - self.verify_module(modules[0], so_path, + self.verify_module(modules[0], so_path, "7295E17C-6668-9E05-CBB5-DEE5003865D5-5267C116") def test_partial_uuid_mismatch(self): """ Breakpad has been known to create minidump files using CvRecord in each module whose signature is set to PDB70 where the UUID only contains the - first 16 bytes of a 20 byte ELF build ID. Code was added to - ProcessMinidump.cpp to deal with this and allows partial UUID matching. - + first 16 bytes of a 20 byte ELF build ID. Code was added to + ProcessMinidump.cpp to deal with this and allows partial UUID matching. + This test verifies that if we have a minidump with a 16 byte UUID, that we are not able to associate a symbol file with a 20 byte UUID only if any of the first 16 bytes do not match. In this case we will see the UUID @@ -163,7 +181,7 @@ modules = self.get_minidump_modules("linux-arm-partial-uuids-mismatch.yaml") self.assertEqual(1, len(modules)) self.verify_module(modules[0], - "/invalid/path/on/current/system/libuuidmismatch.so", + "/invalid/path/on/current/system/libuuidmismatch.so", "7295E17C-6668-9E05-CBB5-DEE5003865D5") def test_relative_module_name(self): diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-same-uuids.yaml b/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-same-uuids.yaml new file mode 100644 --- /dev/null +++ b/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-same-uuids.yaml @@ -0,0 +1,21 @@ +--- !minidump +Streams: + - Type: SystemInfo + Processor Arch: AMD64 + Platform ID: Linux + CSD Version: '15E216' + CPU: + Vendor ID: GenuineIntel + Version Info: 0x00000000 + Feature Info: 0x00000000 + - Type: ModuleList + Modules: + - Base of Image: 0x0000000000001000 + Size of Image: 0x00001000 + Module Name: '/file/does/not/exist/a' + CodeView Record: '52534453112233441122334411223344112233441122334411' + - Base of Image: 0x0000000000003000 + Size of Image: 0x00001000 + Module Name: '/file/does/not/exist/b' + CodeView Record: '52534453112233441122334411223344112233441122334411' +... diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp --- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp +++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp @@ -49,8 +49,8 @@ class PlaceholderObjectFile : public ObjectFile { public: PlaceholderObjectFile(const lldb::ModuleSP &module_sp, - const ModuleSpec &module_spec, lldb::offset_t base, - lldb::offset_t size) + const ModuleSpec &module_spec, lldb::addr_t base, + lldb::addr_t size) : ObjectFile(module_sp, &module_spec.GetFileSpec(), /*file_offset*/ 0, /*length*/ 0, /*data_sp*/ nullptr, /*data_offset*/ 0), m_arch(module_spec.GetArchitecture()), m_uuid(module_spec.GetUUID()), @@ -58,7 +58,10 @@ m_symtab_up = std::make_unique(this); } - ConstString GetPluginName() override { return ConstString("placeholder"); } + static ConstString GetStaticPluginName() { + return ConstString("placeholder"); + } + ConstString GetPluginName() override { return GetStaticPluginName(); } uint32_t GetPluginVersion() override { return 1; } bool ParseHeader() override { return true; } Type CalculateType() override { return eTypeUnknown; } @@ -109,11 +112,12 @@ GetFileSpec(), m_base, m_base + m_size); } + lldb::addr_t GetBaseImageAddress() const { return m_base; } private: ArchSpec m_arch; UUID m_uuid; - lldb::offset_t m_base; - lldb::offset_t m_size; + lldb::addr_t m_base; + lldb::addr_t m_size; }; } // namespace @@ -351,14 +355,15 @@ std::vector filtered_modules = m_minidump_parser->GetFilteredModuleList(); - Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_MODULES)); + Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER)); for (auto module : filtered_modules) { std::string name = cantFail(m_minidump_parser->GetMinidumpFile().getString( module->ModuleNameRVA)); + const uint64_t load_addr = module->BaseOfImage; + const uint64_t load_size = module->SizeOfImage; LLDB_LOG(log, "found module: name: {0} {1:x10}-{2:x10} size: {3}", name, - module->BaseOfImage, module->BaseOfImage + module->SizeOfImage, - module->SizeOfImage); + load_addr, load_addr + load_size, load_size); // check if the process is wow64 - a 32 bit windows process running on a // 64 bit windows @@ -373,7 +378,7 @@ Status error; // Try and find a module with a full UUID that matches. This function will // add the module to the target if it finds one. - lldb::ModuleSP module_sp = GetTarget().GetOrCreateModule(module_spec, + lldb::ModuleSP module_sp = GetTarget().GetOrCreateModule(module_spec, true /* notify */, &error); if (!module_sp) { // Try and find a module without specifying the UUID and only looking for @@ -386,8 +391,8 @@ ModuleSpec basename_module_spec(module_spec); basename_module_spec.GetUUID().Clear(); basename_module_spec.GetFileSpec().GetDirectory().Clear(); - module_sp = GetTarget().GetOrCreateModule(basename_module_spec, - true /* notify */, &error); + module_sp = GetTarget().GetOrCreateModule(basename_module_spec, + true /* notify */, &error); if (module_sp) { // We consider the module to be a match if the minidump UUID is a // prefix of the actual UUID, or if either of the UUIDs are empty. @@ -401,6 +406,19 @@ } } } + if (module_sp) { + // Watch out for place holder modules that have different paths, but the + // same UUID. If the base address is different, create a new module. If + // we don't then we will end up setting the load address of a different + // PlaceholderObjectFile and an assertion will fire. + auto *objfile = module_sp->GetObjectFile(); + if (objfile && objfile->GetPluginName() == + PlaceholderObjectFile::GetStaticPluginName()) { + if (((PlaceholderObjectFile *)objfile)->GetBaseImageAddress() != + load_addr) + module_sp.reset(); + } + } if (!module_sp) { // We failed to locate a matching local object file. Fortunately, the // minidump format encodes enough information about each module's memory @@ -415,12 +433,12 @@ name); module_sp = Module::CreateModuleFromObjectFile( - module_spec, module->BaseOfImage, module->SizeOfImage); + module_spec, load_addr, load_size); GetTarget().GetImages().Append(module_sp, true /* notify */); } bool load_addr_changed = false; - module_sp->SetLoadAddress(GetTarget(), module->BaseOfImage, false, + module_sp->SetLoadAddress(GetTarget(), load_addr, false, load_addr_changed); } }