Author: Aadarsh Keshri
Date: 2026-04-06T12:37:32-07:00
New Revision: 326593b4b47cab59d64acbd456e88abfb77a0f4c

URL: 
https://github.com/llvm/llvm-project/commit/326593b4b47cab59d64acbd456e88abfb77a0f4c
DIFF: 
https://github.com/llvm/llvm-project/commit/326593b4b47cab59d64acbd456e88abfb77a0f4c.diff

LOG: [Support][Modules] Removed prepareForGetLock and its usages. Ensured 
parent directory exists when creating lock file. (#189888)

Following #187372

Added: 
    

Modified: 
    clang/include/clang/Serialization/ModuleCache.h
    clang/lib/DependencyScanning/DependencyScannerImpl.cpp
    clang/lib/DependencyScanning/InProcessModuleCache.cpp
    clang/lib/Frontend/CompilerInstance.cpp
    clang/lib/Serialization/ModuleCache.cpp
    llvm/lib/Support/LockFileManager.cpp
    llvm/unittests/Support/LockFileManagerTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Serialization/ModuleCache.h 
b/clang/include/clang/Serialization/ModuleCache.h
index 6683511b56a05..82c4cde37c5ff 100644
--- a/clang/include/clang/Serialization/ModuleCache.h
+++ b/clang/include/clang/Serialization/ModuleCache.h
@@ -29,10 +29,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 0565f5eebfe04..432656b797eea 100644
--- a/clang/lib/DependencyScanning/InProcessModuleCache.cpp
+++ b/clang/lib/DependencyScanning/InProcessModuleCache.cpp
@@ -84,8 +84,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 a504cde306a35..0b00ad7128c00 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -1529,7 +1529,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 e82875f6f2831..dd0b4b845331e 100644
--- a/clang/lib/Serialization/ModuleCache.cpp
+++ b/clang/lib/Serialization/ModuleCache.cpp
@@ -169,16 +169,6 @@ 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();
-
-    // 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>
   getLock(StringRef ModuleFilename) override {
     return std::make_unique<llvm::LockFileManager>(ModuleFilename);

diff  --git a/llvm/lib/Support/LockFileManager.cpp 
b/llvm/lib/Support/LockFileManager.cpp
index 1a98ec03486db..564cfc5b21aa4 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"
@@ -182,13 +183,32 @@ Expected<bool> LockFileManager::tryLock() {
   }
 
   // Create a lock file that is unique to this instance.
-  UniqueLockFileName = LockFileName;
-  UniqueLockFileName += "-%%%%%%%%";
   int UniqueLockFileID;
-  if (std::error_code EC = sys::fs::createUniqueFile(
-          UniqueLockFileName, UniqueLockFileID, UniqueLockFileName))
-    return createStringError(EC, "failed to create unique file " +
-                                     UniqueLockFileName);
+  {
+    SmallString<128> UniqueLockFilePattern = LockFileName;
+    UniqueLockFilePattern += "-%%%%%%%%";
+    SmallString<128> UniquePath;
+    std::error_code EC = sys::fs::createUniqueFile(
+        UniqueLockFilePattern, UniqueLockFileID, UniquePath);
+    if (EC == errc::no_such_file_or_directory) {
+      SmallString<128> Dir = sys::path::parent_path(UniqueLockFilePattern);
+      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
+      EC = sys::fs::createUniqueFile(UniqueLockFilePattern, UniqueLockFileID,
+                                     UniquePath);
+    }
+
+    if (EC)
+      return createStringError(EC,
+                               "failed to create unique file " + UniquePath);
+
+    UniqueLockFileName = UniquePath;
+  }
 
   // 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


        
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to