[clang] [llvm] [clang-tools-extra] [flang] [flang] add SYSTEM runtime and lowering intrinsics support (PR #74309)

[Yi Wu via llvm-commits]

https://github.com/yi-wu-arm updated https://github.com/llvm/llvm-project/pull/74309

>From 14f8c3e38791cc6b06455b8beffe37a6f7105e03 Mon Sep 17 00:00:00 2001
From: Yi Wu <yi.wu2 at arm.com>
Date: Mon, 4 Dec 2023 10:32:03 +0000
Subject: [PATCH 01/18] Add SYSTEM runtime and lowering intrinsic support

Calls std::system() function and pass the command,
cmd on Windows or shell on Linux.
Command parameter is required, exitstatus is optional.
call system(command)
call system(command, exitstatus)
---
 flang/docs/Intrinsics.md                      |  2 +-
 .../flang/Optimizer/Builder/IntrinsicCall.h   |  1 +
 .../flang/Optimizer/Builder/Runtime/Command.h |  5 +++
 flang/include/flang/Runtime/command.h         |  5 +++
 flang/lib/Evaluate/intrinsics.cpp             | 16 +++++---
 flang/lib/Optimizer/Builder/IntrinsicCall.cpp | 22 ++++++++++
 .../lib/Optimizer/Builder/Runtime/Command.cpp | 13 ++++++
 flang/runtime/command.cpp                     | 19 +++++++++
 flang/test/Lower/Intrinsics/system.f90        | 39 ++++++++++++++++++
 flang/unittests/Runtime/CommandTest.cpp       | 41 +++++++++++++++++++
 10 files changed, 157 insertions(+), 6 deletions(-)
 create mode 100644 flang/test/Lower/Intrinsics/system.f90

diff --git a/flang/docs/Intrinsics.md b/flang/docs/Intrinsics.md
index fef2b4ea4dd8c8..871332399628e9 100644
--- a/flang/docs/Intrinsics.md
+++ b/flang/docs/Intrinsics.md
@@ -751,7 +751,7 @@ This phase currently supports all the intrinsic procedures listed above but the
 | Object characteristic inquiry functions | ALLOCATED, ASSOCIATED, EXTENDS_TYPE_OF, IS_CONTIGUOUS, PRESENT, RANK, SAME_TYPE, STORAGE_SIZE |
 | Type inquiry intrinsic functions | BIT_SIZE, DIGITS, EPSILON, HUGE, KIND, MAXEXPONENT, MINEXPONENT, NEW_LINE, PRECISION, RADIX, RANGE, TINY|
 | Non-standard intrinsic functions | AND, OR, XOR, SHIFT, ZEXT, IZEXT, COSD, SIND, TAND, ACOSD, ASIND, ATAND, ATAN2D, COMPL, EQV, NEQV, INT8, JINT, JNINT, KNINT, QCMPLX, DREAL, DFLOAT, QEXT, QFLOAT, QREAL, DNUM, NUM, JNUM, KNUM, QNUM, RNUM, RAN, RANF, ILEN, SIZEOF, MCLOCK, SECNDS, COTAN, IBCHNG, ISHA, ISHC, ISHL, IXOR, IARG, IARGC, NARGS, GETPID, NUMARG, BADDRESS, IADDR, CACHESIZE, EOF, FP_CLASS, INT_PTR_KIND, ISNAN, MALLOC |
-| Intrinsic subroutines |MVBITS (elemental), CPU_TIME, DATE_AND_TIME, EVENT_QUERY, EXECUTE_COMMAND_LINE, GET_COMMAND, GET_COMMAND_ARGUMENT, GET_ENVIRONMENT_VARIABLE, MOVE_ALLOC, RANDOM_INIT, RANDOM_NUMBER, RANDOM_SEED, SYSTEM_CLOCK |
+| Intrinsic subroutines |MVBITS (elemental), CPU_TIME, DATE_AND_TIME, EVENT_QUERY, EXECUTE_COMMAND_LINE, GET_COMMAND, GET_COMMAND_ARGUMENT, GET_ENVIRONMENT_VARIABLE, MOVE_ALLOC, RANDOM_INIT, RANDOM_NUMBER, RANDOM_SEED, SYSTEM, SYSTEM_CLOCK |
 | Atomic intrinsic subroutines | ATOMIC_ADD |
 | Collective intrinsic subroutines | CO_REDUCE |
 
diff --git a/flang/include/flang/Optimizer/Builder/IntrinsicCall.h b/flang/include/flang/Optimizer/Builder/IntrinsicCall.h
index 5065f11ae9e726..669d076c3e0e7d 100644
--- a/flang/include/flang/Optimizer/Builder/IntrinsicCall.h
+++ b/flang/include/flang/Optimizer/Builder/IntrinsicCall.h
@@ -321,6 +321,7 @@ struct IntrinsicLibrary {
   fir::ExtendedValue genStorageSize(mlir::Type,
                                     llvm::ArrayRef<fir::ExtendedValue>);
   fir::ExtendedValue genSum(mlir::Type, llvm::ArrayRef<fir::ExtendedValue>);
+  void genSystem(mlir::ArrayRef<fir::ExtendedValue> args);
   void genSystemClock(llvm::ArrayRef<fir::ExtendedValue>);
   mlir::Value genTand(mlir::Type, llvm::ArrayRef<mlir::Value>);
   mlir::Value genTrailz(mlir::Type, llvm::ArrayRef<mlir::Value>);
diff --git a/flang/include/flang/Optimizer/Builder/Runtime/Command.h b/flang/include/flang/Optimizer/Builder/Runtime/Command.h
index 976fb3aa0b6fbb..9d6a39639844fc 100644
--- a/flang/include/flang/Optimizer/Builder/Runtime/Command.h
+++ b/flang/include/flang/Optimizer/Builder/Runtime/Command.h
@@ -53,5 +53,10 @@ mlir::Value genGetEnvVariable(fir::FirOpBuilder &, mlir::Location,
                               mlir::Value length, mlir::Value trimName,
                               mlir::Value errmsg);
 
+/// Generate a call to System runtime function which implements
+/// the non-standard System GNU extension.
+void genSystem(fir::FirOpBuilder &, mlir::Location, mlir::Value command,
+               mlir::Value exitstat);
+
 } // namespace fir::runtime
 #endif // FORTRAN_OPTIMIZER_BUILDER_RUNTIME_COMMAND_H
diff --git a/flang/include/flang/Runtime/command.h b/flang/include/flang/Runtime/command.h
index c67d171c8e2f1b..f325faa7bd09fa 100644
--- a/flang/include/flang/Runtime/command.h
+++ b/flang/include/flang/Runtime/command.h
@@ -55,6 +55,11 @@ std::int32_t RTNAME(GetEnvVariable)(const Descriptor &name,
     const Descriptor *value = nullptr, const Descriptor *length = nullptr,
     bool trim_name = true, const Descriptor *errmsg = nullptr,
     const char *sourceFile = nullptr, int line = 0);
+
+// Calls std::system()
+void RTNAME(System)(const Descriptor *command = nullptr,
+    const Descriptor *exitstat = nullptr, const char *sourceFile = nullptr,
+    int line = 0);
 }
 } // namespace Fortran::runtime
 
diff --git a/flang/lib/Evaluate/intrinsics.cpp b/flang/lib/Evaluate/intrinsics.cpp
index c5faf319fafb7d..bb6ac8ac7159a5 100644
--- a/flang/lib/Evaluate/intrinsics.cpp
+++ b/flang/lib/Evaluate/intrinsics.cpp
@@ -1387,6 +1387,11 @@ static const IntrinsicInterface intrinsicSubroutine[]{
             {"get", DefaultInt, Rank::vector, Optionality::optional,
                 common::Intent::Out}},
         {}, Rank::elemental, IntrinsicClass::impureSubroutine},
