Skip to content

[clang][cas] Fix caching builds with -fdiagnostics-absolute-paths #12033

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
benlangmuir wants to merge 2 commits into
base: next
Choose a base branch
from
+83 −3
Open

[clang][cas] Fix caching builds with -fdiagnostics-absolute-paths #12033

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
966a992
[clang][cas] Fix caching builds with -fdiagnostics-absolute-paths
benlangmuir Dec 19, 2025
392f68c
Add empty filename check
benlangmuir Dec 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions clang/include/clang/Basic/DiagnosticOptions.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,9 @@ DIAGOPT(ShowSafeBufferUsageSuggestions, 1, 0)
// TO_UPSTREAM(BoundsSafety)
DIAGOPT(BoundsSafetyAdoptionMode, 1, 0)

// TO_UPSTREAM(CAS)
DIAGOPT(FallbackRealFileSystemAbsolutePaths, 1, 0)

#undef DIAGOPT
#undef ENUM_DIAGOPT
#undef VALUE_DIAGOPT
Expand Down
6 changes: 6 additions & 0 deletions clang/lib/Frontend/CompileJobCache.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,9 @@ std::optional<int> CompileJobCache::initialize(CompilerInstance &Clang) {
return Error::success();
};

// Ensure path canonicalization in text diagnostics.
Invocation.getDiagnosticOpts().FallbackRealFileSystemAbsolutePaths = true;

