Fix MinidumpParser::GetFilteredModuleList() and test it

The MinidumpParser::GetFilteredModuleList() code was attempting to iterate through the entire module list and if it found more than one entry for a given module name, it wanted to pick the MinidumpModule with the lowest address. A bug existed where it wasn't doing that due to "exists" variable being inverted. "exists" was set to true if it was inserted, not if it existed. Furthermore, the order of the modules would be modified by sorting all modules from low address to high address (using MinidumpModule::base_of_image). This fix also maintains the original order which means your executable is at index 0 as intended instead of some random shared library.

Tests were added to ensure this functionality doesn't regress.

Differential Revision: https://reviews.llvm.org/D55614

llvm-svn: 349062

1 files changed, 38 insertions, 0 deletions

diff --git a/lldb/unittests/Process/minidump/MinidumpParserTest.cpp b/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
index 57f27d4f6ba..7508cf23654 100644
--- a/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
+++ b/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
@@ -533,3 +533,41 @@ TEST_F(MinidumpParserTest, ConvertMinidumpContext_x86_32_wow64) {
     }
   }
 }
+
+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);
+}