+    {"system",
+        {{"command", DefaultChar, Rank::scalar},
+            {"exitstat", AnyInt, Rank::scalar, Optionality::optional,
+                common::Intent::InOut}},
+        {}, Rank::elemental, IntrinsicClass::impureSubroutine},
     {"system_clock",
         {{"count", AnyInt, Rank::scalar, Optionality::optional,
              common::Intent::Out},
@@ -1885,7 +1890,7 @@ std::optional<SpecificCall> IntrinsicInterface::Match(
   int elementalRank{0};
   for (std::size_t j{0}; j < dummies; ++j) {
     const IntrinsicDummyArgument &d{dummy[std::min(j, dummyArgPatterns - 1)]};
-    if (const ActualArgument *arg{actualForDummy[j]}) {
+    if (const ActualArgument * arg{actualForDummy[j]}) {
       bool isAssumedRank{IsAssumedRank(*arg)};
       if (isAssumedRank && d.rank != Rank::anyOrAssumedRank &&
           d.rank != Rank::arrayOrAssumedRank) {
@@ -2222,7 +2227,8 @@ std::optional<SpecificCall> IntrinsicInterface::Match(
     case Rank::locReduced:
     case Rank::scalarIfDim:
       if (dummy[*dimArg].optionality == Optionality::required) {
-        if (const Symbol *whole{
+        if (const Symbol *
+            whole{
                 UnwrapWholeSymbolOrComponentDataRef(actualForDummy[*dimArg])}) {
           if (IsOptional(*whole) || IsAllocatableOrObjectPointer(whole)) {
             if (context.languageFeatures().ShouldWarn(
@@ -2300,7 +2306,7 @@ std::optional<SpecificCall> IntrinsicInterface::Match(
   // Rearrange the actual arguments into dummy argument order.
   ActualArguments rearranged(dummies);
   for (std::size_t j{0}; j < dummies; ++j) {
-    if (ActualArgument *arg{actualForDummy[j]}) {
+    if (ActualArgument * arg{actualForDummy[j]}) {
       rearranged[j] = std::move(*arg);
     }
   }
@@ -2937,7 +2943,7 @@ static bool ApplySpecificChecks(SpecificCall &call, FoldingContext &context) {
     ok &= CheckForCoindexedObject(context, call.arguments[3], name, "errmsg");
     if (call.arguments[0] && call.arguments[1]) {
       for (int j{0}; j < 2; ++j) {
-        if (const Symbol *last{GetLastSymbol(call.arguments[j])};
+        if (const Symbol * last{GetLastSymbol(call.arguments[j])};
             last && !IsAllocatable(last->GetUltimate())) {
           context.messages().Say(call.arguments[j]->sourceLocation(),
               "Argument #%d to MOVE_ALLOC must be allocatable"_err_en_US,
@@ -2957,7 +2963,7 @@ static bool ApplySpecificChecks(SpecificCall &call, FoldingContext &context) {
     const auto &arg{call.arguments[0]};
     if (arg) {
       if (const auto *expr{arg->UnwrapExpr()}) {
-        if (const Symbol *symbol{UnwrapWholeSymbolDataRef(*expr)}) {
+        if (const Symbol * symbol{UnwrapWholeSymbolDataRef(*expr)}) {
           ok = symbol->attrs().test(semantics::Attr::OPTIONAL);
         }
       }
diff --git a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
index 24fdbe97856b3a..8fded510a18012 100644
--- a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
+++ b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
@@ -508,6 +508,10 @@ static constexpr IntrinsicHandler handlers[]{
        {"dim", asValue},
        {"mask", asBox, handleDynamicOptional}}},
      /*isElemental=*/false},
+    {"system",
+     &I::genSystem,
+     {{{"command", asBox}, {"exitstat", asBox, handleDynamicOptional}}},
+     /*isElemental=*/false},
     {"system_clock",
      &I::genSystemClock,
      {{{"count", asAddr}, {"count_rate", asAddr}, {"count_max", asAddr}}},
@@ -5316,6 +5320,24 @@ IntrinsicLibrary::genSum(mlir::Type resultType,
                       resultType, args);
 }
 
+// SYSTEM
+void IntrinsicLibrary::genSystem(llvm::ArrayRef<fir::ExtendedValue> args) {
+  assert(args.size() >= 1);
+  mlir::Value command = fir::getBase(args[0]);
+  const fir::ExtendedValue &exitstat = args[1];
+
+  if (!command)
+    fir::emitFatalError(loc, "expected COMMAND parameter");
+
+  mlir::Type boxNoneTy = fir::BoxType::get(builder.getNoneType());
+
+  mlir::Value exitstatBox =
+      isStaticallyPresent(exitstat)
+          ? fir::getBase(exitstat)
+          : builder.create<fir::AbsentOp>(loc, boxNoneTy).getResult();
+
+  fir::runtime::genSystem(builder, loc, command, exitstatBox);
+}
 // SYSTEM_CLOCK
 void IntrinsicLibrary::genSystemClock(llvm::ArrayRef<fir::ExtendedValue> args) {
   assert(args.size() == 3);
diff --git a/flang/lib/Optimizer/Builder/Runtime/Command.cpp b/flang/lib/Optimizer/Builder/Runtime/Command.cpp
index 1d719e7bbd9a2d..00af35a74bf871 100644
--- a/flang/lib/Optimizer/Builder/Runtime/Command.cpp
+++ b/flang/lib/Optimizer/Builder/Runtime/Command.cpp
@@ -88,3 +88,16 @@ mlir::Value fir::runtime::genGetEnvVariable(fir::FirOpBuilder &builder,
       sourceFile, sourceLine);
   return builder.create<fir::CallOp>(loc, runtimeFunc, args).getResult(0);
 }
+
+void fir::runtime::genSystem(fir::FirOpBuilder &builder, mlir::Location loc,
+                             mlir::Value command, mlir::Value exitstat) {
+  auto runtimeFunc =
+      fir::runtime::getRuntimeFunc<mkRTKey(System)>(loc, builder);
+  mlir::FunctionType runtimeFuncTy = runtimeFunc.getFunctionType();
+  mlir::Value sourceFile = fir::factory::locationToFilename(builder, loc);
+  mlir::Value sourceLine =
+      fir::factory::locationToLineNo(builder, loc, runtimeFuncTy.getInput(3));
+  llvm::SmallVector<mlir::Value> args = fir::runtime::createArguments(
+      builder, loc, runtimeFuncTy, command, exitstat, sourceFile, sourceLine);
+  builder.create<fir::CallOp>(loc, runtimeFunc, args);
+}
diff --git a/flang/runtime/command.cpp b/flang/runtime/command.cpp
index 8e6135b5487c05..a0f71147275fd1 100644
--- a/flang/runtime/command.cpp
+++ b/flang/runtime/command.cpp
@@ -282,4 +282,23 @@ std::int32_t RTNAME(GetEnvVariable)(const Descriptor &name,
   return StatOk;
 }
 
+void RTNAME(System)(const Descriptor *command, const Descriptor *exitstat,
+    const char *sourceFile, int line) {
+  Terminator terminator{sourceFile, line};
+
+  if (command) {
+    RUNTIME_CHECK(terminator, IsValidCharDescriptor(command));
+    int status{std::system(command->OffsetElement())};
+    if (exitstat) {
+      RUNTIME_CHECK(terminator, IsValidIntDescriptor(exitstat));
+#ifdef _WIN32
+      StoreLengthToDescriptor(exitstat, status, terminator);
+#else
+      int exitstatVal{WEXITSTATUS(status)};
+      StoreLengthToDescriptor(exitstat, exitstatVal, terminator);
+#endif
+    }
+  }
+}
+
 } // namespace Fortran::runtime
diff --git a/flang/test/Lower/Intrinsics/system.f90 b/flang/test/Lower/Intrinsics/system.f90
new file mode 100644
index 00000000000000..da9ccd9d22ae2f
--- /dev/null
+++ b/flang/test/Lower/Intrinsics/system.f90
@@ -0,0 +1,39 @@
+! RUN: bbc -emit-hlfir %s -o - | FileCheck %s
+
+! CHECK-LABEL: func.func @_QPall_args() {
+! CHECK:         %0 = fir.alloca !fir.char<1,30> {bindc_name = "command", uniq_name = "_QFall_argsEcommand"}
+! CHECK-NEXT:    %1:2 = hlfir.declare %0 typeparams %c30 {uniq_name = "_QFall_argsEcommand"} : (!fir.ref<!fir.char<1,30>>, index) -> (!fir.ref<!fir.char<1,30>>, !fir.ref<!fir.char<1,30>>)
+! CHECK-NEXT:    %2 = fir.alloca i32 {bindc_name = "exitval", uniq_name = "_QFall_argsEexitval"}
+! CHECK-NEXT:    %3:2 = hlfir.declare %2 {uniq_name = "_QFall_argsEexitval"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
+! CHECK-NEXT:    %4 = fir.embox %1#1 : (!fir.ref<!fir.char<1,30>>) -> !fir.box<!fir.char<1,30>>
+! CHECK-NEXT:    %5 = fir.embox %3#1 : (!fir.ref<i32>) -> !fir.box<i32>
+! CHECK-NEXT:    %6 = fir.address_of(@_QQclX6c088457724b07fb671a04391501cd9d) : !fir.ref<!fir.char<1,73>>
+! CHECK-NEXT:    %c21_i32 = arith.constant 21 : i32
+! CHECK-NEXT:    %7 = fir.convert %4 : (!fir.box<!fir.char<1,30>>) -> !fir.box<none>
+! CHECK-NEXT:    %8 = fir.convert %5 : (!fir.box<i32>) -> !fir.box<none>
+! CHECK-NEXT:    %9 = fir.convert %6 : (!fir.ref<!fir.char<1,73>>) -> !fir.ref<i8>
+! CHECK-NEXT:    %10 = fir.call @_FortranASystem(%7, %8, %9, %c21_i32) fastmath<contract> : (!fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none
+! CHECK-NEXT:    return
+! CHECK-NEXT:    }
+subroutine all_args()
+CHARACTER(30) :: command
+INTEGER :: exitVal
+call system(command, exitVal)
+end subroutine all_args
+
+! CHECK-LABEL: func.func @_QPonly_command() {
+! CHECK:         %0 = fir.alloca !fir.char<1,30> {bindc_name = "command", uniq_name = "_QFonly_commandEcommand"}
+! CHECK-NEXT:    %1:2 = hlfir.declare %0 typeparams %c30 {uniq_name = "_QFonly_commandEcommand"} : (!fir.ref<!fir.char<1,30>>, index) -> (!fir.ref<!fir.char<1,30>>, !fir.ref<!fir.char<1,30>>)
+! CHECK-NEXT:    %2 = fir.embox %1#1 : (!fir.ref<!fir.char<1,30>>) -> !fir.box<!fir.char<1,30>>
+! CHECK-NEXT:    %3 = fir.absent !fir.box<none>
+! CHECK-NEXT:    %4 = fir.address_of(@_QQclX6c088457724b07fb671a04391501cd9d) : !fir.ref<!fir.char<1,73>>
+! CHECK-NEXT:    %c38_i32 = arith.constant 38 : i32
+! CHECK-NEXT:    %5 = fir.convert %2 : (!fir.box<!fir.char<1,30>>) -> !fir.box<none> 
+! CHECK-NEXT:    %6 = fir.convert %4 : (!fir.ref<!fir.char<1,73>>) -> !fir.ref<i8> 
+! CHECK-NEXT:    %7 = fir.call @_FortranASystem(%5, %3, %6, %c38_i32) fastmath<contract> : (!fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none
+! CHECK-NEXT:    return
+! CHECK-NEXT:   }
+subroutine only_command()
+CHARACTER(30) :: command
+call system(command)
+end subroutine only_command
diff --git a/flang/unittests/Runtime/CommandTest.cpp b/flang/unittests/Runtime/CommandTest.cpp
index 9f66c7924c86e3..f0bbc721a67073 100644
--- a/flang/unittests/Runtime/CommandTest.cpp
+++ b/flang/unittests/Runtime/CommandTest.cpp
@@ -46,6 +46,17 @@ static OwningPtr<Descriptor> EmptyIntDescriptor() {
   return descriptor;
 }
 
+template <int kind = sizeof(std::int64_t)>
+static OwningPtr<Descriptor> IntDescriptor(const int &value) {
+  OwningPtr<Descriptor> descriptor{Descriptor::Create(TypeCategory::Integer,
+      kind, nullptr, 0, nullptr, CFI_attribute_allocatable)};
+  if (descriptor->Allocate() != 0) {
+    return nullptr;
+  }
+  std::memcpy(descriptor->OffsetElement<int>(), &value, sizeof(int));
+  return descriptor;
+}
+
 class CommandFixture : public ::testing::Test {
 protected:
   CommandFixture(int argc, const char *argv[]) {
@@ -227,6 +238,36 @@ TEST_F(ZeroArguments, GetCommandArgument) {
 
 TEST_F(ZeroArguments, GetCommand) { CheckCommandValue(commandOnlyArgv, 1); }
 
+TEST_F(ZeroArguments, SystemValidCommandExitStat) {
+  OwningPtr<Descriptor> command{CharDescriptor("echo hi")};
+  OwningPtr<Descriptor> exitStat{EmptyIntDescriptor()};
+
+  RTNAME(System)(command.get(), exitStat.get());
+  CheckDescriptorEqInt(exitStat.get(), 0);
+}
+
+TEST_F(ZeroArguments, SystemInvalidCommandExitStat) {
+  OwningPtr<Descriptor> command{CharDescriptor("InvalidCommand")};
+  OwningPtr<Descriptor> exitStat{EmptyIntDescriptor()};
+
+  RTNAME(System)(command.get(), exitStat.get());
+#ifdef _WIN32
+  CheckDescriptorEqInt(exitStat.get(), 1);
+#else
+  CheckDescriptorEqInt(exitStat.get(), 127);
+#endif
+}
+
+TEST_F(ZeroArguments, SystemValidCommandOptionalExitStat) {
+  OwningPtr<Descriptor> command{CharDescriptor("echo hi")};
+  EXPECT_NO_FATAL_FAILURE(RTNAME(System)(command.get(), nullptr));
+}
+
+TEST_F(ZeroArguments, SystemInvalidCommandOptionalExitStat) {
+  OwningPtr<Descriptor> command{CharDescriptor("InvalidCommand")};
+  EXPECT_NO_FATAL_FAILURE(RTNAME(System)(command.get(), nullptr));
+}
+
 static const char *oneArgArgv[]{"aProgram", "anArgumentOfLength20"};
 class OneArgument : public CommandFixture {
 protected:

>From be27d36ca18910b9cf521da26948a3fd1c03e228 Mon Sep 17 00:00:00 2001
From: Yi Wu <yiwu02 at wdev-yiwu02.arm.com>
Date: Mon, 4 Dec 2023 12:01:23 +0000
Subject: [PATCH 02/18] test fixes

---
 flang/test/Lower/Intrinsics/system.f90 | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/flang/test/Lower/Intrinsics/system.f90 b/flang/test/Lower/Intrinsics/system.f90
index da9ccd9d22ae2f..0c3e5fd63047c6 100644
--- a/flang/test/Lower/Intrinsics/system.f90
+++ b/flang/test/Lower/Intrinsics/system.f90
@@ -7,12 +7,10 @@
 ! CHECK-NEXT:    %3:2 = hlfir.declare %2 {uniq_name = "_QFall_argsEexitval"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
 ! CHECK-NEXT:    %4 = fir.embox %1#1 : (!fir.ref<!fir.char<1,30>>) -> !fir.box<!fir.char<1,30>>
 ! CHECK-NEXT:    %5 = fir.embox %3#1 : (!fir.ref<i32>) -> !fir.box<i32>
-! CHECK-NEXT:    %6 = fir.address_of(@_QQclX6c088457724b07fb671a04391501cd9d) : !fir.ref<!fir.char<1,73>>
-! CHECK-NEXT:    %c21_i32 = arith.constant 21 : i32
+! CHECK:         %c19_i32 = arith.constant 19 : i32
 ! CHECK-NEXT:    %7 = fir.convert %4 : (!fir.box<!fir.char<1,30>>) -> !fir.box<none>
 ! CHECK-NEXT:    %8 = fir.convert %5 : (!fir.box<i32>) -> !fir.box<none>
-! CHECK-NEXT:    %9 = fir.convert %6 : (!fir.ref<!fir.char<1,73>>) -> !fir.ref<i8>
-! CHECK-NEXT:    %10 = fir.call @_FortranASystem(%7, %8, %9, %c21_i32) fastmath<contract> : (!fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none
+! CHECK:         %10 = fir.call @_FortranASystem(%7, %8, %9, %c19_i32) fastmath<contract> : (!fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none
 ! CHECK-NEXT:    return
 ! CHECK-NEXT:    }
 subroutine all_args()
@@ -26,11 +24,9 @@ end subroutine all_args
 ! CHECK-NEXT:    %1:2 = hlfir.declare %0 typeparams %c30 {uniq_name = "_QFonly_commandEcommand"} : (!fir.ref<!fir.char<1,30>>, index) -> (!fir.ref<!fir.char<1,30>>, !fir.ref<!fir.char<1,30>>)
 ! CHECK-NEXT:    %2 = fir.embox %1#1 : (!fir.ref<!fir.char<1,30>>) -> !fir.box<!fir.char<1,30>>
 ! CHECK-NEXT:    %3 = fir.absent !fir.box<none>
-! CHECK-NEXT:    %4 = fir.address_of(@_QQclX6c088457724b07fb671a04391501cd9d) : !fir.ref<!fir.char<1,73>>
-! CHECK-NEXT:    %c38_i32 = arith.constant 38 : i32
+! CHECK:         %c34_i32 = arith.constant 34 : i32
 ! CHECK-NEXT:    %5 = fir.convert %2 : (!fir.box<!fir.char<1,30>>) -> !fir.box<none> 
-! CHECK-NEXT:    %6 = fir.convert %4 : (!fir.ref<!fir.char<1,73>>) -> !fir.ref<i8> 
-! CHECK-NEXT:    %7 = fir.call @_FortranASystem(%5, %3, %6, %c38_i32) fastmath<contract> : (!fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none
+! CHECK:         %7 = fir.call @_FortranASystem(%5, %3, %6, %c34_i32) fastmath<contract> : (!fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none
 ! CHECK-NEXT:    return
 ! CHECK-NEXT:   }
 subroutine only_command()

>From d8215296237586aabc180ed36c4220f58ccef8e3 Mon Sep 17 00:00:00 2001
From: Yi Wu <yi.wu2 at arm.com>
Date: Mon, 4 Dec 2023 12:22:38 +0000
Subject: [PATCH 03/18] remove unnecessary clang-format change

---
 flang/lib/Evaluate/intrinsics.cpp | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/flang/lib/Evaluate/intrinsics.cpp b/flang/lib/Evaluate/intrinsics.cpp
index bb6ac8ac7159a5..98dd2b27a08ebf 100644
--- a/flang/lib/Evaluate/intrinsics.cpp
+++ b/flang/lib/Evaluate/intrinsics.cpp
@@ -1890,7 +1890,7 @@ std::optional<SpecificCall> IntrinsicInterface::Match(
   int elementalRank{0};
   for (std::size_t j{0}; j < dummies; ++j) {
     const IntrinsicDummyArgument &d{dummy[std::min(j, dummyArgPatterns - 1)]};
-    if (const ActualArgument * arg{actualForDummy[j]}) {
+    if (const ActualArgument *arg{actualForDummy[j]}) {
       bool isAssumedRank{IsAssumedRank(*arg)};
       if (isAssumedRank && d.rank != Rank::anyOrAssumedRank &&
           d.rank != Rank::arrayOrAssumedRank) {
@@ -2227,8 +2227,7 @@ std::optional<SpecificCall> IntrinsicInterface::Match(
     case Rank::locReduced:
     case Rank::scalarIfDim:
       if (dummy[*dimArg].optionality == Optionality::required) {
-        if (const Symbol *
-            whole{
+        if (const Symbol *whole{
                 UnwrapWholeSymbolOrComponentDataRef(actualForDummy[*dimArg])}) {
           if (IsOptional(*whole) || IsAllocatableOrObjectPointer(whole)) {
             if (context.languageFeatures().ShouldWarn(
@@ -2306,7 +2305,7 @@ std::optional<SpecificCall> IntrinsicInterface::Match(
   // Rearrange the actual arguments into dummy argument order.
   ActualArguments rearranged(dummies);
   for (std::size_t j{0}; j < dummies; ++j) {
-    if (ActualArgument * arg{actualForDummy[j]}) {
+    if (ActualArgument *arg{actualForDummy[j]}) {
       rearranged[j] = std::move(*arg);
     }
   }
@@ -2963,7 +2962,7 @@ static bool ApplySpecificChecks(SpecificCall &call, FoldingContext &context) {
     const auto &arg{call.arguments[0]};
     if (arg) {
       if (const auto *expr{arg->UnwrapExpr()}) {
-        if (const Symbol * symbol{UnwrapWholeSymbolDataRef(*expr)}) {
+        if (const Symbol *symbol{UnwrapWholeSymbolDataRef(*expr)}) {
           ok = symbol->attrs().test(semantics::Attr::OPTIONAL);
         }
       }

>From 9a73cb8b5e30a0963dceb3b8f2e6265f400f2f71 Mon Sep 17 00:00:00 2001
From: Yi Wu <yi.wu2 at arm.com>
Date: Thu, 7 Dec 2023 12:20:05 +0000
Subject: [PATCH 04/18] required parameter use reference not pointer, ensure
 command is null-terminated

---
 flang/include/flang/Runtime/command.h   |  2 +-
 flang/runtime/command.cpp               | 37 ++++++++++++++++++-------
 flang/unittests/Runtime/CommandTest.cpp |  8 +++---
 3 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/flang/include/flang/Runtime/command.h b/flang/include/flang/Runtime/command.h
index f325faa7bd09fa..e477a2da1c5954 100644
--- a/flang/include/flang/Runtime/command.h
+++ b/flang/include/flang/Runtime/command.h
@@ -57,7 +57,7 @@ std::int32_t RTNAME(GetEnvVariable)(const Descriptor &name,
     const char *sourceFile = nullptr, int line = 0);
 
 // Calls std::system()
-void RTNAME(System)(const Descriptor *command = nullptr,
+void RTNAME(System)(const Descriptor &command,
     const Descriptor *exitstat = nullptr, const char *sourceFile = nullptr,
     int line = 0);
 }
diff --git a/flang/runtime/command.cpp b/flang/runtime/command.cpp
index a0f71147275fd1..709e1cb4904611 100644
--- a/flang/runtime/command.cpp
+++ b/flang/runtime/command.cpp
@@ -282,22 +282,39 @@ std::int32_t RTNAME(GetEnvVariable)(const Descriptor &name,
   return StatOk;
 }
 
-void RTNAME(System)(const Descriptor *command, const Descriptor *exitstat,
+const char *ensureNullTerminated(
+    const char *str, size_t length, Terminator &terminator) {
+  if (length < strlen(str)) {
+    char *newCmd{(char *)malloc(length + 1)};
+    if (newCmd == NULL) {
+      terminator.Crash("Command not null-terminated, memory allocation failed "
+                       "for null-terminated newCmd.");
+    }
+
+    strncpy(newCmd, str, length);
+    newCmd[length] = '\0';
+    return newCmd;
+  } else {
+    return str;
+  }
+}
+
+void RTNAME(System)(const Descriptor &command, const Descriptor *exitstat,
     const char *sourceFile, int line) {
   Terminator terminator{sourceFile, line};
 
-  if (command) {
-    RUNTIME_CHECK(terminator, IsValidCharDescriptor(command));
-    int status{std::system(command->OffsetElement())};
-    if (exitstat) {
-      RUNTIME_CHECK(terminator, IsValidIntDescriptor(exitstat));
+  const char *newCmd{ensureNullTerminated(
+      command.OffsetElement(), command.ElementBytes(), terminator)};
+  int status{std::system(newCmd)};
+
+  if (exitstat) {
+    RUNTIME_CHECK(terminator, IsValidIntDescriptor(exitstat));
 #ifdef _WIN32
-      StoreLengthToDescriptor(exitstat, status, terminator);
+    StoreLengthToDescriptor(exitstat, status, terminator);
 #else
-      int exitstatVal{WEXITSTATUS(status)};
-      StoreLengthToDescriptor(exitstat, exitstatVal, terminator);
+    int exitstatVal{WEXITSTATUS(status)};
+    StoreLengthToDescriptor(exitstat, exitstatVal, terminator);
 #endif
-    }
   }
 }
 
diff --git a/flang/unittests/Runtime/CommandTest.cpp b/flang/unittests/Runtime/CommandTest.cpp
index f0bbc721a67073..466e64cf65299e 100644
--- a/flang/unittests/Runtime/CommandTest.cpp
+++ b/flang/unittests/Runtime/CommandTest.cpp
@@ -242,7 +242,7 @@ TEST_F(ZeroArguments, SystemValidCommandExitStat) {
   OwningPtr<Descriptor> command{CharDescriptor("echo hi")};
   OwningPtr<Descriptor> exitStat{EmptyIntDescriptor()};
 
-  RTNAME(System)(command.get(), exitStat.get());
+  RTNAME(System)(*command.get(), exitStat.get());
   CheckDescriptorEqInt(exitStat.get(), 0);
 }
 
@@ -250,7 +250,7 @@ TEST_F(ZeroArguments, SystemInvalidCommandExitStat) {
   OwningPtr<Descriptor> command{CharDescriptor("InvalidCommand")};
   OwningPtr<Descriptor> exitStat{EmptyIntDescriptor()};
 
-  RTNAME(System)(command.get(), exitStat.get());
+  RTNAME(System)(*command.get(), exitStat.get());
 #ifdef _WIN32
   CheckDescriptorEqInt(exitStat.get(), 1);
 #else
@@ -260,12 +260,12 @@ TEST_F(ZeroArguments, SystemInvalidCommandExitStat) {
 
 TEST_F(ZeroArguments, SystemValidCommandOptionalExitStat) {
   OwningPtr<Descriptor> command{CharDescriptor("echo hi")};
-  EXPECT_NO_FATAL_FAILURE(RTNAME(System)(command.get(), nullptr));
+  EXPECT_NO_FATAL_FAILURE(RTNAME(System)(*command.get(), nullptr));
 }
 
 TEST_F(ZeroArguments, SystemInvalidCommandOptionalExitStat) {
   OwningPtr<Descriptor> command{CharDescriptor("InvalidCommand")};
-  EXPECT_NO_FATAL_FAILURE(RTNAME(System)(command.get(), nullptr));
+  EXPECT_NO_FATAL_FAILURE(RTNAME(System)(*command.get(), nullptr));
 }
 
 static const char *oneArgArgv[]{"aProgram", "anArgumentOfLength20"};

>From da3f993b775c0e9a241a38b73eb06a41ea65ecd3 Mon Sep 17 00:00:00 2001
From: Yi Wu <yi.wu2 at arm.com>
Date: Tue, 12 Dec 2023 11:30:16 +0000
Subject: [PATCH 05/18] remove redundant clang-format and minor fixes

---
 flang/lib/Evaluate/intrinsics.cpp | 2 +-
 flang/runtime/command.cpp         | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/flang/lib/Evaluate/intrinsics.cpp b/flang/lib/Evaluate/intrinsics.cpp
index 98dd2b27a08ebf..a1066ef521961e 100644
--- a/flang/lib/Evaluate/intrinsics.cpp
+++ b/flang/lib/Evaluate/intrinsics.cpp
@@ -2942,7 +2942,7 @@ static bool ApplySpecificChecks(SpecificCall &call, FoldingContext &context) {
     ok &= CheckForCoindexedObject(context, call.arguments[3], name, "errmsg");
     if (call.arguments[0] && call.arguments[1]) {
       for (int j{0}; j < 2; ++j) {
-        if (const Symbol * last{GetLastSymbol(call.arguments[j])};
+        if (const Symbol *last{GetLastSymbol(call.arguments[j])};
             last && !IsAllocatable(last->GetUltimate())) {
           context.messages().Say(call.arguments[j]->sourceLocation(),
               "Argument #%d to MOVE_ALLOC must be allocatable"_err_en_US,
diff --git a/flang/runtime/command.cpp b/flang/runtime/command.cpp
index 709e1cb4904611..03efb57e2c8cd7 100644
--- a/flang/runtime/command.cpp
+++ b/flang/runtime/command.cpp
@@ -284,7 +284,7 @@ std::int32_t RTNAME(GetEnvVariable)(const Descriptor &name,
 
 const char *ensureNullTerminated(
     const char *str, size_t length, Terminator &terminator) {
-  if (length < strlen(str)) {
+  if (length <= strlen(str)) {
     char *newCmd{(char *)malloc(length + 1)};
     if (newCmd == NULL) {
       terminator.Crash("Command not null-terminated, memory allocation failed "

>From 2dbeb8162750e9a2bed81b09f20cef66c8967587 Mon Sep 17 00:00:00 2001
From: Yi Wu <yi.wu2 at arm.com>
Date: Thu, 14 Dec 2023 13:19:55 +0000
Subject: [PATCH 06/18] tests fixes: avoid hard-coded SSA

---
 .../test/Lower/Intrinsics/system-optional.f90 | 24 +++++++++
 flang/test/Lower/Intrinsics/system.f90        | 54 ++++++++++---------
 2 files changed, 52 insertions(+), 26 deletions(-)
 create mode 100644 flang/test/Lower/Intrinsics/system-optional.f90

diff --git a/flang/test/Lower/Intrinsics/system-optional.f90 b/flang/test/Lower/Intrinsics/system-optional.f90
new file mode 100644
index 00000000000000..d9f5a79965a98a
--- /dev/null
+++ b/flang/test/Lower/Intrinsics/system-optional.f90
@@ -0,0 +1,24 @@
+! RUN: bbc -emit-hlfir %s -o - | FileCheck %s
+
+! CHECK-LABEL: func.func @_QPall_args(
+! CHECK-SAME:    %[[commandArg:.*]]: !fir.boxchar<1> {fir.bindc_name = "command", fir.optional},
+! CHECK-SAME:    %[[exitvalArg:.*]]: !fir.ref<i32> {fir.bindc_name = "exitval", fir.optional}) {
+subroutine all_args(command, exitVal)
+CHARACTER(*), OPTIONAL :: command
+INTEGER, OPTIONAL :: exitVal
+call system(command, exitVal)
+! CHECK-NEXT:    %[[commandUnbox:.*]]:2 = fir.unboxchar %[[commandArg]] : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index) 
+! CHECK-NEXT:    %[[commandDeclare:.*]]:2 = hlfir.declare %[[commandUnbox]]#0 typeparams %[[commandUnbox]]#1 {fortran_attrs = #fir.var_attrs<optional>, uniq_name = "_QFall_argsEcommand"} : (!fir.ref<!fir.char<1,?>>, index) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>) 
+! CHECK-NEXT:    %[[exitvalDeclare:.*]]:2 = hlfir.declare %[[exitvalArg]] {fortran_attrs = #fir.var_attrs<optional>, uniq_name = "_QFall_argsEexitval"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>) 
+! CHECK-NEXT:    %[[exitvalIsPresent:.*]] = fir.is_present %[[exitvalDeclare]]#0 : (!fir.ref<i32>) -> i1
+! CHECK-NEXT:    %[[commandBox:.*]] = fir.embox %[[commandDeclare]]#1 typeparams %[[commandUnbox]]#1 : (!fir.ref<!fir.char<1,?>>, index) -> !fir.box<!fir.char<1,?>>
+! CHECK-NEXT:    %[[exitvalBox:.*]] = fir.embox %[[exitvalDeclare]]#1 : (!fir.ref<i32>) -> !fir.box<i32>
+! CHECK-NEXT:    %[[absentBox:.*]] = fir.absent !fir.box<i32>
+! CHECK-NEXT:    %[[exitvalSelect:.*]] = arith.select %[[exitvalIsPresent]], %[[exitvalBox]], %[[absentBox]] : !fir.box<i32>
+! CHECK:         %c9_i32 = arith.constant 9 : i32
+! CHECK-NEXT:    %[[command:.*]] = fir.convert %[[commandBox]] : (!fir.box<!fir.char<1,?>>) -> !fir.box<none>
+! CHECK-NEXT:    %[[exitval:.*]] = fir.convert %[[exitvalSelect]] : (!fir.box<i32>) -> !fir.box<none>
+! CHECK:         %[[VAL_12:.*]] = fir.call @_FortranASystem(%[[command]], %[[exitval]], %[[VAL_11:.*]], %c9_i32) fastmath<contract> : (!fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none
+! CHECK-NEXT:    return
+! CHECK-NEXT:    }
+end subroutine all_args
diff --git a/flang/test/Lower/Intrinsics/system.f90 b/flang/test/Lower/Intrinsics/system.f90
index 0c3e5fd63047c6..46fc828a544acc 100644
--- a/flang/test/Lower/Intrinsics/system.f90
+++ b/flang/test/Lower/Intrinsics/system.f90
@@ -1,35 +1,37 @@
 ! RUN: bbc -emit-hlfir %s -o - | FileCheck %s
 
-! CHECK-LABEL: func.func @_QPall_args() {
-! CHECK:         %0 = fir.alloca !fir.char<1,30> {bindc_name = "command", uniq_name = "_QFall_argsEcommand"}
-! CHECK-NEXT:    %1:2 = hlfir.declare %0 typeparams %c30 {uniq_name = "_QFall_argsEcommand"} : (!fir.ref<!fir.char<1,30>>, index) -> (!fir.ref<!fir.char<1,30>>, !fir.ref<!fir.char<1,30>>)
-! CHECK-NEXT:    %2 = fir.alloca i32 {bindc_name = "exitval", uniq_name = "_QFall_argsEexitval"}
-! CHECK-NEXT:    %3:2 = hlfir.declare %2 {uniq_name = "_QFall_argsEexitval"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
-! CHECK-NEXT:    %4 = fir.embox %1#1 : (!fir.ref<!fir.char<1,30>>) -> !fir.box<!fir.char<1,30>>
-! CHECK-NEXT:    %5 = fir.embox %3#1 : (!fir.ref<i32>) -> !fir.box<i32>
-! CHECK:         %c19_i32 = arith.constant 19 : i32
-! CHECK-NEXT:    %7 = fir.convert %4 : (!fir.box<!fir.char<1,30>>) -> !fir.box<none>
-! CHECK-NEXT:    %8 = fir.convert %5 : (!fir.box<i32>) -> !fir.box<none>
-! CHECK:         %10 = fir.call @_FortranASystem(%7, %8, %9, %c19_i32) fastmath<contract> : (!fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none
-! CHECK-NEXT:    return
-! CHECK-NEXT:    }
-subroutine all_args()
-CHARACTER(30) :: command
+! CHECK-LABEL: func.func @_QPall_args(
+! CHECK-SAME:    %[[commandArg:.*]]: !fir.boxchar<1> {fir.bindc_name = "command"}, 
+! CHECK-SAME:    %[[exitvalArg:.*]]: !fir.ref<i32> {fir.bindc_name = "exitval"}) { 
+subroutine all_args(command, exitVal)
+CHARACTER(*) :: command
 INTEGER :: exitVal
 call system(command, exitVal)
+! CHECK-NEXT:    %[[commandUnbox:.*]]:2 = fir.unboxchar %[[commandArg]] : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index) 
+! CHECK-NEXT:    %[[commandDeclare:.*]]:2 = hlfir.declare %[[commandUnbox]]#0 typeparams %[[commandUnbox]]#1 {uniq_name = "_QFall_argsEcommand"} : (!fir.ref<!fir.char<1,?>>, index) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>)
+! CHECK-NEXT:    %[[exitvalDeclare:.*]]:2 = hlfir.declare %[[exitvalArg]] {uniq_name = "_QFall_argsEexitval"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
+! CHECK-NEXT:    %[[commandBox:.*]] = fir.embox %[[commandDeclare]]#1 typeparams %[[commandUnbox]]#1 : (!fir.ref<!fir.char<1,?>>, index) -> !fir.box<!fir.char<1,?>>
+! CHECK-NEXT:    %[[exitvalBox:.*]] = fir.embox %[[exitvalDeclare]]#1 : (!fir.ref<i32>) -> !fir.box<i32>
+! CHECK:         %c9_i32 = arith.constant 9 : i32
+! CHECK-NEXT:    %[[command:.*]] = fir.convert %[[commandBox]] : (!fir.box<!fir.char<1,?>>) -> !fir.box<none> 
+! CHECK-NEXT:    %[[exitval:.*]] = fir.convert %[[exitvalBox]] : (!fir.box<i32>) -> !fir.box<none>
+! CHECK:         %[[VAL_9:.*]] = fir.call @_FortranASystem(%[[command]], %[[exitval]], %[[VAL_8:.*]], %c9_i32) fastmath<contract> : (!fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none
+! CHECK-NEXT:    return
+! CHECK-NEXT:    }
 end subroutine all_args
 
-! CHECK-LABEL: func.func @_QPonly_command() {
-! CHECK:         %0 = fir.alloca !fir.char<1,30> {bindc_name = "command", uniq_name = "_QFonly_commandEcommand"}
-! CHECK-NEXT:    %1:2 = hlfir.declare %0 typeparams %c30 {uniq_name = "_QFonly_commandEcommand"} : (!fir.ref<!fir.char<1,30>>, index) -> (!fir.ref<!fir.char<1,30>>, !fir.ref<!fir.char<1,30>>)
-! CHECK-NEXT:    %2 = fir.embox %1#1 : (!fir.ref<!fir.char<1,30>>) -> !fir.box<!fir.char<1,30>>
-! CHECK-NEXT:    %3 = fir.absent !fir.box<none>
-! CHECK:         %c34_i32 = arith.constant 34 : i32
-! CHECK-NEXT:    %5 = fir.convert %2 : (!fir.box<!fir.char<1,30>>) -> !fir.box<none> 
-! CHECK:         %7 = fir.call @_FortranASystem(%5, %3, %6, %c34_i32) fastmath<contract> : (!fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none
+! CHECK-LABEL: func.func @_QPonly_command(
+! CHECK-SAME:    %[[commandArg:.*]]: !fir.boxchar<1> {fir.bindc_name = "command"}) {
+subroutine only_command(command)
+CHARACTER(*) :: command
+call system(command)
+! CHECK-NEXT:    %[[commandUnbox:.*]]:2 = fir.unboxchar %[[commandArg]] : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)
+! CHECK-NEXT:    %[[commandDeclare:.*]]:2 = hlfir.declare %[[commandUnbox]]#0 typeparams %[[commandUnbox]]#1 {uniq_name = "_QFonly_commandEcommand"} : (!fir.ref<!fir.char<1,?>>, index) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>)
+! CHECK-NEXT:    %[[commandBox:.*]] = fir.embox %[[commandDeclare]]#1 typeparams %[[commandUnbox]]#1 : (!fir.ref<!fir.char<1,?>>, index) -> !fir.box<!fir.char<1,?>>
+! CHECK-NEXT:    %[[absentBox:.*]] = fir.absent !fir.box<none>
+! CHECK:         %c27_i32 = arith.constant 27 : i32
+! CHECK-NEXT:    %[[command:.*]] = fir.convert %[[commandBox]] : (!fir.box<!fir.char<1,?>>) -> !fir.box<none>
+! CHECK:         %[[VAL_7:.*]] = fir.call @_FortranASystem(%[[command]], %[[absentBox]], %[[VAL_6:.*]], %c27_i32) fastmath<contract> : (!fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none
 ! CHECK-NEXT:    return
 ! CHECK-NEXT:   }
-subroutine only_command()
-CHARACTER(30) :: command
-call system(command)
 end subroutine only_command

>From 27aa276d16fcd754377286318f9738f59695b7e5 Mon Sep 17 00:00:00 2001
From: Yi Wu <yi.wu2 at arm.com>
Date: Thu, 14 Dec 2023 13:43:52 +0000
Subject: [PATCH 07/18] move EnsureNullTerminated to tools.h, capitalized and
 free memory after use

---
 flang/runtime/command.cpp | 20 ++------------------
 flang/runtime/tools.h     | 12 ++++++++++++
 2 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/flang/runtime/command.cpp b/flang/runtime/command.cpp
index 03efb57e2c8cd7..4db7ed38e8d20a 100644
--- a/flang/runtime/command.cpp
+++ b/flang/runtime/command.cpp
@@ -282,28 +282,11 @@ std::int32_t RTNAME(GetEnvVariable)(const Descriptor &name,
   return StatOk;
 }
 
-const char *ensureNullTerminated(
-    const char *str, size_t length, Terminator &terminator) {
-  if (length <= strlen(str)) {
-    char *newCmd{(char *)malloc(length + 1)};
-    if (newCmd == NULL) {
-      terminator.Crash("Command not null-terminated, memory allocation failed "
-                       "for null-terminated newCmd.");
-    }
-
-    strncpy(newCmd, str, length);
-    newCmd[length] = '\0';
-    return newCmd;
-  } else {
-    return str;
-  }
-}
-
 void RTNAME(System)(const Descriptor &command, const Descriptor *exitstat,
     const char *sourceFile, int line) {
   Terminator terminator{sourceFile, line};
 
-  const char *newCmd{ensureNullTerminated(
+  const char *newCmd{EnsureNullTerminated(
       command.OffsetElement(), command.ElementBytes(), terminator)};
   int status{std::system(newCmd)};
 
@@ -315,6 +298,7 @@ void RTNAME(System)(const Descriptor &command, const Descriptor *exitstat,
     int exitstatVal{WEXITSTATUS(status)};
     StoreLengthToDescriptor(exitstat, exitstatVal, terminator);
 #endif
+    FreeMemory((void *)newCmd);
   }
 }
 
diff --git a/flang/runtime/tools.h b/flang/runtime/tools.h
index ea659190e14391..735acad728422b 100644
--- a/flang/runtime/tools.h
+++ b/flang/runtime/tools.h
@@ -411,5 +411,17 @@ RT_API_ATTRS void ShallowCopy(const Descriptor &to, const Descriptor &from,
     bool toIsContiguous, bool fromIsContiguous);
 RT_API_ATTRS void ShallowCopy(const Descriptor &to, const Descriptor &from);
 
+inline RT_API_ATTRS const char *EnsureNullTerminated(
+    const char *str, size_t length, Terminator &terminator) {
+  if (length <= std::strlen(str)) {
+    char *newCmd{(char *)AllocateMemoryOrCrash(terminator, length + 1)};
+    std::memcpy(newCmd, str, length);
+    newCmd[length] = '\0';
+    return newCmd;
+  } else {
+    return str;
+  }
+}
+
 } // namespace Fortran::runtime
 #endif // FORTRAN_RUNTIME_TOOLS_H_

>From cb88f6ebe458193cac62199005392fc7bda6e2db Mon Sep 17 00:00:00 2001
From: Yi Wu <yi.wu2 at arm.com>
Date: Thu, 14 Dec 2023 16:59:59 +0000
Subject: [PATCH 08/18] declare in tools.g, define in tools.cpp

---
 flang/runtime/tools.cpp | 12 ++++++++++++
 flang/runtime/tools.h   | 13 ++-----------
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/flang/runtime/tools.cpp b/flang/runtime/tools.cpp
index a027559d9f4a74..b46cec9239d428 100644
--- a/flang/runtime/tools.cpp
+++ b/flang/runtime/tools.cpp
@@ -173,5 +173,17 @@ RT_API_ATTRS void ShallowCopy(const Descriptor &to, const Descriptor &from) {
   ShallowCopy(to, from, to.IsContiguous(), from.IsContiguous());
 }
 
+RT_API_ATTRS const char *EnsureNullTerminated(
+    const char *str, size_t length, Terminator &terminator) {
+  if (length <= std::strlen(str)) {
+    char *newCmd{(char *)AllocateMemoryOrCrash(terminator, length + 1)};
+    std::memcpy(newCmd, str, length);
+    newCmd[length] = '\0';
+    return newCmd;
+  } else {
+    return str;
+  }
+}
+
 RT_OFFLOAD_API_GROUP_END
 } // namespace Fortran::runtime
diff --git a/flang/runtime/tools.h b/flang/runtime/tools.h
index 735acad728422b..30a9add01ffbce 100644
--- a/flang/runtime/tools.h
+++ b/flang/runtime/tools.h
@@ -411,17 +411,8 @@ RT_API_ATTRS void ShallowCopy(const Descriptor &to, const Descriptor &from,
     bool toIsContiguous, bool fromIsContiguous);
 RT_API_ATTRS void ShallowCopy(const Descriptor &to, const Descriptor &from);
 
-inline RT_API_ATTRS const char *EnsureNullTerminated(
-    const char *str, size_t length, Terminator &terminator) {
-  if (length <= std::strlen(str)) {
-    char *newCmd{(char *)AllocateMemoryOrCrash(terminator, length + 1)};
-    std::memcpy(newCmd, str, length);
-    newCmd[length] = '\0';
-    return newCmd;
-  } else {
-    return str;
-  }
-}
+RT_API_ATTRS const char *EnsureNullTerminated(
+    const char *str, size_t length, Terminator &terminator);
 
 } // namespace Fortran::runtime
 #endif // FORTRAN_RUNTIME_TOOLS_H_

>From fb15f831b25286bb95d09968d181e92893608f18 Mon Sep 17 00:00:00 2001
From: Yi Wu <yi.wu2 at arm.com>
Date: Wed, 20 Dec 2023 16:04:42 +0000
Subject: [PATCH 09/18] free memoryin the end

---
 flang/runtime/command.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/flang/runtime/command.cpp b/flang/runtime/command.cpp
index 4db7ed38e8d20a..11187b07fc267f 100644
--- a/flang/runtime/command.cpp
+++ b/flang/runtime/command.cpp
@@ -298,8 +298,8 @@ void RTNAME(System)(const Descriptor &command, const Descriptor *exitstat,
     int exitstatVal{WEXITSTATUS(status)};
     StoreLengthToDescriptor(exitstat, exitstatVal, terminator);
 #endif
-    FreeMemory((void *)newCmd);
   }
+  FreeMemory((void *)newCmd);
 }
 
 } // namespace Fortran::runtime

>From d147dbb031083017ab0ee26f1ac2bf00275d6ef1 Mon Sep 17 00:00:00 2001
From: Yi Wu <yi.wu2 at arm.com>
Date: Fri, 29 Dec 2023 13:18:54 +0000
Subject: [PATCH 10/18] memory deallocation fixes, and use std::size_t

---
 flang/runtime/command.cpp | 5 ++++-
 flang/runtime/tools.cpp   | 6 ++++--
 flang/runtime/tools.h     | 6 +++++-
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/flang/runtime/command.cpp b/flang/runtime/command.cpp
index 11187b07fc267f..9a21313ecb2e6a 100644
--- a/flang/runtime/command.cpp
+++ b/flang/runtime/command.cpp
@@ -299,7 +299,10 @@ void RTNAME(System)(const Descriptor &command, const Descriptor *exitstat,
     StoreLengthToDescriptor(exitstat, exitstatVal, terminator);
 #endif
   }
-  FreeMemory((void *)newCmd);
+  // Deallocate memory if EnsureNullTerminated dynamically allocate a memory
+  if (newCmd != command.OffsetElement()) {
+    FreeMemory((void *)newCmd);
+  }
 }
 
 } // namespace Fortran::runtime
diff --git a/flang/runtime/tools.cpp b/flang/runtime/tools.cpp
index b46cec9239d428..c9b7a9922d6453 100644
--- a/flang/runtime/tools.cpp
+++ b/flang/runtime/tools.cpp
@@ -174,8 +174,10 @@ RT_API_ATTRS void ShallowCopy(const Descriptor &to, const Descriptor &from) {
 }
 
 RT_API_ATTRS const char *EnsureNullTerminated(
-    const char *str, size_t length, Terminator &terminator) {
-  if (length <= std::strlen(str)) {
+    const char *str, std::size_t length, Terminator &terminator) {
+  const void *nullTerminatorPos{std::memchr(str, '\0', length)};
+
+  if (nullTerminatorPos == nullptr) {
     char *newCmd{(char *)AllocateMemoryOrCrash(terminator, length + 1)};
     std::memcpy(newCmd, str, length);
     newCmd[length] = '\0';
diff --git a/flang/runtime/tools.h b/flang/runtime/tools.h
index 30a9add01ffbce..003ac66e7c7c99 100644
--- a/flang/runtime/tools.h
+++ b/flang/runtime/tools.h
@@ -411,8 +411,12 @@ RT_API_ATTRS void ShallowCopy(const Descriptor &to, const Descriptor &from,
     bool toIsContiguous, bool fromIsContiguous);
 RT_API_ATTRS void ShallowCopy(const Descriptor &to, const Descriptor &from);
 
+// Ensures that a character string is null-terminated, allocating a /p length +1
+// size memory for null-terminator if necessary. Returns the original or a newly
+// allocated null-terminated string (responsibility for deallocation is on the
+// caller).
 RT_API_ATTRS const char *EnsureNullTerminated(
-    const char *str, size_t length, Terminator &terminator);
+    const char *str, std::size_t length, Terminator &terminator);
 
 } // namespace Fortran::runtime
 #endif // FORTRAN_RUNTIME_TOOLS_H_

>From cce16643a918059206f7d8d449aad5364b2e079d Mon Sep 17 00:00:00 2001
From: Yi Wu <yi.wu2 at arm.com>
Date: Tue, 16 Jan 2024 12:08:58 +0000
Subject: [PATCH 11/18] refactor: move SYSTEM from command.cpp to execute.cpp

---
 .../flang/Optimizer/Builder/Runtime/Command.h |  5 ---
 .../flang/Optimizer/Builder/Runtime/Execute.h |  5 +++
 flang/include/flang/Runtime/command.h         |  5 ---
 flang/include/flang/Runtime/execute.h         |  5 +++
 .../lib/Optimizer/Builder/Runtime/Command.cpp | 13 ------
 .../lib/Optimizer/Builder/Runtime/Execute.cpp | 13 ++++++
 flang/runtime/command.cpp                     | 23 ----------
 flang/runtime/execute.cpp                     | 23 ++++++++++
 flang/runtime/tools.h                         | 44 -------------------
 9 files changed, 46 insertions(+), 90 deletions(-)

diff --git a/flang/include/flang/Optimizer/Builder/Runtime/Command.h b/flang/include/flang/Optimizer/Builder/Runtime/Command.h
index 9d6a39639844fc..976fb3aa0b6fbb 100644
--- a/flang/include/flang/Optimizer/Builder/Runtime/Command.h
+++ b/flang/include/flang/Optimizer/Builder/Runtime/Command.h
@@ -53,10 +53,5 @@ mlir::Value genGetEnvVariable(fir::FirOpBuilder &, mlir::Location,
                               mlir::Value length, mlir::Value trimName,
                               mlir::Value errmsg);
 
-/// Generate a call to System runtime function which implements
-/// the non-standard System GNU extension.
-void genSystem(fir::FirOpBuilder &, mlir::Location, mlir::Value command,
-               mlir::Value exitstat);
-
 } // namespace fir::runtime
 #endif // FORTRAN_OPTIMIZER_BUILDER_RUNTIME_COMMAND_H
diff --git a/flang/include/flang/Optimizer/Builder/Runtime/Execute.h b/flang/include/flang/Optimizer/Builder/Runtime/Execute.h
index a1e6ef20876049..d6621f00fafef7 100644
--- a/flang/include/flang/Optimizer/Builder/Runtime/Execute.h
+++ b/flang/include/flang/Optimizer/Builder/Runtime/Execute.h
@@ -31,5 +31,10 @@ void genExecuteCommandLine(fir::FirOpBuilder &, mlir::Location,
                            mlir::Value exitstat, mlir::Value cmdstat,
                            mlir::Value cmdmsg);
 
+/// Generate a call to System runtime function which implements
+/// the non-standard System GNU extension.
+void genSystem(fir::FirOpBuilder &, mlir::Location, mlir::Value command,
+               mlir::Value exitstat);
+
 } // namespace fir::runtime
 #endif // FORTRAN_OPTIMIZER_BUILDER_RUNTIME_EXECUTE_H
diff --git a/flang/include/flang/Runtime/command.h b/flang/include/flang/Runtime/command.h
index e477a2da1c5954..c67d171c8e2f1b 100644
--- a/flang/include/flang/Runtime/command.h
+++ b/flang/include/flang/Runtime/command.h
@@ -55,11 +55,6 @@ std::int32_t RTNAME(GetEnvVariable)(const Descriptor &name,
     const Descriptor *value = nullptr, const Descriptor *length = nullptr,
     bool trim_name = true, const Descriptor *errmsg = nullptr,
     const char *sourceFile = nullptr, int line = 0);
-
-// Calls std::system()
-void RTNAME(System)(const Descriptor &command,
-    const Descriptor *exitstat = nullptr, const char *sourceFile = nullptr,
-    int line = 0);
 }
 } // namespace Fortran::runtime
 
diff --git a/flang/include/flang/Runtime/execute.h b/flang/include/flang/Runtime/execute.h
index ca137b9d1823c4..9d40910dccd679 100644
--- a/flang/include/flang/Runtime/execute.h
+++ b/flang/include/flang/Runtime/execute.h
@@ -23,6 +23,11 @@ void RTNAME(ExecuteCommandLine)(const Descriptor &command, bool wait = true,
     const Descriptor *exitstat = nullptr, const Descriptor *cmdstat = nullptr,
     const Descriptor *cmdmsg = nullptr, const char *sourceFile = nullptr,
     int line = 0);
+
+// Calls std::system()
+void RTNAME(System)(const Descriptor &command,
+    const Descriptor *exitstat = nullptr, const char *sourceFile = nullptr,
+    int line = 0);
 }
 } // namespace Fortran::runtime
 
diff --git a/flang/lib/Optimizer/Builder/Runtime/Command.cpp b/flang/lib/Optimizer/Builder/Runtime/Command.cpp
index 00af35a74bf871..1d719e7bbd9a2d 100644
--- a/flang/lib/Optimizer/Builder/Runtime/Command.cpp
+++ b/flang/lib/Optimizer/Builder/Runtime/Command.cpp
@@ -88,16 +88,3 @@ mlir::Value fir::runtime::genGetEnvVariable(fir::FirOpBuilder &builder,
       sourceFile, sourceLine);
   return builder.create<fir::CallOp>(loc, runtimeFunc, args).getResult(0);
 }
-
-void fir::runtime::genSystem(fir::FirOpBuilder &builder, mlir::Location loc,
-                             mlir::Value command, mlir::Value exitstat) {
-  auto runtimeFunc =
-      fir::runtime::getRuntimeFunc<mkRTKey(System)>(loc, builder);
-  mlir::FunctionType runtimeFuncTy = runtimeFunc.getFunctionType();
-  mlir::Value sourceFile = fir::factory::locationToFilename(builder, loc);
-  mlir::Value sourceLine =
-      fir::factory::locationToLineNo(builder, loc, runtimeFuncTy.getInput(3));
-  llvm::SmallVector<mlir::Value> args = fir::runtime::createArguments(
-      builder, loc, runtimeFuncTy, command, exitstat, sourceFile, sourceLine);
-  builder.create<fir::CallOp>(loc, runtimeFunc, args);
-}
diff --git a/flang/lib/Optimizer/Builder/Runtime/Execute.cpp b/flang/lib/Optimizer/Builder/Runtime/Execute.cpp
index 71ee3996ac0da7..ce16b5de65e293 100644
--- a/flang/lib/Optimizer/Builder/Runtime/Execute.cpp
+++ b/flang/lib/Optimizer/Builder/Runtime/Execute.cpp
@@ -42,3 +42,16 @@ void fir::runtime::genExecuteCommandLine(fir::FirOpBuilder &builder,
       sourceFile, sourceLine);
   builder.create<fir::CallOp>(loc, runtimeFunc, args);
 }
+
+void fir::runtime::genSystem(fir::FirOpBuilder &builder, mlir::Location loc,
+                             mlir::Value command, mlir::Value exitstat) {
+  auto runtimeFunc =
+      fir::runtime::getRuntimeFunc<mkRTKey(System)>(loc, builder);
+  mlir::FunctionType runtimeFuncTy = runtimeFunc.getFunctionType();
+  mlir::Value sourceFile = fir::factory::locationToFilename(builder, loc);
+  mlir::Value sourceLine =
+      fir::factory::locationToLineNo(builder, loc, runtimeFuncTy.getInput(3));
+  llvm::SmallVector<mlir::Value> args = fir::runtime::createArguments(
+      builder, loc, runtimeFuncTy, command, exitstat, sourceFile, sourceLine);
+  builder.create<fir::CallOp>(loc, runtimeFunc, args);
+}
diff --git a/flang/runtime/command.cpp b/flang/runtime/command.cpp
index 5b2837c96f694a..7c44890545bd3f 100644
--- a/flang/runtime/command.cpp
+++ b/flang/runtime/command.cpp
@@ -241,27 +241,4 @@ std::int32_t RTNAME(GetEnvVariable)(const Descriptor &name,
   return StatOk;
 }
 
-void RTNAME(System)(const Descriptor &command, const Descriptor *exitstat,
-    const char *sourceFile, int line) {
-  Terminator terminator{sourceFile, line};
-
-  const char *newCmd{EnsureNullTerminated(
-      command.OffsetElement(), command.ElementBytes(), terminator)};
-  int status{std::system(newCmd)};
-
-  if (exitstat) {
-    RUNTIME_CHECK(terminator, IsValidIntDescriptor(exitstat));
-#ifdef _WIN32
-    StoreLengthToDescriptor(exitstat, status, terminator);
-#else
-    int exitstatVal{WEXITSTATUS(status)};
-    StoreLengthToDescriptor(exitstat, exitstatVal, terminator);
-#endif
-  }
-  // Deallocate memory if EnsureNullTerminated dynamically allocate a memory
-  if (newCmd != command.OffsetElement()) {
-    FreeMemory((void *)newCmd);
-  }
-}
-
 } // namespace Fortran::runtime
diff --git a/flang/runtime/execute.cpp b/flang/runtime/execute.cpp
index d38cb8384bc864..a2791a8e155207 100644
--- a/flang/runtime/execute.cpp
+++ b/flang/runtime/execute.cpp
@@ -204,4 +204,27 @@ void RTNAME(ExecuteCommandLine)(const Descriptor &command, bool wait,
   }
 }
 
+void RTNAME(System)(const Descriptor &command, const Descriptor *exitstat,
+    const char *sourceFile, int line) {
+  Terminator terminator{sourceFile, line};
+
+  char *newCmd{EnsureNullTerminated(
+      command.OffsetElement(), command.ElementBytes(), terminator)};
+  int status{std::system(newCmd)};
+
+  if (exitstat) {
+    RUNTIME_CHECK(terminator, IsValidIntDescriptor(exitstat));
+#ifdef _WIN32
+    StoreLengthToDescriptor(exitstat, status, terminator);
+#else
+    int exitstatVal{WEXITSTATUS(status)};
+    StoreIntToDescriptor(exitstat, exitstatVal, terminator);
+#endif
+  }
+  // Deallocate memory if EnsureNullTerminated dynamically allocate a memory
+  if (newCmd != command.OffsetElement()) {
+    FreeMemory(static_cast<char *>(newCmd));
+  }
+}
+
 } // namespace Fortran::runtime
diff --git a/flang/runtime/tools.h b/flang/runtime/tools.h
index 514a0a926f7541..86c06149dc4e4a 100644
--- a/flang/runtime/tools.h
+++ b/flang/runtime/tools.h
@@ -481,50 +481,6 @@ RT_API_ATTRS void CopyAndPad(
   }
 }
 
-// Ensures that a character string is null-terminated, allocating a /p length +1
-// size memory for null-terminator if necessary. Returns the original or a newly
-// allocated null-terminated string (responsibility for deallocation is on the
-// caller).
-RT_API_ATTRS const char *EnsureNullTerminated(
-    const char *str, std::size_t length, Terminator &terminator);
-
-RT_API_ATTRS bool IsValidCharDescriptor(const Descriptor *value);
-
-RT_API_ATTRS bool IsValidIntDescriptor(const Descriptor *intVal);
-
-// Copy a null-terminated character array \p rawValue to descriptor \p value.
-// The copy starts at the given \p offset, if not present then start at 0.
-// If descriptor `errmsg` is provided, error messages will be stored to it.
-// Returns stats specified in standard.
-RT_API_ATTRS std::int32_t CopyCharsToDescriptor(const Descriptor &value,
-    const char *rawValue, std::size_t rawValueLength,
-    const Descriptor *errmsg = nullptr, std::size_t offset = 0);
-
-RT_API_ATTRS void StoreIntToDescriptor(
-    const Descriptor *length, std::int64_t value, Terminator &terminator);
-
-// Defines a utility function for copying and padding characters
-template <typename TO, typename FROM>
-RT_API_ATTRS void CopyAndPad(
-    TO *to, const FROM *from, std::size_t toChars, std::size_t fromChars) {
-  if constexpr (sizeof(TO) != sizeof(FROM)) {
-    std::size_t copyChars{std::min(toChars, fromChars)};
-    for (std::size_t j{0}; j < copyChars; ++j) {
-      to[j] = from[j];
-    }
-    for (std::size_t j{copyChars}; j < toChars; ++j) {
-      to[j] = static_cast<TO>(' ');
-    }
-  } else if (toChars <= fromChars) {
-    std::memcpy(to, from, toChars * sizeof(TO));
-  } else {
-    std::memcpy(to, from, std::min(toChars, fromChars) * sizeof(TO));
-    for (std::size_t j{fromChars}; j < toChars; ++j) {
-      to[j] = static_cast<TO>(' ');
-    }
-  }
-}
-
 
 } // namespace Fortran::runtime
 #endif // FORTRAN_RUNTIME_TOOLS_H_

>From 31c3a42490548d702edd5fffb34eaebf9ad0737c Mon Sep 17 00:00:00 2001
From: Yi Wu <yi.wu2 at arm.com>
Date: Tue, 16 Jan 2024 12:14:33 +0000
Subject: [PATCH 12/18] clang format and change exitstat type from AnyInt to
 DefaultInt to match gfortran gnu standard

---
 flang/lib/Evaluate/intrinsics.cpp | 2 +-
 flang/runtime/tools.h             | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/flang/lib/Evaluate/intrinsics.cpp b/flang/lib/Evaluate/intrinsics.cpp
index 639db1808f126d..068b3cfafcbc11 100644
--- a/flang/lib/Evaluate/intrinsics.cpp
+++ b/flang/lib/Evaluate/intrinsics.cpp
@@ -1389,7 +1389,7 @@ static const IntrinsicInterface intrinsicSubroutine[]{
         {}, Rank::elemental, IntrinsicClass::impureSubroutine},
     {"system",
         {{"command", DefaultChar, Rank::scalar},
-            {"exitstat", AnyInt, Rank::scalar, Optionality::optional,
+            {"exitstat", DefaultInt, Rank::scalar, Optionality::optional,
                 common::Intent::InOut}},
         {}, Rank::elemental, IntrinsicClass::impureSubroutine},
     {"system_clock",
diff --git a/flang/runtime/tools.h b/flang/runtime/tools.h
index 86c06149dc4e4a..89e5069995748b 100644
--- a/flang/runtime/tools.h
+++ b/flang/runtime/tools.h
@@ -481,6 +481,5 @@ RT_API_ATTRS void CopyAndPad(
   }
 }
 
-
 } // namespace Fortran::runtime
 #endif // FORTRAN_RUNTIME_TOOLS_H_

>From 25c26f97d28e5a793e8469c5ce2a3534164eb93d Mon Sep 17 00:00:00 2001
From: Yi Wu <yi.wu2 at arm.com>
Date: Tue, 16 Jan 2024 14:21:31 +0000
Subject: [PATCH 13/18] Windows fixes

---
 flang/runtime/execute.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/flang/runtime/execute.cpp b/flang/runtime/execute.cpp
index a2791a8e155207..1aef04ebfc2a02 100644
--- a/flang/runtime/execute.cpp
+++ b/flang/runtime/execute.cpp
@@ -215,13 +215,13 @@ void RTNAME(System)(const Descriptor &command, const Descriptor *exitstat,
   if (exitstat) {
     RUNTIME_CHECK(terminator, IsValidIntDescriptor(exitstat));
 #ifdef _WIN32
-    StoreLengthToDescriptor(exitstat, status, terminator);
+    StoreIntToDescriptor(exitstat, status, terminator);
 #else
     int exitstatVal{WEXITSTATUS(status)};
     StoreIntToDescriptor(exitstat, exitstatVal, terminator);
 #endif
   }
-  // Deallocate memory if EnsureNullTerminated dynamically allocate a memory
+  // Deallocate memory if EnsureNullTerminated dynamically allocated memory
   if (newCmd != command.OffsetElement()) {
     FreeMemory(static_cast<char *>(newCmd));
   }

>From 836cd28213b11e7d90a183a3553e2bccf8255ad1 Mon Sep 17 00:00:00 2001
From: Yi Wu <yi.wu2 at arm.com>
Date: Tue, 16 Jan 2024 15:06:52 +0000
Subject: [PATCH 14/18] reuse ExecuteCommandLine runtime library and lowering

---
 .../flang/Optimizer/Builder/Runtime/Execute.h |  5 ---
 flang/include/flang/Runtime/execute.h         |  4 ---
 flang/lib/Optimizer/Builder/IntrinsicCall.cpp | 14 ++++++--
 .../lib/Optimizer/Builder/Runtime/Execute.cpp | 13 --------
 flang/runtime/execute.cpp                     |  3 ++
 flang/unittests/Runtime/CommandTest.cpp       | 32 ++++++++++++++++---
 6 files changed, 43 insertions(+), 28 deletions(-)

diff --git a/flang/include/flang/Optimizer/Builder/Runtime/Execute.h b/flang/include/flang/Optimizer/Builder/Runtime/Execute.h
index d6621f00fafef7..a1e6ef20876049 100644
--- a/flang/include/flang/Optimizer/Builder/Runtime/Execute.h
+++ b/flang/include/flang/Optimizer/Builder/Runtime/Execute.h
@@ -31,10 +31,5 @@ void genExecuteCommandLine(fir::FirOpBuilder &, mlir::Location,
                            mlir::Value exitstat, mlir::Value cmdstat,
                            mlir::Value cmdmsg);
 
-/// Generate a call to System runtime function which implements
-/// the non-standard System GNU extension.
-void genSystem(fir::FirOpBuilder &, mlir::Location, mlir::Value command,
-               mlir::Value exitstat);
-
 } // namespace fir::runtime
 #endif // FORTRAN_OPTIMIZER_BUILDER_RUNTIME_EXECUTE_H
diff --git a/flang/include/flang/Runtime/execute.h b/flang/include/flang/Runtime/execute.h
index 9d40910dccd679..ae8655f2484e49 100644
--- a/flang/include/flang/Runtime/execute.h
+++ b/flang/include/flang/Runtime/execute.h
@@ -24,10 +24,6 @@ void RTNAME(ExecuteCommandLine)(const Descriptor &command, bool wait = true,
     const Descriptor *cmdmsg = nullptr, const char *sourceFile = nullptr,
     int line = 0);
 
-// Calls std::system()
-void RTNAME(System)(const Descriptor &command,
-    const Descriptor *exitstat = nullptr, const char *sourceFile = nullptr,
-    int line = 0);
 }
 } // namespace Fortran::runtime
 
diff --git a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
index f57f241badde3a..1c1889396820b6 100644
--- a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
+++ b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
@@ -5923,7 +5923,7 @@ IntrinsicLibrary::genSum(mlir::Type resultType,
 
 // SYSTEM
 void IntrinsicLibrary::genSystem(llvm::ArrayRef<fir::ExtendedValue> args) {
-  assert(args.size() >= 1);
+  assert(args.size() >= 1 && args.size()<=2);
   mlir::Value command = fir::getBase(args[0]);
   const fir::ExtendedValue &exitstat = args[1];
 
@@ -5932,13 +5932,23 @@ void IntrinsicLibrary::genSystem(llvm::ArrayRef<fir::ExtendedValue> args) {
 
   mlir::Type boxNoneTy = fir::BoxType::get(builder.getNoneType());
 
+  mlir::Value waitBool = builder.createBool(loc, true);
   mlir::Value exitstatBox =
       isStaticallyPresent(exitstat)
           ? fir::getBase(exitstat)
           : builder.create<fir::AbsentOp>(loc, boxNoneTy).getResult();
 
-  fir::runtime::genSystem(builder, loc, command, exitstatBox);
+  // Create a dummmy cmdstat to prevent EXECUTE_COMMAND_LINE terminate itself
+  // when cmdstat is assigned with a non-zero value but not present 
+  mlir::Value cmdstatBox = builder.createBox(loc, builder.createIntegerConstant(loc, builder.getIndexType(), 0));
+
+  mlir::Value cmdmsgBox =
+      builder.create<fir::AbsentOp>(loc, boxNoneTy).getResult();
+
+  fir::runtime::genExecuteCommandLine(builder, loc, command, waitBool,
+                                      exitstatBox, cmdstatBox, cmdmsgBox);
 }
+
 // SYSTEM_CLOCK
 void IntrinsicLibrary::genSystemClock(llvm::ArrayRef<fir::ExtendedValue> args) {
   assert(args.size() == 3);
diff --git a/flang/lib/Optimizer/Builder/Runtime/Execute.cpp b/flang/lib/Optimizer/Builder/Runtime/Execute.cpp
index ce16b5de65e293..71ee3996ac0da7 100644
--- a/flang/lib/Optimizer/Builder/Runtime/Execute.cpp
+++ b/flang/lib/Optimizer/Builder/Runtime/Execute.cpp
@@ -42,16 +42,3 @@ void fir::runtime::genExecuteCommandLine(fir::FirOpBuilder &builder,
       sourceFile, sourceLine);
   builder.create<fir::CallOp>(loc, runtimeFunc, args);
 }
-
-void fir::runtime::genSystem(fir::FirOpBuilder &builder, mlir::Location loc,
-                             mlir::Value command, mlir::Value exitstat) {
-  auto runtimeFunc =
-      fir::runtime::getRuntimeFunc<mkRTKey(System)>(loc, builder);
-  mlir::FunctionType runtimeFuncTy = runtimeFunc.getFunctionType();
-  mlir::Value sourceFile = fir::factory::locationToFilename(builder, loc);
-  mlir::Value sourceLine =
-      fir::factory::locationToLineNo(builder, loc, runtimeFuncTy.getInput(3));
-  llvm::SmallVector<mlir::Value> args = fir::runtime::createArguments(
-      builder, loc, runtimeFuncTy, command, exitstat, sourceFile, sourceLine);
-  builder.create<fir::CallOp>(loc, runtimeFunc, args);
-}
diff --git a/flang/runtime/execute.cpp b/flang/runtime/execute.cpp
index 1aef04ebfc2a02..1ad09e2864a89f 100644
--- a/flang/runtime/execute.cpp
+++ b/flang/runtime/execute.cpp
@@ -25,6 +25,8 @@
 #include <unistd.h>
 #endif
 
+#include <stdio.h>
+
 namespace Fortran::runtime {
 
 // cmdstat specified in 16.9.73
@@ -126,6 +128,7 @@ void RTNAME(ExecuteCommandLine)(const Descriptor &command, bool wait,
 
   if (cmdstat) {
     RUNTIME_CHECK(terminator, IsValidIntDescriptor(cmdstat));
+    printf("cmdstat found. \n");
     // Assigned 0 as specifed in standard, if error then overwrite
     StoreIntToDescriptor(cmdstat, CMD_EXECUTED, terminator);
   }
diff --git a/flang/unittests/Runtime/CommandTest.cpp b/flang/unittests/Runtime/CommandTest.cpp
index a99501ab5b245c..ce285629580884 100644
--- a/flang/unittests/Runtime/CommandTest.cpp
+++ b/flang/unittests/Runtime/CommandTest.cpp
@@ -405,18 +405,30 @@ TEST_F(ZeroArguments, ECLInvalidCommandParentNotTerminatedAsync) {
 }
 
 TEST_F(ZeroArguments, SystemValidCommandExitStat) {
+  // envrionment setup for SYSTEM from EXECUTE_COMMAND_LINE runtime
+  OwningPtr<Descriptor> cmdStat{IntDescriptor(202)};
+  bool wait{true};
+  // setup finished
+
   OwningPtr<Descriptor> command{CharDescriptor("echo hi")};
   OwningPtr<Descriptor> exitStat{EmptyIntDescriptor()};
 
-  RTNAME(System)(*command.get(), exitStat.get());
+  RTNAME(ExecuteCommandLine)
+  (*command.get(), wait, exitStat.get(), cmdStat.get(), nullptr);
   CheckDescriptorEqInt(exitStat.get(), 0);
 }
 
 TEST_F(ZeroArguments, SystemInvalidCommandExitStat) {
+  // envrionment setup for SYSTEM from EXECUTE_COMMAND_LINE runtime
+  OwningPtr<Descriptor> cmdStat{IntDescriptor(202)};
+  bool wait{true};
+  // setup finished
+
   OwningPtr<Descriptor> command{CharDescriptor("InvalidCommand")};
   OwningPtr<Descriptor> exitStat{EmptyIntDescriptor()};
 
-  RTNAME(System)(*command.get(), exitStat.get());
+  RTNAME(ExecuteCommandLine)
+  (*command.get(), wait, exitStat.get(), cmdStat.get(), nullptr);
 #ifdef _WIN32
   CheckDescriptorEqInt(exitStat.get(), 1);
 #else
@@ -425,13 +437,25 @@ TEST_F(ZeroArguments, SystemInvalidCommandExitStat) {
 }
 
 TEST_F(ZeroArguments, SystemValidCommandOptionalExitStat) {
+  // envrionment setup for SYSTEM from EXECUTE_COMMAND_LINE runtime
+  OwningPtr<Descriptor> cmdStat{IntDescriptor(202)};
+  bool wait{true};
+  // setup finished
+
   OwningPtr<Descriptor> command{CharDescriptor("echo hi")};
-  EXPECT_NO_FATAL_FAILURE(RTNAME(System)(*command.get(), nullptr));
+  EXPECT_NO_FATAL_FAILURE(RTNAME(ExecuteCommandLine)(
+      *command.get(), wait, nullptr, cmdStat.get(), nullptr));
 }
 
 TEST_F(ZeroArguments, SystemInvalidCommandOptionalExitStat) {
+  // envrionment setup for SYSTEM from EXECUTE_COMMAND_LINE runtime
+  OwningPtr<Descriptor> cmdStat{IntDescriptor(202)};
+  bool wait{true};
+  // setup finished
+
   OwningPtr<Descriptor> command{CharDescriptor("InvalidCommand")};
-  EXPECT_NO_FATAL_FAILURE(RTNAME(System)(*command.get(), nullptr));
+  EXPECT_NO_FATAL_FAILURE(RTNAME(ExecuteCommandLine)(
+      *command.get(), wait, nullptr, cmdStat.get(), nullptr););
 }
 
 static const char *oneArgArgv[]{"aProgram", "anArgumentOfLength20"};

>From 1b908adafed5f75b5fd6bcf52d3cd473df070bf6 Mon Sep 17 00:00:00 2001
From: Yi Wu <yi.wu2 at arm.com>
Date: Wed, 17 Jan 2024 15:26:55 +0000
Subject: [PATCH 15/18] add tests

---
 flang/lib/Optimizer/Builder/IntrinsicCall.cpp | 12 +++-
 .../test/Lower/Intrinsics/system-optional.f90 | 42 +++++++-----
 flang/test/Lower/Intrinsics/system.f90        | 64 ++++++++++++-------
 3 files changed, 75 insertions(+), 43 deletions(-)

diff --git a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
index 1c1889396820b6..b95e6485c13fbc 100644
--- a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
+++ b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
@@ -5923,7 +5923,7 @@ IntrinsicLibrary::genSum(mlir::Type resultType,
 
 // SYSTEM
 void IntrinsicLibrary::genSystem(llvm::ArrayRef<fir::ExtendedValue> args) {
-  assert(args.size() >= 1 && args.size()<=2);
+  assert(args.size() >= 1 && args.size() <= 2);
   mlir::Value command = fir::getBase(args[0]);
   const fir::ExtendedValue &exitstat = args[1];
 
@@ -5939,8 +5939,14 @@ void IntrinsicLibrary::genSystem(llvm::ArrayRef<fir::ExtendedValue> args) {
           : builder.create<fir::AbsentOp>(loc, boxNoneTy).getResult();
 
   // Create a dummmy cmdstat to prevent EXECUTE_COMMAND_LINE terminate itself
-  // when cmdstat is assigned with a non-zero value but not present 
-  mlir::Value cmdstatBox = builder.createBox(loc, builder.createIntegerConstant(loc, builder.getIndexType(), 0));
+  // when cmdstat is assigned with a non-zero value but not present
+  mlir::Value tempValue =
+      builder.createIntegerConstant(loc, builder.getI2Type(), 0);
+  mlir::Value temp = builder.createTemporary(loc, builder.getI2Type());
+  mlir::Value castVal =
+      builder.createConvert(loc, builder.getI2Type(), tempValue);
+  builder.create<fir::StoreOp>(loc, castVal, temp);
+  mlir::Value cmdstatBox = builder.createBox(loc, temp);
 
   mlir::Value cmdmsgBox =
       builder.create<fir::AbsentOp>(loc, boxNoneTy).getResult();
diff --git a/flang/test/Lower/Intrinsics/system-optional.f90 b/flang/test/Lower/Intrinsics/system-optional.f90
index d9f5a79965a98a..2fbdfe4b9f5775 100644
--- a/flang/test/Lower/Intrinsics/system-optional.f90
+++ b/flang/test/Lower/Intrinsics/system-optional.f90
@@ -1,24 +1,34 @@
 ! RUN: bbc -emit-hlfir %s -o - | FileCheck %s
 
 ! CHECK-LABEL: func.func @_QPall_args(
-! CHECK-SAME:    %[[commandArg:.*]]: !fir.boxchar<1> {fir.bindc_name = "command", fir.optional},
-! CHECK-SAME:    %[[exitvalArg:.*]]: !fir.ref<i32> {fir.bindc_name = "exitval", fir.optional}) {
-subroutine all_args(command, exitVal)
+! CHECK-SAME:   %[[commandArg:.*]]: !fir.boxchar<1> {fir.bindc_name = "command", fir.optional},
+! CHECK-SAME:   %[[exitstatArg:.*]]: !fir.ref<i32> {fir.bindc_name = "exitstat", fir.optional}) {
+subroutine all_args(command, exitstat)
 CHARACTER(*), OPTIONAL :: command
-INTEGER, OPTIONAL :: exitVal
-call system(command, exitVal)
-! CHECK-NEXT:    %[[commandUnbox:.*]]:2 = fir.unboxchar %[[commandArg]] : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index) 
-! CHECK-NEXT:    %[[commandDeclare:.*]]:2 = hlfir.declare %[[commandUnbox]]#0 typeparams %[[commandUnbox]]#1 {fortran_attrs = #fir.var_attrs<optional>, uniq_name = "_QFall_argsEcommand"} : (!fir.ref<!fir.char<1,?>>, index) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>) 
-! CHECK-NEXT:    %[[exitvalDeclare:.*]]:2 = hlfir.declare %[[exitvalArg]] {fortran_attrs = #fir.var_attrs<optional>, uniq_name = "_QFall_argsEexitval"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>) 
-! CHECK-NEXT:    %[[exitvalIsPresent:.*]] = fir.is_present %[[exitvalDeclare]]#0 : (!fir.ref<i32>) -> i1
+INTEGER, OPTIONAL :: exitstat
+call system(command, exitstat)
+
+! CHECK-NEXT:    %[[cmdstatVal:.*]] = fir.alloca i2
+! CHECK-NEXT:    %[[commandUnbox:.*]]:2 = fir.unboxchar %[[commandArg]] : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)
+! CHECK-NEXT:    %[[commandDeclare:.*]]:2 = hlfir.declare %[[commandUnbox]]#0 typeparams %[[commandUnbox]]#1 {fortran_attrs = #fir.var_attrs<optional>, uniq_name = "_QFall_argsEcommand"} : (!fir.ref<!fir.char<1,?>>, index) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>)
+! CHECK-NEXT:    %[[exitstatDeclare:.*]]:2 = hlfir.declare %[[exitstatArg]] {fortran_attrs = #fir.var_attrs<optional>, uniq_name = "_QFall_argsEexitstat"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
+! CHECK-NEXT:    %[[exitstatIsPresent:.*]] = fir.is_present %[[exitstatDeclare]]#0 : (!fir.ref<i32>) -> i1
 ! CHECK-NEXT:    %[[commandBox:.*]] = fir.embox %[[commandDeclare]]#1 typeparams %[[commandUnbox]]#1 : (!fir.ref<!fir.char<1,?>>, index) -> !fir.box<!fir.char<1,?>>
-! CHECK-NEXT:    %[[exitvalBox:.*]] = fir.embox %[[exitvalDeclare]]#1 : (!fir.ref<i32>) -> !fir.box<i32>
-! CHECK-NEXT:    %[[absentBox:.*]] = fir.absent !fir.box<i32>
-! CHECK-NEXT:    %[[exitvalSelect:.*]] = arith.select %[[exitvalIsPresent]], %[[exitvalBox]], %[[absentBox]] : !fir.box<i32>
-! CHECK:         %c9_i32 = arith.constant 9 : i32
+! CHECK-NEXT:    %[[exitstatBox:.*]] = fir.embox %[[exitstatDeclare]]#1 : (!fir.ref<i32>) -> !fir.box<i32>
+! CHECK-NEXT:    %[[absentIntBox:.*]] = fir.absent !fir.box<i32>
+! CHECK-NEXT:    %[[exitstatRealBox:.*]] = arith.select %[[exitstatIsPresent]], %[[exitstatBox]], %[[absentIntBox]] : !fir.box<i32>
+! CHECK-NEXT:    %[[true:.*]] = arith.constant true
+! CHECK-NEXT:    %[[c0_i2:.*]] = arith.constant 0 : i2
+! CHECK-NEXT:    fir.store %[[c0_i2]] to %[[cmdstatVal]] : !fir.ref<i2>
+! CHECK-NEXT:    %[[cmdstatBox:.*]] = fir.embox %[[cmdstatVal]] : (!fir.ref<i2>) -> !fir.box<i2>
+! CHECK-NEXT:    %[[absentBox:.*]] = fir.absent !fir.box<none>
+! CHECK:         %[[c9_i32:.*]] = arith.constant 9 : i32
 ! CHECK-NEXT:    %[[command:.*]] = fir.convert %[[commandBox]] : (!fir.box<!fir.char<1,?>>) -> !fir.box<none>
-! CHECK-NEXT:    %[[exitval:.*]] = fir.convert %[[exitvalSelect]] : (!fir.box<i32>) -> !fir.box<none>
-! CHECK:         %[[VAL_12:.*]] = fir.call @_FortranASystem(%[[command]], %[[exitval]], %[[VAL_11:.*]], %c9_i32) fastmath<contract> : (!fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none
+! CHECK-NEXT:    %[[exitstat:.*]] = fir.convert %[[exitstatRealBox]] : (!fir.box<i32>) -> !fir.box<none>
+! CHECK-NEXT:    %[[cmdstat:.*]] = fir.convert %[[cmdstatBox]] : (!fir.box<i2>) -> !fir.box<none>
+! CHECK-NEXT:    %[[VAL_15:.*]] = fir.convert %[[VAL_11:.*]] : (!fir.ref<!fir.char<1,82>>) -> !fir.ref<i8>
+! CHECK-NEXT:    %[[VAL_16:.*]] = fir.call @_FortranAExecuteCommandLine(%[[command]], %[[true]], %[[exitstat]], %[[cmdstat]], %[[absentBox]], %[[VAL_15]], %[[c9_i32]]) fastmath<contract> : (!fir.box<none>, i1, !fir.box<none>, !fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none
 ! CHECK-NEXT:    return
-! CHECK-NEXT:    }
+! CHECK-NEXT:      }
+
 end subroutine all_args
diff --git a/flang/test/Lower/Intrinsics/system.f90 b/flang/test/Lower/Intrinsics/system.f90
index 46fc828a544acc..37034bbc0c4f84 100644
--- a/flang/test/Lower/Intrinsics/system.f90
+++ b/flang/test/Lower/Intrinsics/system.f90
@@ -2,22 +2,30 @@
 
 ! CHECK-LABEL: func.func @_QPall_args(
 ! CHECK-SAME:    %[[commandArg:.*]]: !fir.boxchar<1> {fir.bindc_name = "command"}, 
-! CHECK-SAME:    %[[exitvalArg:.*]]: !fir.ref<i32> {fir.bindc_name = "exitval"}) { 
-subroutine all_args(command, exitVal)
+! CHECK-SAME:    %[[exitstatArg:.*]]: !fir.ref<i32> {fir.bindc_name = "exitstat"}) { 
+subroutine all_args(command, exitstat)
 CHARACTER(*) :: command
-INTEGER :: exitVal
-call system(command, exitVal)
-! CHECK-NEXT:    %[[commandUnbox:.*]]:2 = fir.unboxchar %[[commandArg]] : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index) 
-! CHECK-NEXT:    %[[commandDeclare:.*]]:2 = hlfir.declare %[[commandUnbox]]#0 typeparams %[[commandUnbox]]#1 {uniq_name = "_QFall_argsEcommand"} : (!fir.ref<!fir.char<1,?>>, index) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>)
-! CHECK-NEXT:    %[[exitvalDeclare:.*]]:2 = hlfir.declare %[[exitvalArg]] {uniq_name = "_QFall_argsEexitval"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
-! CHECK-NEXT:    %[[commandBox:.*]] = fir.embox %[[commandDeclare]]#1 typeparams %[[commandUnbox]]#1 : (!fir.ref<!fir.char<1,?>>, index) -> !fir.box<!fir.char<1,?>>
-! CHECK-NEXT:    %[[exitvalBox:.*]] = fir.embox %[[exitvalDeclare]]#1 : (!fir.ref<i32>) -> !fir.box<i32>
-! CHECK:         %c9_i32 = arith.constant 9 : i32
-! CHECK-NEXT:    %[[command:.*]] = fir.convert %[[commandBox]] : (!fir.box<!fir.char<1,?>>) -> !fir.box<none> 
-! CHECK-NEXT:    %[[exitval:.*]] = fir.convert %[[exitvalBox]] : (!fir.box<i32>) -> !fir.box<none>
-! CHECK:         %[[VAL_9:.*]] = fir.call @_FortranASystem(%[[command]], %[[exitval]], %[[VAL_8:.*]], %c9_i32) fastmath<contract> : (!fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none
-! CHECK-NEXT:    return
-! CHECK-NEXT:    }
+INTEGER :: exitstat
+call system(command, exitstat)
+! CHECK-NEXT:   %[[cmdststVal:.*]] = fir.alloca i2
+! CHECK-NEXT:   %[[commandUnbox:.*]]:2 = fir.unboxchar %[[commandArg]] : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)
+! CHECK-NEXT:   %[[commandDeclare:.*]]:2 = hlfir.declare %[[commandUnbox]]#0 typeparams %[[commandUnbox]]#1 {uniq_name = "_QFall_argsEcommand"} : (!fir.ref<!fir.char<1,?>>, index) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>)
+! CHECK-NEXT:   %[[exitstatDeclare:.*]]:2 = hlfir.declare %[[exitstatArg]] {uniq_name = "_QFall_argsEexitstat"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
+! CHECK-NEXT:   %[[commandBox:.*]] = fir.embox %[[commandDeclare]]#1 typeparams %[[commandUnbox]]#1 : (!fir.ref<!fir.char<1,?>>, index) -> !fir.box<!fir.char<1,?>>
+! CHECK-NEXT:   %[[exitstatBox:.*]] = fir.embox %[[exitstatDeclare]]#1 : (!fir.ref<i32>) -> !fir.box<i32>
+! CHECK-NEXT:   %[[true:.*]] = arith.constant true
+! CHECK-NEXT:   %[[c0_i2:.*]] = arith.constant 0 : i2
+! CHECK-NEXT:   fir.store %[[c0_i2:.*]] to %[[cmdststVal]] : !fir.ref<i2>
+! CHECK-NEXT:   %[[cmdstatBox:.*]] = fir.embox %[[cmdststVal]] : (!fir.ref<i2>) -> !fir.box<i2>
+! CHECK-NEXT:   %[[absentBox:.*]] = fir.absent !fir.box<none>
+! CHECK:        %[[c9_i32:.*]] = arith.constant 9 : i32
+! CHECK-NEXT:   %[[command:.*]] = fir.convert %[[commandBox]] : (!fir.box<!fir.char<1,?>>) -> !fir.box<none>
+! CHECK-NEXT:   %[[exitstat:.*]] = fir.convert %[[exitstatBox]] : (!fir.box<i32>) -> !fir.box<none>
+! CHECK-NEXT:   %[[cmdstat:.*]] = fir.convert %[[cmdstatBox]] : (!fir.box<i2>) -> !fir.box<none>
+! CHECK-NEXT:   %[[VAL_12:.*]] = fir.convert %[[VAL_8:.*]] : (!fir.ref<!fir.char<1,73>>) -> !fir.ref<i8>
+! CHECK-NEXT:   %[[VAL_13:.*]] = fir.call @_FortranAExecuteCommandLine(%[[command]], %[[true]], %[[exitstat]], %[[cmdstat]], %[[absentBox]], %[[VAL_12]], %[[c9_i32]]) fastmath<contract> : (!fir.box<none>, i1, !fir.box<none>, !fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none
+! CHECK-NEXT:   return
+! CHECK-NEXT:  }
 end subroutine all_args
 
 ! CHECK-LABEL: func.func @_QPonly_command(
@@ -25,13 +33,21 @@ end subroutine all_args
 subroutine only_command(command)
 CHARACTER(*) :: command
 call system(command)
-! CHECK-NEXT:    %[[commandUnbox:.*]]:2 = fir.unboxchar %[[commandArg]] : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)
-! CHECK-NEXT:    %[[commandDeclare:.*]]:2 = hlfir.declare %[[commandUnbox]]#0 typeparams %[[commandUnbox]]#1 {uniq_name = "_QFonly_commandEcommand"} : (!fir.ref<!fir.char<1,?>>, index) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>)
-! CHECK-NEXT:    %[[commandBox:.*]] = fir.embox %[[commandDeclare]]#1 typeparams %[[commandUnbox]]#1 : (!fir.ref<!fir.char<1,?>>, index) -> !fir.box<!fir.char<1,?>>
-! CHECK-NEXT:    %[[absentBox:.*]] = fir.absent !fir.box<none>
-! CHECK:         %c27_i32 = arith.constant 27 : i32
-! CHECK-NEXT:    %[[command:.*]] = fir.convert %[[commandBox]] : (!fir.box<!fir.char<1,?>>) -> !fir.box<none>
-! CHECK:         %[[VAL_7:.*]] = fir.call @_FortranASystem(%[[command]], %[[absentBox]], %[[VAL_6:.*]], %c27_i32) fastmath<contract> : (!fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none
-! CHECK-NEXT:    return
-! CHECK-NEXT:   }
+! CHECK-NEXT:   %[[cmdstatVal:.*]] = fir.alloca i2
+! CHECK-NEXT:   %[[commandUnbox:.*]]:2 = fir.unboxchar %arg0 : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)
+! CHECK-NEXT:   %[[commandDeclare:.*]]:2 = hlfir.declare %[[commandUnbox]]#0 typeparams %[[commandUnbox]]#1 {uniq_name = "_QFonly_commandEcommand"} : (!fir.ref<!fir.char<1,?>>, index) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>)
+! CHECK-NEXT:   %[[commandBox:.*]] = fir.embox %[[commandDeclare]]#1 typeparams %[[commandUnbox]]#1 : (!fir.ref<!fir.char<1,?>>, index) -> !fir.box<!fir.char<1,?>>
+! CHECK-NEXT:   %[[true:.*]] = arith.constant true
+! CHECK-NEXT:   %[[absentBox:.*]] = fir.absent !fir.box<none>
+! CHECK-NEXT:   %[[c0_i2:.*]] = arith.constant 0 : i2
+! CHECK-NEXT:   fir.store %[[c0_i2:.*]] to %[[cmdstatVal]] : !fir.ref<i2>
+! CHECK-NEXT:   %[[cmdstatBox:.*]] = fir.embox %[[cmdstatVal]] : (!fir.ref<i2>) -> !fir.box<i2>
+! CHECK-NEXT:   %[[absentBox2:.*]] = fir.absent !fir.box<none>
+! CHECK:        %[[c35_i32:.*]] = arith.constant 35 : i32
+! CHECK-NEXT:   %[[command:.*]] = fir.convert %[[commandBox]] : (!fir.box<!fir.char<1,?>>) -> !fir.box<none>
+! CHECK-NEXT:   %[[cmdstst:.*]] = fir.convert %[[cmdstatBox]] : (!fir.box<i2>) -> !fir.box<none>
+! CHECK-NEXT:   %[[VAL_10:.*]]0 = fir.convert %[[VAL_7:.*]] : (!fir.ref<!fir.char<1,73>>) -> !fir.ref<i8>
+! CHECK-NEXT:   %[[VAL_11:.*]]1 = fir.call @_FortranAExecuteCommandLine(%[[command]], %[[true]], %[[absentBox]], %[[cmdstst]], %[[absentBox2]], %[[VAL_10]]0, %[[c35_i32]]) fastmath<contract> : (!fir.box<none>, i1, !fir.box<none>, !fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none
+! CHECK-NEXT:   return
+! CHECK-NEXT:    }
 end subroutine only_command

>From da6089d8a10965f08984da41e1d713b7d08cb853 Mon Sep 17 00:00:00 2001
From: Yi Wu <yi.wu2 at arm.com>
Date: Wed, 17 Jan 2024 15:34:18 +0000
Subject: [PATCH 16/18] remove debug leftover and test fixes based on #78302

---
 flang/include/flang/Runtime/execute.h   |  1 -
 flang/runtime/execute.cpp               |  3 ---
 flang/unittests/Runtime/CommandTest.cpp | 12 ++++++------
 3 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/flang/include/flang/Runtime/execute.h b/flang/include/flang/Runtime/execute.h
index ae8655f2484e49..ca137b9d1823c4 100644
--- a/flang/include/flang/Runtime/execute.h
+++ b/flang/include/flang/Runtime/execute.h
@@ -23,7 +23,6 @@ void RTNAME(ExecuteCommandLine)(const Descriptor &command, bool wait = true,
     const Descriptor *exitstat = nullptr, const Descriptor *cmdstat = nullptr,
     const Descriptor *cmdmsg = nullptr, const char *sourceFile = nullptr,
     int line = 0);
-
 }
 } // namespace Fortran::runtime
 
diff --git a/flang/runtime/execute.cpp b/flang/runtime/execute.cpp
index 1ad09e2864a89f..1aef04ebfc2a02 100644
--- a/flang/runtime/execute.cpp
+++ b/flang/runtime/execute.cpp
@@ -25,8 +25,6 @@
 #include <unistd.h>
 #endif
 
-#include <stdio.h>
-
 namespace Fortran::runtime {
 
 // cmdstat specified in 16.9.73
@@ -128,7 +126,6 @@ void RTNAME(ExecuteCommandLine)(const Descriptor &command, bool wait,
 
   if (cmdstat) {
     RUNTIME_CHECK(terminator, IsValidIntDescriptor(cmdstat));
-    printf("cmdstat found. \n");
     // Assigned 0 as specifed in standard, if error then overwrite
     StoreIntToDescriptor(cmdstat, CMD_EXECUTED, terminator);
   }
diff --git a/flang/unittests/Runtime/CommandTest.cpp b/flang/unittests/Runtime/CommandTest.cpp
index ce285629580884..0b808dbef1ad13 100644
--- a/flang/unittests/Runtime/CommandTest.cpp
+++ b/flang/unittests/Runtime/CommandTest.cpp
@@ -386,8 +386,8 @@ TEST_F(ZeroArguments, ECLValidCommandAndExitStatNoChangeAndCMDStatusSetAsync) {
   RTNAME(ExecuteCommandLine)
   (*command.get(), wait, exitStat.get(), cmdStat.get(), cmdMsg.get());
 
-  CheckDescriptorEqInt(exitStat.get(), 404);
-  CheckDescriptorEqInt(cmdStat.get(), 0);
+  CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 404);
+  CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 0);
   CheckDescriptorEqStr(cmdMsg.get(), "No change");
 }
 
@@ -400,7 +400,7 @@ TEST_F(ZeroArguments, ECLInvalidCommandParentNotTerminatedAsync) {
   EXPECT_NO_FATAL_FAILURE(RTNAME(ExecuteCommandLine)(
       *command.get(), wait, exitStat.get(), nullptr, cmdMsg.get()));
 
-  CheckDescriptorEqInt(exitStat.get(), 404);
+  CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 404);
   CheckDescriptorEqStr(cmdMsg.get(), "No change");
 }
 
@@ -415,7 +415,7 @@ TEST_F(ZeroArguments, SystemValidCommandExitStat) {
 
   RTNAME(ExecuteCommandLine)
   (*command.get(), wait, exitStat.get(), cmdStat.get(), nullptr);
-  CheckDescriptorEqInt(exitStat.get(), 0);
+  CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 0);
 }
 
 TEST_F(ZeroArguments, SystemInvalidCommandExitStat) {
@@ -430,9 +430,9 @@ TEST_F(ZeroArguments, SystemInvalidCommandExitStat) {
   RTNAME(ExecuteCommandLine)
   (*command.get(), wait, exitStat.get(), cmdStat.get(), nullptr);
 #ifdef _WIN32
-  CheckDescriptorEqInt(exitStat.get(), 1);
+  CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 1);
 #else
-  CheckDescriptorEqInt(exitStat.get(), 127);
+  CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 127);
 #endif
 }
 

>From ceac5e97ecabbf78df3b2cd3dc9acec6a2d7d122 Mon Sep 17 00:00:00 2001
From: Yi Wu <yi.wu2 at arm.com>
Date: Wed, 17 Jan 2024 15:36:59 +0000
Subject: [PATCH 17/18] more code clean up

---
 flang/runtime/execute.cpp               | 23 -----------------------
 flang/unittests/Runtime/CommandTest.cpp |  6 +++---
 2 files changed, 3 insertions(+), 26 deletions(-)

diff --git a/flang/runtime/execute.cpp b/flang/runtime/execute.cpp
index 1aef04ebfc2a02..d38cb8384bc864 100644
--- a/flang/runtime/execute.cpp
+++ b/flang/runtime/execute.cpp
@@ -204,27 +204,4 @@ void RTNAME(ExecuteCommandLine)(const Descriptor &command, bool wait,
   }
 }
 
-void RTNAME(System)(const Descriptor &command, const Descriptor *exitstat,
-    const char *sourceFile, int line) {
-  Terminator terminator{sourceFile, line};
-
-  char *newCmd{EnsureNullTerminated(
-      command.OffsetElement(), command.ElementBytes(), terminator)};
-  int status{std::system(newCmd)};
-
-  if (exitstat) {
-    RUNTIME_CHECK(terminator, IsValidIntDescriptor(exitstat));
-#ifdef _WIN32
-    StoreIntToDescriptor(exitstat, status, terminator);
-#else
-    int exitstatVal{WEXITSTATUS(status)};
-    StoreIntToDescriptor(exitstat, exitstatVal, terminator);
-#endif
-  }
-  // Deallocate memory if EnsureNullTerminated dynamically allocated memory
-  if (newCmd != command.OffsetElement()) {
-    FreeMemory(static_cast<char *>(newCmd));
-  }
-}
-
 } // namespace Fortran::runtime
diff --git a/flang/unittests/Runtime/CommandTest.cpp b/flang/unittests/Runtime/CommandTest.cpp
index 0b808dbef1ad13..afc2b76bc3d5a3 100644
--- a/flang/unittests/Runtime/CommandTest.cpp
+++ b/flang/unittests/Runtime/CommandTest.cpp
@@ -386,8 +386,8 @@ TEST_F(ZeroArguments, ECLValidCommandAndExitStatNoChangeAndCMDStatusSetAsync) {
   RTNAME(ExecuteCommandLine)
   (*command.get(), wait, exitStat.get(), cmdStat.get(), cmdMsg.get());
 
-  CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 404);
-  CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 0);
+  CheckDescriptorEqInt(exitStat.get(), 404);
+  CheckDescriptorEqInt(cmdStat.get(), 0);
   CheckDescriptorEqStr(cmdMsg.get(), "No change");
 }
 
@@ -400,7 +400,7 @@ TEST_F(ZeroArguments, ECLInvalidCommandParentNotTerminatedAsync) {
   EXPECT_NO_FATAL_FAILURE(RTNAME(ExecuteCommandLine)(
       *command.get(), wait, exitStat.get(), nullptr, cmdMsg.get()));
 
-  CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 404);
+  CheckDescriptorEqInt(exitStat.get(), 404);
   CheckDescriptorEqStr(cmdMsg.get(), "No change");
 }
 

>From 90fbdc88e3ea3ae905314f50f17e0172048a7bd2 Mon Sep 17 00:00:00 2001
From: Yi Wu <yi.wu2 at arm.com>
Date: Wed, 17 Jan 2024 17:05:14 +0000
Subject: [PATCH 18/18] test fixes

---
 flang/test/Lower/Intrinsics/system-optional.f90 | 3 +--
 flang/test/Lower/Intrinsics/system.f90          | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/flang/test/Lower/Intrinsics/system-optional.f90 b/flang/test/Lower/Intrinsics/system-optional.f90
index 2fbdfe4b9f5775..e30549bf95a8df 100644
--- a/flang/test/Lower/Intrinsics/system-optional.f90
+++ b/flang/test/Lower/Intrinsics/system-optional.f90
@@ -26,8 +26,7 @@ subroutine all_args(command, exitstat)
 ! CHECK-NEXT:    %[[command:.*]] = fir.convert %[[commandBox]] : (!fir.box<!fir.char<1,?>>) -> !fir.box<none>
 ! CHECK-NEXT:    %[[exitstat:.*]] = fir.convert %[[exitstatRealBox]] : (!fir.box<i32>) -> !fir.box<none>
 ! CHECK-NEXT:    %[[cmdstat:.*]] = fir.convert %[[cmdstatBox]] : (!fir.box<i2>) -> !fir.box<none>
-! CHECK-NEXT:    %[[VAL_15:.*]] = fir.convert %[[VAL_11:.*]] : (!fir.ref<!fir.char<1,82>>) -> !fir.ref<i8>
-! CHECK-NEXT:    %[[VAL_16:.*]] = fir.call @_FortranAExecuteCommandLine(%[[command]], %[[true]], %[[exitstat]], %[[cmdstat]], %[[absentBox]], %[[VAL_15]], %[[c9_i32]]) fastmath<contract> : (!fir.box<none>, i1, !fir.box<none>, !fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none
+! CHECK:         %[[VAL_16:.*]] = fir.call @_FortranAExecuteCommandLine(%[[command]], %[[true]], %[[exitstat]], %[[cmdstat]], %[[absentBox]], %[[VAL_15:.*]], %[[c9_i32]]) fastmath<contract> : (!fir.box<none>, i1, !fir.box<none>, !fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none
 ! CHECK-NEXT:    return
 ! CHECK-NEXT:      }
 
diff --git a/flang/test/Lower/Intrinsics/system.f90 b/flang/test/Lower/Intrinsics/system.f90
index 37034bbc0c4f84..851e44e3826129 100644
--- a/flang/test/Lower/Intrinsics/system.f90
+++ b/flang/test/Lower/Intrinsics/system.f90
@@ -46,8 +46,7 @@ subroutine only_command(command)
 ! CHECK:        %[[c35_i32:.*]] = arith.constant 35 : i32
 ! CHECK-NEXT:   %[[command:.*]] = fir.convert %[[commandBox]] : (!fir.box<!fir.char<1,?>>) -> !fir.box<none>
 ! CHECK-NEXT:   %[[cmdstst:.*]] = fir.convert %[[cmdstatBox]] : (!fir.box<i2>) -> !fir.box<none>
-! CHECK-NEXT:   %[[VAL_10:.*]]0 = fir.convert %[[VAL_7:.*]] : (!fir.ref<!fir.char<1,73>>) -> !fir.ref<i8>
-! CHECK-NEXT:   %[[VAL_11:.*]]1 = fir.call @_FortranAExecuteCommandLine(%[[command]], %[[true]], %[[absentBox]], %[[cmdstst]], %[[absentBox2]], %[[VAL_10]]0, %[[c35_i32]]) fastmath<contract> : (!fir.box<none>, i1, !fir.box<none>, !fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none
+! CHECK:        %[[VAL_11:.*]]1 = fir.call @_FortranAExecuteCommandLine(%[[command]], %[[true]], %[[absentBox]], %[[cmdstst]], %[[absentBox2]], %[[VAL_10:.*]]0, %[[c35_i32]]) fastmath<contract> : (!fir.box<none>, i1, !fir.box<none>, !fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none
 ! CHECK-NEXT:   return
 ! CHECK-NEXT:    }
 end subroutine only_command