Update thin-lto cache file atimes when on windows

[gbreynoo]

Summary

Thin-lto cache file atimes are used for expiration based pruning, and since Vista, access file times are not updated by Windows by default:
https://blogs.technet.microsoft.com/filecab/2006/11/07/disabling-last-access-time-in-windows-vista-to-improve-ntfs-performance/

This means on Windows, cache files are currently being pruned from creation time. This change manually updates cache files that are accessed by Thin-lto, when on Windows.

[dmajor]

I don't know this code well enough to be a proper reviewer, but I want to offer moral support for this change. Not having atime updates on NTFS can be frustrating at times.

include/llvm/Support/FileSystem.h

1004

Needs a @param

lib/Support/Windows/Path.inc

1153

This needs a CloseHandle here, right?

[gbreynoo]

Updated to a recent revision and made changes suggested by dmajor

[pcc]

lib/LTO/Caching.cpp

37

I'd rather not add another mysterious bool parameter to getFile for something that only one client needs. I'd prefer if the atime update were made more explicit. Specifically, I think we should do this:

• add an OpenFlag that means "update atime"
• change this code to use openFileForRead passing the new OpenFlag to obtain an FD, and then pass the FD to MemoryBuffer::getOpenFile().

[gbreynoo]

Updated to recent revision and made changes suggested by pcc

[gbreynoo]

ping

[pcc]

You should be able to avoid adding the bool argument to openNativeFileInternal as well.

include/llvm/Support/FileSystem.h

726

Nit: add space between /// and the start of the comment.

lib/LTO/Caching.cpp

37

I think this is leaking the FD; you need to close it after creating the map. Same in ThinLTOCodeGenerator.cpp.

lib/Support/Windows/Path.inc

1133

Move this into nativeAccess.

1180–1181

Move the call to updateAccessTime here (or better yet just inline the code).

[gbreynoo]

Updated to recent revision and made changes suggested by pcc

[pcc]

LGTM

lib/Support/Windows/Path.inc

1189

Close the file handle here.

[gbreynoo]

Updated to recent revision and made change suggested by pcc

include/llvm/Support/FileSystem.h

enum OpenFlags : unsigned {
 
   /// Delete the file on close. Only makes a difference on windows.
   OF_Delete = 4,
 
   /// When a child process is launched, this file should remain open in the
   /// child process.
   OF_ChildInherit = 8,
 };
 
 /// Create a uniquely named file.
 ///
 /// Generates a unique path suitable for a temporary file and then opens it as a
 /// file. The name is based on \a model with '%' replaced by a random char in
 /// [0-9a-f]. If \a model is not an absolute path, the temporary file will be
 /// created in the current directory.
 ///
 /// Example: clang-%%-%%-%%-%%-%%.s => clang-a0-b1-c2-d3-e4.s

 /// finished with it.
 ///
 /// @param Name The path of the file to open, relative or absolute.
 /// @param Disp Value specifying the existing-file behavior.
 /// @param Access Value specifying whether to open the file in read, write, or
 ///               read-write mode.
 /// @param Flags Additional flags.
 /// @param Mode The access permissions of the file, represented in octal.
+///
+/// @param ForceUpdateAtime Set to true to force the files atime to be
+/// updated, when on Windows systems that do not have atime updated by
+/// default.
 /// @returns errc::success if \a Name has been opened, otherwise a
 ///          platform-specific error_code.
 Expected<file_t> openNativeFile(const Twine &Name, CreationDisposition Disp,
                                 FileAccess Access, OpenFlags Flags,
-                                unsigned Mode = 0666);
+                                unsigned Mode = 0666,
+                                bool ForceUpdateAtime = false);
 
 /// @brief Opens the file with the given name in a write-only or read-write
 /// mode, returning its open file descriptor. If the file does not exist, it
 /// is created.
 ///
 /// The caller is responsible for closing the file descriptor once they are
 /// finished with it.
 ///

 /// finished with it.
 ///
 /// @param Name The path of the file to open, relative or absolute.
 /// @param ResultFD If the file could be opened successfully, its descriptor
 ///                 is stored in this location. Otherwise, this is set to -1.
 /// @param RealPath If nonnull, extra work is done to determine the real path
 ///                 of the opened file, and that path is stored in this
 ///                 location.
