[flang] GET_COMMAND_ARGUMENT(LENGTH) runtime implementation

[rovka]

Summary

Implement the ArgumentLength entry point of GET_COMMAND_ARGUMENT. Also
introduce a fixture for the tests.

Note that this also changes the interface for ArgumentLength from
returning a 4-byte integer to returning an 8-byte integer.

[klausler]

flang/runtime/command.cpp

23

It'll probably never happen, but what if the length of the argument is > 2GiB?

[rovka]

Made it fail when strlen returns something we can't represent.

[klausler]

flang/runtime/command.cpp

14

I don't think that this is the right solution. What if the LENGTH= argument to GET_COMMAND_ARGUMENT had been an INTEGER*8 variable? That case should work. It would be better to use a larger result type here.

[rovka]

flang/runtime/command.cpp

14

Oh, I misunderstood, I thought we had agreed on the interface.

I'll upload a new version with 8-byte integers next week, since right now I'm a bit busy with some side issues.

[rovka]

Switched from 4-byte to 8-byte integer.

[rovka]

Also add to the no-cpp-dep test (would be nice to have most of the runtime in there).

[rovka]

Rebase and move the definitions for CountType and LengthType to the header file, as suggested in https://reviews.llvm.org/D109813

[klausler]

flang/include/flang/Runtime/command.h

18

I think that you can use std::int32_t/std::int64_t here.

[rovka]

Switched to std::int32_t/64_t

[klausler]

flang/include/flang/Runtime/command.h

18

I meant that you don't need to have the type aliases CountType, LengthType, &c. either.

34–35

std::int32_t

[rovka]

Remove shorthand types.

[clementval]

I'm seeing build failure with gcc 9.3.0 with this patch.

******************** TEST 'Flang :: Runtime/no-cpp-dep.c' FAILED ********************
Script:
--
: 'RUN: at line 8';   /home/sw/thirdparty/gcc/gcc-9.3.0/linux86-64/bin/gcc -std=c99 llvm-project/flang/test/Runtime/no-cpp-dep.c -Illvm-project/flang/include llvm-project/build/lib/libFortranRuntime.a llvm-project/build/lib/libFortranDecimal.a -lm -o /dev/null
--
Exit Code: 1

Command Output (stderr):
--
In file included from /home/sw/thirdparty/gcc/gcc-9.3.0/linux86-64/lib/gcc/x86_64-pc-linux-gnu/9.3.0/include/stdint.h:9,
                 from llvm-project/flang/test/Runtime/no-cpp-dep.c:12:
/usr/include/stdint.h:26:10: fatal error: bits/libc-header-start.h: No such file or directory
   26 | #include <bits/libc-header-start.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.

--

********************
********************
Failed Tests (1):
  Flang :: Runtime/no-cpp-dep.c


Testing Time: 2.52s
  Unsupported      :    3
  Passed           : 1012
  Expectedly Failed:    3
  Failed           :    1

[rovka]

Hi Valentin,

Thank you for the report!

I've been googling that error message a bit and all I could find were posts
related to compiling with -m32 on a 64-bit system. However the test is
definitely not passing -m32 and you seem to be using an x86-64 compiler, so
I'm not sure what the issue is. Would it be possible for you to try some
experiments on your end, e.g. check if you have libc-header-start.h
anywhere on your system, or installing gcc-multilib?

Thanks,
Diana

[clementval]

In D109227#3027135, @rovka wrote:

Hi Valentin,

Thank you for the report!

I've been googling that error message a bit and all I could find were posts
related to compiling with -m32 on a 64-bit system. However the test is
definitely not passing -m32 and you seem to be using an x86-64 compiler, so
I'm not sure what the issue is. Would it be possible for you to try some
experiments on your end, e.g. check if you have libc-header-start.h
anywhere on your system, or installing gcc-multilib?

Thanks,
Diana

We have libc-header-start.h on the system under /usr/include/x86_64-linux-gnu/bits/libc-header-start.h. Since it's a lab machine I cannot install gcc-multilib easily.

[rovka]

Hi Valentin,

I tried to reproduce this in a Ubuntu Focal container with gcc 9.3.0
installed via apt (and no multilib afaict) and I'm not seeing any failures.
It also seems to work in Compiler Explorer: https://godbolt.org/z/nq8czercf
I think the issue might be limited to your system. If you find some kind of
workaround to get the test to work on your system too I'll be happy to
review any changes.

Cheers,
Diana

[clementval]

In D109227#3032739, @rovka wrote:

Hi Valentin,

I tried to reproduce this in a Ubuntu Focal container with gcc 9.3.0
installed via apt (and no multilib afaict) and I'm not seeing any failures.
It also seems to work in Compiler Explorer: https://godbolt.org/z/nq8czercf
I think the issue might be limited to your system. If you find some kind of
workaround to get the test to work on your system too I'll be happy to
review any changes.

Cheers,
Diana

Thanks for trying to reproduce. No worries, I'll check on my side.

Interestingly it doesn't fail if I run the command by hand. Might be a llvm configuration problem

