[flang-commits] [flang] Revert "[flang] Fix execute_command_line cmdstat is not set when error occurs" (PR #96365)
via flang-commits
flang-commits at lists.llvm.org
Fri Jun 21 15:47:22 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-flang-runtime
Author: Kiran Chandramohan (kiranchandramohan)
<details>
<summary>Changes</summary>
Reverts llvm/llvm-project#<!-- -->93023
Reverting due to buildbot failure.
https://lab.llvm.org/buildbot/#/builders/41/builds/227
test-suite :: Fortran/gfortran/regression/gfortran-regression-execute-regression__execute_command_line_3_f90
---
Full diff: https://github.com/llvm/llvm-project/pull/96365.diff
3 Files Affected:
- (modified) flang/docs/Intrinsics.md (+9-10)
- (modified) flang/runtime/execute.cpp (+25-96)
- (modified) flang/unittests/Runtime/CommandTest.cpp (+17-65)
``````````diff
diff --git a/flang/docs/Intrinsics.md b/flang/docs/Intrinsics.md
index d1f7cd8372e24..8853d4d9e1c79 100644
--- a/flang/docs/Intrinsics.md
+++ b/flang/docs/Intrinsics.md
@@ -893,17 +893,16 @@ used in constant expressions have currently no folding support at all.
##### `CMDSTAT`:
- Synchronous execution:
- - -2: `ASYNC_NO_SUPPORT_ERR` - No error condition occurs, but `WAIT` is present with the value `false`, and the processor does not support asynchronous execution.
- - -1: `NO_SUPPORT_ERR` - The processor does not support command line execution. (system returns -1 with errno `ENOENT`)
- - 0: `CMD_EXECUTED` - Command executed with no error.
+ - -2: No error condition occurs, but `WAIT` is present with the value `false`, and the processor does not support asynchronous execution.
+ - -1: The processor does not support command line execution.
- \+ (positive value): An error condition occurs.
- - 1: `FORK_ERR` - Fork Error (occurs only on POSIX-compatible systems).
- - 2: `EXECL_ERR` - Execution Error (system returns -1 with other errno).
- - 3: `COMMAND_EXECUTION_ERR` - Invalid Command Error (exit code 1).
- - 4: `COMMAND_CANNOT_EXECUTE_ERR` - Command Cannot Execute Error (Linux exit code 126).
- - 5: `COMMAND_NOT_FOUND_ERR` - Command Not Found Error (Linux exit code 127).
- - 6: `INVALID_CL_ERR` - Invalid Command Line Error (covers all other non-zero exit codes).
- - 7: `SIGNAL_ERR` - Signal error (either stopped or killed by signal, occurs only on POSIX-compatible systems).
+ - 1: Fork Error (occurs only on POSIX-compatible systems).
+ - 2: Execution Error (command exits with status -1).
+ - 3: Invalid Command Error (determined by the exit code depending on the system).
+ - On Windows: exit code is 1.
+ - On POSIX-compatible systems: exit code is 127 or 126.
+ - 4: Signal error (either stopped or killed by signal, occurs only on POSIX-compatible systems).
+ - 0: Otherwise.
- Asynchronous execution:
- 0 will always be assigned.
diff --git a/flang/runtime/execute.cpp b/flang/runtime/execute.cpp
index c7f8f386d81f4..0f5bc5059e21d 100644
--- a/flang/runtime/execute.cpp
+++ b/flang/runtime/execute.cpp
@@ -13,10 +13,8 @@
#include "tools.h"
#include "flang/Runtime/descriptor.h"
#include <cstdlib>
-#include <errno.h>
#include <future>
#include <limits>
-
#ifdef _WIN32
#include "flang/Common/windows-include.h"
#else
@@ -34,16 +32,13 @@ namespace Fortran::runtime {
// and the processor does not support asynchronous execution. Otherwise it is
// assigned the value 0
enum CMD_STAT {
- ASYNC_NO_SUPPORT_ERR = -2, // system returns -1 with ENOENT
- NO_SUPPORT_ERR = -1, // Linux setsid() returns -1
- CMD_EXECUTED = 0, // command executed with no error
- FORK_ERR = 1, // Linux fork() returns < 0
- EXECL_ERR = 2, // system returns -1 with other errno
- COMMAND_EXECUTION_ERR = 3, // exit code 1
- COMMAND_CANNOT_EXECUTE_ERR = 4, // Linux exit code 126
- COMMAND_NOT_FOUND_ERR = 5, // Linux exit code 127
- INVALID_CL_ERR = 6, // cover all other non-zero exit code
- SIGNAL_ERR = 7
+ ASYNC_NO_SUPPORT_ERR = -2,
+ NO_SUPPORT_ERR = -1,
+ CMD_EXECUTED = 0,
+ FORK_ERR = 1,
+ EXECL_ERR = 2,
+ INVALID_CL_ERR = 3,
+ SIGNAL_ERR = 4
};
// Override CopyCharsToDescriptor in tools.h, pass string directly
@@ -67,86 +62,24 @@ void CheckAndStoreIntToDescriptor(
// If a condition occurs that would assign a nonzero value to CMDSTAT but
// the CMDSTAT variable is not present, error termination is initiated.
-std::int64_t TerminationCheck(std::int64_t status, const Descriptor *cmdstat,
+int TerminationCheck(int status, const Descriptor *cmdstat,
const Descriptor *cmdmsg, Terminator &terminator) {
- // On both Windows and Linux, errno is set when system returns -1.
if (status == -1) {
- // On Windows, ENOENT means the command interpreter can't be found.
- // On Linux, system calls execl with filepath "/bin/sh", ENOENT means the
- // file pathname does not exist.
- if (errno == ENOENT) {
- if (!cmdstat) {
- terminator.Crash("Command line execution is not supported, system "
- "returns -1 with errno ENOENT.");
- } else {
- StoreIntToDescriptor(cmdstat, NO_SUPPORT_ERR, terminator);
- CheckAndCopyCharsToDescriptor(cmdmsg,
- "Command line execution is not supported, system returns -1 with "
- "errno ENOENT.");
- }
+ if (!cmdstat) {
+ terminator.Crash("Execution error with system status code: %d", status);
} else {
- char err_buffer[30];
- char msg[]{"Execution error with system status code: -1, errno: "};
-#ifdef _WIN32
- if (strerror_s(err_buffer, sizeof(err_buffer), errno) != 0)
-#else
- if (strerror_r(errno, err_buffer, sizeof(err_buffer)) != 0)
-#endif
- terminator.Crash("errno to char msg failed.");
- char *newMsg{static_cast<char *>(AllocateMemoryOrCrash(
- terminator, std::strlen(msg) + std::strlen(err_buffer) + 1))};
- std::strcat(newMsg, err_buffer);
-
- if (!cmdstat) {
- terminator.Crash(newMsg);
- } else {
- StoreIntToDescriptor(cmdstat, EXECL_ERR, terminator);
- CheckAndCopyCharsToDescriptor(cmdmsg, newMsg);
- }
- FreeMemory(newMsg);
+ StoreIntToDescriptor(cmdstat, EXECL_ERR, terminator);
+ CheckAndCopyCharsToDescriptor(cmdmsg, "Execution error");
}
}
-
#ifdef _WIN32
// On WIN32 API std::system returns exit status directly
- std::int64_t exitStatusVal{status};
- if (exitStatusVal != 0) {
- if (!cmdstat) {
- terminator.Crash(
- "Invalid command quit with exit status code: %d", exitStatusVal);
- } else {
- StoreIntToDescriptor(cmdstat, INVALID_CL_ERR, terminator);
- CheckAndCopyCharsToDescriptor(cmdmsg, "Invalid command line");
- }
- }
-#else
- std::int64_t exitStatusVal{WEXITSTATUS(status)};
+ int exitStatusVal{status};
if (exitStatusVal == 1) {
- if (!cmdstat) {
- terminator.Crash("Command line execution failed with exit code: 1.");
- } else {
- StoreIntToDescriptor(cmdstat, COMMAND_EXECUTION_ERR, terminator);
- CheckAndCopyCharsToDescriptor(
- cmdmsg, "Command line execution failed with exit code: 1.");
- }
- } else if (exitStatusVal == 126) {
- if (!cmdstat) {
- terminator.Crash("Command cannot be executed with exit code: 126.");
- } else {
- StoreIntToDescriptor(cmdstat, COMMAND_CANNOT_EXECUTE_ERR, terminator);
- CheckAndCopyCharsToDescriptor(
- cmdmsg, "Command cannot be executed with exit code: 126.");
- }
- } else if (exitStatusVal == 127) {
- if (!cmdstat) {
- terminator.Crash("Command not found with exit code: 127.");
- } else {
- StoreIntToDescriptor(cmdstat, COMMAND_NOT_FOUND_ERR, terminator);
- CheckAndCopyCharsToDescriptor(
- cmdmsg, "Command not found with exit code: 127.");
- }
- // capture all other nonzero exit code
- } else if (exitStatusVal != 0) {
+#else
+ int exitStatusVal{WEXITSTATUS(status)};
+ if (exitStatusVal == 127 || exitStatusVal == 126) {
+#endif
if (!cmdstat) {
terminator.Crash(
"Invalid command quit with exit status code: %d", exitStatusVal);
@@ -155,26 +88,23 @@ std::int64_t TerminationCheck(std::int64_t status, const Descriptor *cmdstat,
CheckAndCopyCharsToDescriptor(cmdmsg, "Invalid command line");
}
}
-#endif
-
#if defined(WIFSIGNALED) && defined(WTERMSIG)
if (WIFSIGNALED(status)) {
if (!cmdstat) {
- terminator.Crash("Killed by signal: %d", WTERMSIG(status));
+ terminator.Crash("killed by signal: %d", WTERMSIG(status));
} else {
StoreIntToDescriptor(cmdstat, SIGNAL_ERR, terminator);
- CheckAndCopyCharsToDescriptor(cmdmsg, "Killed by signal");
+ CheckAndCopyCharsToDescriptor(cmdmsg, "killed by signal");
}
}
#endif
-
#if defined(WIFSTOPPED) && defined(WSTOPSIG)
if (WIFSTOPPED(status)) {
if (!cmdstat) {
- terminator.Crash("Stopped by signal: %d", WSTOPSIG(status));
+ terminator.Crash("stopped by signal: %d", WSTOPSIG(status));
} else {
StoreIntToDescriptor(cmdstat, SIGNAL_ERR, terminator);
- CheckAndCopyCharsToDescriptor(cmdmsg, "Stopped by signal");
+ CheckAndCopyCharsToDescriptor(cmdmsg, "stopped by signal");
}
}
#endif
@@ -204,9 +134,8 @@ void RTNAME(ExecuteCommandLine)(const Descriptor &command, bool wait,
if (wait) {
// either wait is not specified or wait is true: synchronous mode
- std::int64_t status{std::system(newCmd)};
- std::int64_t exitStatusVal{
- TerminationCheck(status, cmdstat, cmdmsg, terminator)};
+ int status{std::system(newCmd)};
+ int exitStatusVal{TerminationCheck(status, cmdstat, cmdmsg, terminator)};
// If sync, assigned processor-dependent exit status. Otherwise unchanged
CheckAndStoreIntToDescriptor(exitstat, exitStatusVal, terminator);
} else {
@@ -244,7 +173,7 @@ void RTNAME(ExecuteCommandLine)(const Descriptor &command, bool wait,
terminator.Crash(
"CreateProcess failed with error code: %lu.", GetLastError());
} else {
- StoreIntToDescriptor(cmdstat, ASYNC_NO_SUPPORT_ERR, terminator);
+ StoreIntToDescriptor(cmdstat, (uint32_t)GetLastError(), terminator);
CheckAndCopyCharsToDescriptor(cmdmsg, "CreateProcess failed.");
}
}
@@ -272,7 +201,7 @@ void RTNAME(ExecuteCommandLine)(const Descriptor &command, bool wait,
}
exit(EXIT_FAILURE);
}
- std::int64_t status{std::system(newCmd)};
+ int status{std::system(newCmd)};
TerminationCheck(status, cmdstat, cmdmsg, terminator);
exit(status);
}
diff --git a/flang/unittests/Runtime/CommandTest.cpp b/flang/unittests/Runtime/CommandTest.cpp
index 20bd7a5b5ff35..08daa4ba37f26 100644
--- a/flang/unittests/Runtime/CommandTest.cpp
+++ b/flang/unittests/Runtime/CommandTest.cpp
@@ -311,8 +311,8 @@ TEST_F(ZeroArguments, GetCommand) { CheckCommandValue(commandOnlyArgv, 1); }
TEST_F(ZeroArguments, ECLValidCommandAndPadSync) {
OwningPtr<Descriptor> command{CharDescriptor("echo hi")};
bool wait{true};
- OwningPtr<Descriptor> exitStat{IntDescriptor(404)};
- OwningPtr<Descriptor> cmdStat{IntDescriptor(202)};
+ OwningPtr<Descriptor> exitStat{EmptyIntDescriptor()};
+ OwningPtr<Descriptor> cmdStat{EmptyIntDescriptor()};
OwningPtr<Descriptor> cmdMsg{CharDescriptor("No change")};
RTNAME(ExecuteCommandLine)
@@ -339,91 +339,40 @@ TEST_F(ZeroArguments, ECLValidCommandStatusSetSync) {
CheckDescriptorEqStr(cmdMsg.get(), "No change");
}
-TEST_F(ZeroArguments, ECLGeneralErrorCommandErrorSync) {
- OwningPtr<Descriptor> command{CharDescriptor("cat GeneralErrorCommand")};
- bool wait{true};
- OwningPtr<Descriptor> exitStat{IntDescriptor(404)};
- OwningPtr<Descriptor> cmdStat{IntDescriptor(202)};
- OwningPtr<Descriptor> cmdMsg{CharDescriptor("cmd msg buffer XXXXXXXXXXXXXX")};
-
- RTNAME(ExecuteCommandLine)
- (*command.get(), wait, exitStat.get(), cmdStat.get(), cmdMsg.get());
-#ifdef _WIN32
- CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 1);
- CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 6);
- CheckDescriptorEqStr(cmdMsg.get(), "Invalid command lineXXXXXXXXX");
-#else
- CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 1);
- CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 3);
- CheckDescriptorEqStr(cmdMsg.get(), "Command line execution failed");
-#endif
-}
-
-TEST_F(ZeroArguments, ECLNotExecutedCommandErrorSync) {
- OwningPtr<Descriptor> command{CharDescriptor(
- "touch NotExecutedCommandFile && chmod -x NotExecutedCommandFile && "
- "./NotExecutedCommandFile")};
- bool wait{true};
- OwningPtr<Descriptor> exitStat{IntDescriptor(404)};
- OwningPtr<Descriptor> cmdStat{IntDescriptor(202)};
- OwningPtr<Descriptor> cmdMsg{CharDescriptor("cmd msg buffer XXXXXXXX")};
-
- RTNAME(ExecuteCommandLine)
- (*command.get(), wait, exitStat.get(), cmdStat.get(), cmdMsg.get());
-#ifdef _WIN32
- CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 1);
- CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 6);
- CheckDescriptorEqStr(cmdMsg.get(), "Invalid command lineXXX");
-#else
- CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 126);
- CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 4);
- CheckDescriptorEqStr(cmdMsg.get(), "Command cannot be execu");
- // removing the file only on Linux (file is not created on Win)
- OwningPtr<Descriptor> commandClean{
- CharDescriptor("rm -f NotExecutedCommandFile")};
- OwningPtr<Descriptor> cmdMsgNoErr{CharDescriptor("No Error")};
- RTNAME(ExecuteCommandLine)
- (*commandClean.get(), wait, exitStat.get(), cmdStat.get(), cmdMsgNoErr.get());
- CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 0);
- CheckDescriptorEqStr(cmdMsgNoErr.get(), "No Error");
- CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 0);
-#endif
-}
-
-TEST_F(ZeroArguments, ECLNotFoundCommandErrorSync) {
- OwningPtr<Descriptor> command{CharDescriptor("NotFoundCommand")};
+TEST_F(ZeroArguments, ECLInvalidCommandErrorSync) {
+ OwningPtr<Descriptor> command{CharDescriptor("InvalidCommand")};
bool wait{true};
OwningPtr<Descriptor> exitStat{IntDescriptor(404)};
OwningPtr<Descriptor> cmdStat{IntDescriptor(202)};
- OwningPtr<Descriptor> cmdMsg{CharDescriptor("unmodified buffer XXXXXXXXX")};
+ OwningPtr<Descriptor> cmdMsg{CharDescriptor("Message ChangedXXXXXXXXX")};
RTNAME(ExecuteCommandLine)
(*command.get(), wait, exitStat.get(), cmdStat.get(), cmdMsg.get());
#ifdef _WIN32
- CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 1);
- CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 6);
- CheckDescriptorEqStr(cmdMsg.get(), "Invalid command lineXXXXXXX");
+ CheckDescriptorEqInt(exitStat.get(), 1);
#else
CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 127);
- CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 5);
- CheckDescriptorEqStr(cmdMsg.get(), "Command not found with exit");
#endif
+ CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 3);
+ CheckDescriptorEqStr(cmdMsg.get(), "Invalid command lineXXXX");
}
TEST_F(ZeroArguments, ECLInvalidCommandTerminatedSync) {
OwningPtr<Descriptor> command{CharDescriptor("InvalidCommand")};
bool wait{true};
+ OwningPtr<Descriptor> exitStat{IntDescriptor(404)};
OwningPtr<Descriptor> cmdMsg{CharDescriptor("No Change")};
#ifdef _WIN32
EXPECT_DEATH(RTNAME(ExecuteCommandLine)(
- *command.get(), wait, nullptr, nullptr, cmdMsg.get()),
+ *command.get(), wait, exitStat.get(), nullptr, cmdMsg.get()),
"Invalid command quit with exit status code: 1");
#else
EXPECT_DEATH(RTNAME(ExecuteCommandLine)(
- *command.get(), wait, nullptr, nullptr, cmdMsg.get()),
- "Command not found with exit code: 127.");
+ *command.get(), wait, exitStat.get(), nullptr, cmdMsg.get()),
+ "Invalid command quit with exit status code: 127");
#endif
+ CheckDescriptorEqInt(exitStat.get(), 404);
CheckDescriptorEqStr(cmdMsg.get(), "No Change");
}
@@ -445,10 +394,13 @@ TEST_F(ZeroArguments, ECLValidCommandAndExitStatNoChangeAndCMDStatusSetAsync) {
TEST_F(ZeroArguments, ECLInvalidCommandParentNotTerminatedAsync) {
OwningPtr<Descriptor> command{CharDescriptor("InvalidCommand")};
bool wait{false};
+ OwningPtr<Descriptor> exitStat{IntDescriptor(404)};
OwningPtr<Descriptor> cmdMsg{CharDescriptor("No change")};
EXPECT_NO_FATAL_FAILURE(RTNAME(ExecuteCommandLine)(
- *command.get(), wait, nullptr, nullptr, cmdMsg.get()));
+ *command.get(), wait, exitStat.get(), nullptr, cmdMsg.get()));
+
+ CheckDescriptorEqInt(exitStat.get(), 404);
CheckDescriptorEqStr(cmdMsg.get(), "No change");
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/96365
More information about the flang-commits
mailing list