[clang] [clang] Absoultify paths in dependency file output (PR #117458)

via cfe-commits cfe-commits at lists.llvm.org
Sat Nov 23 17:58:02 PST 2024


https://github.com/xtexChooser created https://github.com/llvm/llvm-project/pull/117458

This fixes #117438.

If paths in dependency file are not absoulte, make (or ninja) will canonicalize them.
While their canonicalization does not involves symbolic links expansion (for IO performance concerns), leaving a non-absolute path in dependency file may lead to unexpected canonicalization.
For example, '/a/../b', where '/a' is a symlink to '/c/d', it should be '/c/b' but make (and ninja) canonicalizes it as '/b', and fails for file not found.

>From f4b80b9cc44e5505180dff470440b7044e838507 Mon Sep 17 00:00:00 2001
From: xtex <xtexchooser at duck.com>
Date: Sun, 24 Nov 2024 07:06:35 +0800
Subject: [PATCH] [clang] Absoultify paths in dependency file output

This fixes #117438.

If paths in dependency file are not absoulte, make (or ninja) will
canonicalize them.
While their canonicalization does not involves symbolic links
expansion (for IO performance concerns), leaving a
non-absolute path in dependency file may lead to unexpected
canonicalization.
For example, '/a/../b', where '/a' is a symlink to '/c/d', it should be
'/c/b' but make (and ninja) canonicalizes it as '/b', and fails for file
not found.

Signed-off-by: Bingwu Zhang <xtexchooser at duck.com>
---
 clang/include/clang/Frontend/Utils.h  |  1 +
 clang/lib/Frontend/DependencyFile.cpp | 16 ++++++++++++----
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/clang/include/clang/Frontend/Utils.h b/clang/include/clang/Frontend/Utils.h
index 604e42067a3f1e..8ed17179c9824b 100644
--- a/clang/include/clang/Frontend/Utils.h
+++ b/clang/include/clang/Frontend/Utils.h
@@ -120,6 +120,7 @@ class DependencyFileGenerator : public DependencyCollector {
 private:
   void outputDependencyFile(DiagnosticsEngine &Diags);
 
+  llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS;
   std::string OutputFile;
   std::vector<std::string> Targets;
   bool IncludeSystemHeaders;
diff --git a/clang/lib/Frontend/DependencyFile.cpp b/clang/lib/Frontend/DependencyFile.cpp
index 528eae2c5283ea..ce7183b47e67c2 100644
--- a/clang/lib/Frontend/DependencyFile.cpp
+++ b/clang/lib/Frontend/DependencyFile.cpp
@@ -10,11 +10,11 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "clang/Frontend/Utils.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Frontend/DependencyOutputOptions.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
+#include "clang/Frontend/Utils.h"
 #include "clang/Lex/DirectoryLookup.h"
 #include "clang/Lex/ModuleMap.h"
 #include "clang/Lex/PPCallbacks.h"
@@ -23,6 +23,7 @@
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Support/raw_ostream.h"
 #include <optional>
 
@@ -236,6 +237,7 @@ void DependencyFileGenerator::attachToPreprocessor(Preprocessor &PP) {
     PP.SetSuppressIncludeNotFoundError(true);
 
   DependencyCollector::attachToPreprocessor(PP);
+  FS = PP.getFileManager().getVirtualFileSystemPtr();
 }
 
 bool DependencyFileGenerator::sawDependency(StringRef Filename, bool FromModule,
@@ -312,11 +314,17 @@ void DependencyFileGenerator::finishedMainFile(DiagnosticsEngine &Diags) {
 /// https://msdn.microsoft.com/en-us/library/dd9y37ha.aspx for NMake info,
 /// https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx
 /// for Windows file-naming info.
-static void PrintFilename(raw_ostream &OS, StringRef Filename,
+static void PrintFilename(raw_ostream &OS, llvm::vfs::FileSystem *FS,
+                          StringRef Filename,
                           DependencyOutputFormat OutputFormat) {
   // Convert filename to platform native path
   llvm::SmallString<256> NativePath;
   llvm::sys::path::native(Filename.str(), NativePath);
+  // Make path absolute. Make and Ninja canonicalize paths without checking for
+  // symbolic links in the path, for performance concerns.
+  // If there is something like `/bin/../lib64` -> `/usr/lib64`
+  // (where `/bin` links to `/usr/bin`), Make will see them as `/lib64`.
+  FS->makeAbsolute(NativePath);
 
   if (OutputFormat == DependencyOutputFormat::NMake) {
     // Add quotes if needed. These are the characters listed as "special" to
@@ -400,7 +408,7 @@ void DependencyFileGenerator::outputDependencyFile(llvm::raw_ostream &OS) {
       Columns = 2;
     }
     OS << ' ';
-    PrintFilename(OS, File, OutputFormat);
+    PrintFilename(OS, FS.get(), File, OutputFormat);
     Columns += N + 1;
   }
   OS << '\n';
@@ -411,7 +419,7 @@ void DependencyFileGenerator::outputDependencyFile(llvm::raw_ostream &OS) {
     for (auto I = Files.begin(), E = Files.end(); I != E; ++I) {
       if (Index++ == InputFileIndex)
         continue;
-      PrintFilename(OS, *I, OutputFormat);
+      PrintFilename(OS, FS.get(), *I, OutputFormat);
       OS << ":\n";
     }
   }



More information about the cfe-commits mailing list