-
Notifications
You must be signed in to change notification settings - Fork 14.3k
Support: Do not check if a file exists before executing #128821
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support: Do not check if a file exists before executing #128821
Conversation
Let the actual syscall error if the file doesn't exist. This produces a more standard "no such file or directory" phrasing of the error message, and avoids an extra step. The same antipattern appears in the windows code, we should probably fix that one too.
@llvm/pr-subscribers-llvm-support Author: Matt Arsenault (arsenm) ChangesLet the actual syscall error if the file doesn't exist. This produces The same antipattern appears in the windows code, we should probably Full diff: https://github.com/llvm/llvm-project/pull/128821.diff 2 Files Affected:
diff --git a/llvm/lib/Support/Unix/Program.inc b/llvm/lib/Support/Unix/Program.inc
index 0708df1eed0a3..6d68369ad191c 100644
--- a/llvm/lib/Support/Unix/Program.inc
+++ b/llvm/lib/Support/Unix/Program.inc
@@ -168,13 +168,6 @@ static bool Execute(ProcessInfo &PI, StringRef Program,
ArrayRef<std::optional<StringRef>> Redirects,
unsigned MemoryLimit, std::string *ErrMsg,
BitVector *AffinityMask, bool DetachProcess) {
- if (!llvm::sys::fs::exists(Program)) {
- if (ErrMsg)
- *ErrMsg = std::string("Executable \"") + Program.str() +
- std::string("\" doesn't exist!");
- return false;
- }
-
assert(!AffinityMask && "Starting a process with an affinity mask is "
"currently not supported on Unix!");
diff --git a/llvm/test/tools/llvm-rc/windres-preproc.test b/llvm/test/tools/llvm-rc/windres-preproc.test
index 52427862e760b..ad558c9a0c927 100644
--- a/llvm/test/tools/llvm-rc/windres-preproc.test
+++ b/llvm/test/tools/llvm-rc/windres-preproc.test
@@ -20,7 +20,7 @@
;; Test error messages when unable to execute the preprocessor.
; RUN: not llvm-windres --preprocessor intentionally-missing-executable %p/Inputs/empty.rc %t.res 2>&1 | FileCheck %s --check-prefix=CHECK4
-; CHECK4: llvm-rc: Preprocessing failed: Executable "intentionally-missing-executable" doesn't exist!
+; CHECK4: llvm-rc: Preprocessing failed: posix_spawn failed: No such file or directory
;; Test --preprocessor with an argument with spaces.
|
This seems to be encountering problems on AIX as well: |
Should also be fixed by #129252 |
Let the actual syscall error if the file doesn't exist. This produces
a more standard "no such file or directory" phrasing of the error message,
and avoids an extra step.
The same antipattern appears in the windows code, we should probably
fix that one too.