Support, LTO: When pruning a directory, ignore files not matching a prefix.

[pcc]

Summary

This is a safeguard against data loss if the user specifies a directory
that is not a cache directory. Teach the existing cache pruning clients
to create files with appropriate names.

[krasin]

Generally, looks good, modulo the comment.

llvm/lib/Support/CachePruning.cpp

193

We should just ignore files not created by LLVM. If a user decided to put something there, then she has a reason. And there's no actual harm to our code, if these files exist but ignored.

[pcc]

• Address review comments

llvm/lib/Support/CachePruning.cpp

193

Makes sense, done.

[krasin]

Thank you, Peter!

lld/ELF/LTO.cpp

std::vector<InputFile *> BitcodeCompiler::compile() {
   checkError(LTOObj->run(
       [&](size_t Task) {
         return llvm::make_unique<lto::NativeObjectStream>(
             llvm::make_unique<raw_svector_ostream>(Buff[Task]));
       },
       Cache));
 
   if (!Config->ThinLTOCacheDir.empty())
-    pruneCache(Config->ThinLTOCacheDir, Config->ThinLTOCachePolicy);
+    check(pruneCache(Config->ThinLTOCacheDir, Config->ThinLTOCachePolicy),
+          "could not prune ThinLTO cache");
 
   for (unsigned I = 0; I != MaxTasks; ++I) {
     if (Buff[I].empty())
       continue;
     if (Config->SaveTemps) {
       if (I == 0)
         saveBuffer(Buff[I], Config->OutputFile + ".lto.o");
       else

lld/test/ELF/lto/cache.ll

 ; REQUIRES: x86
 
 ; RUN: opt -module-hash -module-summary %s -o %t.o
 ; RUN: opt -module-hash -module-summary %p/Inputs/cache.ll -o %t2.o
 
 ; RUN: rm -Rf %t.cache && mkdir %t.cache
 ; Create a file that will be removed by cache pruning due to age.
-; RUN: touch -t 197001010000 %t.cache/foo
+; RUN: touch -t 197001010000 %t.cache/llvmcache-foo
 ; RUN: ld.lld --thinlto-cache-dir=%t.cache --thinlto-cache-policy prune_after=1h -o %t3 %t2.o %t.o
 
 ; Two cached objects, plus a timestamp file, minus the file we removed.
 ; RUN: ls %t.cache | count 3
 
+; RUN: touch %t.cache/foo
+; RUN: not ld.lld --thinlto-cache-dir=%t.cache --thinlto-cache-policy prune_after=1h -o %t3 %t2.o %t.o 2>&1 | FileCheck --check-prefix=ERROR %s
+; ERROR: could not prune ThinLTO cache: unexpected file found in cache directory: {{.*}}foo
+
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 define void @globalfunc() #0 {
 entry:
   ret void
 }

llvm/include/llvm/Support/CachePruning.h

 /// default constructed CachePruningPolicy object.
 /// For example: "prune_interval=30s:prune_after=24h:cache_size=50%"
 /// which means a pruning interval of 30 seconds, expiration time of 24 hours
 /// and maximum cache size of 50% of available disk space.
 Expected<CachePruningPolicy> parseCachePruningPolicy(StringRef PolicyStr);
 
 /// Peform pruning using the supplied policy, returns true if pruning
 /// occured, i.e. if Policy.Interval was expired.
-bool pruneCache(StringRef Path, CachePruningPolicy Policy);
+///
+/// As a safeguard against data loss if the user specifies a directory that is
+/// not a cache directory, this function requires that all files in this
+/// directory have names matching the patterns "llvmcache*" and will not prune
+/// files with names matching the pattern "llvmcache.*".
+Expected<bool> pruneCache(StringRef Path, CachePruningPolicy Policy);
 
 } // namespace llvm
 
 #endif

llvm/lib/LTO/Caching.cpp

 using namespace llvm::lto;
 
 Expected<NativeObjectCache> lto::localCache(StringRef CacheDirectoryPath,
                                             AddBufferFn AddBuffer) {
   if (std::error_code EC = sys::fs::create_directories(CacheDirectoryPath))
     return errorCodeToError(EC);
 
   return [=](unsigned Task, StringRef Key) -> AddStreamFn {
-    // First, see if we have a cache hit.
+    // This choice of file name allows the cache to be pruned (see pruneCache()
+    // in include/llvm/Support/CachePruning.h).
     SmallString<64> EntryPath;
-    sys::path::append(EntryPath, CacheDirectoryPath, Key);
+    sys::path::append(EntryPath, CacheDirectoryPath, "llvmcache-" + Key);
+    // First, see if we have a cache hit.
     ErrorOr<std::unique_ptr<MemoryBuffer>> MBOrErr =
         MemoryBuffer::getFile(EntryPath);
     if (MBOrErr) {
       AddBuffer(Task, std::move(*MBOrErr));
       return AddStreamFn();
     }
 
     if (MBOrErr.getError() != std::errc::no_such_file_or_directory)
struct CacheStream : NativeObjectStream {
         if (!MBOrErr)
           report_fatal_error(Twine("Failed to open cache file ") + EntryPath +
                              ": " + MBOrErr.getError().message() + "\n");
         AddBuffer(Task, std::move(*MBOrErr));
       }
     };
 
     return [=](size_t Task) -> std::unique_ptr<NativeObjectStream> {
-      // Write to a temporary to avoid race condition
+      // Write to a temporary to avoid race condition. The file name was chosen
+      // to avoid cache pruning.
       int TempFD;
       SmallString<64> TempFilenameModel, TempFilename;
-      sys::path::append(TempFilenameModel, CacheDirectoryPath, "Thin-%%%%%%.tmp.o");
+      sys::path::append(TempFilenameModel, CacheDirectoryPath,
+                        "llvmcache.tmp-%%%%%%.o");
       std::error_code EC =
           sys::fs::createUniqueFile(TempFilenameModel, TempFD, TempFilename,
                                     sys::fs::owner_read | sys::fs::owner_write);
       if (EC) {
         errs() << "Error: " << EC.message() << "\n";
         report_fatal_error("ThinLTO: Can't get a temporary file");
       }
 
       // This CacheStream will move the temporary file into the cache when done.
       return llvm::make_unique<CacheStream>(
           llvm::make_unique<raw_fd_ostream>(TempFD, /* ShouldClose */ true),
           AddBuffer, TempFilename.str(), EntryPath.str(), Task);
     };
   };
 }

