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 the various methods of locating the module in GetRemoteSharedModule
fail, make sure we pass the original module spec to the bail-out call to
the provided resolver function.

Also make sure we consistently use the resolved module spec from the
various success paths.

Thanks to what appears to have been an accidentally inverted condition
(commit 85967fa applied the new condition to a path where GetModuleSpec
returns false, but should have applied it when GetModuleSpec returns
true), without this fix we only pass the original module spec in the
fallback if the original spec has no uuid (or has a uuid that somehow
matches the resolved module's uuid despite the call to GetModuleSpec
failing).  This manifested as a bug when processing a minidump file with
a user-provided sysroot, since in that case the resolver call was being
applied to resolved_module_spec (despite resolution failing), which did
not have the path of its file_spec set.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88099

Files:
  lldb/source/Target/Platform.cpp
  lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py


Index: lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
===================================================================
--- lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
+++ lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
@@ -455,3 +455,29 @@
         check_region(17, 0x40169000, 0x4016b000, True,  True,  False, True,  d)
         check_region(18, 0x4016b000, 0x40176000, True,  True,  False, True,  n)
         check_region(-1, 0x40176000, max_int,    False, False, False, False, n)
+
+    @skipIfLLVMTargetMissing("X86")
+    def test_minidump_sysroot(self):
+        """Test that lldb can find a module referenced in an i386 linux 
minidump using the sysroot."""
+
+        # Copy linux-x86_64 executable to tmp_sysroot/temp/test/ (since it was 
compiled as
+        # /tmp/test/linux-x86_64)
+        tmp_sysroot = os.path.join(
+            self.getBuildDir(), "lldb_i386_mock_sysroot")
+        executable = os.path.join(
+            tmp_sysroot, "tmp", "test", "linux-x86_64")
+        exe_dir = os.path.dirname(executable)
+        lldbutil.mkdir_p(exe_dir)
+        shutil.copyfile("linux-x86_64", executable)
+
+        # Set sysroot and load core
+        self.runCmd("platform select remote-linux --sysroot '%s'" %
+                    tmp_sysroot)
+        self.process_from_yaml("linux-x86_64.yaml")
+        self.check_state()
+
+        # Check that we loaded the module from the sysroot
+        self.assertEqual(self.target.GetNumModules(), 1)
+        module = self.target.GetModuleAtIndex(0)
+        spec = module.GetFileSpec()
+        self.assertEqual(spec.GetDirectory(), exe_dir)
Index: lldb/source/Target/Platform.cpp
===================================================================
--- lldb/source/Target/Platform.cpp
+++ lldb/source/Target/Platform.cpp
@@ -1580,21 +1580,29 @@
       if (error.Success() && module_sp)
         break;
     }
-    if (module_sp)
+    if (module_sp) {
+      resolved_module_spec = arch_module_spec;
       got_module_spec = true;
+    }
   }
 
   if (!got_module_spec) {
     // Get module information from a target.
-    if (!GetModuleSpec(module_spec.GetFileSpec(), 
module_spec.GetArchitecture(),
-                       resolved_module_spec)) {
+    if (GetModuleSpec(module_spec.GetFileSpec(), module_spec.GetArchitecture(),
+                      resolved_module_spec)) {
       if (!module_spec.GetUUID().IsValid() ||
           module_spec.GetUUID() == resolved_module_spec.GetUUID()) {
-        return module_resolver(module_spec);
+        got_module_spec = true;
       }
     }
   }
 
+  if (!got_module_spec) {
+    // Fall back to the given module resolver, which may have its own
+    // search logic.
+    return module_resolver(module_spec);
+  }
+
   // If we are looking for a specific UUID, make sure resolved_module_spec has
   // the same one before we search.
   if (module_spec.GetUUID().IsValid()) {


Index: lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
===================================================================
--- lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
+++ lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
@@ -455,3 +455,29 @@
         check_region(17, 0x40169000, 0x4016b000, True,  True,  False, True,  d)
         check_region(18, 0x4016b000, 0x40176000, True,  True,  False, True,  n)
         check_region(-1, 0x40176000, max_int,    False, False, False, False, n)
+
+    @skipIfLLVMTargetMissing("X86")
+    def test_minidump_sysroot(self):
+        """Test that lldb can find a module referenced in an i386 linux minidump using the sysroot."""
+
+        # Copy linux-x86_64 executable to tmp_sysroot/temp/test/ (since it was compiled as
+        # /tmp/test/linux-x86_64)
+        tmp_sysroot = os.path.join(
+            self.getBuildDir(), "lldb_i386_mock_sysroot")
+        executable = os.path.join(
+            tmp_sysroot, "tmp", "test", "linux-x86_64")
+        exe_dir = os.path.dirname(executable)
+        lldbutil.mkdir_p(exe_dir)
+        shutil.copyfile("linux-x86_64", executable)
+
+        # Set sysroot and load core
+        self.runCmd("platform select remote-linux --sysroot '%s'" %
+                    tmp_sysroot)
+        self.process_from_yaml("linux-x86_64.yaml")
+        self.check_state()
+
+        # Check that we loaded the module from the sysroot
+        self.assertEqual(self.target.GetNumModules(), 1)
+        module = self.target.GetModuleAtIndex(0)
+        spec = module.GetFileSpec()
+        self.assertEqual(spec.GetDirectory(), exe_dir)
Index: lldb/source/Target/Platform.cpp
===================================================================
--- lldb/source/Target/Platform.cpp
+++ lldb/source/Target/Platform.cpp
@@ -1580,21 +1580,29 @@
       if (error.Success() && module_sp)
         break;
     }
-    if (module_sp)
+    if (module_sp) {
+      resolved_module_spec = arch_module_spec;
       got_module_spec = true;
+    }
   }
 
   if (!got_module_spec) {
     // Get module information from a target.
-    if (!GetModuleSpec(module_spec.GetFileSpec(), module_spec.GetArchitecture(),
-                       resolved_module_spec)) {
+    if (GetModuleSpec(module_spec.GetFileSpec(), module_spec.GetArchitecture(),
+                      resolved_module_spec)) {
       if (!module_spec.GetUUID().IsValid() ||
           module_spec.GetUUID() == resolved_module_spec.GetUUID()) {
-        return module_resolver(module_spec);
+        got_module_spec = true;
       }
     }
   }
 
+  if (!got_module_spec) {
+    // Fall back to the given module resolver, which may have its own
+    // search logic.
+    return module_resolver(module_spec);
+  }
+
   // If we are looking for a specific UUID, make sure resolved_module_spec has
   // the same one before we search.
   if (module_spec.GetUUID().IsValid()) {
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
  • [Lldb-commits] [PATCH] ... Joseph Tremoulet via Phabricator via lldb-commits

Reply via email to