[flang] Add runtime support for GET_COMMAND

[rovka]

Summary

Implement the GET_COMMAND intrinsic.
Add 2 new parameters (sourceFile and line) so we can create
a terminator for RUNTIME_CHECKs.

[jeanPerier]

The implementation looks good to me, thanks. I have the same comment as @klausler in D118776. We recently realized that lowering was not dealing properly with argument that may be dynamically optional:

subroutine foo(command, length, errmsg, status)
  character(*), optional :: command, length, errmsg
  integer, optional :: status
  call get_command(command, length, errmsg, status)
end subroutine

It is not until the runtime that we will know which arguments are present here. So splitting the intrinsic in different entry points/returning the status by value (which made sense to me before), turns out to push some complexity in lowering here. I am not sure about get_command, but some intrinsics have requirements that at exactly one optional argument be present (RANDOM_SEED), and it is possible but cumbersome to deal with checking these constraints at runtime in inlined code. Having split entry points prevents the runtime from being able to check this. Since these intrinsics are not "hot code", I think moving to a single entry points approach and let the runtime check optionality constraints and dispatching makes more sense.

flang/runtime/command.cpp

181

should this be max(0, executionEnvironment.argc - 1) ? I think it is highly unlikely that argc be zero, but looking at some stack overflow threads, that does not look impossible.

[rovka]

I finally got around to updating this to write the length to an int descriptor.
Also:
Added the source & line parameters so we can have RUNTIME_CHECKS.
Added a test fixture with a completely empty argv.

I'm going to update the other intrinsics in the file to the same model (i.e. single entry point with int descriptors, RUNTIME_CHECKs for invalid descriptors, support for empty argv), and with that I can probably remove some redundant checks from some of the helpers. That's going to be a different patch though.

[jeanPerier]

flang/runtime/command.cpp

189

Could this be moved right after stat = CopyToDescriptor(*value, " ", 1, errmsg, offset); on line 184 ?
It seems related to it but having it after the if {} where the copy happens makes it harder for me to be sure this is related.

flang/unittests/Runtime/CommandTest.cpp

396

This test seems to fail on windows, see the build bot:

Note: Google Test filter = OnlyValidArguments.GetCommandShortLength
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from OnlyValidArguments
[ RUN      ] OnlyValidArguments.GetCommandShortLength
C:\ws\w2\llvm-project\premerge-checks\flang\unittests\Runtime\CommandTest.cpp(77): error: Expected equality of these values:
  *value->OffsetElement<std::int64_t>()
    Which is: 8589934643
  expected
    Which is: 51

I think the value->OffsetElement<std::int64_t>() in CheckDescriptorEqInt may be undefined behavior if the integer type in value is not an std::int64_t (like here where it is a short).

[rovka]

flang/unittests/Runtime/CommandTest.cpp

396

Oops, I thought I had looked at the premerge checks for this one, but it must've slipped my mind 😳 Thanks for looking into it.

[rovka]

• Replaced CopyToDescriptor with CheckAndCopyToDescriptor.
• Made DescriptorEqInt a template, so we can check with the intended integer type.

[jeanPerier]

Looks good, thanks

[jeanPerier]

This comment was removed by jeanPerier.

