[Lldb-commits] [PATCH] D89155: [lldb] Minidump: check for .text hash match with directory

[Joseph Tremoulet via Phabricator via lldb-commits]

JosephTremoulet created this revision.
Herald added a reviewer: JDevlieghere.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
JosephTremoulet requested review of this revision.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89155

Files:
  lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
  lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py


Index: lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
===================================================================
--- lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
+++ lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
@@ -22,11 +22,14 @@
     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):
@@ -201,6 +204,25 @@
         # 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
Index: lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
===================================================================
--- lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -523,9 +523,13 @@
       // 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) {
+        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.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D89155.297300.patch
Type: text/x-patch
Size: 3621 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20201009/c203e5ea/attachment.bin>