flang/include/flang/Runtime/command.h

 #define FORTRAN_RUNTIME_COMMAND_H_
 
 #include "flang/Runtime/cpp-type.h"
 #include "flang/Runtime/entry-names.h"
 
 namespace Fortran::runtime {
 class Descriptor;
 
+using CountType = CppTypeFor<TypeCategory::Integer, 4>;
+using LengthType = CppTypeFor<TypeCategory::Integer, 8>;
+
 extern "C" {
 // 16.9.51 COMMAND_ARGUMENT_COUNT
 //
 // Lowering may need to cast the result to match the precision of the default
 // integer kind.
-CppTypeFor<TypeCategory::Integer, 4> RTNAME(ArgumentCount)();
+CountType RTNAME(ArgumentCount)();
 
 // 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.
 CppTypeFor<TypeCategory::Integer, 4> RTNAME(ArgumentValue)(
-    CppTypeFor<TypeCategory::Integer, 4> n, const Descriptor *value,
-    const Descriptor *errmsg);
+    CountType n, const Descriptor *value, const Descriptor *errmsg);
 
 // Try to get the significant length of the n'th argument.
 // Returns 0 if it doesn't manage.
-CppTypeFor<TypeCategory::Integer, 4> RTNAME(ArgumentLength)(
-    CppTypeFor<TypeCategory::Integer, 4> n);
+LengthType RTNAME(ArgumentLength)(CountType n);
 }
 } // namespace Fortran::runtime
 
 #endif // FORTRAN_RUNTIME_COMMAND_H_

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 <limits>
 
 namespace Fortran::runtime {
-CppTypeFor<TypeCategory::Integer, 4> RTNAME(ArgumentCount)() {
+CountType RTNAME(ArgumentCount)() {
   int argc{executionEnvironment.argc};
   if (argc > 1) {
     // C counts the command name as one of the arguments, but Fortran doesn't.
     return argc - 1;
   }
   return 0;
 }
+
+LengthType RTNAME(ArgumentLength)(CountType n) {
+  if (n < 0 || n >= executionEnvironment.argc) {
+    return 0;
+  }
+
+  std::size_t length{std::strlen(executionEnvironment.argv[n])};
+  if constexpr (sizeof(std::size_t) <= sizeof(LengthType)) {
+    return static_cast<LengthType>(length);
+  } else {
+    std::size_t max{std::numeric_limits<LengthType>::max()};
+    return length > max ? 0 // Just fail.
+                        : static_cast<LengthType>(length);
+  }
+}
 } // namespace Fortran::runtime

flang/test/Runtime/no-cpp-dep.c

 /*
 This test makes sure that flang's runtime does not depend on the C++ runtime
 library. It tries to link this simple file against libFortranRuntime.a with
 a C compiler.
 
 REQUIRES: c-compiler
 
-RUN: %cc -std=c90 %s -I%include %libruntime %libdecimal -o /dev/null
+RUN: %cc -std=c99 %s -I%include %libruntime %libdecimal -lm -o /dev/null
 */
 
 #include "flang/Runtime/entry-names.h"
+#include <stdint.h>
 
 /*
 Manually add declarations for the runtime functions that we want to make sure
 we're testing. We can't include any headers directly since they likely contain
 C++ code that would explode here.
 */
+struct Descriptor;
+
 double RTNAME(CpuTime)();
 
+void RTNAME(ProgramStart)(int, const char *[], const char *[]);
+int32_t RTNAME(ArgumentCount)();
+int64_t RTNAME(ArgumentLength)(int32_t);
+
 int main() {
   double x = RTNAME(CpuTime)();
-  return x;
+  RTNAME(ProgramStart)(0, 0, 0);
+  int32_t c = RTNAME(ArgumentCount)();
+  int32_t l = RTNAME(ArgumentLength)(0);
+  return x + c + l;
 }

flang/unittests/Runtime/CommandTest.cpp

 //===-- flang/unittests/RuntimeGTest/CommandTest.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 "gtest/gtest.h"
 #include "flang/Runtime/descriptor.h"
 #include "flang/Runtime/main.h"
 
 using namespace Fortran::runtime;
 
-TEST(ArgumentCount, ZeroArguments) {
-  const char *argv[]{"aProgram"};
-  RTNAME(ProgramStart)(1, argv, {});
-  EXPECT_EQ(0, RTNAME(ArgumentCount)());
+class CommandFixture : public ::testing::Test {
+protected:
+  CommandFixture(int argc, const char *argv[]) {
+    RTNAME(ProgramStart)(argc, argv, {});
   }
+};
 
-TEST(ArgumentCount, OneArgument) {
-  const char *argv[]{"aProgram", "anArgument"};
-  RTNAME(ProgramStart)(2, argv, {});
-  EXPECT_EQ(1, RTNAME(ArgumentCount)());
+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(ArgumentCount, SeveralArguments) {
-  const char *argv[]{"aProgram", "arg1", "arg2", "arg3", "arg4"};
-  RTNAME(ProgramStart)(5, argv, {});
+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));
+}
+
+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, ArgumentCount) {
   EXPECT_EQ(4, RTNAME(ArgumentCount)());
 }
+
+TEST_F(SeveralArguments, ArgumentLength) {
+  EXPECT_EQ(0, RTNAME(ArgumentLength)(-1));
+  EXPECT_EQ(8, RTNAME(ArgumentLength)(0));
+  EXPECT_EQ(16, RTNAME(ArgumentLength)(1));
+  EXPECT_EQ(0, RTNAME(ArgumentLength)(2));
+  EXPECT_EQ(22, RTNAME(ArgumentLength)(3));
+  EXPECT_EQ(1, RTNAME(ArgumentLength)(4));
+  EXPECT_EQ(0, RTNAME(ArgumentLength)(5));
+}