[all-commits] [llvm/llvm-project] 389bfb: [LLD] [COFF] Don't try to detect MSVC installation...

[R. Voggenauer via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 389bfbd66d6f78b5fc60e51e620e7f767fc867f0
      https://github.com/llvm/llvm-project/commit/389bfbd66d6f78b5fc60e51e620e7f767fc867f0
  Author: Martin Storsjö <martin at martin.st>
  Date:   2023-02-17 (Fri, 17 Feb 2023)

  Changed paths:
    M lld/COFF/Driver.cpp
    M lld/test/COFF/winsysroot.test

  Log Message:
  -----------
  [LLD] [COFF] Don't try to detect MSVC installations in mingw mode

In mingw mode, all linker paths are passed explicitly to the linker
by the compiler driver. Don't try to implicitly add linker paths
from the LIB environment variable or by detecting an MSVC
installation directory.

If the /winsysroot command line parameter is explicitly passed to
lld-link while /lldmingw is specified, it could be considered reasonable
to actually include those paths. However, modifying the code to
handle only the /winsysroot case but not the other ones, when the
mingw mode has been enabled, seems like much more code complexity
for a mostly hypothetical case.

Add a test for this when case when using LIB. (The code paths for
trying to detect an MSVC installation aren't really regression tested.)

Also fix an issue in the existing test for "Check that when /winsysroot
is specified, %LIB% is ignored.", where the LIB variable pointed
to a nonexistent directory, so the test would pass even if /winsysroot
wouldn't be specified.

Differential Revision: https://reviews.llvm.org/D144084


  Commit: a1e80c69223a091e6f0fc84df33a464604c8bbc1
      https://github.com/llvm/llvm-project/commit/a1e80c69223a091e6f0fc84df33a464604c8bbc1
  Author: R. Voggenauer <rvogg at users.noreply.github.com>
  Date:   2023-02-17 (Fri, 17 Feb 2023)

  Changed paths:
    M llvm/lib/Support/Windows/Path.inc

  Log Message:
  -----------
  [Support] [Windows] Don't check file access time in equivalent(file_status, file_status)

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.

Differential Revision: https://reviews.llvm.org/D144172


Compare: https://github.com/llvm/llvm-project/compare/3d84f4268dd9...a1e80c69223a