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

310–311 ↗

(On Diff #34329)

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

310–311 ↗

(On Diff #34329)

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.

llvm/trunk/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);
 }

llvm/trunk/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;

llvm/trunk/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 &&