llvm.org GIT mirror llvm / ec179d7
LTO: Fix a potential race condition in the caching API. After the call to sys::fs::exists succeeds, indicating a cache hit, we call AddFile and the client will open the file using the supplied path. If the client is using cache pruning, there is a potential race between the pruner and the client. To avoid this, change the caching API so that it provides a MemoryBuffer to the client, and have clients use that MemoryBuffer where possible. This scheme won't work with the gold plugin because the plugin API expects a file path. So we have the gold plugin use the buffer identifier as a path and live with the race for now. (Note that the gold plugin isn't actually affected by the problem at the moment because it doesn't support cache pruning.) This effectively reverts r279883 modulo the change to use the existing path in the gold plugin. Differential Revision: https://reviews.llvm.org/D31063 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@298020 91177308-0d34-0410-b5e6-96231b3b80d8 Peter Collingbourne 3 years ago
4 changed file(s) with 41 addition(s) and 22 deletion(s). Raw diff Collapse all Expand all
2323 /// This type defines the callback to add a pre-existing native object file
2424 /// (e.g. in a cache).
2525 ///
26 /// File callbacks must be thread safe.
27 typedef std::function AddFileFn;
26 /// MB->getBufferIdentifier() is a valid path for the file at the time that it
27 /// was opened, but clients should prefer to access MB directly in order to
28 /// avoid a potential race condition.
29 ///
30 /// Buffer callbacks must be thread safe.
31 typedef std::function MB)>
32 AddBufferFn;
2833
2934 /// Create a local file system cache which uses the given cache directory and
3035 /// file callback. This function also creates the cache directory if it does not
3136 /// already exist.
3237 Expected localCache(StringRef CacheDirectoryPath,
33 AddFileFn AddFile);
38 AddBufferFn AddBuffer);
3439
3540 } // namespace lto
3641 } // namespace llvm
2121 using namespace llvm::lto;
2222
2323 Expected lto::localCache(StringRef CacheDirectoryPath,
24 AddFileFn AddFile) {
24 AddBufferFn AddBuffer) {
2525 if (std::error_code EC = sys::fs::create_directories(CacheDirectoryPath))
2626 return errorCodeToError(EC);
2727
2929 // First, see if we have a cache hit.
3030 SmallString<64> EntryPath;
3131 sys::path::append(EntryPath, CacheDirectoryPath, Key);
32 if (sys::fs::exists(EntryPath)) {
33 AddFile(Task, EntryPath);
32 ErrorOr> MBOrErr =
33 MemoryBuffer::getFile(EntryPath);
34 if (MBOrErr) {
35 AddBuffer(Task, std::move(*MBOrErr));
3436 return AddStreamFn();
3537 }
3638
39 if (MBOrErr.getError() != std::errc::no_such_file_or_directory)
40 report_fatal_error(Twine("Failed to open cache file ") + EntryPath +
41 ": " + MBOrErr.getError().message() + "\n");
42
3743 // This native object stream is responsible for commiting the resulting
38 // file to the cache and calling AddFile to add it to the link.
44 // file to the cache and calling AddBuffer to add it to the link.
3945 struct CacheStream : NativeObjectStream {
40 AddFileFn AddFile;
46 AddBufferFn AddBuffer;
4147 std::string TempFilename;
4248 std::string EntryPath;
4349 unsigned Task;
4450
45 CacheStream(std::unique_ptr OS, AddFileFn AddFile,
51 CacheStream(std::unique_ptr OS, AddBufferFn AddBuffer,
4652 std::string TempFilename, std::string EntryPath,
4753 unsigned Task)
48 : NativeObjectStream(std::move(OS)), AddFile(std::move(AddFile)),
54 : NativeObjectStream(std::move(OS)), AddBuffer(std::move(AddBuffer)),
4955 TempFilename(std::move(TempFilename)),
5056 EntryPath(std::move(EntryPath)), Task(Task) {}
5157
5258 ~CacheStream() {
59 // FIXME: This code could race with the cache pruner, but it is unlikely
60 // that the cache pruner will choose to remove a newly created file.
61
5362 // Make sure the file is closed before committing it.
5463 OS.reset();
5564 // This is atomic on POSIX systems.
5665 if (auto EC = sys::fs::rename(TempFilename, EntryPath))
5766 report_fatal_error(Twine("Failed to rename temporary file ") +
5867 TempFilename + ": " + EC.message() + "\n");
59 AddFile(Task, EntryPath);
68
69 ErrorOr> MBOrErr =
70 MemoryBuffer::getFile(EntryPath);
71 if (!MBOrErr)
72 report_fatal_error(Twine("Failed to open cache file ") + EntryPath +
73 ": " + MBOrErr.getError().message() + "\n");
74 AddBuffer(Task, std::move(*MBOrErr));
6075 }
6176 };
6277
7691 // This CacheStream will move the temporary file into the cache when done.
7792 return llvm::make_unique(
7893 llvm::make_unique(TempFD, /* ShouldClose */ true),
79 AddFile, TempFilename.str(), EntryPath.str(), Task);
94 AddBuffer, TempFilename.str(), EntryPath.str(), Task);
8095 };
8196 };
8297 }
830830 llvm::make_unique(FD, true));
831831 };
832832
833 auto AddFile = [&](size_t Task, StringRef Path) { Filenames[Task] = Path; };
833 auto AddBuffer = [&](size_t Task, std::unique_ptr MB) {
834 // Note that this requires that the memory buffers provided to AddBuffer are
835 // backed by a file.
836 Filenames[Task] = MB->getBufferIdentifier();
837 };
834838
835839 NativeObjectCache Cache;
836840 if (!options::cache_dir.empty())
837 Cache = check(localCache(options::cache_dir, AddFile));
841 Cache = check(localCache(options::cache_dir, AddBuffer));
838842
839843 check(Lto->run(AddStream, Cache));
840844
274274 return llvm::make_unique(std::move(S));
275275 };
276276
277 auto AddFile = [&](size_t Task, StringRef Path) {
278 auto ReloadedBufferOrErr = MemoryBuffer::getFile(Path);
279 if (auto EC = ReloadedBufferOrErr.getError())
280 report_fatal_error(Twine("Can't reload cached file '") + Path + "': " +
281 EC.message() + "\n");
282
283 *AddStream(Task)->OS << (*ReloadedBufferOrErr)->getBuffer();
277 auto AddBuffer = [&](size_t Task, std::unique_ptr MB) {
278 *AddStream(Task)->OS << MB->getBuffer();
284279 };
285280
286281 NativeObjectCache Cache;
287282 if (!CacheDir.empty())
288 Cache = check(localCache(CacheDir, AddFile), "failed to create cache");
283 Cache = check(localCache(CacheDir, AddBuffer), "failed to create cache");
289284
290285 check(Lto.run(AddStream, Cache), "LTO::run failed");
291286 }