clayborg created this revision.
clayborg added reviewers: labath, zturner.
Herald added a subscriber: javed.absar.

Breakpad had bugs in earlier versions where it would take a 20 byte ELF build 
ID and put it into the minidump file as a 16 byte PDB70 UUID with an age of 
zero. This would make it impossible to do postmortem debugging with one of 
these older minidump files.

This fix allows partial matching of UUIDs. To do this we first try and match 
with the full UUID value, and then fall back to removing the original directory 
path from the module specification and we remove the UUID requirement, and then 
manually do the matching ourselves. This allows scripts to find symbols files 
using a symbol server, place them all in a directory, use the "setting set 
target.exec-search-paths" setting to specify the directory, and then load the 
core file. The Target::GetSharedModule() can then find the correct file without 
doing any other matching and load it.

Tests were added to cover a partial UUID match where the breakpad file has a 16 
byte UUID and the actual file on disk has a 20 byte UUID, both where the first 
16 bytes match, and don't match.


https://reviews.llvm.org/D60001

Files:
  
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
  
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/libuuidmatch.so
  
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/libuuidmismatch.so
  
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-partial-uuids-match.dmp
  
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-partial-uuids-mismatch.dmp
  source/Plugins/Process/minidump/ProcessMinidump.cpp

Index: source/Plugins/Process/minidump/ProcessMinidump.cpp
===================================================================
--- source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -351,6 +351,8 @@
   std::vector<const MinidumpModule *> filtered_modules =
       m_minidump_parser->GetFilteredModuleList();
 