+///
+/// @param ForceUpdateAtime Set to true to force the files atime to be
+/// updated, when on Windows systems that do not have atime updated by
+/// default.
 /// @returns errc::success if \a Name has been opened, otherwise a
 ///          platform-specific error_code.
 std::error_code openFileForRead(const Twine &Name, int &ResultFD,
                                 OpenFlags Flags = OF_None,
-                                SmallVectorImpl<char> *RealPath = nullptr);
+                                SmallVectorImpl<char> *RealPath = nullptr,
+                                bool ForceUpdateAtime = false);
 
 /// @brief Opens the file with the given name in a read-only mode, returning
 /// its open file descriptor.
 ///
 /// The caller is responsible for closing the freeing the file once they are
 /// finished with it.
 ///
 /// @param Name The path of the file to open, relative or absolute.
 /// @param RealPath If nonnull, extra work is done to determine the real path
 ///                 of the opened file, and that path is stored in this
 ///                 location.
+///
+/// @param ForceUpdateAtime Set to true to force the files atime to be
+/// updated, when on Windows systems that do not have atime updated by
+/// default.
 /// @returns a platform-specific file descriptor if \a Name has been opened,
 ///          otherwise an error object.
 Expected<file_t>
 openNativeFileForRead(const Twine &Name, OpenFlags Flags = OF_None,
-                      SmallVectorImpl<char> *RealPath = nullptr);
+                      SmallVectorImpl<char> *RealPath = nullptr,
+                      bool ForceUpdateAtime = false);
 
 /// @brief Close the file object.  This should be used instead of ::close for
 /// portability.
 ///
 /// @param F On input, this is the file to close.  On output, the file is
 /// set to kInvalidFile.
 void closeFile(file_t &F);
 

include/llvm/Support/MemoryBuffer.h

public:
   /// Open the specified file as a MemoryBuffer, returning a new MemoryBuffer
   /// if successful, otherwise returning null. If FileSize is specified, this
   /// means that the client knows that the file exists and that it has the
   /// specified size.
   ///
   /// \param IsVolatile Set to true to indicate that the contents of the file
   /// can change outside the user's control, e.g. when libclang tries to parse
   /// while the user is editing/updating the file or if the file is on an NFS.
+  ///
+  /// \param ForceUpdateAtime Set to true to force the files atime to be
+  /// updated, when on Windows systems that do not have atime updated by
+  /// default.
   static ErrorOr<std::unique_ptr<MemoryBuffer>>
   getFile(const Twine &Filename, int64_t FileSize = -1,
-          bool RequiresNullTerminator = true, bool IsVolatile = false);
+          bool RequiresNullTerminator = true, bool IsVolatile = false,
+          bool ForceUpdateAtime = false);
 
   /// Read all of the specified file into a MemoryBuffer as a stream
   /// (i.e. until EOF reached). This is useful for special files that
   /// look like a regular file but have 0 size (e.g. /proc/cpuinfo on Linux).
   static ErrorOr<std::unique_ptr<MemoryBuffer>>
   getFileAsStream(const Twine &Filename);
 
   /// Given an already-open file descriptor, map some slice of it into a

lib/LTO/Caching.cpp