llvm::PrefixMapper PrefixMapper;
llvm::SmallVector<llvm::MappedPrefix> Split;
llvm::MappedPrefix::transformPairs(CacheOpts.PathPrefixMappings, Split);
Expand Down Expand Up @@ -638,6 +641,9 @@ Expected<std::optional<int>> CompileJobCache::replayCachedResult(

assert(!Clang.getDiagnostics().hasErrorOccurred());

// Ensure path canonicalization in text diagnostics.
Clang.getDiagnosticOpts().FallbackRealFileSystemAbsolutePaths = true;

std::optional<llvm::cas::CASID> MCOutputID;
ObjectStoreCachingOutputs CachingOutputs(
Clang, WorkingDir, std::move(PrefixMapper), WriteOutputAsCASID,
Expand Down
19 changes: 16 additions & 3 deletions clang/lib/Frontend/TextDiagnostic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@
#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/ConvertUTF.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/IOSandbox.h"
#include "llvm/Support/Locale.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/VirtualFileSystem.h"
#include <algorithm>
#include <optional>

Expand Down Expand Up @@ -813,9 +816,7 @@ void TextDiagnostic::printDiagnosticMessage(raw_ostream &OS,
}

void TextDiagnostic::emitFilename(StringRef Filename, const SourceManager &SM) {
#ifdef _WIN32
SmallString<4096> TmpFilename;
#endif
SmallString<256> TmpFilename;
if (DiagOpts.AbsolutePath) {
auto File = SM.getFileManager().getOptionalFileRef(Filename);
if (File) {
Expand Down Expand Up @@ -843,6 +844,18 @@ void TextDiagnostic::emitFilename(StringRef Filename, const SourceManager &SM) {
Filename = SM.getFileManager().getCanonicalName(*File);
#endif
}

// TO_UPSTREAM(CAS)
if (DiagOpts.FallbackRealFileSystemAbsolutePaths && !Filename.empty() &&
!llvm::sys::path::is_absolute(Filename)) {
// When caching, the above may fail to canonicalize due to using the
// IncludeTreeFileSystem. In that case, we can safely fallback to the real
// filesytem since the text diagnostic output is not captured directly.
auto BypassSandbox = llvm::sys::sandbox::scopedDisable();
auto FS = llvm::vfs::getRealFileSystem();
if (FS->getRealPath(Filename, TmpFilename) == std::error_code())
Copy link

@cachemeifyoucan cachemeifyoucan Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain again with llvm::sys::path::make_absolute is not sufficient here?

Copy link
Author

@benlangmuir benlangmuir Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My goal for this PR is to match the behaviour without caching, which uses getCanonicalName -> getRealPath.

As to why it's done this way in the first place: I dug through the history of changes and review comments on this code and it seems like the motivation is to eliminate . and .. components in paths, which seems to be motivated by two issues: 1) some editors apparently were opening a separate tab for (a/b/../c and a/c), and 2) there are some cases where the extra dots make the absolute path really long. The code on Windows handles this directly, but it's not correct on other systems.

As a separate change, I am thinking of proposing that this behaviour change to make it instead do basically what Windows is doing (make absolute and remove dots), but add an additional check on non-Windows system that the two paths resolve to the same file. That will hopefully be cheaper in most cases, since stat() will be cheaper than realpath(). If the paths really are different files then we can fallback to only removing single dots. Even if there is some possibility it leaves some of the file paths "too long" in rare situations with symlinks, it seems like a better tradeoff to me overall.

Filename = TmpFilename;
}
}

OS << Filename;
Expand Down
38 changes: 38 additions & 0 deletions clang/test/CAS/include-tree-fdiagnostics-absolute-paths.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// RUN: rm -rf %t
// RUN: split-file %s %t
// RUN: cd %t
// RUN: env LLVM_CACHE_CAS_PATH=%t/cas %clang-cache \
// RUN: %clang subdir/tu.c -I. -fdiagnostics-absolute-paths -E \
// RUN: -Rcompile-job-cache > %t/stdout-miss 2> %t/stderr-miss
// RUN: FileCheck %s -DPREFIX=%/t -check-prefix=PP -input-file=%t/stdout-miss
// RUN: FileCheck %s -DPREFIX=%/t -check-prefix=DIAG -input-file=%t/stderr-miss
// RUN: FileCheck %s -DPREFIX=%/t -check-prefix=MISS -input-file=%t/stderr-miss

// Again, but with a cache hit.
// RUN: env LLVM_CACHE_CAS_PATH=%t/cas %clang-cache \
// RUN: %clang subdir/tu.c -I. -fdiagnostics-absolute-paths -E \
// RUN: -Rcompile-job-cache > %t/stdout-hit 2> %t/stderr-hit
// RUN: FileCheck %s -DPREFIX=%/t -check-prefix=PP -input-file=%t/stdout-hit
// RUN: FileCheck %s -DPREFIX=%/t -check-prefix=DIAG -input-file=%t/stderr-hit
// RUN: FileCheck %s -DPREFIX=%/t -check-prefix=HIT -input-file=%t/stderr-hit

// Preprocessor does not force absolute paths.
// PP: # 1 "subdir/tu.c"
// PP: # 1 "./h1.h"
// PP: const char *filename = "./h1.h";

// Diagnostics force absolute paths.
// DIAG: [[PREFIX]]/subdir/tu.c:1:2: warning: "from tu"
// DIAG: In file included from [[PREFIX]]/subdir/tu.c
// DIAG: [[PREFIX]]/h1.h:1:2: warning: "from h1"

// MISS: remark: compile job cache miss
// HIT: remark: compile job cache hit

//--- subdir/tu.c
#warning "from tu"
#include "h1.h"

//--- h1.h
#warning "from h1"
const char *filename = __FILE__;
19 changes: 19 additions & 0 deletions clang/test/CAS/libclang-replay-job.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,25 @@
// RUN: diff -u %t/t1.d %t/a/b/rel.d
// FIXME: Get clang's `-working-directory` to affect relative path for serialized diagnostics.

// Check with -fdiagnostics-absolute-path
// RUN: cd %t
// RUN: c-index-test core -replay-cached-job -cas-path %t/cas @%t/cache-key \
// RUN: -fcas-plugin-path %llvmshlibdir/libCASPluginTest%pluginext \
// RUN: -fcas-plugin-option no-logging \
// RUN: -working-dir %t \
// RUN: -- @%t/cc1.rsp \
// RUN: -fdiagnostics-absolute-paths \
// RUN: -Rcompile-job-cache-hit \
// RUN: -dependency-file %t/t3.d -o %t/output3.o 2> %t/output3.txt
// RUN: FileCheck %s -input-file=%t/output2.txt -check-prefix=REL_DIAG
// RUN: FileCheck %s -input-file=%t/output3.txt -check-prefix=ABS_DIAG
// RUN: diff %t/output1.o %t/output3.o
// RUN: diff -u %t/t1.d %t/t3.d
// REL_DIAG: remark: compile job cache hit for
// REL_DIAG: {{^}}main.c:1:2: warning:
// ABS_DIAG: remark: compile job cache hit for
// ABS_DIAG: libclang-replay-job.c.tmp{{.}}main.c:1:2: warning:

// Use relative path to inputs and outputs.
//--- cdb.json.template
[{
Expand Down
1 change: 1 addition & 0 deletions clang/test/Modules/feature-availability.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// RUN: rm -rf %t
// RUN: %clang_cc1 -triple arm64-apple-macosx -fmodules %S/Inputs/feature-availability/module.modulemap -fmodule-name=Feature1 -emit-module -o %t/feature1.pcm
// RUN: %clang_cc1 -triple arm64-apple-macosx -fmodules %S/Inputs/feature-availability/module.modulemap -fmodule-name=Feature2 -fmodule-file=%t/feature1.pcm -emit-module -o %t/feature2.pcm
// RUN: %clang_cc1 -triple arm64-apple-macosx -fmodules -fmodule-file=%t/feature2.pcm -I %S/Inputs/feature-availability -emit-llvm -o - %s | FileCheck %s
Expand Down