[llvm] 6b92bb4 - [Support] [DebugInfo] Lazily create cache dir.
Daniel Thornburgh via llvm-commits
llvm-commits at lists.llvm.org
Thu Jan 20 11:27:23 PST 2022
Author: Daniel Thornburgh
Date: 2022-01-20T19:27:15Z
New Revision: 6b92bb47901f3a2d4a9aa683b0365088113a729e
URL: https://github.com/llvm/llvm-project/commit/6b92bb47901f3a2d4a9aa683b0365088113a729e
DIFF: https://github.com/llvm/llvm-project/commit/6b92bb47901f3a2d4a9aa683b0365088113a729e.diff
LOG: [Support] [DebugInfo] Lazily create cache dir.
This change defers creating Support/Caching.cpp's cache directory until
it actually writes to the cache.
This allows using Caching library in a read-only fashion. If read-only,
the cache is guaranteed not to write to disk. This keeps tools using
DebugInfod (currently llvm-symbolizer) hermetic when not configured to
perform remote lookups.
Reviewed By: phosek
Differential Revision: https://reviews.llvm.org/D117589
Added:
Modified:
llvm/include/llvm/Support/Caching.h
llvm/lib/Support/Caching.cpp
llvm/test/ThinLTO/X86/cache.ll
llvm/test/ThinLTO/X86/empty_module_with_cache.ll
llvm/unittests/Debuginfod/DebuginfodTests.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/Support/Caching.h b/llvm/include/llvm/Support/Caching.h
index 5c30a822ef388..bef23ae757f2e 100644
--- a/llvm/include/llvm/Support/Caching.h
+++ b/llvm/include/llvm/Support/Caching.h
@@ -62,10 +62,11 @@ using AddBufferFn =
std::function<void(unsigned Task, std::unique_ptr<MemoryBuffer> MB)>;
/// Create a local file system cache which uses the given cache name, temporary
-/// file prefix, cache directory and file callback. This function also creates
-/// the cache directory if it does not already exist. The cache name appears in
-/// error messages for errors during caching. The temporary file prefix is used
-/// in the temporary file naming scheme used when writing files atomically.
+/// file prefix, cache directory and file callback. This function does not
+/// immediately create the cache directory if it does not yet exist; this is
+/// done lazily the first time a file is added. The cache name appears in error
+/// messages for errors during caching. The temporary file prefix is used in the
+/// temporary file naming scheme used when writing files atomically.
Expected<FileCache> localCache(
Twine CacheNameRef, Twine TempFilePrefixRef, Twine CacheDirectoryPathRef,
AddBufferFn AddBuffer = [](size_t Task, std::unique_ptr<MemoryBuffer> MB) {
diff --git a/llvm/lib/Support/Caching.cpp b/llvm/lib/Support/Caching.cpp
index 8c685640f791a..d6902f660e39e 100644
--- a/llvm/lib/Support/Caching.cpp
+++ b/llvm/lib/Support/Caching.cpp
@@ -30,8 +30,6 @@ Expected<FileCache> llvm::localCache(Twine CacheNameRef,
Twine TempFilePrefixRef,
Twine CacheDirectoryPathRef,
AddBufferFn AddBuffer) {
- if (std::error_code EC = sys::fs::create_directories(CacheDirectoryPathRef))
- return errorCodeToError(EC);
// Create local copies which are safely captured-by-copy in lambdas
SmallString<64> CacheName, TempFilePrefix, CacheDirectoryPath;
@@ -140,6 +138,12 @@ Expected<FileCache> llvm::localCache(Twine CacheNameRef,
};
return [=](size_t Task) -> Expected<std::unique_ptr<CachedFileStream>> {
+ // Create the cache directory if not already done. Doing this lazily
+ // ensures the filesystem isn't mutated until the cache is.
+ if (std::error_code EC = sys::fs::create_directories(
+ CacheDirectoryPath, /*IgnoreExisting=*/true))
+ return errorCodeToError(EC);
+
// Write to a temporary to avoid race condition
SmallString<64> TempFilenameModel;
sys::path::append(TempFilenameModel, CacheDirectoryPath,
diff --git a/llvm/test/ThinLTO/X86/cache.ll b/llvm/test/ThinLTO/X86/cache.ll
index 406a1a456f89c..009b78713316a 100644
--- a/llvm/test/ThinLTO/X86/cache.ll
+++ b/llvm/test/ThinLTO/X86/cache.ll
@@ -20,7 +20,7 @@
; RUN: -r=%t2.bc,_main,plx \
; RUN: -r=%t2.bc,_globalfunc,lx \
; RUN: -r=%t.bc,_globalfunc,plx
-; RUN: ls %t.cache.noindex | count 0
+; RUN: not ls %t.cache.noindex
; Repeat again, *with* hash this time.
diff --git a/llvm/test/ThinLTO/X86/empty_module_with_cache.ll b/llvm/test/ThinLTO/X86/empty_module_with_cache.ll
index 8e58d9f0db959..693f264b8ff26 100644
--- a/llvm/test/ThinLTO/X86/empty_module_with_cache.ll
+++ b/llvm/test/ThinLTO/X86/empty_module_with_cache.ll
@@ -28,7 +28,7 @@
; RUN: rm -Rf %t.cache
; RUN: llvm-lto2 run -o %t.o %t2.bc %t.bc -cache-dir %t.cache \
; RUN: -r=%t2.bc,_main,plx
-; RUN: ls %t.cache | count 0
+; RUN: not ls %t.cache
target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
diff --git a/llvm/unittests/Debuginfod/DebuginfodTests.cpp b/llvm/unittests/Debuginfod/DebuginfodTests.cpp
index fc5eb705969e7..5312912599e93 100644
--- a/llvm/unittests/Debuginfod/DebuginfodTests.cpp
+++ b/llvm/unittests/Debuginfod/DebuginfodTests.cpp
@@ -17,6 +17,17 @@
#define setenv(name, var, ignore) _putenv_s(name, var)
#endif
+#define ASSERT_NO_ERROR(x) \
+ if (std::error_code ASSERT_NO_ERROR_ec = x) { \
+ SmallString<128> MessageStorage; \
+ raw_svector_ostream Message(MessageStorage); \
+ Message << #x ": did not return errc::success.\n" \
+ << "error number: " << ASSERT_NO_ERROR_ec.value() << "\n" \
+ << "error message: " << ASSERT_NO_ERROR_ec.message() << "\n"; \
+ GTEST_FATAL_FAILURE_(MessageStorage.c_str()); \
+ } else { \
+ }
+
using namespace llvm;
// Check that the Debuginfod client can find locally cached artifacts.
@@ -40,11 +51,12 @@ TEST(DebuginfodClient, CacheHit) {
// Check that the Debuginfod client returns an Error when it fails to find an
// artifact.
TEST(DebuginfodClient, CacheMiss) {
- // Set the cache path to a temp directory to avoid permissions issues if $HOME
- // is not writable.
- SmallString<32> TempDir;
- sys::path::system_temp_directory(true, TempDir);
- setenv("DEBUGINFOD_CACHE_PATH", TempDir.c_str(),
+ SmallString<32> CacheDir;
+ ASSERT_NO_ERROR(
+ sys::fs::createUniqueDirectory("debuginfod-unittest", CacheDir));
+ sys::path::append(CacheDir, "cachedir");
+ ASSERT_FALSE(sys::fs::exists(CacheDir));
+ setenv("DEBUGINFOD_CACHE_PATH", CacheDir.c_str(),
/*replace=*/1);
// Ensure there are no urls to guarantee a cache miss.
setenv("DEBUGINFOD_URLS", "", /*replace=*/1);
@@ -52,4 +64,6 @@ TEST(DebuginfodClient, CacheMiss) {
Expected<std::string> PathOrErr = getCachedOrDownloadArtifact(
/*UniqueKey=*/"nonexistent-key", /*UrlPath=*/"/null");
EXPECT_THAT_EXPECTED(PathOrErr, Failed<StringError>());
+ // A cache miss with no possible URLs should not create the cache directory.
+ EXPECT_FALSE(sys::fs::exists(CacheDir));
}
More information about the llvm-commits
mailing list