Expected<NativeObjectCache> lto::localCache(StringRef CacheDirectoryPath,
 
   return [=](unsigned Task, StringRef Key) -> AddStreamFn {
     // 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, "llvmcache-" + Key);
     // First, see if we have a cache hit.
     ErrorOr<std::unique_ptr<MemoryBuffer>> MBOrErr =
-        MemoryBuffer::getFile(EntryPath);
+        MemoryBuffer::getFile(EntryPath, -1, true, false, true);
     if (MBOrErr) {
       AddBuffer(Task, std::move(*MBOrErr));
       return AddStreamFn();
     }
 
     if (MBOrErr.getError() != errc::no_such_file_or_directory)
       report_fatal_error(Twine("Failed to open cache file ") + EntryPath +
                          ": " + MBOrErr.getError().message() + "\n");

lib/LTO/ThinLTOCodeGenerator.cpp

#endif
 
   // 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())
       return std::error_code();
-    return MemoryBuffer::getFile(EntryPath);
+    return MemoryBuffer::getFile(EntryPath, -1, true, false, true);
   }
 
   // Cache the Produced object file
   void write(const MemoryBuffer &OutputBuffer) {
     if (EntryPath.empty())
       return;
 
     // Write to a temporary to avoid race condition

lib/Support/MemoryBuffer.cpp

MemoryBuffer::BufferKind getBufferKind() const override {
     return MemoryBuffer::MemoryBuffer_Malloc;
   }
 };
 }
 
 template <typename MB>
 static ErrorOr<std::unique_ptr<MB>>
 getFileAux(const Twine &Filename, int64_t FileSize, uint64_t MapSize,
-           uint64_t Offset, bool RequiresNullTerminator, bool IsVolatile);
+           uint64_t Offset, bool RequiresNullTerminator, bool IsVolatile,
+           bool ForceUpdateAtime);
 
 std::unique_ptr<MemoryBuffer>
 MemoryBuffer::getMemBuffer(StringRef InputData, StringRef BufferName,
                            bool RequiresNullTerminator) {
   auto *Ret = new (NamedBufferAlloc(BufferName))
       MemoryBufferMem<MemoryBuffer>(InputData, RequiresNullTerminator);
   return std::unique_ptr<MemoryBuffer>(Ret);
 }
 
 std::unique_ptr<MemoryBuffer>
 MemoryBuffer::getMemBuffer(MemoryBufferRef Ref, bool RequiresNullTerminator) {
   return std::unique_ptr<MemoryBuffer>(getMemBuffer(
       Ref.getBuffer(), Ref.getBufferIdentifier(), RequiresNullTerminator));
 }
 
 static ErrorOr<std::unique_ptr<WritableMemoryBuffer>>
 getMemBufferCopyImpl(StringRef InputData, const Twine &BufferName) {
-  auto Buf = WritableMemoryBuffer::getNewUninitMemBuffer(InputData.size(), BufferName);
+  auto Buf =
+      WritableMemoryBuffer::getNewUninitMemBuffer(InputData.size(), BufferName);
   if (!Buf)
     return make_error_code(errc::not_enough_memory);
   memcpy(Buf->getBufferStart(), InputData.data(), InputData.size());
   return std::move(Buf);
 }
 
 std::unique_ptr<MemoryBuffer>
 MemoryBuffer::getMemBufferCopy(StringRef InputData, const Twine &BufferName) {
MemoryBuffer::getFileOrSTDIN(const Twine &Filename, int64_t FileSize,
   StringRef NameRef = Filename.toStringRef(NameBuf);
 
   if (NameRef == "-")
     return getSTDIN();
   return getFile(Filename, FileSize, RequiresNullTerminator);
 }
 
 ErrorOr<std::unique_ptr<MemoryBuffer>>
-MemoryBuffer::getFileSlice(const Twine &FilePath, uint64_t MapSize, 
+MemoryBuffer::getFileSlice(const Twine &FilePath, uint64_t MapSize,
                            uint64_t Offset, bool IsVolatile) {
   return getFileAux<MemoryBuffer>(FilePath, -1, MapSize, Offset, false,
-                                  IsVolatile);
+                                  IsVolatile, /*ForceUpdateAtime*/ false);
 }
 
 //===----------------------------------------------------------------------===//
 // MemoryBuffer::getFile implementation.
 //===----------------------------------------------------------------------===//
 
 namespace {
 /// Memory maps a file descriptor using sys::fs::mapped_file_region.
 ///
 /// This handles converting the offset into a legal offset on the platform.
-template<typename MB>
-class MemoryBufferMMapFile : public MB {
+template <typename MB> class MemoryBufferMMapFile : public MB {
   sys::fs::mapped_file_region MFR;
 
   static uint64_t getLegalMapOffset(uint64_t Offset) {
     return Offset & ~(sys::fs::mapped_file_region::alignment() - 1);
   }
 
   static uint64_t getLegalMapSize(uint64_t Len, uint64_t Offset) {
     return Len + (Offset - getLegalMapOffset(Offset));
do {
     if (ReadBytes == -1)
       return std::error_code(errno, std::generic_category());
     Buffer.set_size(Buffer.size() + ReadBytes);
   } while (ReadBytes != 0);
 
   return getMemBufferCopyImpl(Buffer, BufferName);
 }
 
-
 ErrorOr<std::unique_ptr<MemoryBuffer>>
 MemoryBuffer::getFile(const Twine &Filename, int64_t FileSize,
-                      bool RequiresNullTerminator, bool IsVolatile) {
+                      bool RequiresNullTerminator, bool IsVolatile,
+                      bool ForceUpdateAtime) {
   return getFileAux<MemoryBuffer>(Filename, FileSize, FileSize, 0,
-                                  RequiresNullTerminator, IsVolatile);
+                                  RequiresNullTerminator, IsVolatile,
+                                  ForceUpdateAtime);
 }
 
 template <typename MB>
 static ErrorOr<std::unique_ptr<MB>>
 getOpenFileImpl(int FD, const Twine &Filename, uint64_t FileSize,
                 uint64_t MapSize, int64_t Offset, bool RequiresNullTerminator,
                 bool IsVolatile);
 
 template <typename MB>
 static ErrorOr<std::unique_ptr<MB>>
 getFileAux(const Twine &Filename, int64_t FileSize, uint64_t MapSize,
-           uint64_t Offset, bool RequiresNullTerminator, bool IsVolatile) {
+           uint64_t Offset, bool RequiresNullTerminator, bool IsVolatile,
+           bool ForceUpdateAtime) {
   int FD;
-  std::error_code EC = sys::fs::openFileForRead(Filename, FD, sys::fs::OF_None);
+  std::error_code EC = sys::fs::openFileForRead(Filename, FD, sys::fs::OF_None,
+                                                nullptr, ForceUpdateAtime);
 
   if (EC)
     return EC;
 
   auto Ret = getOpenFileImpl<MB>(FD, Filename, FileSize, MapSize, Offset,
                                  RequiresNullTerminator, IsVolatile);
   close(FD);
   return Ret;
 }
 
 ErrorOr<std::unique_ptr<WritableMemoryBuffer>>
 WritableMemoryBuffer::getFile(const Twine &Filename, int64_t FileSize,
                               bool IsVolatile) {
-  return getFileAux<WritableMemoryBuffer>(Filename, FileSize, FileSize, 0,
-                                          /*RequiresNullTerminator*/ false,
-                                          IsVolatile);
+  return getFileAux<WritableMemoryBuffer>(
+      Filename, FileSize, FileSize, 0,
+      /*RequiresNullTerminator*/ false, IsVolatile, /*ForceUpdateAtime*/ false);
 }
 
 ErrorOr<std::unique_ptr<WritableMemoryBuffer>>
 WritableMemoryBuffer::getFileSlice(const Twine &Filename, uint64_t MapSize,
                                    uint64_t Offset, bool IsVolatile) {
   return getFileAux<WritableMemoryBuffer>(Filename, -1, MapSize, Offset, false,
-                                          IsVolatile);
+                                          IsVolatile,
+                                          /*ForceUpdateAtime*/ false);
 }
 
 std::unique_ptr<WritableMemoryBuffer>
 WritableMemoryBuffer::getNewUninitMemBuffer(size_t Size, const Twine &BufferName) {
   using MemBuffer = MemoryBufferMem<WritableMemoryBuffer>;
   // Allocate space for the MemoryBuffer, the data and the name. It is important
   // that MemoryBuffer and data are aligned so PointerIntPair works with them.
   // TODO: Is 16-byte alignment enough?  We copy small object files with large

lib/Support/Unix/Path.inc

Expected<int> openNativeFile(const Twine &Name, CreationDisposition Disp,
   std::error_code EC = openFile(Name, FD, Disp, Access, Flags, Mode);
   if (EC)
     return errorCodeToError(EC);
   return FD;
 }
 
 std::error_code openFileForRead(const Twine &Name, int &ResultFD,
                                 OpenFlags Flags,
-                                SmallVectorImpl<char> *RealPath) {
+                                SmallVectorImpl<char> *RealPath,
+                                bool /* ForceUpdateAtime */) {
   std::error_code EC =
       openFile(Name, ResultFD, CD_OpenExisting, FA_Read, Flags, 0666);
   if (EC)
     return EC;
 
   // Attempt to get the real name of the file, if the user asked
   if(!RealPath)
     return std::error_code();

lib/Support/Windows/Path.inc

if (Access & FA_Read)
     Result |= GENERIC_READ;
   if (Access & FA_Write)
     Result |= GENERIC_WRITE;
   if (Flags & OF_Delete)
     Result |= DELETE;
   return Result;
 }
 
+std::error_code updateAccessTime (HANDLE Handle) {
+  FILETIME FileTime;
+  SYSTEMTIME SystemTime;
+  GetSystemTime(&SystemTime);
+  if (SystemTimeToFileTime(&SystemTime, &FileTime) != 0) {
+    if (SetFileTime(Handle, NULL, &FileTime, NULL) != 0) {
+      return std::error_code();
+    }
+  }
+  DWORD LastError = ::GetLastError();
+  return mapWindowsError(LastError);
+}
+
 static std::error_code openNativeFileInternal(const Twine &Name,
                                               file_t &ResultFile, DWORD Disp,
                                               DWORD Access, DWORD Flags,
-                                              bool Inherit = false) {
+                                              bool Inherit = false,
+                                              bool ForceUpdateAtime = false) {
   SmallVector<wchar_t, 128> PathUTF16;
   if (std::error_code EC = widenPath(Name, PathUTF16))
     return EC;
 
   SECURITY_ATTRIBUTES SA;
   SA.nLength = sizeof(SA);
   SA.lpSecurityDescriptor = nullptr;
   SA.bInheritHandle = Inherit;
 
+  if (ForceUpdateAtime)
+    Access |= FILE_WRITE_ATTRIBUTES;
+
   HANDLE H =
       ::CreateFileW(PathUTF16.begin(), Access,
                     FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, &SA,
                     Disp, Flags, NULL);
   if (H == INVALID_HANDLE_VALUE) {
     DWORD LastError = ::GetLastError();
     std::error_code EC = mapWindowsError(LastError);
     // Provide a better error message when trying to open directories.
     // This only runs if we failed to open the file, so there is probably
     // no performances issues.
     if (LastError != ERROR_ACCESS_DENIED)
       return EC;
     if (is_directory(Name))
       return make_error_code(errc::is_a_directory);
     return EC;
   }
   ResultFile = H;
+  if (ForceUpdateAtime) {
+    if (std::error_code EC = updateAccessTime(H))
+      return EC;
+  }
+
   return std::error_code();
 }
 
 Expected<file_t> openNativeFile(const Twine &Name, CreationDisposition Disp,
                                 FileAccess Access, OpenFlags Flags,
-                                unsigned Mode) {
+                                unsigned Mode, bool ForceUpdateAtime) {
   // Verify that we don't have both "append" and "excl".
   assert((!(Disp == CD_CreateNew) || !(Flags & OF_Append)) &&
          "Cannot specify both 'CreateNew' and 'Append' file creation flags!");
 
   DWORD NativeFlags = nativeOpenFlags(Flags);
   DWORD NativeDisp = nativeDisposition(Disp, Flags);
   DWORD NativeAccess = nativeAccess(Access, Flags);
 
   bool Inherit = false;
   if (Flags & OF_ChildInherit)
     Inherit = true;
 
   file_t Result;
-  std::error_code EC = openNativeFileInternal(
-      Name, Result, NativeDisp, NativeAccess, NativeFlags, Inherit);
+  std::error_code EC =
+      openNativeFileInternal(Name, Result, NativeDisp, NativeAccess,
+                             NativeFlags, Inherit, ForceUpdateAtime);
   if (EC)
     return errorCodeToError(EC);
   return Result;
 }
 
 std::error_code openFile(const Twine &Name, int &ResultFD,
                          CreationDisposition Disp, FileAccess Access,
                          OpenFlags Flags, unsigned int Mode) {
   Expected<file_t> Result = openNativeFile(Name, Disp, Access, Flags);
   if (!Result)
     return errorToErrorCode(Result.takeError());
 
   return nativeFileToFd(*Result, ResultFD, Flags);
 }
 
 static std::error_code directoryRealPath(const Twine &Name,
                                          SmallVectorImpl<char> &RealPath) {
   file_t File;
   std::error_code EC = openNativeFileInternal(
       Name, File, OPEN_EXISTING, GENERIC_READ, FILE_FLAG_BACKUP_SEMANTICS);
   if (EC)
     return EC;
 
   EC = realPathFromHandle(File, RealPath);
   ::CloseHandle(File);
   return EC;
 }
 
 std::error_code openFileForRead(const Twine &Name, int &ResultFD,
                                 OpenFlags Flags,
-                                SmallVectorImpl<char> *RealPath) {
-  Expected<HANDLE> NativeFile = openNativeFileForRead(Name, Flags, RealPath);
+                                SmallVectorImpl<char> *RealPath,
+                                bool ForceUpdateAtime) {
+  Expected<HANDLE> NativeFile =
+      openNativeFileForRead(Name, Flags, RealPath, ForceUpdateAtime);
   return nativeFileToFd(std::move(NativeFile), ResultFD, OF_None);
 }
 
 Expected<file_t> openNativeFileForRead(const Twine &Name, OpenFlags Flags,
-                                       SmallVectorImpl<char> *RealPath) {
-  Expected<file_t> Result =
-      openNativeFile(Name, CD_OpenExisting, FA_Read, Flags);
+                                       SmallVectorImpl<char> *RealPath,
+                                       bool ForceUpdateAtime) {
+  Expected<file_t> Result = openNativeFile(Name, CD_OpenExisting, FA_Read,
+                                           Flags, 0666, ForceUpdateAtime);
 
   // Fetch the real name of the file, if the user asked
   if (Result && RealPath)
     realPathFromHandle(*Result, *RealPath);
 
   return Result;
 }
 

test/ThinLTO/X86/cache.ll

 ; llvmcache.timestamp is current,
 ; cache file is older than default cache expiration value.
 ; RUN: rm -Rf %t.cache && mkdir %t.cache
 ; RUN: touch -t 197001011200 %t.cache/llvmcache-foo
 ; RUN: touch %t.cache/llvmcache.timestamp
 ; RUN: llvm-lto -thinlto-action=run -exported-symbol=globalfunc %t2.bc %t.bc -thinlto-cache-dir %t.cache --thinlto-cache-pruning-interval 0
 ; RUN: not ls %t.cache/llvmcache-foo
 
+; Populate the cache with files with "old" access times, then check llvm-lto updates these file times
+; A negative pruning interval is used to avoid removing cache entries
+; RUN: rm -Rf %t.cache && mkdir %t.cache
+; RUN: llvm-lto -thinlto-action=run -exported-symbol=globalfunc %t2.bc %t.bc -thinlto-cache-dir %t.cache
+; RUN: touch -a -t 197001011200 %t.cache/llvmcache-*
+; RUN: llvm-lto -thinlto-action=run -exported-symbol=globalfunc %t2.bc %t.bc -thinlto-cache-dir %t.cache --thinlto-cache-pruning-interval -1
+; RUN: ls -ltu %t.cache/* | not grep 1970-01-01
+
+; Populate the cache with files with "old" access times, then check llvm-lto2 updates these file times
+; 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:  -r=%t2.bc,_globalfunc,lx \
+; RUN:  -r=%t.bc,_globalfunc,plx
+; RUN: touch -a -t 197001011200 %t.cache/llvmcache-*
+; RUN: llvm-lto2 run -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 -ltu %t.cache/* | not grep 1970-01-01
+
 ; Verify that specifying max size for the cache directory prunes it to this
 ; size, removing the largest files first.
 ; RUN: rm -Rf %t.cache && mkdir %t.cache
 ; Create cache files with different sizes.
 ; Only 8B, 16B and 76B files should stay after pruning.
 ; RUN: "%python" -c "with open(r'%t.cache/llvmcache-foo-1024', 'w') as file: file.truncate(1024)"
 ; RUN: "%python" -c "with open(r'%t.cache/llvmcache-foo-16', 'w') as file: file.truncate(16)"
 ; RUN: "%python" -c "with open(r'%t.cache/llvmcache-foo-8', 'w') as file: file.truncate(8)"

 ; RUN: not ls %t.cache/llvmcache-foo-77
 
 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
-}
+}
 No newline at end of file

unittests/Support/Path.cpp

 #include "llvm/Support/Host.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/raw_ostream.h"
 #include "gtest/gtest.h"
 #include "gmock/gmock.h"
 
 #ifdef _WIN32
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/Support/Chrono.h"
 #include <windows.h>
 #include <winerror.h>
 #endif
 
 #ifdef LLVM_ON_UNIX
 #include <pwd.h>
 #include <sys/stat.h>
 #endif
TEST_F(FileSystemTest, OpenFileForRead) {
   if (!ResultPath.empty()) {
     // The paths returned by createTemporaryFile and getPathFromOpenFD
     // should reference the same file on disk.
     fs::UniqueID D1, D2;
     ASSERT_NO_ERROR(fs::getUniqueID(Twine(TempPath), D1));
     ASSERT_NO_ERROR(fs::getUniqueID(Twine(ResultPath), D2));
     ASSERT_EQ(D1, D2);
   }
+  ::close(FileDescriptor);
+  ::close(FileDescriptor2);
+
+#ifdef _WIN32
+  // Since Windows Vista, file access time is not updated by default.
+  // This is instead updated manually by openFileForRead.
+  // https://blogs.technet.microsoft.com/filecab/2006/11/07/disabling-last-access-time-in-windows-vista-to-improve-ntfs-performance/
+  // This part of the unit test is Windows specific as the updating of
+  // access times can be disabled on Linux using /etc/fstab.
+
+  // Set access time to UNIX epoch.
+  ASSERT_NO_ERROR(sys::fs::openFileForWrite(Twine(TempPath), FileDescriptor,
+                                            fs::CD_OpenExisting));
+  TimePoint<> Epoch(std::chrono::milliseconds(0));
+  ASSERT_NO_ERROR(fs::setLastModificationAndAccessTime(FileDescriptor, Epoch));
+  ::close(FileDescriptor);
 
+  // Open the file and ensure access time is updated, when forced.
+  bool ForceATime = true;
+  ASSERT_NO_ERROR(fs::openFileForRead(Twine(TempPath), FileDescriptor,
+                                      fs::OF_None, &ResultPath, ForceATime));
+
+  sys::fs::file_status Status;
+  ASSERT_NO_ERROR(sys::fs::status(FileDescriptor, Status));
+  auto FileAccessTime = Status.getLastAccessedTime();
+
+  ASSERT_NE(Epoch, FileAccessTime);
   ::close(FileDescriptor);
+
+  // Ideally this test would include a case when ATime is not forced to update,
+  // however the expected behaviour will differ depending on the configuration
+  // of the Windows file system.
+#endif
 }
 
 static void createFileWithData(const Twine &Path, bool ShouldExistBefore,
                                fs::CreationDisposition Disp, StringRef Data) {
   int FD;
   ASSERT_EQ(ShouldExistBefore, fs::exists(Path));
   ASSERT_NO_ERROR(fs::openFileForWrite(Path, FD, Disp));
   FileDescriptorCloser Closer(FD);