[Lldb-commits] [lldb] [lldb] Fallback to comparing modules by their canonical paths (PR #210041)

[via lldb-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Charles Zablit (charles-zablit)

<details>
<summary>Changes</summary>

`Module::MatchesModuleSpec` compares 2 modules paths using their `FileSpec`. If one of the modules is a symlink (or a subst drive on Windows), that comparison will fail even if the modules are the same. This causes breakpoints to be resolved twice.

This patch adds the `FileSpecsResolveToSameFile` method as a fallback to the `Module::MatchesModuleSpec` comparison. If the comparison fails, try to compare the module using their real path (canonicalized paths). It also adds a regression test.

---
Full diff: https://github.com/llvm/llvm-project/pull/210041.diff


4 Files Affected:

- (modified) lldb/source/Core/Module.cpp (+23-1) 
- (added) lldb/test/API/functionalities/breakpoint/breakpoint_via_symlink/Makefile (+3) 
- (added) lldb/test/API/functionalities/breakpoint/breakpoint_via_symlink/TestBreakpointViaSymlink.py (+70) 
- (added) lldb/test/API/functionalities/breakpoint/breakpoint_via_symlink/main.c (+3) 


``````````diff
diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index dcdfd67d3e303..1493c0291c572 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -54,6 +54,7 @@
 #endif
 
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/DJB.h"
 #include "llvm/Support/FileSystem.h"
@@ -1455,6 +1456,25 @@ bool Module::SetLoadAddress(Target &target, lldb::addr_t value,
   return false;
 }
 
+// A module's stored path can be an alias of the same on-disk file that a lookup
+// spec refers to:
+//   - Windows: a subst/mapped drive or a symbolic link
+//   - POSIX: a symbolic link
+// In those cases, MatchesModuleSpec fails even though both name the same file.
+// This is a fall back which compares the canonicalized real paths.
+static bool FileSpecsResolveToSameFile(const FileSpec &pattern,
+                                       const FileSpec &file) {
+  if (!pattern.GetDirectory() || !file)
+    return false;
+  FileSystem &fs = FileSystem::Instance();
+  llvm::SmallString<256> pattern_real, file_real;
+  if (fs.GetRealPath(pattern.GetPath(), pattern_real) ||
+      fs.GetRealPath(file.GetPath(), file_real))
+    return false;
+  return FileSpec::Equal(FileSpec(pattern_real), FileSpec(file_real),
+                         /*full=*/true);
+}
+
 bool Module::MatchesModuleSpec(const ModuleSpec &module_ref) {
   const UUID &uuid = module_ref.GetUUID();
 
@@ -1465,7 +1485,9 @@ bool Module::MatchesModuleSpec(const ModuleSpec &module_ref) {
 
   const FileSpec &file_spec = module_ref.GetFileSpec();
   if (!FileSpec::Match(file_spec, m_file) &&
-      !FileSpec::Match(file_spec, m_platform_file))
+      !FileSpec::Match(file_spec, m_platform_file) &&
+      !FileSpecsResolveToSameFile(file_spec, m_file) &&
+      !FileSpecsResolveToSameFile(file_spec, m_platform_file))
     return false;
 
   const FileSpec &platform_file_spec = module_ref.GetPlatformFileSpec();
diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_via_symlink/Makefile b/lldb/test/API/functionalities/breakpoint/breakpoint_via_symlink/Makefile
new file mode 100644
index 0000000000000..10495940055b6
--- /dev/null
+++ b/lldb/test/API/functionalities/breakpoint/breakpoint_via_symlink/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_via_symlink/TestBreakpointViaSymlink.py b/lldb/test/API/functionalities/breakpoint/breakpoint_via_symlink/TestBreakpointViaSymlink.py
new file mode 100644
index 0000000000000..defb23cc16f37
--- /dev/null
+++ b/lldb/test/API/functionalities/breakpoint/breakpoint_via_symlink/TestBreakpointViaSymlink.py
@@ -0,0 +1,70 @@
+"""
+Test that creating a target through a symlinked (or otherwise aliased)
+executable does not create a duplicate module at launch.
+
+When a target is created via a path that is not the file's canonical path (a
+symlink, or on Windows a subst/mapped drive), the running process reports its
+*real* image path at launch. Module matching needs to compare the canonicalized
+paths.
+"""
+
+import os
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestBreakpointViaSymlink(TestBase):
+    NO_DEBUG_INFO_TESTCASE = True
+
+    @skipIfRemote
+    def test_breakpoint_resolves_once_via_symlink(self):
+        self.build()
+        real_exe = self.getBuildArtifact("a.out")
+
+        # A symlink whose real path differs from the path used to create the
+        # target.
+        link_exe = self.getBuildArtifact("a.out.symlink")
+        try:
+            if os.path.lexists(link_exe):
+                os.remove(link_exe)
+            os.symlink(real_exe, link_exe)
+        except (OSError, NotImplementedError) as e:
+            # Windows needs Developer Mode / SeCreateSymbolicLinkPrivilege.
+            self.skipTest("could not create a symlink: %s" % e)
+        self.addTearDownHook(lambda: os.remove(link_exe))
+        self.assertNotEqual(os.path.realpath(link_exe), link_exe)
+
+        target = self.dbg.CreateTarget(link_exe)
+        self.assertTrue(target, VALID_TARGET)
+
+        bkpt = target.BreakpointCreateBySourceRegex(
+            "break here", lldb.SBFileSpec("main.c")
+        )
+        # Resolves against the preloaded module: exactly one location.
+        self.assertEqual(
+            bkpt.GetNumLocations(), 1, "one breakpoint location before launch"
+        )
+
+        process = target.LaunchSimple(None, None, self.get_process_working_directory())
+        self.assertState(process.GetState(), lldb.eStateStopped)
+
+        # After launch the running image may report its real path. With module
+        # matching canonicalizing paths, the loaded module is the one lldb already
+        # preloaded, so the breakpoint keeps exactly one resolved location.
+        self.assertEqual(bkpt.GetNumLocations(), 1)
+        self.assertEqual(bkpt.GetNumResolvedLocations(), 1)
+
+        # The executable appears as a single module (no duplicate). The module
+        # keeps the path it was created with (the symlink), so compare by real
+        # path.
+        exe_real = os.path.realpath(real_exe)
+        matches = [
+            m
+            for m in target.module_iter()
+            if os.path.realpath(m.GetFileSpec().fullpath) == exe_real
+        ]
+        self.assertEqual(
+            len(matches), 1, "exactly one executable module, got %d" % len(matches)
+        )
diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_via_symlink/main.c b/lldb/test/API/functionalities/breakpoint/breakpoint_via_symlink/main.c
new file mode 100644
index 0000000000000..9ba28de2ae01c
--- /dev/null
+++ b/lldb/test/API/functionalities/breakpoint/breakpoint_via_symlink/main.c
@@ -0,0 +1,3 @@
+int main(int argc, char **argv) {
+  return 0; // break here
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/210041