https://github.com/keith updated https://github.com/llvm/llvm-project/pull/189475
>From 20b57302be76dd3638abfc854e259db4d6f2e494 Mon Sep 17 00:00:00 2001 From: Keith Smiley <[email protected]> Date: Mon, 30 Mar 2026 12:13:06 -0700 Subject: [PATCH 1/2] [clang] Fix incorrect filenames for hardlinks Previously hardlinked files would be discovered based on their inode, leading to incorrect filepaths printed in some cases. Fixes https://github.com/llvm/llvm-project/issues/26953 Fixes https://github.com/llvm/llvm-project/issues/58726 Related https://reviews.llvm.org/D137304 --- clang/include/clang/Basic/SourceManager.h | 6 ++ clang/lib/Basic/SourceManager.cpp | 58 ++++++++++----- .../Preprocessor/hardlink-include-names.c | 42 +++++++++++ clang/unittests/Basic/SourceManagerTest.cpp | 72 +++++++++++++++++++ 4 files changed, 162 insertions(+), 16 deletions(-) create mode 100644 clang/test/Preprocessor/hardlink-include-names.c diff --git a/clang/include/clang/Basic/SourceManager.h b/clang/include/clang/Basic/SourceManager.h index bc9e97863556d..912146096195e 100644 --- a/clang/include/clang/Basic/SourceManager.h +++ b/clang/include/clang/Basic/SourceManager.h @@ -714,6 +714,12 @@ class SourceManager : public RefCountedBase<SourceManager> { /// as they do not refer to a file. std::vector<SrcMgr::ContentCache*> MemBufferInfos; + /// Per-FileID content caches for aliased file references. + /// + /// These caches preserve the spelling used for a particular include while + /// sharing file contents with the canonical cache in \c FileInfos. + std::vector<SrcMgr::ContentCache *> FileIDContentCaches; + /// The table of SLocEntries that are local to this module. /// /// Positive FileIDs are indexes into this table. Entry 0 indicates an invalid diff --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp index b6cc6ec9365f5..bb4f4c6d87645 100644 --- a/clang/lib/Basic/SourceManager.cpp +++ b/clang/lib/Basic/SourceManager.cpp @@ -47,6 +47,21 @@ using llvm::MemoryBuffer; #define DEBUG_TYPE "source-manager" +static SrcMgr::ContentCache * +cloneContentCache(llvm::BumpPtrAllocator &Alloc, + const ContentCache &Other) { + auto *Clone = new (Alloc.Allocate<ContentCache>()) ContentCache; + Clone->OrigEntry = Other.OrigEntry; + Clone->ContentsEntry = Other.ContentsEntry; + Clone->Filename = Other.Filename; + Clone->BufferOverridden = Other.BufferOverridden; + Clone->IsFileVolatile = Other.IsFileVolatile; + Clone->IsTransient = Other.IsTransient; + Clone->IsBufferInvalid = Other.IsBufferInvalid; + Clone->setUnownedBuffer(Other.getBufferIfLoaded()); + return Clone; +} + // Reaching a limit of 2^31 results in a hard error. This metric allows to track // if particular invocation of the compiler is close to it. STATISTIC(MaxUsedSLocBytes, "Maximum number of bytes used by source locations " @@ -324,9 +339,23 @@ SourceManager::~SourceManager() { ContentCacheAlloc.Deallocate(I->second); } } + for (unsigned i = 0, e = FileIDContentCaches.size(); i != e; ++i) { + if (FileIDContentCaches[i]) { + FileIDContentCaches[i]->~ContentCache(); + ContentCacheAlloc.Deallocate(FileIDContentCaches[i]); + } + } } void SourceManager::clearIDTables() { + for (unsigned i = 0, e = FileIDContentCaches.size(); i != e; ++i) { + if (FileIDContentCaches[i]) { + FileIDContentCaches[i]->~ContentCache(); + ContentCacheAlloc.Deallocate(FileIDContentCaches[i]); + } + } + FileIDContentCaches.clear(); + MainFileID = FileID(); LocalSLocEntryTable.clear(); LocalLocOffsetTable.clear(); @@ -361,17 +390,6 @@ bool SourceManager::isMainFile(const FileEntry &SourceFile) { void SourceManager::initializeForReplay(const SourceManager &Old) { assert(MainFileID.isInvalid() && "expected uninitialized SourceManager"); - auto CloneContentCache = [&](const ContentCache *Cache) -> ContentCache * { - auto *Clone = new (ContentCacheAlloc.Allocate<ContentCache>()) ContentCache; - Clone->OrigEntry = Cache->OrigEntry; - Clone->ContentsEntry = Cache->ContentsEntry; - Clone->BufferOverridden = Cache->BufferOverridden; - Clone->IsFileVolatile = Cache->IsFileVolatile; - Clone->IsTransient = Cache->IsTransient; - Clone->setUnownedBuffer(Cache->getBufferIfLoaded()); - return Clone; - }; - // Ensure all SLocEntries are loaded from the external source. for (unsigned I = 0, N = Old.LoadedSLocEntryTable.size(); I != N; ++I) if (!Old.SLocEntryLoaded[I]) @@ -382,7 +400,7 @@ void SourceManager::initializeForReplay(const SourceManager &Old) { SrcMgr::ContentCache *&Slot = FileInfos[FileInfo.first]; if (Slot) continue; - Slot = CloneContentCache(FileInfo.second); + Slot = cloneContentCache(ContentCacheAlloc, *FileInfo.second); } } @@ -542,14 +560,21 @@ FileID SourceManager::createFileID(FileEntryRef SourceFile, SourceLocation::UIntTy LoadedOffset) { SrcMgr::ContentCache &IR = getOrCreateContentCache(SourceFile, isSystem(FileCharacter)); + SrcMgr::ContentCache *Cache = &IR; + + if (IR.OrigEntry && !IR.OrigEntry->isSameRef(SourceFile)) { + Cache = cloneContentCache(ContentCacheAlloc, IR); + Cache->OrigEntry = SourceFile; + FileIDContentCaches.push_back(Cache); + } // If this is a named pipe, immediately load the buffer to ensure subsequent // calls to ContentCache::getSize() are accurate. - if (IR.ContentsEntry->isNamedPipe()) - (void)IR.getBufferOrNone(Diag, getFileManager(), SourceLocation()); + if (Cache->ContentsEntry->isNamedPipe()) + (void)Cache->getBufferOrNone(Diag, getFileManager(), SourceLocation()); - return createFileIDImpl(IR, SourceFile.getName(), IncludePos, FileCharacter, - LoadedID, LoadedOffset); + return createFileIDImpl(*Cache, SourceFile.getName(), IncludePos, + FileCharacter, LoadedID, LoadedOffset); } /// Create a new FileID that represents the specified memory buffer. @@ -2310,6 +2335,7 @@ SourceManager::MemoryBufferSizes SourceManager::getMemoryBufferSizes() const { size_t SourceManager::getDataStructureSizes() const { size_t size = llvm::capacity_in_bytes(MemBufferInfos) + + llvm::capacity_in_bytes(FileIDContentCaches) + llvm::capacity_in_bytes(LocalSLocEntryTable) + llvm::capacity_in_bytes(LoadedSLocEntryTable) + llvm::capacity_in_bytes(SLocEntryLoaded) + diff --git a/clang/test/Preprocessor/hardlink-include-names.c b/clang/test/Preprocessor/hardlink-include-names.c new file mode 100644 index 0000000000000..675ba30438ce9 --- /dev/null +++ b/clang/test/Preprocessor/hardlink-include-names.c @@ -0,0 +1,42 @@ +// Test that symlinked and hardlinked files are reported with their correct +// filenames +// +// REQUIRES: symlinks +// RUN: rm -rf %t +// RUN: split-file %s %t +// RUN: ln %t/foo.inc %t/bar.inc +// RUN: ln -s %t/foo.inc %t/baz.inc + +// Test 1: -E should show both filenames. +// RUN: %clang_cc1 -E %t/main.c -o - | FileCheck --check-prefix=PP %s +// PP: # 1 "{{.*(/|\\\\)}}foo.inc" 1 +// PP: # 1 "{{.*(/|\\\\)}}bar.inc" 1 +// PP: # 1 "{{.*(/|\\\\)}}baz.inc" 1 + +// Test 2: .d should list both filenames. +// RUN: %clang_cc1 -dependency-file %t/deps.d -MT main.o %t/main.c -fsyntax-only +// RUN: FileCheck --check-prefix=DEPS -input-file=%t/deps.d %s +// DEPS: foo.inc +// DEPS: bar.inc +// DEPS: baz.inc + +// Test 3: --show-includes should list both filenames. +// RUN: %clang_cc1 --show-includes -o /dev/null %t/main.c | \ +// RUN: FileCheck --check-prefix=SHOW %s +// SHOW: Note: including file: {{.*}}foo.inc +// SHOW: Note: including file: {{.*}}bar.inc +// SHOW: Note: including file: {{.*}}baz.inc + +//--- main.c +const char *a = +#include "foo.inc" +; +const char *b = +#include "bar.inc" +; +const char *c = +#include "baz.inc" +; + +//--- foo.inc +"contents" diff --git a/clang/unittests/Basic/SourceManagerTest.cpp b/clang/unittests/Basic/SourceManagerTest.cpp index 7c8aae5c5834f..c4d1763eec2ef 100644 --- a/clang/unittests/Basic/SourceManagerTest.cpp +++ b/clang/unittests/Basic/SourceManagerTest.cpp @@ -10,6 +10,7 @@ #include "clang/Basic/Diagnostic.h" #include "clang/Basic/DiagnosticOptions.h" #include "clang/Basic/FileManager.h" +#include "clang/Basic/FileSystemStatCache.h" #include "clang/Basic/LangOptions.h" #include "clang/Basic/TargetInfo.h" #include "clang/Basic/TargetOptions.h" @@ -22,6 +23,7 @@ #include "llvm/Config/llvm-config.h" #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/Process.h" +#include "llvm/Support/VirtualFileSystem.h" #include "gmock/gmock.h" #include "gtest/gtest.h" #include <cstddef> @@ -38,6 +40,41 @@ class SourceManagerTestHelper { namespace { +#ifndef _WIN32 + +class FakeStatCache : public FileSystemStatCache { + llvm::StringMap<llvm::vfs::Status, llvm::BumpPtrAllocator> StatCalls; + +public: + void InjectFile(StringRef Path, ino_t INode) { + StatCalls[Path] = llvm::vfs::Status( + Path, llvm::sys::fs::UniqueID(1, INode), /*MTime=*/{}, + /*User=*/0, /*Group=*/0, /*Size=*/0, + llvm::sys::fs::file_type::regular_file, + llvm::sys::fs::perms::all_all); + } + + void InjectDirectory(StringRef Path, ino_t INode) { + StatCalls[Path] = llvm::vfs::Status( + Path, llvm::sys::fs::UniqueID(1, INode), /*MTime=*/{}, + /*User=*/0, /*Group=*/0, /*Size=*/0, + llvm::sys::fs::file_type::directory_file, + llvm::sys::fs::perms::all_all); + } + + std::error_code getStat(StringRef Path, llvm::vfs::Status &Status, + bool isFile, std::unique_ptr<llvm::vfs::File> *F, + llvm::vfs::FileSystem &FS) override { + auto It = StatCalls.find(Path); + if (It == StatCalls.end()) + return std::make_error_code(std::errc::no_such_file_or_directory); + Status = It->second; + return std::error_code(); + } +}; + +#endif + // The test fixture. class SourceManagerTest : public ::testing::Test { protected: @@ -417,6 +454,41 @@ TEST_F(SourceManagerTest, getLineNumber) { ASSERT_NO_FATAL_FAILURE(SourceMgr.getLineNumber(mainFileID, 1, nullptr)); } +#ifndef _WIN32 + +TEST_F(SourceManagerTest, aliasedFilesKeepRequestedNamesPerFileID) { + auto StatCache = std::make_unique<FakeStatCache>(); + StatCache->InjectDirectory("dir", 40); + StatCache->InjectFile("dir/foo.h", 41); + StatCache->InjectFile("dir/bar.h", 41); + FileMgr.setStatCache(std::move(StatCache)); + + auto FooOrErr = FileMgr.getFileRef("dir/foo.h"); + auto BarOrErr = FileMgr.getFileRef("dir/bar.h"); + ASSERT_TRUE(static_cast<bool>(FooOrErr)); + ASSERT_TRUE(static_cast<bool>(BarOrErr)); + + FileEntryRef Foo = *FooOrErr; + FileEntryRef Bar = *BarOrErr; + EXPECT_FALSE(Foo.isSameRef(Bar)); + EXPECT_EQ(Foo, Bar); + + SourceMgr.overrideFileContents(Foo, llvm::MemoryBuffer::getMemBuffer("x\n")); + + FileID FooID = SourceMgr.createFileID(Foo, SourceLocation(), SrcMgr::C_User); + FileID BarID = SourceMgr.createFileID(Bar, SourceLocation(), SrcMgr::C_User); + + SourceLocation FooLoc = SourceMgr.getLocForStartOfFile(FooID); + SourceLocation BarLoc = SourceMgr.getLocForStartOfFile(BarID); + + EXPECT_EQ("dir/foo.h", SourceMgr.getFilename(FooLoc)); + EXPECT_EQ("dir/bar.h", SourceMgr.getFilename(BarLoc)); + EXPECT_STREQ("dir/foo.h", SourceMgr.getPresumedLoc(FooLoc).getFilename()); + EXPECT_STREQ("dir/bar.h", SourceMgr.getPresumedLoc(BarLoc).getFilename()); +} + +#endif + struct FakeExternalSLocEntrySource : ExternalSLocEntrySource { bool ReadSLocEntry(int ID) override { return {}; } int getSLocEntryID(SourceLocation::UIntTy SLocOffset) override { return 0; } >From 4b817c561a026a579ca15d3cd534f3b58fbdfea7 Mon Sep 17 00:00:00 2001 From: Keith Smiley <[email protected]> Date: Mon, 30 Mar 2026 13:48:28 -0700 Subject: [PATCH 2/2] format --- clang/lib/Basic/SourceManager.cpp | 5 ++--- clang/unittests/Basic/SourceManagerTest.cpp | 13 ++++++------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp index bb4f4c6d87645..b27a07dc677b6 100644 --- a/clang/lib/Basic/SourceManager.cpp +++ b/clang/lib/Basic/SourceManager.cpp @@ -47,9 +47,8 @@ using llvm::MemoryBuffer; #define DEBUG_TYPE "source-manager" -static SrcMgr::ContentCache * -cloneContentCache(llvm::BumpPtrAllocator &Alloc, - const ContentCache &Other) { +static SrcMgr::ContentCache *cloneContentCache(llvm::BumpPtrAllocator &Alloc, + const ContentCache &Other) { auto *Clone = new (Alloc.Allocate<ContentCache>()) ContentCache; Clone->OrigEntry = Other.OrigEntry; Clone->ContentsEntry = Other.ContentsEntry; diff --git a/clang/unittests/Basic/SourceManagerTest.cpp b/clang/unittests/Basic/SourceManagerTest.cpp index c4d1763eec2ef..2594d35a19960 100644 --- a/clang/unittests/Basic/SourceManagerTest.cpp +++ b/clang/unittests/Basic/SourceManagerTest.cpp @@ -50,16 +50,15 @@ class FakeStatCache : public FileSystemStatCache { StatCalls[Path] = llvm::vfs::Status( Path, llvm::sys::fs::UniqueID(1, INode), /*MTime=*/{}, /*User=*/0, /*Group=*/0, /*Size=*/0, - llvm::sys::fs::file_type::regular_file, - llvm::sys::fs::perms::all_all); + llvm::sys::fs::file_type::regular_file, llvm::sys::fs::perms::all_all); } void InjectDirectory(StringRef Path, ino_t INode) { - StatCalls[Path] = llvm::vfs::Status( - Path, llvm::sys::fs::UniqueID(1, INode), /*MTime=*/{}, - /*User=*/0, /*Group=*/0, /*Size=*/0, - llvm::sys::fs::file_type::directory_file, - llvm::sys::fs::perms::all_all); + StatCalls[Path] = + llvm::vfs::Status(Path, llvm::sys::fs::UniqueID(1, INode), /*MTime=*/{}, + /*User=*/0, /*Group=*/0, /*Size=*/0, + llvm::sys::fs::file_type::directory_file, + llvm::sys::fs::perms::all_all); } std::error_code getStat(StringRef Path, llvm::vfs::Status &Status, _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
