[flang-commits] [flang] [flang] Fix execute_command_line cmdstat is not set when error occurs (PR #93023)
Yi Wu via flang-commits
flang-commits at lists.llvm.org
Thu May 30 07:19:40 PDT 2024
https://github.com/yi-wu-arm updated https://github.com/llvm/llvm-project/pull/93023
>From 006cefc035030fe80dc55030d931cde1c78ac448 Mon Sep 17 00:00:00 2001
From: YI WU <YI.WU2 at arm.com>
Date: Wed, 22 May 2024 12:06:24 +0100
Subject: [PATCH 1/6] [flang] Fix execute_command_line cmdstat is not set when
error occurs
Fixes: https://github.com/llvm/llvm-project/issues/92929
---
flang/docs/Intrinsics.md | 2 --
flang/runtime/execute.cpp | 14 ++++++++------
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/flang/docs/Intrinsics.md b/flang/docs/Intrinsics.md
index 41129b10083b1..7fa718626217b 100644
--- a/flang/docs/Intrinsics.md
+++ b/flang/docs/Intrinsics.md
@@ -899,8 +899,6 @@ used in constant expressions have currently no folding support at all.
- 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:
diff --git a/flang/runtime/execute.cpp b/flang/runtime/execute.cpp
index 0f5bc5059e21d..b9543e6582bae 100644
--- a/flang/runtime/execute.cpp
+++ b/flang/runtime/execute.cpp
@@ -72,14 +72,14 @@ int TerminationCheck(int status, const Descriptor *cmdstat,
CheckAndCopyCharsToDescriptor(cmdmsg, "Execution error");
}
}
+
#ifdef _WIN32
// On WIN32 API std::system returns exit status directly
int exitStatusVal{status};
- if (exitStatusVal == 1) {
#else
int exitStatusVal{WEXITSTATUS(status)};
- if (exitStatusVal == 127 || exitStatusVal == 126) {
#endif
+ if (exitStatusVal != 0) {
if (!cmdstat) {
terminator.Crash(
"Invalid command quit with exit status code: %d", exitStatusVal);
@@ -88,23 +88,25 @@ int TerminationCheck(int status, const Descriptor *cmdstat,
CheckAndCopyCharsToDescriptor(cmdmsg, "Invalid command line");
}
}
+
#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
>From 4e2799deb1017ee3d5a0070d6c85b2ca991eef62 Mon Sep 17 00:00:00 2001
From: YI WU <YI.WU2 at arm.com>
Date: Tue, 28 May 2024 10:11:40 +0100
Subject: [PATCH 2/6] cmdstat will set to 3 when error then set to specific
value
---
flang/runtime/execute.cpp | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/flang/runtime/execute.cpp b/flang/runtime/execute.cpp
index b9543e6582bae..b430137e7f143 100644
--- a/flang/runtime/execute.cpp
+++ b/flang/runtime/execute.cpp
@@ -64,15 +64,6 @@ void CheckAndStoreIntToDescriptor(
// the CMDSTAT variable is not present, error termination is initiated.
int TerminationCheck(int status, const Descriptor *cmdstat,
const Descriptor *cmdmsg, Terminator &terminator) {
- if (status == -1) {
- if (!cmdstat) {
- terminator.Crash("Execution error with system status code: %d", status);
- } else {
- StoreIntToDescriptor(cmdstat, EXECL_ERR, terminator);
- CheckAndCopyCharsToDescriptor(cmdmsg, "Execution error");
- }
- }
-
#ifdef _WIN32
// On WIN32 API std::system returns exit status directly
int exitStatusVal{status};
@@ -88,6 +79,14 @@ int TerminationCheck(int status, const Descriptor *cmdstat,
CheckAndCopyCharsToDescriptor(cmdmsg, "Invalid command line");
}
}
+ if (status == -1) {
+ if (!cmdstat) {
+ terminator.Crash("Execution error with system status code: %d", status);
+ } else {
+ StoreIntToDescriptor(cmdstat, EXECL_ERR, terminator);
+ CheckAndCopyCharsToDescriptor(cmdmsg, "Execution error");
+ }
+ }
#if defined(WIFSIGNALED) && defined(WTERMSIG)
if (WIFSIGNALED(status)) {
>From 3b285a1024f3115a6af6da1b0d7d637548a0bf1c Mon Sep 17 00:00:00 2001
From: YI WU <YI.WU2 at arm.com>
Date: Tue, 28 May 2024 20:00:10 +0100
Subject: [PATCH 3/6] slightly more detailed cmdmsg
---
flang/runtime/execute.cpp | 3 ++-
flang/unittests/Runtime/CommandTest.cpp | 6 ++++--
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/flang/runtime/execute.cpp b/flang/runtime/execute.cpp
index b430137e7f143..c937a688a3bb1 100644
--- a/flang/runtime/execute.cpp
+++ b/flang/runtime/execute.cpp
@@ -76,7 +76,8 @@ int TerminationCheck(int status, const Descriptor *cmdstat,
"Invalid command quit with exit status code: %d", exitStatusVal);
} else {
StoreIntToDescriptor(cmdstat, INVALID_CL_ERR, terminator);
- CheckAndCopyCharsToDescriptor(cmdmsg, "Invalid command line");
+ CheckAndCopyCharsToDescriptor(cmdmsg,
+ "Invalid command line: check for exitstat and console printout");
}
}
if (status == -1) {
diff --git a/flang/unittests/Runtime/CommandTest.cpp b/flang/unittests/Runtime/CommandTest.cpp
index 08daa4ba37f26..591911d1df2c7 100644
--- a/flang/unittests/Runtime/CommandTest.cpp
+++ b/flang/unittests/Runtime/CommandTest.cpp
@@ -344,7 +344,8 @@ TEST_F(ZeroArguments, ECLInvalidCommandErrorSync) {
bool wait{true};
OwningPtr<Descriptor> exitStat{IntDescriptor(404)};
OwningPtr<Descriptor> cmdStat{IntDescriptor(202)};
- OwningPtr<Descriptor> cmdMsg{CharDescriptor("Message ChangedXXXXXXXXX")};
+ OwningPtr<Descriptor> cmdMsg{CharDescriptor(
+ "cmdmsg will not modify the remaining buffer XXXXXXXXXXXXXXXXXXXXX")};
RTNAME(ExecuteCommandLine)
(*command.get(), wait, exitStat.get(), cmdStat.get(), cmdMsg.get());
@@ -354,7 +355,8 @@ TEST_F(ZeroArguments, ECLInvalidCommandErrorSync) {
CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 127);
#endif
CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 3);
- CheckDescriptorEqStr(cmdMsg.get(), "Invalid command lineXXXX");
+ CheckDescriptorEqStr(cmdMsg.get(),
+ "Invalid command line: check for exitstat and console printoutXXXX");
}
TEST_F(ZeroArguments, ECLInvalidCommandTerminatedSync) {
>From 7c8b5e8c44c3f6afadb0e947d66bbc7820c49439 Mon Sep 17 00:00:00 2001
From: YI WU <YI.WU2 at arm.com>
Date: Thu, 30 May 2024 14:23:59 +0100
Subject: [PATCH 4/6] Revert "slightly more detailed cmdmsg"
This reverts commit 3b285a1024f3115a6af6da1b0d7d637548a0bf1c.
---
flang/runtime/execute.cpp | 3 +--
flang/unittests/Runtime/CommandTest.cpp | 6 ++----
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/flang/runtime/execute.cpp b/flang/runtime/execute.cpp
index c937a688a3bb1..b430137e7f143 100644
--- a/flang/runtime/execute.cpp
+++ b/flang/runtime/execute.cpp
@@ -76,8 +76,7 @@ int TerminationCheck(int status, const Descriptor *cmdstat,
"Invalid command quit with exit status code: %d", exitStatusVal);
} else {
StoreIntToDescriptor(cmdstat, INVALID_CL_ERR, terminator);
- CheckAndCopyCharsToDescriptor(cmdmsg,
- "Invalid command line: check for exitstat and console printout");
+ CheckAndCopyCharsToDescriptor(cmdmsg, "Invalid command line");
}
}
if (status == -1) {
diff --git a/flang/unittests/Runtime/CommandTest.cpp b/flang/unittests/Runtime/CommandTest.cpp
index 591911d1df2c7..08daa4ba37f26 100644
--- a/flang/unittests/Runtime/CommandTest.cpp
+++ b/flang/unittests/Runtime/CommandTest.cpp
@@ -344,8 +344,7 @@ TEST_F(ZeroArguments, ECLInvalidCommandErrorSync) {
bool wait{true};
OwningPtr<Descriptor> exitStat{IntDescriptor(404)};
OwningPtr<Descriptor> cmdStat{IntDescriptor(202)};
- OwningPtr<Descriptor> cmdMsg{CharDescriptor(
- "cmdmsg will not modify the remaining buffer XXXXXXXXXXXXXXXXXXXXX")};
+ OwningPtr<Descriptor> cmdMsg{CharDescriptor("Message ChangedXXXXXXXXX")};
RTNAME(ExecuteCommandLine)
(*command.get(), wait, exitStat.get(), cmdStat.get(), cmdMsg.get());
@@ -355,8 +354,7 @@ TEST_F(ZeroArguments, ECLInvalidCommandErrorSync) {
CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 127);
#endif
CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 3);
- CheckDescriptorEqStr(cmdMsg.get(),
- "Invalid command line: check for exitstat and console printoutXXXX");
+ CheckDescriptorEqStr(cmdMsg.get(), "Invalid command lineXXXX");
}
TEST_F(ZeroArguments, ECLInvalidCommandTerminatedSync) {
>From cde8a346dca02effc4ccb691a77413c18862ca81 Mon Sep 17 00:00:00 2001
From: YI WU <YI.WU2 at arm.com>
Date: Thu, 30 May 2024 14:32:15 +0100
Subject: [PATCH 5/6] add cmdmsg to common exit code 1, 126, 127 for Linux
for Windows, all three of them has a exit code of 1
so can't write cmdmsg for them
---
flang/runtime/execute.cpp | 31 +++++++++++++++
flang/unittests/Runtime/CommandTest.cpp | 50 +++++++++++++++++++++++--
2 files changed, 77 insertions(+), 4 deletions(-)
diff --git a/flang/runtime/execute.cpp b/flang/runtime/execute.cpp
index b430137e7f143..fe85971052a1e 100644
--- a/flang/runtime/execute.cpp
+++ b/flang/runtime/execute.cpp
@@ -79,6 +79,37 @@ int TerminationCheck(int status, const Descriptor *cmdstat,
CheckAndCopyCharsToDescriptor(cmdmsg, "Invalid command line");
}
}
+
+#ifndef _WIN32
+ if (exitStatusVal == 1) {
+ if (!cmdstat) {
+ terminator.Crash("General Error with exit status code: 1");
+ } else {
+ StoreIntToDescriptor(cmdstat, INVALID_CL_ERR, terminator);
+ CheckAndCopyCharsToDescriptor(cmdmsg,
+ "General Error: a catch-all exit code for a variety of general "
+ "errors.");
+ }
+ } else if (exitStatusVal == 126) {
+ if (!cmdstat) {
+ terminator.Crash("Command cannot execute with exit status code: 126");
+ } else {
+ StoreIntToDescriptor(cmdstat, INVALID_CL_ERR, terminator);
+ CheckAndCopyCharsToDescriptor(cmdmsg,
+ "Command cannot execute: command was found, but it could not be "
+ "executed.");
+ }
+ } else if (exitStatusVal == 127) {
+ if (!cmdstat) {
+ terminator.Crash("Command not found with exit status code: 127");
+ } else {
+ StoreIntToDescriptor(cmdstat, INVALID_CL_ERR, terminator);
+ CheckAndCopyCharsToDescriptor(cmdmsg,
+ "Command not found: command was not found in the system's PATH");
+ }
+ }
+#endif
+
if (status == -1) {
if (!cmdstat) {
terminator.Crash("Execution error with system status code: %d", status);
diff --git a/flang/unittests/Runtime/CommandTest.cpp b/flang/unittests/Runtime/CommandTest.cpp
index 08daa4ba37f26..25a878039ff0f 100644
--- a/flang/unittests/Runtime/CommandTest.cpp
+++ b/flang/unittests/Runtime/CommandTest.cpp
@@ -339,22 +339,64 @@ TEST_F(ZeroArguments, ECLValidCommandStatusSetSync) {
CheckDescriptorEqStr(cmdMsg.get(), "No change");
}
-TEST_F(ZeroArguments, ECLInvalidCommandErrorSync) {
- OwningPtr<Descriptor> command{CharDescriptor("InvalidCommand")};
+TEST_F(ZeroArguments, ECLGeneralErrorCommandErrorSync) {
+ OwningPtr<Descriptor> command{CharDescriptor("cat general-error")};
+ 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(exitStat.get(), 1);
+ CheckDescriptorEqStr(cmdMsg.get(), "Invalid command lineXXX");
+#else
+ CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 1);
+ CheckDescriptorEqStr(cmdMsg.get(), "General Error: a catch-");
+#endif
+ CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 3);
+}
+
+TEST_F(ZeroArguments, ECLNotExecutedCommandErrorSync) {
+ OwningPtr<Descriptor> command{CharDescriptor("cd NotExecutedCommand")};
+ 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(exitStat.get(), 1);
+ CheckDescriptorEqStr(cmdMsg.get(), "Invalid command lineXXX");
+#else
+ CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 126);
+ CheckDescriptorEqStr(cmdMsg.get(), "Command cannot execute:");
+#endif
+ CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 3);
+}
+
+TEST_F(ZeroArguments, ECLNotFoundCommandErrorSync) {
+ OwningPtr<Descriptor> command{CharDescriptor("NotFoundCommand")};
bool wait{true};
OwningPtr<Descriptor> exitStat{IntDescriptor(404)};
OwningPtr<Descriptor> cmdStat{IntDescriptor(202)};
- OwningPtr<Descriptor> cmdMsg{CharDescriptor("Message ChangedXXXXXXXXX")};
+ OwningPtr<Descriptor> cmdMsg{CharDescriptor(
+ "cmdmsg will not modify the remaining buffer XXXXXXXXXXXXXXXXXXXX")};
RTNAME(ExecuteCommandLine)
(*command.get(), wait, exitStat.get(), cmdStat.get(), cmdMsg.get());
#ifdef _WIN32
CheckDescriptorEqInt(exitStat.get(), 1);
+ CheckDescriptorEqStr(cmdMsg.get(),
+ "Invalid command lineXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX");
#else
CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 127);
+ CheckDescriptorEqStr(cmdMsg.get(),
+ "Command not found: command was not found in the system's PATHXXX");
#endif
CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 3);
- CheckDescriptorEqStr(cmdMsg.get(), "Invalid command lineXXXX");
}
TEST_F(ZeroArguments, ECLInvalidCommandTerminatedSync) {
>From ebd933cab5c38af7519991eea71ac2066adf9875 Mon Sep 17 00:00:00 2001
From: YI WU <YI.WU2 at arm.com>
Date: Thu, 30 May 2024 15:18:26 +0100
Subject: [PATCH 6/6] rewrite if-else flow
---
flang/runtime/execute.cpp | 23 ++++++++++-------------
flang/unittests/Runtime/CommandTest.cpp | 2 +-
2 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/flang/runtime/execute.cpp b/flang/runtime/execute.cpp
index fe85971052a1e..0718ce07a4ab7 100644
--- a/flang/runtime/execute.cpp
+++ b/flang/runtime/execute.cpp
@@ -69,18 +69,6 @@ int TerminationCheck(int status, const Descriptor *cmdstat,
int exitStatusVal{status};
#else
int exitStatusVal{WEXITSTATUS(status)};
-#endif
- 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");
- }
- }
-
-#ifndef _WIN32
if (exitStatusVal == 1) {
if (!cmdstat) {
terminator.Crash("General Error with exit status code: 1");
@@ -107,8 +95,17 @@ int TerminationCheck(int status, const Descriptor *cmdstat,
CheckAndCopyCharsToDescriptor(cmdmsg,
"Command not found: command was not found in the system's PATH");
}
- }
+ } else
#endif
+ 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");
+ }
+ }
if (status == -1) {
if (!cmdstat) {
diff --git a/flang/unittests/Runtime/CommandTest.cpp b/flang/unittests/Runtime/CommandTest.cpp
index 25a878039ff0f..a4518eb3c2c97 100644
--- a/flang/unittests/Runtime/CommandTest.cpp
+++ b/flang/unittests/Runtime/CommandTest.cpp
@@ -340,7 +340,7 @@ TEST_F(ZeroArguments, ECLValidCommandStatusSetSync) {
}
TEST_F(ZeroArguments, ECLGeneralErrorCommandErrorSync) {
- OwningPtr<Descriptor> command{CharDescriptor("cat general-error")};
+ OwningPtr<Descriptor> command{CharDescriptor("cat GeneralErrorCommand")};
bool wait{true};
OwningPtr<Descriptor> exitStat{IntDescriptor(404)};
OwningPtr<Descriptor> cmdStat{IntDescriptor(202)};
More information about the flang-commits
mailing list