clayborg created this revision.
clayborg added reviewers: labath, aadsm, wallace.
Herald added a subscriber: mgrang.
Herald added a project: LLDB.
clayborg requested review of this revision.
Herald added a subscriber: JDevlieghere.
Breakpad creates minidump files that can a module loaded multiple times. We
found that when a process mmap's the object file for a library, this can
confuse breakpad into creating multiple modules in the module list. This patch
fixes the GetFilteredModules() to check the linux maps for permissions and use
the one that has execute permissions. Typically when people mmap a file into
memory they don't map it as executable. This helps people to correctly load
minidump files for post mortem analysis.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D86375
Files:
lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
lldb/source/Plugins/Process/minidump/MinidumpParser.h
lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
lldb/unittests/Process/minidump/MinidumpParserTest.cpp
Index: lldb/unittests/Process/minidump/MinidumpParserTest.cpp
===================================================================
--- lldb/unittests/Process/minidump/MinidumpParserTest.cpp
+++ lldb/unittests/Process/minidump/MinidumpParserTest.cpp
@@ -689,6 +689,42 @@
EXPECT_EQ(0x0000000000001000u, filtered_modules[0]->BaseOfImage);
}
+TEST_F(MinidumpParserTest, MinidumpDuplicateModuleExecutableAddress) {
+ ASSERT_THAT_ERROR(SetUpFromYaml(R"(
+--- !minidump
+Streams:
+ - Type: ModuleList
+ Modules:
+ - Base of Image: 0x400d9000
+ Size of Image: 0x00002000
+ Module Name: '/usr/lib/libc.so'
+ CodeView Record: ''
+ - Base of Image: 0x400ee000
+ Size of Image: 0x00001000
+ Module Name: '/usr/lib/libc.so'
+ CodeView Record: ''
+ - Type: LinuxMaps
+ Text: |
+ 400d9000-400db000 r--p 00000000 b3:04 227 /usr/lib/libc.so
+ 400dc000-400dd000 rw-p 00000000 00:00 0
+ 400ee000-400ef000 r-xp 00010000 b3:04 227 /usr/lib/libc.so
+ 400fc000-400fd000 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
+ // is marked as executable. 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
+ // memory region of the base of image address and make sure it is executable
+ // and prefer that one.
+ std::vector<const minidump::Module *> filtered_modules =
+ parser->GetFilteredModuleList();
+ ASSERT_EQ(1u, filtered_modules.size());
+ EXPECT_EQ(0x400ee000u, filtered_modules[0]->BaseOfImage);
+}
+
TEST_F(MinidumpParserTest, MinidumpModuleOrder) {
ASSERT_THAT_ERROR(SetUpFromYaml(R"(
--- !minidump
@@ -721,4 +757,3 @@
parser->GetMinidumpFile().getString(filtered_modules[1]->ModuleNameRVA),
llvm::HasValue("/tmp/b"));
}
-
Index: lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
===================================================================
--- lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -338,32 +338,6 @@
return ArchSpec(triple);
}
-static MemoryRegionInfo GetMemoryRegionInfo(const MemoryRegionInfos ®ions,
- lldb::addr_t load_addr) {
- MemoryRegionInfo region;
- auto pos = llvm::upper_bound(regions, load_addr);
- if (pos != regions.begin() &&
- std::prev(pos)->GetRange().Contains(load_addr)) {
- return *std::prev(pos);
- }
-
- if (pos == regions.begin())
- region.GetRange().SetRangeBase(0);
- else
- region.GetRange().SetRangeBase(std::prev(pos)->GetRange().GetRangeEnd());
-
- if (pos == regions.end())
- region.GetRange().SetRangeEnd(UINT64_MAX);
- else
- region.GetRange().SetRangeEnd(pos->GetRange().GetRangeBase());
-
- region.SetReadable(MemoryRegionInfo::eNo);
- region.SetWritable(MemoryRegionInfo::eNo);
- region.SetExecutable(MemoryRegionInfo::eNo);
- region.SetMapped(MemoryRegionInfo::eNo);
- return region;
-}
-
void ProcessMinidump::BuildMemoryRegions() {
if (m_memory_regions)
return;
@@ -388,7 +362,7 @@
MemoryRegionInfo::RangeType section_range(load_addr,
section_sp->GetByteSize());
MemoryRegionInfo region =
- ::GetMemoryRegionInfo(*m_memory_regions, load_addr);
+ MinidumpParser::GetMemoryRegionInfo(*m_memory_regions, load_addr);
if (region.GetMapped() != MemoryRegionInfo::eYes &&
region.GetRange().GetRangeBase() <= section_range.GetRangeBase() &&
section_range.GetRangeEnd() <= region.GetRange().GetRangeEnd()) {
@@ -409,7 +383,7 @@
Status ProcessMinidump::GetMemoryRegionInfo(lldb::addr_t load_addr,
MemoryRegionInfo ®ion) {
BuildMemoryRegions();
- region = ::GetMemoryRegionInfo(*m_memory_regions, load_addr);
+ region = MinidumpParser::GetMemoryRegionInfo(*m_memory_regions, load_addr);
return Status();
}
Index: lldb/source/Plugins/Process/minidump/MinidumpParser.h
===================================================================
--- lldb/source/Plugins/Process/minidump/MinidumpParser.h
+++ lldb/source/Plugins/Process/minidump/MinidumpParser.h
@@ -96,6 +96,9 @@
llvm::object::MinidumpFile &GetMinidumpFile() { return *m_file; }
+ static MemoryRegionInfo GetMemoryRegionInfo(const MemoryRegionInfos ®ions,
+ lldb::addr_t load_addr);
+
private:
MinidumpParser(lldb::DataBufferSP data_sp,
std::unique_ptr<llvm::object::MinidumpFile> file);
Index: lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
===================================================================
--- lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
+++ lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -267,6 +267,22 @@
return {};
}
+static bool
+CreateRegionsCacheFromLinuxMaps(MinidumpParser &parser,
+ std::vector<MemoryRegionInfo> ®ions) {
+ auto data = parser.GetStream(StreamType::LinuxMaps);
+ if (data.empty())
+ return false;
+ ParseLinuxMapRegions(llvm::toStringRef(data),
+ [&](const lldb_private::MemoryRegionInfo ®ion,
+ const lldb_private::Status &status) -> bool {
+ if (status.Success())
+ regions.push_back(region);
+ return true;
+ });
+ return !regions.empty();
+}
+
std::vector<const minidump::Module *> MinidumpParser::GetFilteredModuleList() {
Log *log = GetLogIfAnyCategoriesSet(LIBLLDB_LOG_MODULES);
auto ExpectedModules = GetMinidumpFile().getModuleList();
@@ -276,6 +292,15 @@
return {};
}
+ // Create memory regions from the linux maps only. We do this to avoid issues
+ // with breakpad generated minidumps where if someone has mmap'ed a shared
+ // library into memory to accesss its data in the object file, we can get a
+ // minidump with two mappings for a binary: one whose base image points to a
+ // memory region that is read + execute and one that is read only.
+ MemoryRegionInfos linux_regions;
+ if (CreateRegionsCacheFromLinuxMaps(*this, linux_regions))
+ llvm::sort(linux_regions);
+
// map module_name -> filtered_modules index
typedef llvm::StringMap<size_t> MapType;
MapType module_name_to_filtered_index;
@@ -304,10 +329,25 @@
// "filtered_modules.size()" above.
filtered_modules.push_back(&module);
} else {
+ auto dup_module = filtered_modules[iter->second];
+ if (!linux_regions.empty()) {
+ // We have complete memory region information so check for the case
+ // where someone might have mmap'ed the module into memory to read data
+ // from the object file. In this case, the mapping that has execute
+ // permissions is the right one even if it isn't the lowest.
+ MemoryRegionInfo dup_region(
+ GetMemoryRegionInfo(linux_regions, dup_module->BaseOfImage));
+ MemoryRegionInfo mod_region(
+ GetMemoryRegionInfo(linux_regions, module.BaseOfImage));
+ if (dup_region.GetExecutable() != MemoryRegionInfo::eYes &&
+ mod_region.GetExecutable() == MemoryRegionInfo::eYes) {
+ filtered_modules[iter->second] = &module;
+ continue;
+ }
+ }
// This module has been seen. Modules are sometimes mentioned multiple
// 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.BaseOfImage < dup_module->BaseOfImage)
filtered_modules[iter->second] = &module;
}
@@ -412,22 +452,6 @@
}
static bool
-CreateRegionsCacheFromLinuxMaps(MinidumpParser &parser,
- std::vector<MemoryRegionInfo> ®ions) {
- auto data = parser.GetStream(StreamType::LinuxMaps);
- if (data.empty())
- return false;
- ParseLinuxMapRegions(llvm::toStringRef(data),
- [&](const lldb_private::MemoryRegionInfo ®ion,
- const lldb_private::Status &status) -> bool {
- if (status.Success())
- regions.push_back(region);
- return true;
- });
- return !regions.empty();
-}
-
-static bool
CreateRegionsCacheFromMemoryInfoList(MinidumpParser &parser,
std::vector<MemoryRegionInfo> ®ions) {
Log *log = GetLogIfAnyCategoriesSet(LIBLLDB_LOG_MODULES);
@@ -500,10 +524,10 @@
uint64_t base_rva;
std::tie(memory64_list, base_rva) =
MinidumpMemoryDescriptor64::ParseMemory64List(data);
-
+
if (memory64_list.empty())
return false;
-
+
regions.reserve(memory64_list.size());
for (const auto &memory_desc : memory64_list) {
if (memory_desc.data_size == 0)
@@ -597,3 +621,30 @@
}
return "unknown stream type";
}
+
+MemoryRegionInfo
+MinidumpParser::GetMemoryRegionInfo(const MemoryRegionInfos ®ions,
+ lldb::addr_t load_addr) {
+ MemoryRegionInfo region;
+ auto pos = llvm::upper_bound(regions, load_addr);
+ if (pos != regions.begin() &&
+ std::prev(pos)->GetRange().Contains(load_addr)) {
+ return *std::prev(pos);
+ }
+
+ if (pos == regions.begin())
+ region.GetRange().SetRangeBase(0);
+ else
+ region.GetRange().SetRangeBase(std::prev(pos)->GetRange().GetRangeEnd());
+
+ if (pos == regions.end())
+ region.GetRange().SetRangeEnd(UINT64_MAX);
+ else
+ region.GetRange().SetRangeEnd(pos->GetRange().GetRangeBase());
+
+ region.SetReadable(MemoryRegionInfo::eNo);
+ region.SetWritable(MemoryRegionInfo::eNo);
+ region.SetExecutable(MemoryRegionInfo::eNo);
+ region.SetMapped(MemoryRegionInfo::eNo);
+ return region;
+}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