+  Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_MODULES));
+
   for (auto module : filtered_modules) {
     llvm::Optional<std::string> name =
         m_minidump_parser->GetMinidumpString(module->module_name_rva);
@@ -358,7 +360,6 @@
     if (!name)
       continue;
 
-    Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_MODULES));
     if (log) {
       log->Printf("ProcessMinidump::%s found module: name: %s %#010" PRIx64
                   "-%#010" PRIx64 " size: %" PRIu32,
@@ -374,14 +375,67 @@
       m_is_wow64 = true;
     }
 
+    if (log) {
+      log->Printf("ProcessMinidump::%s load module: name: %s", __FUNCTION__,
+                  name.getValue().c_str());
+    }
+
     const auto uuid = m_minidump_parser->GetModuleUUID(module);
     auto file_spec = FileSpec(name.getValue(), GetArchitecture().GetTriple());
     FileSystem::Instance().Resolve(file_spec);
     ModuleSpec module_spec(file_spec, uuid);
     module_spec.GetArchitecture() = GetArchitecture();
     Status error;
+    // Try and find a module with a full UUID that matches. This function will
+    // add the module to the target if it finds one, so we might need to remove
+    // it if there is an error.
     lldb::ModuleSP module_sp = GetTarget().GetSharedModule(module_spec, &error);
-    if (!module_sp || error.Fail()) {
+    if (error.Fail()) {
+      GetTarget().GetImages().Remove(module_sp);
+      module_sp.reset();
+    }
+    if (!module_sp) {
+      // Try and find a module without specifying the UUID and only looking for
+      // the file given a basename. We then will look for a partial UUID match
+      // if we find any matches. This function will add the module to the
+      // target if it finds one, so we might need to remove it if there is an
+      // error.
+      ModuleSpec basename_module_spec(module_spec);
+      basename_module_spec.GetUUID().Clear();
+      basename_module_spec.GetFileSpec().GetDirectory().Clear();
+      module_sp = GetTarget().GetSharedModule(basename_module_spec, &error);
+      if (error.Fail()) {
+        // Remove the module from the target if one got added but there
+        // was an error.
+        GetTarget().GetImages().Remove(module_sp);
+        module_sp.reset();
+      }
+      if (module_sp) {
+        // Verify that the first bytes of the UUID matches.
+        const auto dmp_bytes = uuid.GetBytes();
+        if (!dmp_bytes.empty()) {
+          const auto mod_bytes = module_sp->GetUUID().GetBytes();
+          // We have a valid UUID in the minidump, now check if the module
+          // has a UUID. If the module doesn't have a UUID then we can call
+          // this a mtach
+          if (!mod_bytes.empty()) {
+            // We have a valid UUID in the module and in the minidump.
+            
+            // If the minidump UUID is larger than the module uuid that we
+            // found or the UUID doesn't start with the UUID from the minidump
+            // then this file isn't a match and we need to remove it from the
+            // target since GetTarget().GetSharedModule() will add the module to
+            // the target.
+            if ((dmp_bytes.size() > mod_bytes.size()) ||
+                memcmp(dmp_bytes.data(), mod_bytes.data(), dmp_bytes.size()) != 0) {
+              GetTarget().GetImages().Remove(module_sp);
+              module_sp.reset();
+            }
+          }
+        }
+      }
+    }
+    if (!module_sp) {
       // We failed to locate a matching local object file. Fortunately, the
       // minidump format encodes enough information about each module's memory
       // range to allow us to create placeholder modules.
@@ -400,11 +454,6 @@
       GetTarget().GetImages().Append(module_sp);
     }
 
-    if (log) {
-      log->Printf("ProcessMinidump::%s load module: name: %s", __FUNCTION__,
-                  name.getValue().c_str());
-    }
-
     bool load_addr_changed = false;
     module_sp->SetLoadAddress(GetTarget(), module->base_of_image, false,
                               load_addr_changed);
Index: packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
===================================================================
--- packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
+++ packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
@@ -132,3 +132,46 @@
         self.assertEqual(2, len(modules))
         self.verify_module(modules[0], "/not/exist/a", None)
         self.verify_module(modules[1], "/not/exist/b", None)
+
+    def test_partial_uuid_match(self):
+        """
+            Breakpad has been known to create minidump files using CvRecord in each
+            module whose signature is set to PDB70 where the UUID only contains the
+            first 16 bytes of a 20 byte ELF build ID. Code was added to 
+            ProcessMinidump.cpp to deal with this and allows partial UUID matching. 
+
+            This test verifies that if we have a minidump with a 16 byte UUID, that
+            we are able to associate a symbol file with a 20 byte UUID only if the
+            first 16 bytes match. In this case we will see the path from the file
+            we found in the test directory and the 20 byte UUID from the actual
+            file, not the 16 byte shortened UUID from the minidump.
+        """
+        self.dbg.CreateTarget(None)
+        self.target = self.dbg.GetSelectedTarget()
+        self.process = self.target.LoadCore("linux-arm-partial-uuids-match.dmp")
+        modules = self.target.modules
+        self.assertEqual(1, len(modules))
+        self.verify_module(modules[0],
+                           "libuuidmatch.so", 
+                           "7295E17C-6668-9E05-CBB5-DEE5003865D5-5267C116")
+
+    def test_partial_uuid_mismatch(self):
+        """
+            Breakpad has been known to create minidump files using CvRecord in each
+            module whose signature is set to PDB70 where the UUID only contains the
+            first 16 bytes of a 20 byte ELF build ID. Code was added to 
+            ProcessMinidump.cpp to deal with this and allows partial UUID matching. 
+            
+            This test verifies that if we have a minidump with a 16 byte UUID, that
+            we are not able to associate a symbol file with a 20 byte UUID only if
+            any of the first 16 bytes do not match. In this case we will see the UUID
+            from the minidump file and the path from the minidump file.
+        """
+        self.dbg.CreateTarget(None)
+        self.target = self.dbg.GetSelectedTarget()
+        self.process = self.target.LoadCore("linux-arm-partial-uuids-mismatch.dmp")
+        modules = self.target.modules
+        self.assertEqual(1, len(modules))
+        self.verify_module(modules[0],
+                           "/invalid/path/on/current/system/libuuidmismatch.so", 
+                           "7295E17C-6668-9E05-CBB5-DEE5003865D5")
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to