[lldb] Minidump: check for .text hash match with directory

[JosephTremoulet]

Summary

When opening a minidump, we might discover that it reports a UUID for a
module that doesn't match the build ID, but rather a hash of the .text
section (according to either of two different hash functions, used by
breakpad and Facebook respectively). The current logic searches for a
module by filename only to check the hash; this change updates it to
first search by directory+filename. This is important when the
directory specified in the minidump must be interpreted relative to a
user-provided sysoort, as the leaf directory won't be in the search path
in that case.

Also add a regression test; without this change, module validation fails
because we have just the placeholder module which reports as its path
the platform path in the minidump.

[clayborg]

lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp

533

This whole "if" statement will need to be tried with both files. We might end up finding a file in the sysroot that is a valid file, but does not actually match the .text hash. Then the search by basename, which will search using the "target.exec-search-paths" or "target.debug-file-search-paths" settings might find another file that you might have downloaded, and it will need to go through all of the .text checks as well.

So as we now have more than 1 file to check, this "if (module_sp)" and its contents should be made into a function. Look at the code above I would create a method of ProcessMinidump that does all this:

ModuleSP ProcessMinidump::GetOrCreateModule(UUID minidump_uuid, ModuleSpec module_spec) {
  ModuleSP module_sp = GetTarget().GetOrCreateModule(module_spec, true /* notify */, &error);
  if (!module_sp)
    return module_sp;
  // We consider the module to be a match if the minidump UUID is a
  // prefix of the actual UUID, or if either of the UUIDs are empty.
  const auto dmp_bytes = minidump_uuid.GetBytes();
  const auto mod_bytes = module_sp->GetUUID().GetBytes();
  const bool match = dmp_bytes.empty() || mod_bytes.empty() ||
  mod_bytes.take_front(dmp_bytes.size()) == dmp_bytes;
  if (match) {
    LLDB_LOG(log, "Partial uuid match for {0}.", name);
    return module_sp;
  }

  // Breakpad generates minindump files, and if there is no GNU build
  // ID in the binary, it will calculate a UUID by hashing first 4096
  // bytes of the .text section and using that as the UUID for a module
  // in the minidump. Facebook uses a modified breakpad client that
  // uses a slightly modified this hash to avoid collisions. Check for
  // UUIDs from the minindump that match these cases and accept the
  // module we find if they do match.
  std::vector<uint8_t> breakpad_uuid;
  std::vector<uint8_t> facebook_uuid;
  HashElfTextSection(module_sp, breakpad_uuid, facebook_uuid);
  if (dmp_bytes == llvm::ArrayRef<uint8_t>(breakpad_uuid)) {
    LLDB_LOG(log, "Breakpad .text hash match for {0}.", name);
    return module_sp;
  }
  if (dmp_bytes == llvm::ArrayRef<uint8_t>(facebook_uuid)) {
    LLDB_LOG(log, "Facebook .text hash match for {0}.", name);
    return module_sp;
  }
  // The UUID wasn't a partial match and didn't match the .text hash
  // so remove the module from the target, we will need to create a
  // placeholder object file.
  GetTarget().GetImages().Remove(module_sp);
  module_sp.reset();
  return module_sp;
}

Then call this function once with basename_module_spec with a full path, and if no matching module is returned, call the remove the directory and call it again.

[JosephTremoulet]

• Handle finding wrong version before right one

[JosephTremoulet]

Thanks for the speedy review! I've updated it to include a testcase that shows the problem you described, and include the fix you suggested.

[clayborg]

Sorry for the delay, I was out of town until today. Changes look good! Always good to have more robust support for finding binaries in the sysroot.

lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp

