[PATCH] D144172: [Support] [Windows] Don't check file access time in equivalent(file_status, file_status)

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo created this revision.
mstorsjo added reviewers: mehdi_amini, alvinhochun, mati865, hans, thakis.
Herald added a subscriber: hiraditya.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: LLVM.

The sys::fs::equivalent(file_status, file_status) function is meant to
judge whether two file_status structs denote the same individual file.
On Unix, this is implemented by only comparing the st_dev and st_ino
numbers (from stat), ignoring all other fields.

On Windows, lacking reliable fields corresponding to st_dev and st_ino,
equivalent(file_status, file_status) compares essentially all fields.
However, since 1e39ef331b2b78baec84fdb577d497511cc46bd5
(https://reviews.llvm.org/D18456), file_status also contains the file
access time.

Including the file access time in equivalent(file_status, file_status)
makes it possible to spuriously break. In particular, when invoking
equivalent(Twine, Twine), with two paths, there's a race condition - the
function calls status() on both input paths. Even if the two paths
are the same, the comparison can fail, if the file was accessed
between the two status() calls.

Thus, it seems like the inclusion of the access time in
equivalent(file_status, file_status) was a mistake.

This race condition can cause spurious failures when linking with
LLD/ELF, where LLD uses equivalent() to check whether a file
exists within a specific sysroot, and that sysroot is accessed by other
processes concurrently.

This fixes downstream issue
https://github.com/msys2/MINGW-packages/issues/15695.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144172

Files:
  llvm/lib/Support/Windows/Path.inc


Index: llvm/lib/Support/Windows/Path.inc
===================================================================
--- llvm/lib/Support/Windows/Path.inc
+++ llvm/lib/Support/Windows/Path.inc
@@ -650,8 +650,6 @@
   return A.FileIndexHigh == B.FileIndexHigh &&
          A.FileIndexLow == B.FileIndexLow && A.FileSizeHigh == B.FileSizeHigh &&
          A.FileSizeLow == B.FileSizeLow &&
-         A.LastAccessedTimeHigh == B.LastAccessedTimeHigh &&
-         A.LastAccessedTimeLow == B.LastAccessedTimeLow &&
          A.LastWriteTimeHigh == B.LastWriteTimeHigh &&
          A.LastWriteTimeLow == B.LastWriteTimeLow &&
          A.VolumeSerialNumber == B.VolumeSerialNumber;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D144172.497933.patch
Type: text/x-patch
Size: 667 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230216/4dfa37c3/attachment.bin>