4 files changed, 66 insertions, 19 deletions

diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
index 6b94d4fede3..adaa01c76d3 100644
--- a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
+++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -274,36 +274,45 @@ llvm::ArrayRef<MinidumpModule> MinidumpParser::GetModuleList() {
 
 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;
 }
 
diff --git a/lldb/unittests/Process/minidump/Inputs/modules-dup-min-addr.dmp b/lldb/unittests/Process/minidump/Inputs/modules-dup-min-addr.dmp
new file mode 100644
index 00000000000..a7a1c4afe85
--- /dev/null
+++ b/lldb/unittests/Process/minidump/Inputs/modules-dup-min-addr.dmp
diff --git a/lldb/unittests/Process/minidump/Inputs/modules-order.dmp b/lldb/unittests/Process/minidump/Inputs/modules-order.dmp
new file mode 100644
index 00000000000..cab926b2129
--- /dev/null
+++ b/lldb/unittests/Process/minidump/Inputs/modules-order.dmp
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);
+}