[lld] [lld][ELF] Only convert dependdency filename to native form on Windows (PR #160927)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lld

Author: Ruoyu Zhong (ZhongRuoyu)

<details>
<summary>Changes</summary>

Currently, `llvm::sys::path::native` is used unconditionally when generating dependency filenames. This is correct on Windows, where backslashes are valid path separators, but can be incorrect on non-Windows platforms, because in that case backslashes in the filenames are converted to forward slashes, while they should be treated as ordinary characters.

This fixes the following inconsistency between `ld.lld` and `ld.bfd`:

```console
$ cat test.s
.globl _start
_start:
$ cc -c test.s -o "back\\slash.o"
$ ld.lld -o test "back\\slash.o" --dependency-file=/dev/stdout
test: \
 back/slash.o

back/slash.o:
$ ld.bfd -o test "back\\slash.o" --dependency-file=/dev/stdout
test: \
  back\slash.o

back\slash.o:
```

In addition, while we're here, use 2-space indentation when printing out dependency filenames (consistent with Clang and `ld.bfd`), and escape the target filename too.


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


3 Files Affected:

- (modified) lld/ELF/Driver.cpp (+8-3) 
- (added) lld/test/ELF/dependency-file-backslash-as-normal-char.s (+13) 
- (added) lld/test/ELF/dependency-file-backslash-as-path-separator.s (+13) 


``````````diff
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index 1beab8d33f4ba..8483e01364982 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -2482,10 +2482,14 @@ static void writeDependencyFile(Ctx &ctx) {
   // We use the same escape rules as Clang/GCC which are accepted by Make/Ninja:
   // * A space is escaped by a backslash which itself must be escaped.
   // * A hash sign is escaped by a single backslash.
-  // * $ is escapes as $$.
+  // * $ is escaped as $$.
   auto printFilename = [](raw_fd_ostream &os, StringRef filename) {
     llvm::SmallString<256> nativePath;
+#ifdef _WIN32
     llvm::sys::path::native(filename.str(), nativePath);
+#else
+    nativePath = filename;
+#endif
     llvm::sys::path::remove_dots(nativePath, /*remove_dot_dot=*/true);
     for (unsigned i = 0, e = nativePath.size(); i != e; ++i) {
       if (nativePath[i] == '#') {
@@ -2502,9 +2506,10 @@ static void writeDependencyFile(Ctx &ctx) {
     }
   };
 
-  os << ctx.arg.outputFile << ":";
+  printFilename(os, ctx.arg.outputFile);
+  os << ":";
   for (StringRef path : ctx.arg.dependencyFiles) {
-    os << " \\\n ";
+    os << " \\\n  ";
     printFilename(os, path);
   }
   os << "\n";
diff --git a/lld/test/ELF/dependency-file-backslash-as-normal-char.s b/lld/test/ELF/dependency-file-backslash-as-normal-char.s
new file mode 100644
index 0000000000000..278e06d41d683
--- /dev/null
+++ b/lld/test/ELF/dependency-file-backslash-as-normal-char.s
@@ -0,0 +1,13 @@
+# REQUIRES: x86, !system-windows
+# RUN: mkdir -p %t
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o "%t/back\\slash.o"
+# RUN: ld.lld -o %t/foo.exe "%t/back\\slash.o" --dependency-file=%t/foo.d
+# RUN: FileCheck --match-full-lines -DFILE=%t %s < %t/foo.d
+
+# CHECK:      [[FILE]]/foo.exe: \
+# CHECK-NEXT:   [[FILE]]/back\slash.o
+# CHECK-EMPTY:
+# CHECK-NEXT: [[FILE]]/back\slash.o:
+
+.global _start
+_start:
diff --git a/lld/test/ELF/dependency-file-backslash-as-path-separator.s b/lld/test/ELF/dependency-file-backslash-as-path-separator.s
new file mode 100644
index 0000000000000..6e738626c4127
--- /dev/null
+++ b/lld/test/ELF/dependency-file-backslash-as-path-separator.s
@@ -0,0 +1,13 @@
+# REQUIRES: x86, system-windows
+# RUN: mkdir -p %t/back
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o "%t/back\\slash.o"
+# RUN: ld.lld -o %t/foo.exe "%t/back\\slash.o" --dependency-file=%t/foo.d
+# RUN: FileCheck --match-full-lines -DFILE=%t %s < %t/foo.d
+
+# CHECK:      [[FILE]]\foo.exe: \
+# CHECK-NEXT:   [[FILE]]\back\slash.o
+# CHECK-EMPTY:
+# CHECK-NEXT: [[FILE]]\back\slash.o:
+
+.global _start
+_start:

``````````

</details>


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