LTO: don't fatal when value for cache key already exists

[inglorion]

Summary

LTO/Caching.cpp uses file rename to atomically set the value for a
cache key. On Windows, this fails when the destination file already
exists. Previously, LLVM would report_fatal_error in such
cases. However, because the old and the new value for the cache key
are supposed to be equivalent, it actually doesn't matter which one we
keep. This change makes it so that failing the rename when an openable
file with the desired name already exists causes us to report success
instead of fataling.

[pcc]

llvm/lib/LTO/Caching.cpp

83

I think this can race with the cache pruner.

Maybe in this case you could create a copy of MBOrErr in memory, then close and delete the temporary file and return the copy to the client.

[inglorion]

changes requested by @pcc

[inglorion]

no need to inclune Support/Process.h anymore

[pcc]

LGTM

llvm/lib/Support/Windows/Path.inc

737–739

This is part of an unrelated change, right?

[inglorion]

rebased to remove unrelated changes

llvm/lib/LTO/Caching.cpp

 //===----------------------------------------------------------------------===//
 
 #include "llvm/LTO/Caching.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Process.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace llvm;
 using namespace llvm::lto;
 
 Expected<NativeObjectCache> lto::localCache(StringRef CacheDirectoryPath,
                                             AddBufferFn AddBuffer) {
   if (std::error_code EC = sys::fs::create_directories(CacheDirectoryPath))
struct CacheStream : NativeObjectStream {
         ErrorOr<std::unique_ptr<MemoryBuffer>> MBOrErr =
             MemoryBuffer::getFile(TempFilename);
         if (!MBOrErr)
           report_fatal_error(Twine("Failed to open new cache file ") +
                              TempFilename + ": " +
                              MBOrErr.getError().message() + "\n");
 
         // This is atomic on POSIX systems.
-        if (auto EC = sys::fs::rename(TempFilename, EntryPath))
+        // On Windows, it can fail with permission denied if the destination
+        // file already exists. Since the existing file should be semantically
+        // equivalent to the one we are trying to write, we give AddBuffer
+        // a copy of the bytes we wrote in that case. We do this instead of
+        // just using the existing file, because the pruner might delete the
+        // file before we get a chance to use it.
+        auto EC = sys::fs::rename(TempFilename, EntryPath);
+        if (EC == errc::permission_denied) {
+          auto MBCopy = MemoryBuffer::getMemBufferCopy(
+              (*MBOrErr)->getBuffer(), EntryPath);
+          MBOrErr = std::move(MBCopy);
+          sys::fs::remove(TempFilename);
+        } else if (EC) {
           report_fatal_error(Twine("Failed to rename temporary file ") +
                              TempFilename + " to " + EntryPath + ": " +
                              EC.message() + "\n");
+        }
 
         AddBuffer(Task, std::move(*MBOrErr), EntryPath);
       }
     };
 
     return [=](size_t Task) -> std::unique_ptr<NativeObjectStream> {
       // Write to a temporary to avoid race condition
       int TempFD;

llvm/lib/Support/Windows/Path.inc

std::error_code mapped_file_region::init(int FD, uint64_t Offset,
   switch (Mode) {
   case readonly:  flprotect = PAGE_READONLY; break;
   case readwrite: flprotect = PAGE_READWRITE; break;
   case priv:      flprotect = PAGE_WRITECOPY; break;
   }
 
   HANDLE FileMappingHandle =
       ::CreateFileMappingW(FileHandle, 0, flprotect,
-                           (Offset + Size) >> 32,
-                           (Offset + Size) & 0xffffffff,
+                           Size >> 32,
+                           Size & 0xffffffff,
                            0);
   if (FileMappingHandle == NULL) {
     std::error_code ec = mapWindowsError(GetLastError());
     return ec;
   }
 
   DWORD dwDesiredAccess;
   switch (Mode) {
   case readonly:  dwDesiredAccess = FILE_MAP_READ; break;