Skip to content

Commit 4356356

Browse files
kiranchandramohankiranchandramohan
authored
Revert "[flang] Fix execute_command_line cmdstat is not set when error occurs" (#96365)
Reverts #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
1 parent 73a2232 commit 4356356

File tree

3 files changed

+51
-171
lines changed

3 files changed

+51
-171
lines changed

‎flang/docs/Intrinsics.md

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -893,17 +893,16 @@ used in constant expressions have currently no folding support at all.
893893
##### `CMDSTAT`:
894894

895895
- Synchronous execution:
896-
- -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.
897-
- -1: `NO_SUPPORT_ERR` - The processor does not support command line execution. (system returns -1 with errno `ENOENT`)
898-
- 0: `CMD_EXECUTED` - Command executed with no error.
896+
- -2: No error condition occurs, but `WAIT` is present with the value `false`, and the processor does not support asynchronous execution.
897+
- -1: The processor does not support command line execution.
899898
- \+ (positive value): An error condition occurs.
900-
- 1: `FORK_ERR` - Fork Error (occurs only on POSIX-compatible systems).
901-
- 2: `EXECL_ERR` - Execution Error (system returns -1 with other errno).
902-
- 3: `COMMAND_EXECUTION_ERR` - Invalid Command Error (exit code 1).
903-
- 4: `COMMAND_CANNOT_EXECUTE_ERR` - Command Cannot Execute Error (Linux exit code 126).
904-
- 5: `COMMAND_NOT_FOUND_ERR` - Command Not Found Error (Linux exit code 127).
905-
- 6: `INVALID_CL_ERR` - Invalid Command Line Error (covers all other non-zero exit codes).
906-
- 7: `SIGNAL_ERR` - Signal error (either stopped or killed by signal, occurs only on POSIX-compatible systems).
899+
- 1: Fork Error (occurs only on POSIX-compatible systems).
900+
- 2: Execution Error (command exits with status -1).
901+
- 3: Invalid Command Error (determined by the exit code depending on the system).
902+
- On Windows: exit code is 1.
903+
- On POSIX-compatible systems: exit code is 127 or 126.
904+
- 4: Signal error (either stopped or killed by signal, occurs only on POSIX-compatible systems).
905+
- 0: Otherwise.
907906
- Asynchronous execution:
908907
- 0 will always be assigned.
909908

‎flang/runtime/execute.cpp

Lines changed: 25 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,8 @@
1313
#include "tools.h"
1414
#include "flang/Runtime/descriptor.h"
1515
#include <cstdlib>
16-
#include <errno.h>
1716
#include <future>
1817
#include <limits>
19-
2018
#ifdef _WIN32
2119
#include "flang/Common/windows-include.h"
2220
#else
@@ -34,16 +32,13 @@ namespace Fortran::runtime {
3432
// and the processor does not support asynchronous execution. Otherwise it is
3533
// assigned the value 0
3634
enum CMD_STAT {
37-
ASYNC_NO_SUPPORT_ERR = -2, // system returns -1 with ENOENT
38-
NO_SUPPORT_ERR = -1, // Linux setsid() returns -1
39-
CMD_EXECUTED = 0, // command executed with no error
40-
FORK_ERR = 1, // Linux fork() returns < 0
41-
EXECL_ERR = 2, // system returns -1 with other errno
42-
COMMAND_EXECUTION_ERR = 3, // exit code 1
43-
COMMAND_CANNOT_EXECUTE_ERR = 4, // Linux exit code 126
44-
COMMAND_NOT_FOUND_ERR = 5, // Linux exit code 127
45-
INVALID_CL_ERR = 6, // cover all other non-zero exit code
46-
SIGNAL_ERR = 7
35+
ASYNC_NO_SUPPORT_ERR = -2,
36+
NO_SUPPORT_ERR = -1,
37+
CMD_EXECUTED = 0,
38+
FORK_ERR = 1,
39+
EXECL_ERR = 2,
40+
INVALID_CL_ERR = 3,
41+
SIGNAL_ERR = 4
4742
};
4843

4944
// Override CopyCharsToDescriptor in tools.h, pass string directly
@@ -67,86 +62,24 @@ void CheckAndStoreIntToDescriptor(
6762

6863
// If a condition occurs that would assign a nonzero value to CMDSTAT but
6964
// the CMDSTAT variable is not present, error termination is initiated.
70-
std::int64_t TerminationCheck(std::int64_t status, const Descriptor *cmdstat,
65+
int TerminationCheck(int status, const Descriptor *cmdstat,
7166
const Descriptor *cmdmsg, Terminator &terminator) {
72-
// On both Windows and Linux, errno is set when system returns -1.
7367
if (status == -1) {
74-
// On Windows, ENOENT means the command interpreter can't be found.
75-
// On Linux, system calls execl with filepath "/bin/sh", ENOENT means the
76-
// file pathname does not exist.
77-
if (errno == ENOENT) {
78-
if (!cmdstat) {
79-
terminator.Crash("Command line execution is not supported, system "
80-
"returns -1 with errno ENOENT.");
81-
} else {
82-
StoreIntToDescriptor(cmdstat, NO_SUPPORT_ERR, terminator);
83-
CheckAndCopyCharsToDescriptor(cmdmsg,
84-
"Command line execution is not supported, system returns -1 with "
85-
"errno ENOENT.");
86-
}
68+
if (!cmdstat) {
69+
terminator.Crash("Execution error with system status code: %d", status);
8770
} else {
88-
char err_buffer[30];
89-
char msg[]{"Execution error with system status code: -1, errno: "};
90-
#ifdef _WIN32
91-
if (strerror_s(err_buffer, sizeof(err_buffer), errno) != 0)
92-
#else
93-
if (strerror_r(errno, err_buffer, sizeof(err_buffer)) != 0)
94-
#endif
95-
terminator.Crash("errno to char msg failed.");
96-
char *newMsg{static_cast<char *>(AllocateMemoryOrCrash(
97-
terminator, std::strlen(msg) + std::strlen(err_buffer) + 1))};
98-
std::strcat(newMsg, err_buffer);
99-
100-
if (!cmdstat) {
101-
terminator.Crash(newMsg);
102-
} else {
103-
StoreIntToDescriptor(cmdstat, EXECL_ERR, terminator);
104-
CheckAndCopyCharsToDescriptor(cmdmsg, newMsg);
105-
}
106-
FreeMemory(newMsg);
71+
StoreIntToDescriptor(cmdstat, EXECL_ERR, terminator);
72+
CheckAndCopyCharsToDescriptor(cmdmsg, "Execution error");
10773
}
10874
}
109-
11075
#ifdef _WIN32
11176
// On WIN32 API std::system returns exit status directly
112-
std::int64_t exitStatusVal{status};
113-
if (exitStatusVal != 0) {
114-
if (!cmdstat) {
115-
terminator.Crash(
116-
"Invalid command quit with exit status code: %d", exitStatusVal);
117-
} else {
118-
StoreIntToDescriptor(cmdstat, INVALID_CL_ERR, terminator);
119-
CheckAndCopyCharsToDescriptor(cmdmsg, "Invalid command line");
120-
}
121-
}
122-
#else
123-
std::int64_t exitStatusVal{WEXITSTATUS(status)};
77+
int exitStatusVal{status};
12478
if (exitStatusVal == 1) {
125-
if (!cmdstat) {
126-
terminator.Crash("Command line execution failed with exit code: 1.");
127-
} else {
128-
StoreIntToDescriptor(cmdstat, COMMAND_EXECUTION_ERR, terminator);
129-
CheckAndCopyCharsToDescriptor(
130-
cmdmsg, "Command line execution failed with exit code: 1.");
131-
}
132-
} else if (exitStatusVal == 126) {
133-
if (!cmdstat) {
134-
terminator.Crash("Command cannot be executed with exit code: 126.");
135-
} else {
136-
StoreIntToDescriptor(cmdstat, COMMAND_CANNOT_EXECUTE_ERR, terminator);
137-
CheckAndCopyCharsToDescriptor(
138-
cmdmsg, "Command cannot be executed with exit code: 126.");
139-
}
140-
} else if (exitStatusVal == 127) {
141-
if (!cmdstat) {
142-
terminator.Crash("Command not found with exit code: 127.");
143-
} else {
144-
StoreIntToDescriptor(cmdstat, COMMAND_NOT_FOUND_ERR, terminator);
145-
CheckAndCopyCharsToDescriptor(
146-
cmdmsg, "Command not found with exit code: 127.");
147-
}
148-
// capture all other nonzero exit code
149-
} else if (exitStatusVal != 0) {
79+
#else
80+
int exitStatusVal{WEXITSTATUS(status)};
81+
if (exitStatusVal == 127 || exitStatusVal == 126) {
82+
#endif
15083
if (!cmdstat) {
15184
terminator.Crash(
15285
"Invalid command quit with exit status code: %d", exitStatusVal);
@@ -155,26 +88,23 @@ std::int64_t TerminationCheck(std::int64_t status, const Descriptor *cmdstat,
15588
CheckAndCopyCharsToDescriptor(cmdmsg, "Invalid command line");
15689
}
15790
}
158-
#endif
159-
16091
#if defined(WIFSIGNALED) && defined(WTERMSIG)
16192
if (WIFSIGNALED(status)) {
16293
if (!cmdstat) {
163-
terminator.Crash("Killed by signal: %d", WTERMSIG(status));
94+
terminator.Crash("killed by signal: %d", WTERMSIG(status));
16495
} else {
16596
StoreIntToDescriptor(cmdstat, SIGNAL_ERR, terminator);
166-
CheckAndCopyCharsToDescriptor(cmdmsg, "Killed by signal");
97+
CheckAndCopyCharsToDescriptor(cmdmsg, "killed by signal");
16798
}
16899
}
169100
#endif
170-
171101
#if defined(WIFSTOPPED) && defined(WSTOPSIG)
172102
if (WIFSTOPPED(status)) {
173103
if (!cmdstat) {
174-
terminator.Crash("Stopped by signal: %d", WSTOPSIG(status));
104+
terminator.Crash("stopped by signal: %d", WSTOPSIG(status));
175105
} else {
176106
StoreIntToDescriptor(cmdstat, SIGNAL_ERR, terminator);
177-
CheckAndCopyCharsToDescriptor(cmdmsg, "Stopped by signal");
107+
CheckAndCopyCharsToDescriptor(cmdmsg, "stopped by signal");
178108
}
179109
}
180110
#endif
@@ -204,9 +134,8 @@ void RTNAME(ExecuteCommandLine)(const Descriptor &command, bool wait,
204134

205135
if (wait) {
206136
// either wait is not specified or wait is true: synchronous mode
207-
std::int64_t status{std::system(newCmd)};
208-
std::int64_t exitStatusVal{
209-
TerminationCheck(status, cmdstat, cmdmsg, terminator)};
137+
int status{std::system(newCmd)};
138+
int exitStatusVal{TerminationCheck(status, cmdstat, cmdmsg, terminator)};
210139
// If sync, assigned processor-dependent exit status. Otherwise unchanged
211140
CheckAndStoreIntToDescriptor(exitstat, exitStatusVal, terminator);
212141
} else {
@@ -244,7 +173,7 @@ void RTNAME(ExecuteCommandLine)(const Descriptor &command, bool wait,
244173
terminator.Crash(
245174
"CreateProcess failed with error code: %lu.", GetLastError());
246175
} else {
247-
StoreIntToDescriptor(cmdstat, ASYNC_NO_SUPPORT_ERR, terminator);
176+
StoreIntToDescriptor(cmdstat, (uint32_t)GetLastError(), terminator);
248177
CheckAndCopyCharsToDescriptor(cmdmsg, "CreateProcess failed.");
249178
}
250179
}
@@ -272,7 +201,7 @@ void RTNAME(ExecuteCommandLine)(const Descriptor &command, bool wait,
272201
}
273202
exit(EXIT_FAILURE);
274203
}
275-
std::int64_t status{std::system(newCmd)};
204+
int status{std::system(newCmd)};
276205
TerminationCheck(status, cmdstat, cmdmsg, terminator);
277206
exit(status);
278207
}

‎flang/unittests/Runtime/CommandTest.cpp

Lines changed: 17 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -311,8 +311,8 @@ TEST_F(ZeroArguments, GetCommand) { CheckCommandValue(commandOnlyArgv, 1); }
311311
TEST_F(ZeroArguments, ECLValidCommandAndPadSync) {
312312
OwningPtr<Descriptor> command{CharDescriptor("echo hi")};
313313
bool wait{true};
314-
OwningPtr<Descriptor> exitStat{IntDescriptor(404)};
315-
OwningPtr<Descriptor> cmdStat{IntDescriptor(202)};
314+
OwningPtr<Descriptor> exitStat{EmptyIntDescriptor()};
315+
OwningPtr<Descriptor> cmdStat{EmptyIntDescriptor()};
316316
OwningPtr<Descriptor> cmdMsg{CharDescriptor("No change")};
317317

318318
RTNAME(ExecuteCommandLine)
@@ -339,91 +339,40 @@ TEST_F(ZeroArguments, ECLValidCommandStatusSetSync) {
339339
CheckDescriptorEqStr(cmdMsg.get(), "No change");
340340
}
341341

342-
TEST_F(ZeroArguments, ECLGeneralErrorCommandErrorSync) {
343-
OwningPtr<Descriptor> command{CharDescriptor("cat GeneralErrorCommand")};
344-
bool wait{true};
345-
OwningPtr<Descriptor> exitStat{IntDescriptor(404)};
346-
OwningPtr<Descriptor> cmdStat{IntDescriptor(202)};
347-
OwningPtr<Descriptor> cmdMsg{CharDescriptor("cmd msg buffer XXXXXXXXXXXXXX")};
348-
349-
RTNAME(ExecuteCommandLine)
350-
(*command.get(), wait, exitStat.get(), cmdStat.get(), cmdMsg.get());
351-
#ifdef _WIN32
352-
CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 1);
353-
CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 6);
354-
CheckDescriptorEqStr(cmdMsg.get(), "Invalid command lineXXXXXXXXX");
355-
#else
356-
CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 1);
357-
CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 3);
358-
CheckDescriptorEqStr(cmdMsg.get(), "Command line execution failed");
359-
#endif
360-
}
361-
362-
TEST_F(ZeroArguments, ECLNotExecutedCommandErrorSync) {
363-
OwningPtr<Descriptor> command{CharDescriptor(
364-
"touch NotExecutedCommandFile && chmod -x NotExecutedCommandFile && "
365-
"./NotExecutedCommandFile")};
366-
bool wait{true};
367-
OwningPtr<Descriptor> exitStat{IntDescriptor(404)};
368-
OwningPtr<Descriptor> cmdStat{IntDescriptor(202)};
369-
OwningPtr<Descriptor> cmdMsg{CharDescriptor("cmd msg buffer XXXXXXXX")};
370-
371-
RTNAME(ExecuteCommandLine)
372-
(*command.get(), wait, exitStat.get(), cmdStat.get(), cmdMsg.get());
373-
#ifdef _WIN32
374-
CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 1);
375-
CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 6);
376-
CheckDescriptorEqStr(cmdMsg.get(), "Invalid command lineXXX");
377-
#else
378-
CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 126);
379-
CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 4);
380-
CheckDescriptorEqStr(cmdMsg.get(), "Command cannot be execu");
381-
// removing the file only on Linux (file is not created on Win)
382-
OwningPtr<Descriptor> commandClean{
383-
CharDescriptor("rm -f NotExecutedCommandFile")};
384-
OwningPtr<Descriptor> cmdMsgNoErr{CharDescriptor("No Error")};
385-
RTNAME(ExecuteCommandLine)
386-
(*commandClean.get(), wait, exitStat.get(), cmdStat.get(), cmdMsgNoErr.get());
387-
CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 0);
388-
CheckDescriptorEqStr(cmdMsgNoErr.get(), "No Error");
389-
CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 0);
390-
#endif
391-
}
392-
393-
TEST_F(ZeroArguments, ECLNotFoundCommandErrorSync) {
394-
OwningPtr<Descriptor> command{CharDescriptor("NotFoundCommand")};
342+
TEST_F(ZeroArguments, ECLInvalidCommandErrorSync) {
343+
OwningPtr<Descriptor> command{CharDescriptor("InvalidCommand")};
395344
bool wait{true};
396345
OwningPtr<Descriptor> exitStat{IntDescriptor(404)};
397346
OwningPtr<Descriptor> cmdStat{IntDescriptor(202)};
398-
OwningPtr<Descriptor> cmdMsg{CharDescriptor("unmodified buffer XXXXXXXXX")};
347+
OwningPtr<Descriptor> cmdMsg{CharDescriptor("Message ChangedXXXXXXXXX")};
399348

400349
RTNAME(ExecuteCommandLine)
401350
(*command.get(), wait, exitStat.get(), cmdStat.get(), cmdMsg.get());
402351
#ifdef _WIN32
403-
CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 1);
404-
CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 6);
405-
CheckDescriptorEqStr(cmdMsg.get(), "Invalid command lineXXXXXXX");
352+
CheckDescriptorEqInt(exitStat.get(), 1);
406353
#else
407354
CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 127);
408-
CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 5);
409-
CheckDescriptorEqStr(cmdMsg.get(), "Command not found with exit");
410355
#endif
356+
CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 3);
357+
CheckDescriptorEqStr(cmdMsg.get(), "Invalid command lineXXXX");
411358
}
412359

