[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

562–563

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.h

 protected:
   void Clear();
 
   bool UpdateThreadList(ThreadList &old_thread_list,
                         ThreadList &new_thread_list) override;
 
   void ReadModuleList();
 
+  lldb::ModuleSP GetOrCreateModule(lldb_private::UUID minidump_uuid,
+                                   llvm::StringRef name,
+                                   lldb_private::ModuleSpec module_spec);
+
   JITLoaderList &GetJITLoaders() override;
 
 private:
   FileSpec m_core_file;
   lldb::DataBufferSP m_core_data;
   llvm::ArrayRef<minidump::Thread> m_thread_list;
   const minidump::ExceptionStream *m_active_exception;
   lldb::CommandObjectSP m_command_sp;

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

else
       context = m_minidump_parser->GetThreadContextWow64(thread);
 
     lldb::ThreadSP thread_sp(new ThreadMinidump(*this, thread, context));
     new_thread_list.AddThread(thread_sp);
   }
   return new_thread_list.GetSize(false) > 0;
 }
 
+ModuleSP ProcessMinidump::GetOrCreateModule(UUID minidump_uuid,
+                                            llvm::StringRef name,
+                                            ModuleSpec module_spec) {
+  Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
+  Status error;
+
+  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;
+}
+
 void ProcessMinidump::ReadModuleList() {
   std::vector<const minidump::Module *> filtered_modules =
       m_minidump_parser->GetFilteredModuleList();
 
   Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
 
   for (auto module : filtered_modules) {
     std::string name = cantFail(m_minidump_parser->GetMinidumpFile().getString(
for (auto module : filtered_modules) {
     const auto uuid = m_minidump_parser->GetModuleUUID(module);
     auto file_spec = FileSpec(name, GetArchitecture().GetTriple());
     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.
     lldb::ModuleSP module_sp = GetTarget().GetOrCreateModule(module_spec,
                                                      true /* notify */, &error);
-    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 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();
-      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
-          // 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);
-          } else if (dmp_bytes == llvm::ArrayRef<uint8_t>(facebook_uuid)) {
-            LLDB_LOG(log, "Facebook .text hash match for {0}.", name);
-          } else {
-            // 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();
-          }
-        } else {
-          LLDB_LOG(log, "Partial uuid match for {0}.", name);
-        }
+      LLDB_LOG(log, "Full uuid match for {0}.", name);
+      // We couldn't find a module with an exactly-matching UUID.  Sometimes
+      // a minidump UUID is only a partial match or is a hash.  So try again
+      // without specifying the UUID, then again without specifying the
+      // directory if that fails.  This will allow us to find modules with
+      // partial matches or hash UUIDs in user-provided sysroots or search
+      // directories (target.exec-search-paths).
+      ModuleSpec partial_module_spec = module_spec;
+      partial_module_spec.GetUUID().Clear();
+      module_sp = GetOrCreateModule(uuid, name, partial_module_spec);
+      if (!module_sp) {
+        partial_module_spec.GetFileSpec().GetDirectory().Clear();
+        module_sp = GetOrCreateModule(uuid, name, partial_module_spec);
       }
-    } else {
-      LLDB_LOG(log, "Full uuid match for {0}.", name);
     }
     if (module_sp) {
       // Watch out for place holder modules that have different paths, but the
       // same UUID. If the base address is different, create a new module. If
       // we don't then we will end up setting the load address of a different
       // PlaceholderObjectFile and an assertion will fire.
       auto *objfile = module_sp->GetObjectFile();
       if (objfile && objfile->GetPluginName() ==

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_hash_match_sysroot_decoy(self):
+        """
+            Check that we can match the breakpad .text section hash when there is
+            a module with the right name but wrong contents under a user-provided
+            sysroot, and the right module is at the given search path..
+        """
+        sysroot_path = os.path.join(self.getBuildDir(), "mock_sysroot")
+        # Create the directory under the sysroot where the minidump reports
+        # the module.
+        decoy_dir = os.path.join(sysroot_path, "invalid", "path", "on", "current", "system")
+        decoy_path = os.path.join(decoy_dir, "libbreakpad.so")
+        lldbutil.mkdir_p(decoy_dir)
+        self.yaml2obj("libbreakpad-decoy.yaml", decoy_path)
+        self.runCmd("platform select remote-linux --sysroot '%s'" % sysroot_path)
+        so_dir = os.path.join(self.getBuildDir(), "searchpath_dir")
+        so_path = os.path.join(so_dir, "libbreakpad.so")
+        lldbutil.mkdir_p(so_dir)
+        self.yaml2obj("libbreakpad.yaml", so_path)
+        self.runCmd('settings set target.exec-search-paths "%s"' % so_dir)
+        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.
         """

lldb/test/API/functionalities/postmortem/minidump-new/libbreakpad-decoy.yaml

+# This has different .text contents than libbreakpad-yaml,
+# to simulate having different versions of the module (to
+# test that we pick the one matching the minidump UUID).
+--- !ELF
+FileHeader:
+  Class:           ELFCLASS32
+  Data:            ELFDATA2LSB
+  Type:            ET_DYN
+  Machine:         EM_ARM
+  Flags:           [ EF_ARM_SOFT_FLOAT, EF_ARM_EABI_VER5 ]
+Sections:
+Sections:
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:         0x0000000000010000
+    AddressAlign:    0x0000000000000004
+    Content:         040000001400000003000000474E5500CC