From dfd2711f8f70fca45d2ddbca0eede7ad957ec307 Mon Sep 17 00:00:00 2001 From: Yi Wu Date: Thu, 27 Jun 2024 10:53:56 +0100 Subject: Revert "Revert "[flang] Fix execute_command_line cmdstat is not set when error occurs" (#96365)" (#96774) The fix broke llvm-test-suite, so it was reverted previously. With test fixes added in https://github.com/llvm/llvm-test-suite/pull/137, it should now pass the tests This reverts commit 435635652fd226fa292abcff6a10d3df9dbd74e3. --- flang/docs/Intrinsics.md | 19 ++--- flang/runtime/execute.cpp | 121 +++++++++++++++++++++++++------- flang/unittests/Runtime/CommandTest.cpp | 82 +++++++++++++++++----- 3 files changed, 171 insertions(+), 51 deletions(-) diff --git a/flang/docs/Intrinsics.md b/flang/docs/Intrinsics.md index 8853d4d..d1f7cd8 100644 --- a/flang/docs/Intrinsics.md +++ b/flang/docs/Intrinsics.md @@ -893,16 +893,17 @@ used in constant expressions have currently no folding support at all. ##### `CMDSTAT`: - Synchronous execution: - - -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. + - -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. - \+ (positive value): An error condition occurs. - - 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. + - 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). - Asynchronous execution: - 0 will always be assigned. diff --git a/flang/runtime/execute.cpp b/flang/runtime/execute.cpp index 0f5bc50..c7f8f38 100644 --- a/flang/runtime/execute.cpp +++ b/flang/runtime/execute.cpp @@ -13,8 +13,10 @@ #include "tools.h" #include "flang/Runtime/descriptor.h" #include +#include #include #include + #ifdef _WIN32 #include "flang/Common/windows-include.h" #else @@ -32,13 +34,16 @@ 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, - NO_SUPPORT_ERR = -1, - CMD_EXECUTED = 0, - FORK_ERR = 1, - EXECL_ERR = 2, - INVALID_CL_ERR = 3, - SIGNAL_ERR = 4 + 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 }; // Override CopyCharsToDescriptor in tools.h, pass string directly @@ -62,24 +67,86 @@ 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. -int TerminationCheck(int status, const Descriptor *cmdstat, +std::int64_t TerminationCheck(std::int64_t status, const Descriptor *cmdstat, const Descriptor *cmdmsg, Terminator &terminator) { + // On both Windows and Linux, errno is set when system returns -1. if (status == -1) { - if (!cmdstat) { - terminator.Crash("Execution error with system status code: %d", status); + // 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."); + } } else { - StoreIntToDescriptor(cmdstat, EXECL_ERR, terminator); - CheckAndCopyCharsToDescriptor(cmdmsg, "Execution error"); + 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(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); } } + #ifdef _WIN32 // On WIN32 API std::system returns exit status directly - int exitStatusVal{status}; - if (exitStatusVal == 1) { + 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 - int exitStatusVal{WEXITSTATUS(status)}; - if (exitStatusVal == 127 || exitStatusVal == 126) { -#endif + std::int64_t exitStatusVal{WEXITSTATUS(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) { if (!cmdstat) { terminator.Crash( "Invalid command quit with exit status code: %d", exitStatusVal); @@ -88,23 +155,26 @@ int TerminationCheck(int 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 @@ -134,8 +204,9 @@ void RTNAME(ExecuteCommandLine)(const Descriptor &command, bool wait, if (wait) { // either wait is not specified or wait is true: synchronous mode - int status{std::system(newCmd)}; - int exitStatusVal{TerminationCheck(status, cmdstat, cmdmsg, terminator)}; + std::int64_t status{std::system(newCmd)}; + std::int64_t exitStatusVal{ + TerminationCheck(status, cmdstat, cmdmsg, terminator)}; // If sync, assigned processor-dependent exit status. Otherwise unchanged CheckAndStoreIntToDescriptor(exitstat, exitStatusVal, terminator); } else { @@ -173,7 +244,7 @@ void RTNAME(ExecuteCommandLine)(const Descriptor &command, bool wait, terminator.Crash( "CreateProcess failed with error code: %lu.", GetLastError()); } else { - StoreIntToDescriptor(cmdstat, (uint32_t)GetLastError(), terminator); + StoreIntToDescriptor(cmdstat, ASYNC_NO_SUPPORT_ERR, terminator); CheckAndCopyCharsToDescriptor(cmdmsg, "CreateProcess failed."); } } @@ -201,7 +272,7 @@ void RTNAME(ExecuteCommandLine)(const Descriptor &command, bool wait, } exit(EXIT_FAILURE); } - int status{std::system(newCmd)}; + std::int64_t 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 08daa4b..20bd7a5 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 command{CharDescriptor("echo hi")}; bool wait{true}; - OwningPtr exitStat{EmptyIntDescriptor()}; - OwningPtr cmdStat{EmptyIntDescriptor()}; + OwningPtr exitStat{IntDescriptor(404)}; + OwningPtr cmdStat{IntDescriptor(202)}; OwningPtr cmdMsg{CharDescriptor("No change")}; RTNAME(ExecuteCommandLine) @@ -339,40 +339,91 @@ TEST_F(ZeroArguments, ECLValidCommandStatusSetSync) { CheckDescriptorEqStr(cmdMsg.get(), "No change"); } -TEST_F(ZeroArguments, ECLInvalidCommandErrorSync) { - OwningPtr command{CharDescriptor("InvalidCommand")}; +TEST_F(ZeroArguments, ECLGeneralErrorCommandErrorSync) { + OwningPtr command{CharDescriptor("cat GeneralErrorCommand")}; + bool wait{true}; + OwningPtr exitStat{IntDescriptor(404)}; + OwningPtr cmdStat{IntDescriptor(202)}; + OwningPtr cmdMsg{CharDescriptor("cmd msg buffer XXXXXXXXXXXXXX")}; + + RTNAME(ExecuteCommandLine) + (*command.get(), wait, exitStat.get(), cmdStat.get(), cmdMsg.get()); +#ifdef _WIN32 + CheckDescriptorEqInt(exitStat.get(), 1); + CheckDescriptorEqInt(cmdStat.get(), 6); + CheckDescriptorEqStr(cmdMsg.get(), "Invalid command lineXXXXXXXXX"); +#else + CheckDescriptorEqInt(exitStat.get(), 1); + CheckDescriptorEqInt(cmdStat.get(), 3); + CheckDescriptorEqStr(cmdMsg.get(), "Command line execution failed"); +#endif +} + +TEST_F(ZeroArguments, ECLNotExecutedCommandErrorSync) { + OwningPtr command{CharDescriptor( + "touch NotExecutedCommandFile && chmod -x NotExecutedCommandFile && " + "./NotExecutedCommandFile")}; + bool wait{true}; + OwningPtr exitStat{IntDescriptor(404)}; + OwningPtr cmdStat{IntDescriptor(202)}; + OwningPtr cmdMsg{CharDescriptor("cmd msg buffer XXXXXXXX")}; + + RTNAME(ExecuteCommandLine) + (*command.get(), wait, exitStat.get(), cmdStat.get(), cmdMsg.get()); +#ifdef _WIN32 + CheckDescriptorEqInt(exitStat.get(), 1); + CheckDescriptorEqInt(cmdStat.get(), 6); + CheckDescriptorEqStr(cmdMsg.get(), "Invalid command lineXXX"); +#else + CheckDescriptorEqInt(exitStat.get(), 126); + CheckDescriptorEqInt(cmdStat.get(), 4); + CheckDescriptorEqStr(cmdMsg.get(), "Command cannot be execu"); + // removing the file only on Linux (file is not created on Win) + OwningPtr commandClean{ + CharDescriptor("rm -f NotExecutedCommandFile")}; + OwningPtr cmdMsgNoErr{CharDescriptor("No Error")}; + RTNAME(ExecuteCommandLine) + (*commandClean.get(), wait, exitStat.get(), cmdStat.get(), cmdMsgNoErr.get()); + CheckDescriptorEqInt(exitStat.get(), 0); + CheckDescriptorEqStr(cmdMsgNoErr.get(), "No Error"); + CheckDescriptorEqInt(cmdStat.get(), 0); +#endif +} + +TEST_F(ZeroArguments, ECLNotFoundCommandErrorSync) { + OwningPtr command{CharDescriptor("NotFoundCommand")}; bool wait{true}; OwningPtr exitStat{IntDescriptor(404)}; OwningPtr cmdStat{IntDescriptor(202)}; - OwningPtr cmdMsg{CharDescriptor("Message ChangedXXXXXXXXX")}; + OwningPtr cmdMsg{CharDescriptor("unmodified buffer XXXXXXXXX")}; RTNAME(ExecuteCommandLine) (*command.get(), wait, exitStat.get(), cmdStat.get(), cmdMsg.get()); #ifdef _WIN32 - CheckDescriptorEqInt(exitStat.get(), 1); + CheckDescriptorEqInt(exitStat.get(), 1); + CheckDescriptorEqInt(cmdStat.get(), 6); + CheckDescriptorEqStr(cmdMsg.get(), "Invalid command lineXXXXXXX"); #else CheckDescriptorEqInt(exitStat.get(), 127); + CheckDescriptorEqInt(cmdStat.get(), 5); + CheckDescriptorEqStr(cmdMsg.get(), "Command not found with exit"); #endif - CheckDescriptorEqInt(cmdStat.get(), 3); - CheckDescriptorEqStr(cmdMsg.get(), "Invalid command lineXXXX"); } TEST_F(ZeroArguments, ECLInvalidCommandTerminatedSync) { OwningPtr command{CharDescriptor("InvalidCommand")}; bool wait{true}; - OwningPtr exitStat{IntDescriptor(404)}; OwningPtr cmdMsg{CharDescriptor("No Change")}; #ifdef _WIN32 EXPECT_DEATH(RTNAME(ExecuteCommandLine)( - *command.get(), wait, exitStat.get(), nullptr, cmdMsg.get()), + *command.get(), wait, nullptr, nullptr, cmdMsg.get()), "Invalid command quit with exit status code: 1"); #else EXPECT_DEATH(RTNAME(ExecuteCommandLine)( - *command.get(), wait, exitStat.get(), nullptr, cmdMsg.get()), - "Invalid command quit with exit status code: 127"); + *command.get(), wait, nullptr, nullptr, cmdMsg.get()), + "Command not found with exit code: 127."); #endif - CheckDescriptorEqInt(exitStat.get(), 404); CheckDescriptorEqStr(cmdMsg.get(), "No Change"); } @@ -394,13 +445,10 @@ TEST_F(ZeroArguments, ECLValidCommandAndExitStatNoChangeAndCMDStatusSetAsync) { TEST_F(ZeroArguments, ECLInvalidCommandParentNotTerminatedAsync) { OwningPtr command{CharDescriptor("InvalidCommand")}; bool wait{false}; - OwningPtr exitStat{IntDescriptor(404)}; OwningPtr cmdMsg{CharDescriptor("No change")}; EXPECT_NO_FATAL_FAILURE(RTNAME(ExecuteCommandLine)( - *command.get(), wait, exitStat.get(), nullptr, cmdMsg.get())); - - CheckDescriptorEqInt(exitStat.get(), 404); + *command.get(), wait, nullptr, nullptr, cmdMsg.get())); CheckDescriptorEqStr(cmdMsg.get(), "No change"); } -- cgit v1.1