413360
TEST_F(ZeroArguments, ECLInvalidCommandTerminatedSync) {
414361
OwningPtr<Descriptor> command{CharDescriptor("InvalidCommand")};
415362
bool wait{true};
363+
OwningPtr<Descriptor> exitStat{IntDescriptor(404)};
416364
OwningPtr<Descriptor> cmdMsg{CharDescriptor("No Change")};
417365

418366
#ifdef _WIN32
419367
EXPECT_DEATH(RTNAME(ExecuteCommandLine)(
420-
*command.get(), wait, nullptr, nullptr, cmdMsg.get()),
368+
*command.get(), wait, exitStat.get(), nullptr, cmdMsg.get()),
421369
"Invalid command quit with exit status code: 1");
422370
#else
423371
EXPECT_DEATH(RTNAME(ExecuteCommandLine)(
424-
*command.get(), wait, nullptr, nullptr, cmdMsg.get()),
425-
"Command not found with exit code: 127.");
372+
*command.get(), wait, exitStat.get(), nullptr, cmdMsg.get()),
373+
"Invalid command quit with exit status code: 127");
426374
#endif
375+
CheckDescriptorEqInt(exitStat.get(), 404);
427376
CheckDescriptorEqStr(cmdMsg.get(), "No Change");
428377
}
429378

@@ -445,10 +394,13 @@ TEST_F(ZeroArguments, ECLValidCommandAndExitStatNoChangeAndCMDStatusSetAsync) {
445394
TEST_F(ZeroArguments, ECLInvalidCommandParentNotTerminatedAsync) {
446395
OwningPtr<Descriptor> command{CharDescriptor("InvalidCommand")};
447396
bool wait{false};
397+
OwningPtr<Descriptor> exitStat{IntDescriptor(404)};
448398
OwningPtr<Descriptor> cmdMsg{CharDescriptor("No change")};
449399

450400
EXPECT_NO_FATAL_FAILURE(RTNAME(ExecuteCommandLine)(
451-
*command.get(), wait, nullptr, nullptr, cmdMsg.get()));
401+
*command.get(), wait, exitStat.get(), nullptr, cmdMsg.get()));
402+
403+
CheckDescriptorEqInt(exitStat.get(), 404);
452404
CheckDescriptorEqStr(cmdMsg.get(), "No change");
453405
}
454406

0 commit comments

Comments
 (0)