if (!module_sp) {
       // 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 need to remove the module from the target
       // if the UUID doesn't match during our manual UUID verification. This
       // allows the "target.exec-search-paths" setting to specify one or more
       // directories that contain executables that can be searched for matches.
       ModuleSpec basename_module_spec(module_spec);
       basename_module_spec.GetUUID().Clear();
+      module_sp = GetTarget().GetOrCreateModule(basename_module_spec,
+                                                true /* notify */, &error);
+      if (!module_sp) {
         basename_module_spec.GetFileSpec().GetDirectory().Clear();
         module_sp = GetTarget().GetOrCreateModule(basename_module_spec,
                                                   true /* notify */, &error);
+      }
       if (module_sp) {
         // We consider the module to be a match if the minidump UUID is a
         // prefix of the actual UUID, or if either of the UUIDs are empty.
         const auto dmp_bytes = uuid.GetBytes();
         const auto mod_bytes = module_sp->GetUUID().GetBytes();
         const bool match = dmp_bytes.empty() || mod_bytes.empty() ||
             mod_bytes.take_front(dmp_bytes.size()) == dmp_bytes;
         if (!match) {
           // Breakpad generates minindump files, and if there is no GNU build

lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py

class MiniDumpUUIDTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
     NO_DEBUG_INFO_TESTCASE = True
 
     def verify_module(self, module, verify_path, verify_uuid):
         # Compare the filename and the directory separately. We are avoiding
         # SBFileSpec.fullpath because it causes a slash/backslash confusion
-        # on Windows.
+        # on Windows.  Similarly, we compare the directories using normcase
+        # because they may contain a Linux-style relative path from the
+        # minidump appended to a Windows-style root path from the host.
         self.assertEqual(
             os.path.basename(verify_path), module.GetFileSpec().basename)
         self.assertEqual(
-            os.path.dirname(verify_path), module.GetFileSpec().dirname or "")
+            os.path.normcase(os.path.dirname(verify_path)),
+            os.path.normcase(module.GetFileSpec().dirname or ""))
         self.assertEqual(verify_uuid, module.GetUUIDString())
 
     def get_minidump_modules(self, yaml_file):
         minidump_path = self.getBuildArtifact(os.path.basename(yaml_file) + ".dmp")
         self.yaml2obj(yaml_file, minidump_path)
         self.target = self.dbg.CreateTarget(None)
         self.process = self.target.LoadCore(minidump_path)
         return self.target.modules
def test_breakpad_hash_match(self):
         cmd = 'settings set target.exec-search-paths "%s"' % (os.path.dirname(so_path))
         self.dbg.HandleCommand(cmd)
         modules = self.get_minidump_modules("linux-arm-breakpad-uuid-match.yaml")
         self.assertEqual(1, len(modules))
         # LLDB makes up it own UUID as well when there is no build ID so we
         # will check that this matches.
         self.verify_module(modules[0], so_path, "D9C480E8")
 
+    def test_breakpad_hash_match_sysroot(self):
+        """
+            Check that we can match the breakpad .text section hash when the
+            module is located under a user-provided sysroot.
+        """
+        sysroot_path = os.path.join(self.getBuildDir(), "mock_sysroot")
+        # Create the directory under the sysroot where the minidump reports
+        # the module.
+        so_dir = os.path.join(sysroot_path, "invalid", "path", "on", "current", "system")
+        so_path = os.path.join(so_dir, "libbreakpad.so")
+        lldbutil.mkdir_p(so_dir)
+        self.yaml2obj("libbreakpad.yaml", so_path)
+        self.runCmd("platform select remote-linux --sysroot '%s'" % sysroot_path)
+        modules = self.get_minidump_modules("linux-arm-breakpad-uuid-match.yaml")
+        self.assertEqual(1, len(modules))
+        # LLDB makes up its own UUID as well when there is no build ID so we
+        # will check that this matches.
+        self.verify_module(modules[0], so_path, "D9C480E8")
+
     def test_breakpad_overflow_hash_match(self):
         """
             This is a similar to test_breakpad_hash_match, but it verifies that
             if the .text section does not end on a 16 byte boundary, then it
             will overflow into the next section's data by up to 15 bytes. This
             verifies that we are able to match what breakpad does as it will do
             this.
         """