diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp --- a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp +++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp @@ -391,19 +391,23 @@ filtered_modules.push_back(&module); } else { // We have a duplicate module entry. Check the linux regions to see if - // the module we already have is not really a mapped executable. If it - // isn't check to see if the current duplicate module entry is a real - // mapped executable, and if so, replace it. This can happen when a - // process mmap's in the file for an executable in order to read bytes - // from the executable file. A memory region mapping will exist for the - // mmap'ed version and for the loaded executable, but only one will have - // a consecutive region that is executable in the memory regions. + // either module is not really a mapped executable. If one but not the + // other is a real mapped executable, prefer the executable one. This + // can happen when a process mmap's in the file for an executable in + // order to read bytes from the executable file. A memory region mapping + // will exist for the mmap'ed version and for the loaded executable, but + // only one will have a consecutive region that is executable in the + // memory regions. auto dup_module = filtered_modules[iter->second]; ConstString name(*ExpectedName); - if (!CheckForLinuxExecutable(name, linux_regions, - dup_module->BaseOfImage) && - CheckForLinuxExecutable(name, linux_regions, module.BaseOfImage)) { - filtered_modules[iter->second] = &module; + bool is_executable = + CheckForLinuxExecutable(name, linux_regions, module.BaseOfImage); + bool dup_is_executable = + CheckForLinuxExecutable(name, linux_regions, dup_module->BaseOfImage); + + if (is_executable != dup_is_executable) { + if (is_executable) + filtered_modules[iter->second] = &module; continue; } // This module has been seen. Modules are sometimes mentioned multiple diff --git a/lldb/unittests/Process/minidump/MinidumpParserTest.cpp b/lldb/unittests/Process/minidump/MinidumpParserTest.cpp --- a/lldb/unittests/Process/minidump/MinidumpParserTest.cpp +++ b/lldb/unittests/Process/minidump/MinidumpParserTest.cpp @@ -792,6 +792,47 @@ EXPECT_EQ(0x400d0000u, filtered_modules[0]->BaseOfImage); } +TEST_F(MinidumpParserTest, MinidumpDuplicateModuleMappedSecondHigh) { + ASSERT_THAT_ERROR(SetUpFromYaml(R"( +--- !minidump +Streams: + - Type: ModuleList + Modules: + - Base of Image: 0x400d3000 + Size of Image: 0x00002000 + Module Name: '/usr/lib/libc.so' + CodeView Record: '' + - Base of Image: 0x400d0000 + Size of Image: 0x00001000 + Module Name: '/usr/lib/libc.so' + CodeView Record: '' + - Type: LinuxMaps + Text: | + 400d0000-400d2000 r--p 00000000 b3:04 227 /usr/lib/libc.so + 400d2000-400d3000 rw-p 00000000 00:00 0 + 400d3000-400d4000 r-xp 00010000 b3:04 227 /usr/lib/libc.so + 400d4000-400d5000 rwxp 00001000 b3:04 227 /usr/lib/libc.so +... +)"), + llvm::Succeeded()); + // If we have a module mentioned twice in the module list, and we have full + // linux maps for all of the memory regions, make sure we pick the one that + // has a consecutive region with a matching path that has executable + // permissions. If clients open an object file with mmap, breakpad can create + // multiple mappings for a library errnoneously and the lowest address isn't + // always the right address. In this case we check the consective memory + // regions whose path matches starting at the base of image address and make + // sure one of the regions is executable and prefer that one. + // + // This test will make sure that if the executable is second in the module + // list, that it will become the selected module in the filtered list, even + // if the non-executable module was loaded at a lower base address. + std::vector filtered_modules = + parser->GetFilteredModuleList(); + ASSERT_EQ(1u, filtered_modules.size()); + EXPECT_EQ(0x400d3000u, filtered_modules[0]->BaseOfImage); +} + TEST_F(MinidumpParserTest, MinidumpDuplicateModuleSeparateCode) { ASSERT_THAT_ERROR(SetUpFromYaml(R"( --- !minidump