This revision was automatically updated to reflect the committed changes.
Closed by commit rL349062: Fix MinidumpParser::GetFilteredModuleList() and test
it (authored by gclayton, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D55614?vs=177902&id=178086#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55614/new/
https://reviews.llvm.org/D55614
Files:
lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
lldb/trunk/unittests/Process/minidump/Inputs/modules-dup-min-addr.dmp
lldb/trunk/unittests/Process/minidump/Inputs/modules-order.dmp
lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp
Index: lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp
===================================================================
--- lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp
+++ lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp
@@ -533,3 +533,41 @@
}
}
}
+
+TEST_F(MinidumpParserTest, MinidumpDuplicateModuleMinAddress) {
+ SetUpData("modules-dup-min-addr.dmp");
+ // Test that if we have two modules in the module list:
+ // /tmp/a with range [0x2000-0x3000)
+ // /tmp/a with range [0x1000-0x2000)
+ // 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<const MinidumpModule *> filtered_modules =
+ parser->GetFilteredModuleList();
+ EXPECT_EQ(1, filtered_modules.size());
+ EXPECT_EQ(0x0000000000001000, filtered_modules[0]->base_of_image);
+}
+
+TEST_F(MinidumpParserTest, MinidumpModuleOrder) {
+ SetUpData("modules-order.dmp");
+ // Test that if we have two modules in the module list:
+ // /tmp/a with range [0x2000-0x3000)
+ // /tmp/b with range [0x1000-0x2000)
+ // That we end up with two modules in the filtered list with the same ranges
+ // 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<const MinidumpModule *> filtered_modules =
+ parser->GetFilteredModuleList();
+ llvm::Optional<std::string> name;
+ EXPECT_EQ(2, filtered_modules.size());
+ EXPECT_EQ(0x0000000000002000, 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_EQ(0x0000000000001000, 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);
+}
Index: lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
===================================================================
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
+++ lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -274,36 +274,45 @@
std::vector<const MinidumpModule *> MinidumpParser::GetFilteredModuleList() {
llvm::ArrayRef<MinidumpModule> modules = GetModuleList();
- // map module_name -> pair(load_address, pointer to module struct in memory)
- llvm::StringMap<std::pair<uint64_t, const MinidumpModule *>> lowest_addr;
+ // map module_name -> filtered_modules index
+ typedef llvm::StringMap<size_t> MapType;
+ MapType module_name_to_filtered_index;
std::vector<const MinidumpModule *> filtered_modules;
-
+
llvm::Optional<std::string> name;
std::string module_name;
for (const auto &module : modules) {
name = GetMinidumpString(module.module_name_rva);
-
+
if (!name)
continue;
-
+
module_name = name.getValue();
-
- auto iter = lowest_addr.end();
- bool exists;
- std::tie(iter, exists) = lowest_addr.try_emplace(
- module_name, std::make_pair(module.base_of_image, &module));
-
- if (exists && module.base_of_image < iter->second.first)
- iter->second = std::make_pair(module.base_of_image, &module);
- }
-
- filtered_modules.reserve(lowest_addr.size());
- for (const auto &module : lowest_addr) {
- filtered_modules.push_back(module.second.second);
+
+ 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());
+
+ if (inserted) {
+ // This module has not been seen yet, insert it into filtered_modules at
+ // the index that was inserted into module_name_to_filtered_index using
+ // "filtered_modules.size()" above.
+ filtered_modules.push_back(&module);
+ } else {
+ // 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.base_of_image < dup_module->base_of_image)
+ filtered_modules[iter->second] = &module;
+ }
}
-
return filtered_modules;
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