llvm/lib/LTO/ThinLTOCodeGenerator.cpp

#endif
 
     // Include the hash for the preserved symbols.
     for (auto &Entry : PreservedSymbols) {
       if (DefinedFunctions.count(Entry))
         Hasher.update(
             ArrayRef<uint8_t>((const uint8_t *)&Entry, sizeof(GlobalValue::GUID)));
     }
 
-    sys::path::append(EntryPath, CachePath, toHex(Hasher.result()));
+    // This choice of file name allows the cache to be pruned (see pruneCache()
+    // in include/llvm/Support/CachePruning.h).
+    sys::path::append(EntryPath, CachePath,
+                      "llvmcache-" + toHex(Hasher.result()));
   }
 
   // Access the path to this entry in the cache.
   StringRef getEntryPath() { return EntryPath; }
 
   // Try loading the buffer for this cache entry.
   ErrorOr<std::unique_ptr<MemoryBuffer>> tryLoadingBuffer() {
     if (EntryPath.empty())
for (auto IndexCount : ModulesOrdering) {
           return;
         }
         ProducedBinaryFiles[count] = writeGeneratedObject(
             count, CacheEntryPath, SavedObjectsDirectoryPath, *OutputBuffer);
       }, IndexCount);
     }
   }
 
-  pruneCache(CacheOptions.Path, CacheOptions.Policy);
+  Expected<bool> PruneOrErr = pruneCache(CacheOptions.Path, CacheOptions.Policy);
+  if (!PruneOrErr)
+    report_fatal_error("Could not prune ThinLTO cache: " +
+                       toString(PruneOrErr.takeError()));
 
   // If statistics were requested, print them out now.
   if (llvm::AreStatisticsEnabled())
     llvm::PrintStatistics();
 }

llvm/lib/Support/CachePruning.cpp

if (Key == "prune_interval") {
                                      inconvertibleErrorCode());
     }
   }
 
   return Policy;
 }
 
 /// Prune the cache of files that haven't been accessed in a long time.
