Skip to content

Commit ca2ab74

Browse files
authored
[clang] Canonicalize absolute paths in dependency file (#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.
1 parent 1623c43 commit ca2ab74

File tree

5 files changed

+22
-7
lines changed

5 files changed

+22
-7
lines changed

clang/include/clang/Frontend/Utils.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ class DependencyFileGenerator : public DependencyCollector {
120120
private:
121121
void outputDependencyFile(DiagnosticsEngine &Diags);
122122

123+
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS;
123124
std::string OutputFile;
124125
std::vector<std::string> Targets;
125126
bool IncludeSystemHeaders;

clang/lib/Frontend/DependencyFile.cpp

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,11 @@
1010
//
1111
//===----------------------------------------------------------------------===//
1212

13-
#include "clang/Frontend/Utils.h"
1413
#include "clang/Basic/FileManager.h"
1514
#include "clang/Basic/SourceManager.h"
1615
#include "clang/Frontend/DependencyOutputOptions.h"
1716
#include "clang/Frontend/FrontendDiagnostic.h"
17+
#include "clang/Frontend/Utils.h"
1818
#include "clang/Lex/DirectoryLookup.h"
1919
#include "clang/Lex/ModuleMap.h"
2020
#include "clang/Lex/PPCallbacks.h"
@@ -23,8 +23,10 @@
2323
#include "llvm/ADT/StringSet.h"
2424
#include "llvm/Support/FileSystem.h"
2525
#include "llvm/Support/Path.h"
26+
#include "llvm/Support/VirtualFileSystem.h"
2627
#include "llvm/Support/raw_ostream.h"
2728
#include <optional>
29+
#include <system_error>
2830

2931
using namespace clang;
3032

@@ -236,6 +238,7 @@ void DependencyFileGenerator::attachToPreprocessor(Preprocessor &PP) {
236238
PP.SetSuppressIncludeNotFoundError(true);
237239

238240
DependencyCollector::attachToPreprocessor(PP);
241+
FS = PP.getFileManager().getVirtualFileSystemPtr();
239242
}
240243

241244
bool DependencyFileGenerator::sawDependency(StringRef Filename, bool FromModule,
@@ -312,11 +315,22 @@ void DependencyFileGenerator::finishedMainFile(DiagnosticsEngine &Diags) {
312315
/// https://msdn.microsoft.com/en-us/library/dd9y37ha.aspx for NMake info,
313316
/// https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx
314317
/// for Windows file-naming info.
315-
static void PrintFilename(raw_ostream &OS, StringRef Filename,
318+
static void printFilename(raw_ostream &OS, llvm::vfs::FileSystem *FS,
319+
StringRef Filename,
316320
DependencyOutputFormat OutputFormat) {
317321
// Convert filename to platform native path
318322
llvm::SmallString<256> NativePath;
319323
llvm::sys::path::native(Filename.str(), NativePath);
324+
// Resolve absolute path. Make and Ninja canonicalize paths
325+
// without checking for symbolic links in the path, for performance concerns.
326+
// If there is something like `/bin/../lib64` -> `/usr/lib64`
327+
// (where `/bin` links to `/usr/bin`), Make will see them as `/lib64`.
328+
if (FS != nullptr && llvm::sys::path::is_absolute(NativePath)) {
329+
llvm::SmallString<256> NativePathTmp = NativePath;
330+
std::error_code EC = FS->getRealPath(NativePathTmp, NativePath);
331+
if (EC)
332+
NativePath = NativePathTmp;
333+
}
320334

321335
if (OutputFormat == DependencyOutputFormat::NMake) {
322336
// Add quotes if needed. These are the characters listed as "special" to
@@ -400,7 +414,7 @@ void DependencyFileGenerator::outputDependencyFile(llvm::raw_ostream &OS) {
400414
Columns = 2;
401415
}
402416
OS << ' ';
403-
PrintFilename(OS, File, OutputFormat);
417+
printFilename(OS, FS.get(), File, OutputFormat);
404418
Columns += N + 1;
405419
}
406420
OS << '\n';
@@ -411,7 +425,7 @@ void DependencyFileGenerator::outputDependencyFile(llvm::raw_ostream &OS) {
411425
for (auto I = Files.begin(), E = Files.end(); I != E; ++I) {
412426
if (Index++ == InputFileIndex)
413427
continue;
414-
PrintFilename(OS, *I, OutputFormat);
428+
printFilename(OS, FS.get(), *I, OutputFormat);
415429
OS << ":\n";
416430
}
417431
}

clang/test/Frontend/dependency-gen-symlink.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
// CHECK: dependency-gen-symlink.c.o
1616
// CHECK: dependency-gen-symlink.c
1717
// CHECK: a/header.h
18-
// CHECK: b/header.h
18+
// CHECK-NOT: b/header.h
1919
// CHECK-NOT: with-header-guard.h
2020
#include "a/header.h"
2121
#include "b/header.h"

clang/test/Frontend/dependency-gen-windows-duplicates.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
// RUN: %clang -MD -MF - %t.dir/test.c -fsyntax-only -I %t.dir/subdir | FileCheck %s
1010
// CHECK: test.o:
1111
// CHECK-NEXT: \test.c
12-
// CHECK-NEXT: \SubDir\X.h
12+
// CHECK-NEXT: \subdir\x.h
1313
// File x.h must appear only once (case insensitive check).
1414
// CHECK-NOT: {{\\|/}}{{x|X}}.{{h|H}}
1515

clang/test/VFS/external-names.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,4 +47,4 @@
4747

4848
// RUN: %clang_cc1 -D REINCLUDE -I %t -ivfsoverlay %t.yaml -Eonly %s -MTfoo -dependency-file %t.dep
4949
// RUN: cat %t.dep | FileCheck --check-prefix=CHECK-DEP %s
50-
// CHECK-DEP-NOT: Inputs
50+
// CHECK-DEP: Inputs{{..?}}external-names.h

0 commit comments

Comments
 (0)