[llvm] [TableGen] correctly escape dependency filenames (PR #160834)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-tablegen

Author: Ruoyu Zhong (ZhongRuoyu)

<details>
<summary>Changes</summary>

Currently, `*-tblgen` do not escape special characters in dependency filenames. This can lead to unnecessary rebuilds when the filenames contain characters that require escaping, as the build system may not treat them correctly.

For instance, when building LLVM in a directory that contains spaces (`/home/user/repos/llvm project` in the example below), the build system always rebuilds a large portion of the project despite nothing having changed, due to an unescaped space in the dependency filename:

```console
$ ninja -C build -d explain
ninja: Entering directory `build'
ninja explain: /home/user/repos/llvm is dirty
...
$ cat build/include/llvm/IR/IntrinsicsRISCV.h.d
IntrinsicsRISCV.h: /home/user/repos/llvm project/llvm/include/llvm/CodeGen/SDNodeProperties.td ...
```

Fix this by escaping special characters in dependency filenames using backslashes. This is consistent with how GCC, Clang [^1] and lld [^2] handle this.

After this change (notice the escaped space):

```console
$ cat build/include/llvm/IR/IntrinsicsRISCV.h.d
IntrinsicsRISCV.h: /home/user/repos/llvm\ project/llvm/include/llvm/CodeGen/SDNodeProperties.td ...
```

[^1]: https://github.com/llvm/llvm-project/blob/2cacf7117ba0fb7c134413a1a51302f8d6649052/clang/lib/Frontend/DependencyFile.cpp#L267-L344
[^2]: https://github.com/llvm/llvm-project/blob/2cacf7117ba0fb7c134413a1a51302f8d6649052/lld/ELF/Driver.cpp#L2482-L2503

N.B. `git clang-format` wanted me to format some other parts of the source so there is a second commit a8d9a90d36bc896a949c9a9ae6de1c57a2f6baa6 that does that.


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


1 Files Affected:

- (modified) llvm/lib/TableGen/Main.cpp (+42-13) 


``````````diff
diff --git a/llvm/lib/TableGen/Main.cpp b/llvm/lib/TableGen/Main.cpp
index f545706d6fe30..76b161d087f60 100644
--- a/llvm/lib/TableGen/Main.cpp
+++ b/llvm/lib/TableGen/Main.cpp
@@ -17,12 +17,14 @@
 #include "llvm/TableGen/Main.h"
 #include "TGLexer.h"
 #include "TGParser.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/SMLoc.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/ToolOutputFile.h"
@@ -37,26 +39,24 @@
 #include <utility>
 using namespace llvm;
 
-static cl::opt<std::string>
-OutputFilename("o", cl::desc("Output filename"), cl::value_desc("filename"),
-               cl::init("-"));
+static cl::opt<std::string> OutputFilename("o", cl::desc("Output filename"),
+                                           cl::value_desc("filename"),
+                                           cl::init("-"));
 
-static cl::opt<std::string>
-DependFilename("d",
-               cl::desc("Dependency filename"),
-               cl::value_desc("filename"),
-               cl::init(""));
+static cl::opt<std::string> DependFilename("d", cl::desc("Dependency filename"),
+                                           cl::value_desc("filename"),
+                                           cl::init(""));
 
 static cl::opt<std::string>
-InputFilename(cl::Positional, cl::desc("<input file>"), cl::init("-"));
+    InputFilename(cl::Positional, cl::desc("<input file>"), cl::init("-"));
 
 static cl::list<std::string>
 IncludeDirs("I", cl::desc("Directory of include files"),
             cl::value_desc("directory"), cl::Prefix);
 
 static cl::list<std::string>
-MacroNames("D", cl::desc("Name of the macro to be defined"),
-            cl::value_desc("macro name"), cl::Prefix);
+    MacroNames("D", cl::desc("Name of the macro to be defined"),
+               cl::value_desc("macro name"), cl::Prefix);
 
 static cl::opt<bool>
 WriteIfChanged("write-if-changed", cl::desc("Only write output if it changed"));
@@ -83,6 +83,35 @@ static int reportError(const char *ProgName, Twine Msg) {
   return 1;
 }
 
+/// Escape a filename in the dependency file so that it is correctly
+/// interpreted by `make`. This is consistent with Clang, GCC, and lld.
+static TGLexer::DependenciesSetTy::value_type escapeDependencyFilename(
+    const TGLexer::DependenciesSetTy::value_type &Filename) {
+  std::string Res;
+  raw_string_ostream OS(Res);
+
+  SmallString<256> NativePath;
+  sys::path::native(Filename, NativePath);
+
+  for (unsigned I = 0, E = NativePath.size(); I != E; ++I) {
+    if (NativePath[I] == '#') {
+      OS << '\\';
+    } else if (NativePath[I] == ' ') {
+      OS << '\\';
+      unsigned J = I;
+      while (J > 0 && NativePath[--J] == '\\') {
+        OS << '\\';
+      }
+    } else if (NativePath[I] == '$') {
+      OS << '$';
+    }
+    OS << NativePath[I];
+  }
+
+  OS.flush();
+  return Res;
+}
+
 /// Create a dependency file for `-d` option.
 ///
 /// This functionality is really only for the benefit of the build system.
@@ -96,9 +125,9 @@ static int createDependencyFile(const TGParser &Parser, const char *argv0) {
   if (EC)
     return reportError(argv0, "error opening " + DependFilename + ":" +
                                   EC.message() + "\n");
-  DepOut.os() << OutputFilename << ":";
+  DepOut.os() << escapeDependencyFilename(OutputFilename) << ":";
   for (const auto &Dep : Parser.getDependencies()) {
-    DepOut.os() << ' ' << Dep;
+    DepOut.os() << ' ' << escapeDependencyFilename(Dep);
   }
   DepOut.os() << "\n";
   DepOut.keep();

``````````

</details>


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