ScanDirForExecutable on Windows fails to find executables with the "exe" extension in name

[iid_iunknown]

Summary

Hello Reid

This is a patch for the problem we discussed some time ago. Would you be able to take a look and share your opinion, please?
Thank you.

Description

When the driver tries to locate a program by its name, e.g. a linker, it scans the paths provided by the toolchain using the ScanDirForExecutable function. If the lookup fails, the driver uses llvm::sys::findProgramByName. Unlike llvm::sys::findProgramByName, ScanDirForExecutable is not aware of file extensions. If the program has the "exe" extension in its name, which is very common on Windows, ScanDirForExecutable won't find it under the toolchain-provided paths.

This patch changes the Windows version of the "access" function called by ScanDirForExecutable to respect file extensions, similarly to llvm::sys::findProgramByName.

[rnk]

I'm not sure I like this approach. This makes the following code do the wrong thing when foo.exe exists but foo doesn't:

if (exists("foo"))
  // try to read it and fail

I think a better approach would be to change only can_execute to deal with PATHEXT.

[iid_iunknown]

In D12711#242038, @rnk wrote:

This makes the following code do the wrong thing...

Thank you for your remark! Yes, you are right. I will update the patch.

[iid_iunknown]

The patch updated according to the Reid's remarks. File extensions are now handled by sys::fs::can_execute.

[rnk]

Seems pretty close. Do you need someone to commit this for you?

lib/Support/Windows/Path.inc

309–310

I think can_execute should really be tied to what llvm::sys::ExecteAndWait can run. ExecuteAndWait uses CreateProcess, which ignores PATHEXT, but does check for files ending in .exe. So, if possible, I would simplify this whole thing to:

return !access(Path, AccessMode::Execute) || !access(Path + ".exe", AccessMode::Execute);

[iid_iunknown]

In D12711#242494, @rnk wrote:

Do you need someone to commit this for you?

Yes.

lib/Support/Windows/Path.inc

309–310

Yes, other extensions are not that common for Windows executables. Anyway, it would be fine for my case.
Does it mean findProgramByName may be simplified in the same way?

[iid_iunknown]

sys::fs::can_execute now ignores PATHEXT similarly to ExecuteAndWait. The "exe" extension is used only.

include/llvm/Support/FileSystem.h

 inline bool exists(const Twine &Path) {
   return !access(Path, AccessMode::Exist);
 }
 
 /// @brief Can we execute this file?
 ///
 /// @param Path Input path.
 /// @returns True if we can execute it, false otherwise.
-inline bool can_execute(const Twine &Path) {
-  return !access(Path, AccessMode::Execute);
-}
+bool can_execute(const Twine &Path);
 
 /// @brief Can we write this file?
 ///
 /// @param Path Input path.
 /// @returns True if we can write to it, false otherwise.
 inline bool can_write(const Twine &Path) {
   return !access(Path, AccessMode::Write);
 }

lib/Support/Unix/Path.inc

if (0 != stat(P.begin(), &buf))
       return errc::permission_denied;
     if (!S_ISREG(buf.st_mode))
       return errc::permission_denied;
   }
 
   return std::error_code();
 }
 
+bool can_execute(const Twine &Path) {
+  return !access(Path, AccessMode::Execute);
+}
+
 bool equivalent(file_status A, file_status B) {
   assert(status_known(A) && status_known(B));
   return A.fs_st_dev == B.fs_st_dev &&
          A.fs_st_ino == B.fs_st_ino;
 }
 
 std::error_code equivalent(const Twine &A, const Twine &B, bool &result) {
   file_status fsA, fsB;

lib/Support/Windows/Path.inc

std::error_code access(const Twine &Path, AccessMode Mode) {
   }
 
   if (Mode == AccessMode::Write && (Attributes & FILE_ATTRIBUTE_READONLY))
     return errc::permission_denied;
 
   return std::error_code();
 }
 
+bool can_execute(const Twine &Path) {
+  return !access(Path, AccessMode::Execute) ||
+         !access(Path + ".exe", AccessMode::Execute);
+}
+
 bool equivalent(file_status A, file_status B) {
   assert(status_known(A) && status_known(B));
   return A.FileIndexHigh      == B.FileIndexHigh &&
          A.FileIndexLow       == B.FileIndexLow &&
          A.FileSizeHigh       == B.FileSizeHigh &&
          A.FileSizeLow        == B.FileSizeLow &&
          A.LastWriteTimeHigh  == B.LastWriteTimeHigh &&
          A.LastWriteTimeLow   == B.LastWriteTimeLow &&
          A.VolumeSerialNumber == B.VolumeSerialNumber;