https://github.com/Aadarsh-Keshri updated https://github.com/llvm/llvm-project/pull/189888
>From 440e267a7027514bc00850a8cbbe1ad45b696645 Mon Sep 17 00:00:00 2001 From: Aadarsh-Keshri <[email protected]> Date: Wed, 1 Apr 2026 12:11:32 +0530 Subject: [PATCH 1/4] Ensure parent directory exists when creating lock file --- clang/lib/Serialization/ModuleCache.cpp | 5 ---- llvm/lib/Support/LockFileManager.cpp | 27 ++++++++++++++++-- .../unittests/Support/LockFileManagerTest.cpp | 28 +++++++++++++++++++ 3 files changed, 52 insertions(+), 8 deletions(-) diff --git a/clang/lib/Serialization/ModuleCache.cpp b/clang/lib/Serialization/ModuleCache.cpp index 658da6e3b7145..7c57d143d2e03 100644 --- a/clang/lib/Serialization/ModuleCache.cpp +++ b/clang/lib/Serialization/ModuleCache.cpp @@ -109,11 +109,6 @@ class CrossProcessModuleCache : public ModuleCache { void prepareForGetLock(StringRef ModuleFilename) override { // This is a compiler-internal input/output, let's bypass the sandbox. auto BypassSandbox = llvm::sys::sandbox::scopedDisable(); - - // FIXME: Do this in LockFileManager and only if the directory doesn't - // exist. - StringRef Dir = llvm::sys::path::parent_path(ModuleFilename); - llvm::sys::fs::create_directories(Dir); } std::unique_ptr<llvm::AdvisoryLock> diff --git a/llvm/lib/Support/LockFileManager.cpp b/llvm/lib/Support/LockFileManager.cpp index 1a98ec03486db..205339f9366bd 100644 --- a/llvm/lib/Support/LockFileManager.cpp +++ b/llvm/lib/Support/LockFileManager.cpp @@ -16,6 +16,7 @@ #include "llvm/Support/FileSystem.h" #include "llvm/Support/IOSandbox.h" #include "llvm/Support/MemoryBuffer.h" +#include "llvm/Support/Path.h" #include "llvm/Support/Process.h" #include "llvm/Support/Signals.h" #include "llvm/Support/raw_ostream.h" @@ -185,10 +186,30 @@ Expected<bool> LockFileManager::tryLock() { UniqueLockFileName = LockFileName; UniqueLockFileName += "-%%%%%%%%"; int UniqueLockFileID; - if (std::error_code EC = sys::fs::createUniqueFile( - UniqueLockFileName, UniqueLockFileID, UniqueLockFileName)) - return createStringError(EC, "failed to create unique file " + + + { + std::error_code EC = sys::fs::createUniqueFile( + UniqueLockFileName, UniqueLockFileID, UniqueLockFileName); + if (EC == errc::no_such_file_or_directory) { + SmallString<128> Dir = sys::path::parent_path(UniqueLockFileName); + if (!Dir.empty()) { + if (std::error_code DirEC = sys::fs::create_directories(Dir)) + return createStringError(DirEC, + "failed to create lock directory " + Dir); + } + + // Retry creating lock file + UniqueLockFileName = LockFileName; + UniqueLockFileName += "-%%%%%%%%"; + + EC = sys::fs::createUniqueFile(UniqueLockFileName, UniqueLockFileID, UniqueLockFileName); + } + + if (EC) + return createStringError(EC, "failed to create unique file " + + UniqueLockFileName); + } // Clean up the unique file on signal or scope exit. RemoveUniqueLockFileOnSignal RemoveUniqueFile(UniqueLockFileName); diff --git a/llvm/unittests/Support/LockFileManagerTest.cpp b/llvm/unittests/Support/LockFileManagerTest.cpp index bd61b6c36efb3..ce5fd4d5063ce 100644 --- a/llvm/unittests/Support/LockFileManagerTest.cpp +++ b/llvm/unittests/Support/LockFileManagerTest.cpp @@ -104,4 +104,32 @@ TEST(LockFileManagerTest, RelativePath) { ASSERT_FALSE(chdir(OrigPath)); } +TEST(LockFileManagerTest, NonExistentDirectory) { + TempDir LockFileManagerTestDir("LockFileManagerTestDir", /*Unique*/ true); + + SmallString<64> NonExistentDir(LockFileManagerTestDir.path()); + sys::path::append(NonExistentDir, "does_not_exist"); + + SmallString<64> LockedFile(NonExistentDir); + sys::path::append(LockedFile, "file"); + + SmallString<64> FileLock(LockedFile); + FileLock += ".lock"; + + // Ensure directory truly does not exist before test + EXPECT_FALSE(sys::fs::exists(NonExistentDir)); + + { + LockFileManager Locked(LockedFile); + EXPECT_THAT_EXPECTED(Locked.tryLock(), HasValue(true)); + + // Directory and lock file should now exist + EXPECT_TRUE(sys::fs::exists(NonExistentDir)); + EXPECT_TRUE(sys::fs::exists(FileLock)); + } + + // After destruction, lock file should be removed + EXPECT_FALSE(sys::fs::exists(FileLock)); +} + } // end anonymous namespace >From 4ce3c51a66bc2484f99e6266b5fce987c83974aa Mon Sep 17 00:00:00 2001 From: Aadarsh-Keshri <[email protected]> Date: Sat, 4 Apr 2026 14:05:16 +0530 Subject: [PATCH 2/4] Deleted prepareForGetLock and its usages. Tweaked lock file creation logic --- .../include/clang/Serialization/ModuleCache.h | 4 --- .../DependencyScannerImpl.cpp | 1 - .../InProcessModuleCache.cpp | 2 -- clang/lib/Frontend/CompilerInstance.cpp | 1 - clang/lib/Serialization/ModuleCache.cpp | 6 +---- llvm/lib/Support/LockFileManager.cpp | 25 ++++++++++--------- 6 files changed, 14 insertions(+), 25 deletions(-) diff --git a/clang/include/clang/Serialization/ModuleCache.h b/clang/include/clang/Serialization/ModuleCache.h index c6795c5dc358a..621bde0829f74 100644 --- a/clang/include/clang/Serialization/ModuleCache.h +++ b/clang/include/clang/Serialization/ModuleCache.h @@ -24,10 +24,6 @@ class InMemoryModuleCache; /// operations the compiler might want to perform on the cache. class ModuleCache { public: - /// May perform any work that only needs to be performed once for multiple - /// calls \c getLock() with the same module filename. - virtual void prepareForGetLock(StringRef ModuleFilename) = 0; - /// Returns lock for the given module file. The lock is initially unlocked. virtual std::unique_ptr<llvm::AdvisoryLock> getLock(StringRef ModuleFilename) = 0; diff --git a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp index e2fea6f86017a..1940d2f275c73 100644 --- a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp +++ b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp @@ -657,7 +657,6 @@ struct AsyncModuleCompile : PPCallbacks { return; } - ModCache.prepareForGetLock(ModuleFileName); auto Lock = ModCache.getLock(ModuleFileName); bool Owned; llvm::Error LockErr = Lock->tryLock().moveInto(Owned); diff --git a/clang/lib/DependencyScanning/InProcessModuleCache.cpp b/clang/lib/DependencyScanning/InProcessModuleCache.cpp index cd7385c8f38c2..05e8591129981 100644 --- a/clang/lib/DependencyScanning/InProcessModuleCache.cpp +++ b/clang/lib/DependencyScanning/InProcessModuleCache.cpp @@ -82,8 +82,6 @@ class InProcessModuleCache : public ModuleCache { public: InProcessModuleCache(ModuleCacheEntries &Entries) : Entries(Entries) {} - void prepareForGetLock(StringRef Filename) override {} - std::unique_ptr<llvm::AdvisoryLock> getLock(StringRef Filename) override { auto &Entry = [&]() -> ModuleCacheEntry & { std::lock_guard<std::mutex> Lock(Entries.Mutex); diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index 33919bc1c4634..bad85491e1362 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -1491,7 +1491,6 @@ compileModuleBehindLockOrRead(CompilerInstance &ImportingInstance, << ModuleFileName << Module->Name; auto &ModuleCache = ImportingInstance.getModuleCache(); - ModuleCache.prepareForGetLock(ModuleFileName); while (true) { auto Lock = ModuleCache.getLock(ModuleFileName); diff --git a/clang/lib/Serialization/ModuleCache.cpp b/clang/lib/Serialization/ModuleCache.cpp index 7c57d143d2e03..16c533e587177 100644 --- a/clang/lib/Serialization/ModuleCache.cpp +++ b/clang/lib/Serialization/ModuleCache.cpp @@ -106,13 +106,9 @@ class CrossProcessModuleCache : public ModuleCache { InMemoryModuleCache InMemory; public: - void prepareForGetLock(StringRef ModuleFilename) override { - // This is a compiler-internal input/output, let's bypass the sandbox. - auto BypassSandbox = llvm::sys::sandbox::scopedDisable(); - } - std::unique_ptr<llvm::AdvisoryLock> getLock(StringRef ModuleFilename) override { + auto BypassSandbox = llvm::sys::sandbox::scopedDisable(); return std::make_unique<llvm::LockFileManager>(ModuleFilename); } diff --git a/llvm/lib/Support/LockFileManager.cpp b/llvm/lib/Support/LockFileManager.cpp index 205339f9366bd..e948b6ced5ae4 100644 --- a/llvm/lib/Support/LockFileManager.cpp +++ b/llvm/lib/Support/LockFileManager.cpp @@ -183,15 +183,17 @@ Expected<bool> LockFileManager::tryLock() { } // Create a lock file that is unique to this instance. - UniqueLockFileName = LockFileName; - UniqueLockFileName += "-%%%%%%%%"; + SmallString<128> UniqueLockFilePattern = LockFileName; + UniqueLockFilePattern += "-%%%%%%%%"; + int UniqueLockFileID; { - std::error_code EC = sys::fs::createUniqueFile( - UniqueLockFileName, UniqueLockFileID, UniqueLockFileName); + SmallString<128> UniquePath = UniqueLockFilePattern; + std::error_code EC = + sys::fs::createUniqueFile(UniquePath, UniqueLockFileID, UniquePath); if (EC == errc::no_such_file_or_directory) { - SmallString<128> Dir = sys::path::parent_path(UniqueLockFileName); + SmallString<128> Dir = sys::path::parent_path(UniqueLockFilePattern); if (!Dir.empty()) { if (std::error_code DirEC = sys::fs::create_directories(Dir)) return createStringError(DirEC, @@ -199,16 +201,15 @@ Expected<bool> LockFileManager::tryLock() { } // Retry creating lock file - UniqueLockFileName = LockFileName; - UniqueLockFileName += "-%%%%%%%%"; - - EC = sys::fs::createUniqueFile(UniqueLockFileName, UniqueLockFileID, - UniqueLockFileName); + UniquePath = UniqueLockFilePattern; + EC = sys::fs::createUniqueFile(UniquePath, UniqueLockFileID, UniquePath); } if (EC) - return createStringError(EC, "failed to create unique file " + - UniqueLockFileName); + return createStringError(EC, + "failed to create unique file " + UniquePath); + + UniqueLockFileName = UniquePath; } // Clean up the unique file on signal or scope exit. >From caf661a27eac2dee41cb357128808d2a187f39f3 Mon Sep 17 00:00:00 2001 From: Aadarsh-Keshri <[email protected]> Date: Sun, 5 Apr 2026 11:53:04 +0530 Subject: [PATCH 3/4] Limited the scope of UniqueLockFilePattern. --- llvm/lib/Support/LockFileManager.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/llvm/lib/Support/LockFileManager.cpp b/llvm/lib/Support/LockFileManager.cpp index e948b6ced5ae4..e1c039190ef7b 100644 --- a/llvm/lib/Support/LockFileManager.cpp +++ b/llvm/lib/Support/LockFileManager.cpp @@ -183,12 +183,10 @@ Expected<bool> LockFileManager::tryLock() { } // Create a lock file that is unique to this instance. - SmallString<128> UniqueLockFilePattern = LockFileName; - UniqueLockFilePattern += "-%%%%%%%%"; - int UniqueLockFileID; - { + SmallString<128> UniqueLockFilePattern = LockFileName; + UniqueLockFilePattern += "-%%%%%%%%"; SmallString<128> UniquePath = UniqueLockFilePattern; std::error_code EC = sys::fs::createUniqueFile(UniquePath, UniqueLockFileID, UniquePath); >From 5ad1b3f68c2c0ea9aefd0a594365dad05c3365c9 Mon Sep 17 00:00:00 2001 From: Aadarsh-Keshri <[email protected]> Date: Mon, 6 Apr 2026 20:50:51 +0530 Subject: [PATCH 4/4] Removed unwanted BypassSandbox initialization --- clang/lib/Serialization/ModuleCache.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/clang/lib/Serialization/ModuleCache.cpp b/clang/lib/Serialization/ModuleCache.cpp index 16c533e587177..70131658872a7 100644 --- a/clang/lib/Serialization/ModuleCache.cpp +++ b/clang/lib/Serialization/ModuleCache.cpp @@ -108,7 +108,6 @@ class CrossProcessModuleCache : public ModuleCache { public: std::unique_ptr<llvm::AdvisoryLock> getLock(StringRef ModuleFilename) override { - auto BypassSandbox = llvm::sys::sandbox::scopedDisable(); return std::make_unique<llvm::LockFileManager>(ModuleFilename); } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
