[PATCH] D45076: Prevent data races in concurrent ThinLTO processes

[Katya Romanova via Phabricator via llvm-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL328904: Prevent data races in concurrent ThinLTO processes. (authored by kromanova, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D45076?vs=140482&id=140490#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D45076

Files:
  llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp


Index: llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp
===================================================================
--- llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp
+++ llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp
@@ -400,27 +400,24 @@
 
     // Write to a temporary to avoid race condition
     SmallString<128> TempFilename;
+    SmallString<128> CachePath(EntryPath);
     int TempFD;
-    std::error_code EC =
-        sys::fs::createTemporaryFile("Thin", "tmp.o", TempFD, TempFilename);
+    llvm::sys::path::remove_filename(CachePath);
+    sys::path::append(TempFilename, CachePath, "Thin-%%%%%%.tmp.o");
+    std::error_code EC = 
+      sys::fs::createUniqueFile(TempFilename, TempFD, TempFilename);
     if (EC) {
       errs() << "Error: " << EC.message() << "\n";
       report_fatal_error("ThinLTO: Can't get a temporary file");
     }
     {
       raw_fd_ostream OS(TempFD, /* ShouldClose */ true);
       OS << OutputBuffer.getBuffer();
     }
-    // Rename to final destination (hopefully race condition won't matter here)
+    // Rename temp file to final destination; rename is atomic 
     EC = sys::fs::rename(TempFilename, EntryPath);
-    if (EC) {
+    if (EC)
       sys::fs::remove(TempFilename);
-      raw_fd_ostream OS(EntryPath, EC, sys::fs::F_None);
-      if (EC)
-        report_fatal_error(Twine("Failed to open ") + EntryPath +
-                           " to save cached entry\n");
-      OS << OutputBuffer.getBuffer();
-    }
   }
 };
 
@@ -1027,15 +1024,15 @@
         if (SavedObjectsDirectoryPath.empty()) {
           // We need to generated a memory buffer for the linker.
           if (!CacheEntryPath.empty()) {
-            // Cache is enabled, reload from the cache
-            // We do this to lower memory pressuree: the buffer is on the heap
-            // and releasing it frees memory that can be used for the next input
-            // file. The final binary link will read from the VFS cache
-            // (hopefully!) or from disk if the memory pressure wasn't too high.
+            // When cache is enabled, reload from the cache if possible. 
+            // Releasing the buffer from the heap and reloading it from the
+            // cache file with mmap helps us to lower memory pressure. 
+            // The freed memory can be used for the next input file. 
+            // The final binary link will read from the VFS cache (hopefully!)
+            // or from disk (if the memory pressure was too high).
             auto ReloadedBufferOrErr = CacheEntry.tryLoadingBuffer();
             if (auto EC = ReloadedBufferOrErr.getError()) {
-              // On error, keeping the preexisting buffer and printing a
-              // diagnostic is more friendly than just crashing.
+              // On error, keep the preexisting buffer and print a diagnostic.
               errs() << "error: can't reload cached file '" << CacheEntryPath
                      << "': " << EC.message() << "\n";
             } else {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D45076.140490.patch
Type: text/x-patch
Size: 2987 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180330/26030fc3/attachment.bin>