flang/include/flang/Runtime/command.h

 // integer kind.
 std::int32_t RTNAME(ArgumentCount)();
 
 // 16.9.82 GET_COMMAND
 // Try to get the value of the whole command. All of the parameters are
 // optional.
 // Return a STATUS as described in the standard.
 std::int32_t RTNAME(GetCommand)(const Descriptor *command = nullptr,
-    const Descriptor *length = nullptr, const Descriptor *errmsg = nullptr);
+    const Descriptor *length = nullptr, const Descriptor *errmsg = nullptr,
+    const char *sourceFile = nullptr, int line = 0);
 
 // 16.9.83 GET_COMMAND_ARGUMENT
 // We're breaking up the interface into several different functions, since most
 // of the parameters are optional.
 
 // Try to get the value of the n'th argument.
 // Returns a STATUS as described in the standard.
 std::int32_t RTNAME(ArgumentValue)(

flang/runtime/command.cpp

 //===-- runtime/command.cpp -----------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "flang/Runtime/command.h"
 #include "environment.h"
 #include "stat.h"
 #include "terminator.h"
+#include "tools.h"
 #include "flang/Runtime/descriptor.h"
 #include <cstdlib>
 #include <limits>
 
 namespace Fortran::runtime {
 std::int32_t RTNAME(ArgumentCount)() {
   int argc{executionEnvironment.argc};
   if (argc > 1) {

 }
 
 static bool IsValidCharDescriptor(const Descriptor *value) {
   return value && value->IsAllocated() &&
       value->type() == TypeCode(TypeCategory::Character, 1) &&
       value->rank() == 0;
 }
 
-static void FillWithSpaces(const Descriptor *value) {
-  std::memset(value->OffsetElement(), ' ', value->ElementBytes());
+static bool IsValidIntDescriptor(const Descriptor *length) {
+  auto typeCode{length->type().GetCategoryAndKind()};
+  // Check that our descriptor is allocated and is a scalar integer with
+  // kind != 1 (i.e. with a large enough decimal exponent range).
+  return length->IsAllocated() && length->rank() == 0 &&
+      length->type().IsInteger() && typeCode && typeCode->second != 1;
+}
+
+static void FillWithSpaces(const Descriptor &value, std::size_t offset = 0) {
+  if (offset < value.ElementBytes()) {
+    std::memset(
+        value.OffsetElement(offset), ' ', value.ElementBytes() - offset);
+  }
 }
 
 static std::int32_t CopyToDescriptor(const Descriptor &value,
-    const char *rawValue, std::int64_t rawValueLength,
-    const Descriptor *errmsg) {
-  std::int64_t toCopy{std::min(
-      rawValueLength, static_cast<std::int64_t>(value.ElementBytes()))};
-  std::memcpy(value.OffsetElement(), rawValue, toCopy);
+    const char *rawValue, std::int64_t rawValueLength, const Descriptor *errmsg,
+    std::size_t offset = 0) {
+
+  std::int64_t toCopy{std::min(rawValueLength,
+      static_cast<std::int64_t>(value.ElementBytes() - offset))};
+  if (toCopy < 0) {
+    return ToErrmsg(errmsg, StatValueTooShort);
+  }
+
+  std::memcpy(value.OffsetElement(offset), rawValue, toCopy);
 
   if (rawValueLength > toCopy) {
     return ToErrmsg(errmsg, StatValueTooShort);
   }
 
   return StatOk;
 }
 
+static std::int32_t CheckAndCopyToDescriptor(const Descriptor *value,
+    const char *rawValue, const Descriptor *errmsg, std::size_t &offset) {
+  bool haveValue{IsValidCharDescriptor(value)};
+
+  std::int64_t len{StringLength(rawValue)};
+  if (len <= 0) {
+    if (haveValue) {
+      FillWithSpaces(*value);
+    }
+    return ToErrmsg(errmsg, StatMissingArgument);
+  }
+
+  std::int32_t stat{StatOk};
+  if (haveValue) {
+    stat = CopyToDescriptor(*value, rawValue, len, errmsg, offset);
+  }
+
+  offset += len;
+  return stat;
+}
+
 std::int32_t RTNAME(ArgumentValue)(
     std::int32_t n, const Descriptor *value, const Descriptor *errmsg) {
   if (IsValidCharDescriptor(value)) {
-    FillWithSpaces(value);
+    FillWithSpaces(*value);
   }
 
   if (n < 0 || n >= executionEnvironment.argc) {
     return ToErrmsg(errmsg, StatInvalidArgumentNumber);
   }
 
   if (IsValidCharDescriptor(value)) {
     const char *arg{executionEnvironment.argv[n]};
     std::int64_t argLen{StringLength(arg)};
     if (argLen <= 0) {
       return ToErrmsg(errmsg, StatMissingArgument);
     }
 
     return CopyToDescriptor(*value, arg, argLen, errmsg);
   }
 
   return StatOk;
 }
 
+template <int KIND> struct FitsInIntegerKind {
+  bool operator()(std::int64_t value) {
+    return value <= std::numeric_limits<Fortran::runtime::CppTypeFor<
+                        Fortran::common::TypeCategory::Integer, KIND>>::max();
+  }
+};
+
+std::int32_t RTNAME(GetCommand)(const Descriptor *value,
+    const Descriptor *length, const Descriptor *errmsg, const char *sourceFile,
+    int line) {
+  Terminator terminator{sourceFile, line};
+
+  auto storeLength = [&](std::int64_t value) {
+    auto typeCode{length->type().GetCategoryAndKind()};
+    int kind{typeCode->second};
+    Fortran::runtime::ApplyIntegerKind<Fortran::runtime::StoreIntegerAt, void>(
+        kind, terminator, *length, /* atIndex = */ 0, value);
+  };
+
+  if (value) {
+    RUNTIME_CHECK(terminator, IsValidCharDescriptor(value));
+  }
+
+  // Store 0 in case we error out later on.
+  if (length) {
+    RUNTIME_CHECK(terminator, IsValidIntDescriptor(length));
+    storeLength(0);
+  }
+
+  auto shouldContinue = [&](std::int32_t stat) -> bool {
+    // We continue as long as everything is ok OR the value descriptor is
+    // too short, but we still need to compute the length.
+    return stat == StatOk || (length && stat == StatValueTooShort);
+  };
+
+  std::size_t offset{0};
+
+  if (executionEnvironment.argc == 0) {
+    return CheckAndCopyToDescriptor(value, "", errmsg, offset);
+  }
+
+  // value = argv[0]
+  std::int32_t stat{CheckAndCopyToDescriptor(
+      value, executionEnvironment.argv[0], errmsg, offset)};
+  if (!shouldContinue(stat)) {
+    return stat;
+  }
+
+  // value += " " + argv[1:n]
+  for (std::int32_t i{1}; i < executionEnvironment.argc; ++i) {
+    if (value) {
+      stat = CopyToDescriptor(*value, " ", 1, errmsg, offset);
+      if (!shouldContinue(stat)) {
+        return stat;
+      }
+    }
+    offset++;
+
+    stat = CheckAndCopyToDescriptor(
+        value, executionEnvironment.argv[i], errmsg, offset);
+    if (!shouldContinue(stat)) {
+      return stat;
+    }
+  }
+
+  auto fitsInLength = [&](std::int64_t value) -> bool {
+    auto typeCode{length->type().GetCategoryAndKind()};
+    int kind{typeCode->second};
+    return Fortran::runtime::ApplyIntegerKind<FitsInIntegerKind, bool>(
+        kind, terminator, value);
+  };
+
+  if (length && fitsInLength(offset)) {
+    storeLength(offset);
+  }
+
+  // value += spaces for padding
+  if (value) {
+    FillWithSpaces(*value, offset);
+  }
+
+  return stat;
+}
+
 static std::size_t LengthWithoutTrailingSpaces(const Descriptor &d) {
   std::size_t s{d.ElementBytes() - 1};
   while (*d.OffsetElement(s) == ' ') {
     --s;
   }
   return s + 1;
 }
 
const char *value{executionEnvironment.GetEnv(
       name.OffsetElement(), nameLength, terminator)};
   return value;
 }
 
 std::int32_t RTNAME(EnvVariableValue)(const Descriptor &name,
     const Descriptor *value, bool trim_name, const Descriptor *errmsg,
     const char *sourceFile, int line) {
   if (IsValidCharDescriptor(value)) {
-    FillWithSpaces(value);
+    FillWithSpaces(*value);
   }
 
   const char *rawValue{GetEnvVariableValue(name, trim_name, sourceFile, line)};
   if (!rawValue) {
     return ToErrmsg(errmsg, StatMissingEnvVariable);
   }
 
   if (IsValidCharDescriptor(value)) {

flang/unittests/Runtime/CommandTest.cpp

OwningPtr<Descriptor> descriptor{Descriptor::Create(
       sizeof(char), n, nullptr, 0, nullptr, CFI_attribute_allocatable)};
   if (descriptor->Allocate() != 0) {
     return nullptr;
   }
   std::memcpy(descriptor->OffsetElement(), value, n);
   return descriptor;
 }
 
+template <int kind = sizeof(std::int64_t)>
+static OwningPtr<Descriptor> EmptyIntDescriptor() {
+  OwningPtr<Descriptor> descriptor{Descriptor::Create(TypeCategory::Integer,
+      kind, nullptr, 0, nullptr, CFI_attribute_allocatable)};
+  if (descriptor->Allocate() != 0) {
+    return nullptr;
+  }
+  return descriptor;
+}
+
 class CommandFixture : public ::testing::Test {
 protected:
   CommandFixture(int argc, const char *argv[]) {
     RTNAME(ProgramStart)(argc, argv, {});
   }
 
   std::string GetPaddedStr(const char *text, std::size_t len) const {
     std::string res{text};
     assert(res.length() <= len && "No room to pad");
     res.append(len - res.length(), ' ');
     return res;
   }
 
   void CheckDescriptorEqStr(
       const Descriptor *value, const std::string &expected) const {
+    ASSERT_NE(value, nullptr);
     EXPECT_EQ(std::strncmp(value->OffsetElement(), expected.c_str(),
                   value->ElementBytes()),
         0)
         << "expected: " << expected << "\n"
         << "value: "
         << std::string{value->OffsetElement(), value->ElementBytes()};
   }
 
+  void CheckDescriptorEqInt(
+      const Descriptor *value, const std::int64_t expected) const {
+    if (expected != -1) {
+      ASSERT_NE(value, nullptr);
+      EXPECT_EQ(*value->OffsetElement<std::int64_t>(), expected);
+    }
+  }
+
   template <typename RuntimeCall>
   void CheckValue(RuntimeCall F, const char *expectedValue,
-      std::int32_t expectedStatus = 0,
+      std::int64_t expectedLength = -1, std::int32_t expectedStatus = 0,
       const char *expectedErrMsg = "shouldn't change") const {
     OwningPtr<Descriptor> value{CreateEmptyCharDescriptor()};
     ASSERT_NE(value, nullptr);
 
+    OwningPtr<Descriptor> length{
+        expectedLength == -1 ? nullptr : EmptyIntDescriptor()};
+
     OwningPtr<Descriptor> errmsg{CharDescriptor(expectedErrMsg)};
+    ASSERT_NE(errmsg, nullptr);
 
     std::string expectedValueStr{
         GetPaddedStr(expectedValue, value->ElementBytes())};
 
-    EXPECT_EQ(F(value.get(), errmsg.get()), expectedStatus);
+    EXPECT_EQ(F(value.get(), length.get(), errmsg.get()), expectedStatus);
     CheckDescriptorEqStr(value.get(), expectedValueStr);
+    CheckDescriptorEqInt(length.get(), expectedLength);
     CheckDescriptorEqStr(errmsg.get(), expectedErrMsg);
   }
 
   void CheckArgumentValue(const char *expectedValue, int n) const {
     SCOPED_TRACE(n);
     SCOPED_TRACE("Checking argument:");
     CheckValue(
-        [&](const Descriptor *value, const Descriptor *errmsg) {
+        [&](const Descriptor *value, const Descriptor *,
+            const Descriptor *errmsg) {
           return RTNAME(ArgumentValue)(n, value, errmsg);
         },
         expectedValue);
   }
 
+  void CheckCommandValue(const char *args[], int n) const {
+    SCOPED_TRACE("Checking command:");
+    ASSERT_GE(n, 1);
+    std::string expectedValue{args[0]};
+    for (int i = 1; i < n; i++) {
+      expectedValue += " " + std::string{args[i]};
+    }
+    CheckValue(
+        [&](const Descriptor *value, const Descriptor *length,
+            const Descriptor *errmsg) {
+          return RTNAME(GetCommand)(value, length, errmsg);
+        },
+        expectedValue.c_str(), expectedValue.size());
+  }
+
   void CheckEnvVarValue(
       const char *expectedValue, const char *name, bool trimName = true) const {
     SCOPED_TRACE(name);
     SCOPED_TRACE("Checking environment variable");
     CheckValue(
-        [&](const Descriptor *value, const Descriptor *errmsg) {
+        [&](const Descriptor *value, const Descriptor *,
+            const Descriptor *errmsg) {
           return RTNAME(EnvVariableValue)(*CharDescriptor(name), value,
               trimName, errmsg, /*sourceFile=*/nullptr, /*line=*/0);
         },
         expectedValue);
   }
 
   void CheckMissingEnvVarValue(const char *name, bool trimName = true) const {
     SCOPED_TRACE(name);
     SCOPED_TRACE("Checking missing environment variable");
 
     ASSERT_EQ(nullptr, std::getenv(name))
         << "Environment variable " << name << " not expected to exist";
 
     OwningPtr<Descriptor> nameDescriptor{CharDescriptor(name)};
     EXPECT_EQ(0, RTNAME(EnvVariableLength)(*nameDescriptor, trimName));
     CheckValue(
-        [&](const Descriptor *value, const Descriptor *errmsg) {
+        [&](const Descriptor *value, const Descriptor *,
+            const Descriptor *errmsg) {
           return RTNAME(EnvVariableValue)(*nameDescriptor, value, trimName,
               errmsg, /*sourceFile=*/nullptr, /*line=*/0);
         },
-        "", 1, "Missing environment variable");
+        "", -1, 1, "Missing environment variable");
   }
 
   void CheckMissingArgumentValue(int n, const char *errStr = nullptr) const {
     OwningPtr<Descriptor> value{CreateEmptyCharDescriptor()};
     ASSERT_NE(value, nullptr);
 
     OwningPtr<Descriptor> err{errStr ? CreateEmptyCharDescriptor() : nullptr};
 
     EXPECT_GT(RTNAME(ArgumentValue)(n, value.get(), err.get()), 0);
 
     std::string spaces(value->ElementBytes(), ' ');
     CheckDescriptorEqStr(value.get(), spaces);
 
     if (errStr) {
       std::string paddedErrStr(GetPaddedStr(errStr, err->ElementBytes()));
       CheckDescriptorEqStr(err.get(), paddedErrStr);
     }
   }
+
+  void CheckMissingCommandValue(const char *errStr = nullptr) const {
+    OwningPtr<Descriptor> value{CreateEmptyCharDescriptor()};
+    ASSERT_NE(value, nullptr);
+
+    OwningPtr<Descriptor> length{EmptyIntDescriptor()};
+    ASSERT_NE(length, nullptr);
+
+    OwningPtr<Descriptor> err{errStr ? CreateEmptyCharDescriptor() : nullptr};
+
+    EXPECT_GT(RTNAME(GetCommand)(value.get(), length.get(), err.get()), 0);
+
+    std::string spaces(value->ElementBytes(), ' ');
+    CheckDescriptorEqStr(value.get(), spaces);
+
+    CheckDescriptorEqInt(length.get(), 0);
+
+    if (errStr) {
+      std::string paddedErrStr(GetPaddedStr(errStr, err->ElementBytes()));
+      CheckDescriptorEqStr(err.get(), paddedErrStr);
+    }
+  }
+};
+
+class NoArgv : public CommandFixture {
+protected:
+  NoArgv() : CommandFixture(0, nullptr) {}
 };
 
+// TODO: Test other intrinsics with this fixture.
+
+TEST_F(NoArgv, GetCommand) { CheckMissingCommandValue(); }
+
 static const char *commandOnlyArgv[]{"aProgram"};
 class ZeroArguments : public CommandFixture {
 protected:
   ZeroArguments() : CommandFixture(1, commandOnlyArgv) {}
 };
 
 TEST_F(ZeroArguments, ArgumentCount) { EXPECT_EQ(0, RTNAME(ArgumentCount)()); }
 
 TEST_F(ZeroArguments, ArgumentLength) {
   EXPECT_EQ(0, RTNAME(ArgumentLength)(-1));
   EXPECT_EQ(8, RTNAME(ArgumentLength)(0));
   EXPECT_EQ(0, RTNAME(ArgumentLength)(1));
 }
 
 TEST_F(ZeroArguments, ArgumentValue) {
   CheckArgumentValue(commandOnlyArgv[0], 0);
 }
 
+TEST_F(ZeroArguments, GetCommand) { CheckCommandValue(commandOnlyArgv, 1); }
+
 static const char *oneArgArgv[]{"aProgram", "anArgumentOfLength20"};
 class OneArgument : public CommandFixture {
 protected:
   OneArgument() : CommandFixture(2, oneArgArgv) {}
 };
 
 TEST_F(OneArgument, ArgumentCount) { EXPECT_EQ(1, RTNAME(ArgumentCount)()); }
 
 TEST_F(OneArgument, ArgumentLength) {
   EXPECT_EQ(0, RTNAME(ArgumentLength)(-1));
   EXPECT_EQ(8, RTNAME(ArgumentLength)(0));
   EXPECT_EQ(20, RTNAME(ArgumentLength)(1));
   EXPECT_EQ(0, RTNAME(ArgumentLength)(2));
 }
 
 TEST_F(OneArgument, ArgumentValue) {
   CheckArgumentValue(oneArgArgv[0], 0);
   CheckArgumentValue(oneArgArgv[1], 1);
 }
 
+TEST_F(OneArgument, GetCommand) { CheckCommandValue(oneArgArgv, 2); }
+
 static const char *severalArgsArgv[]{
     "aProgram", "16-char-long-arg", "", "-22-character-long-arg", "o"};
 class SeveralArguments : public CommandFixture {
 protected:
   SeveralArguments()
       : CommandFixture(sizeof(severalArgsArgv) / sizeof(*severalArgsArgv),
             severalArgsArgv) {}
 };

 
 TEST_F(SeveralArguments, MissingArguments) {
   CheckMissingArgumentValue(-1, "Invalid argument number");
   CheckMissingArgumentValue(2, "Missing argument");
   CheckMissingArgumentValue(5, "Invalid argument number");
   CheckMissingArgumentValue(5);
 }
 
-TEST_F(SeveralArguments, ValueTooShort) {
+TEST_F(SeveralArguments, ArgValueTooShort) {
   OwningPtr<Descriptor> tooShort{CreateEmptyCharDescriptor<15>()};
   ASSERT_NE(tooShort, nullptr);
   EXPECT_EQ(RTNAME(ArgumentValue)(1, tooShort.get(), nullptr), -1);
   CheckDescriptorEqStr(tooShort.get(), severalArgsArgv[1]);
 
   OwningPtr<Descriptor> errMsg{CreateEmptyCharDescriptor()};
   ASSERT_NE(errMsg, nullptr);
 
   EXPECT_EQ(RTNAME(ArgumentValue)(1, tooShort.get(), errMsg.get()), -1);
 
   std::string expectedErrMsg{
       GetPaddedStr("Value too short", errMsg->ElementBytes())};
   CheckDescriptorEqStr(errMsg.get(), expectedErrMsg);
 }
 
-TEST_F(SeveralArguments, ErrMsgTooShort) {
+TEST_F(SeveralArguments, ArgErrMsgTooShort) {
   OwningPtr<Descriptor> errMsg{CreateEmptyCharDescriptor<3>()};
   EXPECT_GT(RTNAME(ArgumentValue)(-1, nullptr, errMsg.get()), 0);
   CheckDescriptorEqStr(errMsg.get(), "Inv");
 }
 
+TEST_F(SeveralArguments, GetCommand) {
+  CheckMissingCommandValue();
+  CheckMissingCommandValue("Missing argument");
+}
+
+TEST_F(SeveralArguments, CommandErrMsgTooShort) {
+  OwningPtr<Descriptor> value{CreateEmptyCharDescriptor()};
+  OwningPtr<Descriptor> length{EmptyIntDescriptor()};
+  OwningPtr<Descriptor> errMsg{CreateEmptyCharDescriptor<3>()};
+
+  EXPECT_GT(RTNAME(GetCommand)(value.get(), length.get(), errMsg.get()), 0);
+
+  std::string spaces(value->ElementBytes(), ' ');
+  CheckDescriptorEqStr(value.get(), spaces);
+  CheckDescriptorEqInt(length.get(), 0);
+  CheckDescriptorEqStr(errMsg.get(), "Mis");
+}
+
+TEST_F(SeveralArguments, GetCommandCanTakeNull) {
+  EXPECT_GT(RTNAME(GetCommand)(nullptr, nullptr, nullptr), 0);
+}
+
+static const char *onlyValidArgsArgv[]{
+    "aProgram", "-f", "has/a/few/slashes", "has\\a\\few\\backslashes"};
+class OnlyValidArguments : public CommandFixture {
+protected:
+  OnlyValidArguments()
+      : CommandFixture(sizeof(onlyValidArgsArgv) / sizeof(*onlyValidArgsArgv),
+            onlyValidArgsArgv) {}
+};
+
+TEST_F(OnlyValidArguments, GetCommand) {
+  CheckCommandValue(onlyValidArgsArgv, 4);
+}
+
+TEST_F(OnlyValidArguments, CommandValueTooShort) {
+  OwningPtr<Descriptor> tooShort{CreateEmptyCharDescriptor<50>()};
+  ASSERT_NE(tooShort, nullptr);
+  OwningPtr<Descriptor> length{EmptyIntDescriptor()};
+  ASSERT_NE(length, nullptr);
+
+  EXPECT_EQ(RTNAME(GetCommand)(tooShort.get(), length.get(), nullptr), -1);
+
+  CheckDescriptorEqStr(
+      tooShort.get(), "aProgram -f has/a/few/slashes has\\a\\few\\backslashe");
+  CheckDescriptorEqInt(length.get(), 51);
+
+  OwningPtr<Descriptor> errMsg{CreateEmptyCharDescriptor()};
+  ASSERT_NE(errMsg, nullptr);
+
+  EXPECT_EQ(-1, RTNAME(GetCommand)(tooShort.get(), nullptr, errMsg.get()));
+
+  std::string expectedErrMsg{
+      GetPaddedStr("Value too short", errMsg->ElementBytes())};
+  CheckDescriptorEqStr(errMsg.get(), expectedErrMsg);
+}
+
+TEST_F(OnlyValidArguments, GetCommandCanTakeNull) {
+  EXPECT_EQ(0, RTNAME(GetCommand)(nullptr, nullptr, nullptr));
+
+  OwningPtr<Descriptor> value{CreateEmptyCharDescriptor()};
+  ASSERT_NE(value, nullptr);
+  OwningPtr<Descriptor> length{EmptyIntDescriptor()};
+  ASSERT_NE(length, nullptr);
+
+  EXPECT_EQ(0, RTNAME(GetCommand)(value.get(), nullptr, nullptr));
+  CheckDescriptorEqStr(value.get(),
+      GetPaddedStr("aProgram -f has/a/few/slashes has\\a\\few\\backslashes",
+          value->ElementBytes()));
+
+  EXPECT_EQ(0, RTNAME(GetCommand)(nullptr, length.get(), nullptr));
+  CheckDescriptorEqInt(length.get(), 51);
+}
+
+TEST_F(OnlyValidArguments, GetCommandShortLength) {
+  OwningPtr<Descriptor> length{EmptyIntDescriptor<sizeof(short)>()};
+  ASSERT_NE(length, nullptr);
+
+  EXPECT_EQ(0, RTNAME(GetCommand)(nullptr, length.get(), nullptr));
+  CheckDescriptorEqInt(length.get(), 51);
+}
+
 class EnvironmentVariables : public CommandFixture {
 protected:
   EnvironmentVariables() : CommandFixture(0, nullptr) {
     SetEnv("NAME", "VALUE");
     SetEnv("EMPTY", "");
   }
 
   // If we have access to setenv, we can run some more fine-grained tests.