-bool llvm::pruneCache(StringRef Path, CachePruningPolicy Policy) {
+Expected<bool> llvm::pruneCache(StringRef Path, CachePruningPolicy Policy) {
   using namespace std::chrono;
 
   if (Path.empty())
     return false;
 
   bool isPathDir;
   if (sys::fs::is_directory(Path, isPathDir))
     return false;
Expected<bool> llvm::pruneCache(StringRef Path, CachePruningPolicy Policy) {
 
   // Walk the entire directory cache, looking for unused files.
   std::error_code EC;
   SmallString<128> CachePathNative;
   sys::path::native(Path, CachePathNative);
   // Walk all of the files within this directory.
   for (sys::fs::directory_iterator File(CachePathNative, EC), FileEnd;
        File != FileEnd && !EC; File.increment(EC)) {
-    // Do not touch the timestamp.
-    if (File->path() == TimestampFile)
+    StringRef Filename = sys::path::filename(File->path());
+
+    // Do not touch any files beginning with the string "llvmcache.". This
+    // includes the timestamp file as well as temporary files created by
+    // clients.
+    if (Filename.startswith("llvmcache."))
       continue;
 
+    // Abort if we see a file with an unexpected name.
+    if (!Filename.startswith("llvmcache"))
+      return make_error<StringError>(
+          "unexpected file found in cache directory: " + File->path(),
+          inconvertibleErrorCode());
+
     // Look at this file. If we can't stat it, there's nothing interesting
     // there.
     if (sys::fs::status(File->path(), FileStatus)) {
       DEBUG(dbgs() << "Ignore " << File->path() << " (can't stat)\n");
       continue;
     }
 
     // If the file hasn't been used recently enough, delete it

llvm/test/ThinLTO/X86/cache.ll

 ; RUN: ls %t.cache | count 0
 
 
 ; Repeat again, *with* hash this time.
 
 ; RUN: opt -module-hash -module-summary %s -o %t.bc
 ; RUN: opt -module-hash -module-summary %p/Inputs/cache.ll -o %t2.bc
 
-; Verify that enabling caching is working
+; Verify that enabling caching is working, and that the pruner only removes
+; files matching the pattern "llvmcache*" if they do not match the pattern
+; "llvmcache.*".
 ; RUN: rm -Rf %t.cache && mkdir %t.cache
+; RUN: touch -t 197001010000 %t.cache/llvmcache-foo %t.cache/llvmcache.foo
 ; RUN: llvm-lto -thinlto-action=run -exported-symbol=globalfunc %t2.bc  %t.bc -thinlto-cache-dir %t.cache
+; RUN: ls %t.cache | count 4
 ; RUN: ls %t.cache/llvmcache.timestamp
-; RUN: ls %t.cache | count 3
+; RUN: ls %t.cache/llvmcache.foo
+; RUN: not ls %t.cache/llvmcache-foo
+; RUN: ls %t.cache/llvmcache-* | count 2
+
+; Verify that enabling caching fails with a cache directory containing
+; unexpected files.
+; RUN: rm -Rf %t.cache && mkdir %t.cache
+; RUN: touch %t.cache/foo
+; RUN: not llvm-lto -thinlto-action=run -exported-symbol=globalfunc %t2.bc  %t.bc -thinlto-cache-dir %t.cache 2>&1 | FileCheck --check-prefix=ERROR %s
+; ERROR: Could not prune ThinLTO cache: unexpected file found in cache directory: {{.*}}foo
 
 ; Verify that enabling caching is working with llvm-lto2
 ; RUN: rm -Rf %t.cache
 ; RUN: llvm-lto2 -o %t.o %t2.bc  %t.bc -cache-dir %t.cache \
 ; RUN:  -r=%t2.bc,_main,plx \
 ; RUN:  -r=%t2.bc,_globalfunc,lx \
 ; RUN:  -r=%t.bc,_globalfunc,plx
 ; RUN: ls %t.cache | count 2
+; RUN: ls %t.cache/llvmcache-* | count 2
 
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx10.11.0"
 
 define void @globalfunc() #0 {
 entry:
   ret void
 }