[llvm] [flang-rt] Fixes EXECUTE_COMMAND_LINE() status management and double buffering (PR #184285)
Eugene Epshteyn via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 2 19:26:01 PST 2026
https://github.com/eugeneepshteyn created https://github.com/llvm/llvm-project/pull/184285
EXECUTE_COMMAND_LINE() without CMDSTAT initiated termination in runtime if the command returned non-zero status code. For example, EXECUTE_COMMAND_LINE('false') on Linux would cause "fatal Fortran runtime error... : Command line execution failed with exit code: 1." This is too strict: EXECUTE_COMMAND_LINE() successfully called 'false', it's just 'false' happened to return non-zero status code. ifx and gfortran don't initiate termination in such case. Changed EXECUTE_COMMAND_LINE() implementation to behave in similar fashion.
Also during testing discovered that when the output of the program that uses EXECUTE_COMMAND_LINE(... WAIT=.false.) is piped to a file, the resulting file has duplicated output lines. This was because fork() command also ends up duplicating parent's buffered output to the child. Added flush of all units and C stdio before calling fork().
>From 44cb155b86e2675c7ebdeadf1bda3a201d58f38b Mon Sep 17 00:00:00 2001
From: Eugene Epshteyn <eepshteyn at nvidia.com>
Date: Fri, 27 Feb 2026 12:24:54 -0500
Subject: [PATCH 1/4] [flang-rt] Relaxed status checks for
EXECUTE_COMMAND_LINE()
---
flang-rt/lib/runtime/execute.cpp | 119 ++++++++++++++-----------------
1 file changed, 55 insertions(+), 64 deletions(-)
diff --git a/flang-rt/lib/runtime/execute.cpp b/flang-rt/lib/runtime/execute.cpp
index 779e040bebebc..2239fa429ec07 100644
--- a/flang-rt/lib/runtime/execute.cpp
+++ b/flang-rt/lib/runtime/execute.cpp
@@ -34,12 +34,12 @@ 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
+ ASYNC_NO_SUPPORT_ERR = -2, // Linux setsid() returns -1
+ NO_SUPPORT_ERR = -1, // system returns -1 with ENOENT
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_EXECUTION_ERR = 3, // Unexpected execution error
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
@@ -74,62 +74,81 @@ std::int64_t TerminationCheck(std::int64_t status, const Descriptor *cmdstat,
// 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.
+ constexpr char msg[] =
+ "Command line execution is not supported, system returns -1 with errno ENOENT.";
if (errno == ENOENT) {
if (!cmdstat) {
- terminator.Crash("Command line execution is not supported, system "
- "returns -1 with errno ENOENT.");
+ terminator.Crash(msg);
} else {
StoreIntToDescriptor(cmdstat, NO_SUPPORT_ERR, terminator);
- CheckAndCopyCharsToDescriptor(cmdmsg,
- "Command line execution is not supported, system returns -1 with "
- "errno ENOENT.");
+ CheckAndCopyCharsToDescriptor(cmdmsg, msg);
+ return status;
}
} else {
- char err_buffer[30];
- char msg[]{"Execution error with system status code: -1, errno: "};
+ char msg[256]{"Execution error with system status code: -1, errno: "};
+ // Append the output of strerror*() to the end of msg. Note that upon
+ // success, the output of strerror*() is always null-terminated.
+ size_t appendIndex = std::strlen(msg);
#ifdef _WIN32
- if (strerror_s(err_buffer, sizeof(err_buffer), errno) != 0)
+ if (strerror_s(msg + appendIndex, sizeof(msg) - appendIndex, errno) != 0)
#else
- if (strerror_r(errno, err_buffer, sizeof(err_buffer)) != 0)
+ if (strerror_r(errno, msg + appendIndex, sizeof(msg) - appendIndex) != 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);
+ terminator.Crash(msg);
} else {
StoreIntToDescriptor(cmdstat, EXECL_ERR, terminator);
- CheckAndCopyCharsToDescriptor(cmdmsg, newMsg);
+ CheckAndCopyCharsToDescriptor(cmdmsg, msg);
+ return status;
}
- FreeMemory(newMsg);
}
}
-#ifdef _WIN32
- // On WIN32 API std::system returns exit status directly
+ // On WIN32 API std::system() returns exit status directly. On other OS'es,
+ // special status codes are handled below.
std::int64_t exitStatusVal{status};
- if (exitStatusVal != 0) {
+#ifndef _WIN32
+
+#if defined(WIFSIGNALED) && defined(WTERMSIG)
+ if (WIFSIGNALED(status)) {
if (!cmdstat) {
- terminator.Crash(
- "Invalid command quit with exit status code: %d", exitStatusVal);
+ terminator.Crash("Killed by signal: %d", WTERMSIG(status));
} else {
- StoreIntToDescriptor(cmdstat, INVALID_CL_ERR, terminator);
- CheckAndCopyCharsToDescriptor(cmdmsg, "Invalid command line");
+ StoreIntToDescriptor(cmdstat, SIGNAL_ERR, terminator);
+ CheckAndCopyCharsToDescriptor(cmdmsg, "Killed by signal");
+ return WTERMSIG(status);
}
}
-#else
- std::int64_t exitStatusVal{WEXITSTATUS(status)};
- if (exitStatusVal == 1) {
+#endif
+
+#if defined(WIFSTOPPED) && defined(WSTOPSIG)
+ if (WIFSTOPPED(status)) {
if (!cmdstat) {
- terminator.Crash("Command line execution failed with exit code: 1.");
+ terminator.Crash("Stopped by signal: %d", WSTOPSIG(status));
+ } else {
+ StoreIntToDescriptor(cmdstat, SIGNAL_ERR, terminator);
+ CheckAndCopyCharsToDescriptor(cmdmsg, "Stopped by signal");
+ return WSTOPSIG(status);
+ }
+ }
+#endif
+
+#if defined(WIFEXITED) && defined(WEXITSTATUS)
+ // WEXITSTATUS() returns valid value only if WIFEXITED(status) is true
+ if (!WIFEXITED(status)) {
+ if (!cmdstat) {
+ terminator.Crash("Unexpected execution error: %d", status);
} else {
StoreIntToDescriptor(cmdstat, COMMAND_EXECUTION_ERR, terminator);
- CheckAndCopyCharsToDescriptor(
- cmdmsg, "Command line execution failed with exit code: 1.");
+ CheckAndCopyCharsToDescriptor(cmdmsg, "Unexpected execution error");
+ return status;
}
- } else if (exitStatusVal == 126) {
+ }
+ exitStatusVal = WEXITSTATUS(status);
+ // Status codes 126 and 127 are specific to Unix shell.
+ if (exitStatusVal == 126) {
if (!cmdstat) {
terminator.Crash("Command cannot be executed with exit code: 126.");
} else {
@@ -145,39 +164,11 @@ std::int64_t TerminationCheck(std::int64_t status, const Descriptor *cmdstat,
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);
- } else {
- StoreIntToDescriptor(cmdstat, INVALID_CL_ERR, terminator);
- CheckAndCopyCharsToDescriptor(cmdmsg, "Invalid command line");
- }
- }
-#endif
-
-#if defined(WIFSIGNALED) && defined(WTERMSIG)
- if (WIFSIGNALED(status)) {
- if (!cmdstat) {
- terminator.Crash("Killed by signal: %d", WTERMSIG(status));
- } else {
- StoreIntToDescriptor(cmdstat, SIGNAL_ERR, terminator);
- CheckAndCopyCharsToDescriptor(cmdmsg, "Killed by signal");
- }
}
-#endif
-
-#if defined(WIFSTOPPED) && defined(WSTOPSIG)
- if (WIFSTOPPED(status)) {
- if (!cmdstat) {
- terminator.Crash("Stopped by signal: %d", WSTOPSIG(status));
- } else {
- StoreIntToDescriptor(cmdstat, SIGNAL_ERR, terminator);
- CheckAndCopyCharsToDescriptor(cmdmsg, "Stopped by signal");
- }
- }
-#endif
+#endif // WIFEXITED and WEXITSTATUS
+#endif // Not _WIN32
+ // At this point, any other status code is not known to be a "crashable
+ // offense" and will be returned in EXITSTAT if provided.
return exitStatusVal;
}
>From 7166e2ccfccbb079681a7141d4d13eb7024ac830 Mon Sep 17 00:00:00 2001
From: Eugene Epshteyn <eepshteyn at nvidia.com>
Date: Mon, 2 Mar 2026 21:29:20 -0500
Subject: [PATCH 2/4] Fixed the issue of duplicate output for async
execute_command_line()
---
flang-rt/lib/runtime/execute.cpp | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/flang-rt/lib/runtime/execute.cpp b/flang-rt/lib/runtime/execute.cpp
index 2239fa429ec07..b129bf7f90eba 100644
--- a/flang-rt/lib/runtime/execute.cpp
+++ b/flang-rt/lib/runtime/execute.cpp
@@ -6,12 +6,14 @@
//
//===----------------------------------------------------------------------===//
+#include "unit.h"
#include "flang/Runtime/execute.h"
#include "flang-rt/runtime/descriptor.h"
#include "flang-rt/runtime/environment.h"
#include "flang-rt/runtime/stat.h"
#include "flang-rt/runtime/terminator.h"
#include "flang-rt/runtime/tools.h"
+#include <cstdio>
#include <cstdlib>
#include <errno.h>
#include <future>
@@ -241,6 +243,13 @@ void RTNAME(ExecuteCommandLine)(const Descriptor &command, bool wait,
}
FreeMemory(wcmd);
#else
+ // Flush all the output streams before fork() in order to avoid parent's
+ // buffered output to be replicated on the child. (Note: the issue of
+ // duplicated output didn't happen for regular terminal output, but was
+ // easy to reproduce when piping the output to a file.)
+ io::IoErrorHandler handler{terminator};
+ io::ExternalFileUnit::FlushAll(handler);
+ std::fflush(nullptr); // Also flush stdio streams
pid_t pid{fork()};
if (pid < 0) {
if (!cmdstat) {
>From 712773c65b991f17c5b86ec1548a7fb655780fec Mon Sep 17 00:00:00 2001
From: Eugene Epshteyn <eepshteyn at nvidia.com>
Date: Mon, 2 Mar 2026 21:49:55 -0500
Subject: [PATCH 3/4] Modified execute_command_line() unit tests
---
flang-rt/unittests/Runtime/CommandTest.cpp | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/flang-rt/unittests/Runtime/CommandTest.cpp b/flang-rt/unittests/Runtime/CommandTest.cpp
index 6919a98105b8a..4509c9a34c798 100644
--- a/flang-rt/unittests/Runtime/CommandTest.cpp
+++ b/flang-rt/unittests/Runtime/CommandTest.cpp
@@ -349,13 +349,8 @@ TEST_F(ZeroArguments, ECLGeneralErrorCommandErrorSync) {
RTNAME(ExecuteCommandLine)
(*command.get(), wait, exitStat.get(), cmdStat.get(), cmdMsg.get());
CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 1);
-#if defined(_WIN32)
- CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 6);
- CheckDescriptorEqStr(cmdMsg.get(), "Invalid command lineXXXXXXXXX");
-#else
- CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 3);
- CheckDescriptorEqStr(cmdMsg.get(), "Command line execution failed");
-#endif
+ CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 0);
+ CheckDescriptorEqStr(cmdMsg.get(), "cmd msg buffer XXXXXXXXXXXXXX");
}
TEST_F(ZeroArguments, ECLNotExecutedCommandErrorSync) {
@@ -371,8 +366,8 @@ TEST_F(ZeroArguments, ECLNotExecutedCommandErrorSync) {
(*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");
+ CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 0);
+ CheckDescriptorEqStr(cmdMsg.get(), "cmd msg buffer XXXXXXXX");
#else
CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 126);
CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 4);
@@ -400,8 +395,8 @@ TEST_F(ZeroArguments, ECLNotFoundCommandErrorSync) {
(*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<std::int64_t>(cmdStat.get(), 0);
+ CheckDescriptorEqStr(cmdMsg.get(), "unmodified buffer XXXXXXXXX");
#else
CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 127);
CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 5);
>From 217e98bac315a3498c463a49314f32e1c70fc960 Mon Sep 17 00:00:00 2001
From: Eugene Epshteyn <eepshteyn at nvidia.com>
Date: Mon, 2 Mar 2026 21:50:59 -0500
Subject: [PATCH 4/4] clang-format
---
flang-rt/lib/runtime/execute.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/flang-rt/lib/runtime/execute.cpp b/flang-rt/lib/runtime/execute.cpp
index b129bf7f90eba..8da7069f57009 100644
--- a/flang-rt/lib/runtime/execute.cpp
+++ b/flang-rt/lib/runtime/execute.cpp
@@ -6,8 +6,8 @@
//
//===----------------------------------------------------------------------===//
-#include "unit.h"
#include "flang/Runtime/execute.h"
+#include "unit.h"
#include "flang-rt/runtime/descriptor.h"
#include "flang-rt/runtime/environment.h"
#include "flang-rt/runtime/stat.h"
@@ -76,8 +76,8 @@ std::int64_t TerminationCheck(std::int64_t status, const Descriptor *cmdstat,
// 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.
- constexpr char msg[] =
- "Command line execution is not supported, system returns -1 with errno ENOENT.";
+ constexpr char msg[] = "Command line execution is not supported, system "
+ "returns -1 with errno ENOENT.";
if (errno == ENOENT) {
if (!cmdstat) {
terminator.Crash(msg);
More information about the llvm-commits
mailing list