From 6359a4cdbfb83c67a2a776baba4e3f4fb8334853 Mon Sep 17 00:00:00 2001 From: Diana Picus Date: Tue, 28 Sep 2021 12:02:52 +0000 Subject: [PATCH] Revert "[flang] GET_COMMAND_ARGUMENT(VALUE) runtime implementation" This reverts commit 0446f1299f6be9fd35bc5f458c78b34dca3105f6 and df6302311f88d0fbc666b6277d029aa371039945. There's a warning on flang-aarch64-latest-gcc related to strncpy using the result of strlen as a bound. I'll recommit with a fix. --- flang/include/flang/Runtime/magic-numbers.h | 8 --- flang/runtime/command.cpp | 55 ++------------- flang/runtime/stat.cpp | 7 -- flang/runtime/stat.h | 3 - flang/test/Runtime/no-cpp-dep.c | 3 - flang/unittests/Runtime/CommandTest.cpp | 104 ---------------------------- 6 files changed, 5 insertions(+), 175 deletions(-) diff --git a/flang/include/flang/Runtime/magic-numbers.h b/flang/include/flang/Runtime/magic-numbers.h index 0f1957b..f84b7ec 100644 --- a/flang/include/flang/Runtime/magic-numbers.h +++ b/flang/include/flang/Runtime/magic-numbers.h @@ -39,12 +39,4 @@ start at 100 so as to never conflict with those codes. #define FORTRAN_RUNTIME_STAT_STOPPED_IMAGE 104 #define FORTRAN_RUNTIME_STAT_UNLOCKED 105 #define FORTRAN_RUNTIME_STAT_UNLOCKED_FAILED_IMAGE 106 - -#if 0 -Status codes for GET_COMMAND_ARGUMENT. The status for 'value too short' needs -to be -1, the others must be positive. -#endif -#define FORTRAN_RUNTIME_STAT_INVALID_ARG_NUMBER 107 -#define FORTRAN_RUNTIME_STAT_MISSING_ARG 108 -#define FORTRAN_RUNTIME_STAT_VALUE_TOO_SHORT -1 #endif diff --git a/flang/runtime/command.cpp b/flang/runtime/command.cpp index a133b70..1a55061 100644 --- a/flang/runtime/command.cpp +++ b/flang/runtime/command.cpp @@ -8,8 +8,6 @@ #include "flang/Runtime/command.h" #include "environment.h" -#include "stat.h" -#include "flang/Runtime/descriptor.h" #include namespace Fortran::runtime { @@ -22,8 +20,11 @@ std::int32_t RTNAME(ArgumentCount)() { return 0; } -// Returns the length of the \p n'th argument. Assumes \p n is valid. -static std::int64_t ArgumentLength(std::int32_t n) { +std::int64_t RTNAME(ArgumentLength)(std::int32_t 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(std::int64_t)) { return static_cast(length); @@ -33,50 +34,4 @@ static std::int64_t ArgumentLength(std::int32_t n) { : static_cast(length); } } - -std::int64_t RTNAME(ArgumentLength)(std::int32_t n) { - if (n < 0 || n >= executionEnvironment.argc) { - return 0; - } - - return ArgumentLength(n); -} - -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()); -} - -std::int32_t RTNAME(ArgumentValue)( - std::int32_t n, const Descriptor *value, const Descriptor *errmsg) { - if (IsValidCharDescriptor(value)) { - FillWithSpaces(value); - } - - if (n < 0 || n >= executionEnvironment.argc) { - return ToErrmsg(errmsg, StatInvalidArgumentNumber); - } - - if (IsValidCharDescriptor(value)) { - std::int64_t argLen{ArgumentLength(n)}; - if (argLen <= 0) { - return ToErrmsg(errmsg, StatMissingArgument); - } - - std::int64_t toCopy{ - std::min(argLen, static_cast(value->ElementBytes()))}; - std::strncpy(value->OffsetElement(), executionEnvironment.argv[n], toCopy); - - if (argLen > toCopy) { - return ToErrmsg(errmsg, StatValueTooShort); - } - } - - return StatOk; -} } // namespace Fortran::runtime diff --git a/flang/runtime/stat.cpp b/flang/runtime/stat.cpp index d28187c..c0620f4 100644 --- a/flang/runtime/stat.cpp +++ b/flang/runtime/stat.cpp @@ -50,13 +50,6 @@ const char *StatErrorString(int stat) { case StatUnlockedFailedImage: return "Failed image unlocked"; - case StatInvalidArgumentNumber: - return "Invalid argument number"; - case StatMissingArgument: - return "Missing argument"; - case StatValueTooShort: - return "Value too short"; - default: return nullptr; } diff --git a/flang/runtime/stat.h b/flang/runtime/stat.h index 5042f4b..5c1828e 100644 --- a/flang/runtime/stat.h +++ b/flang/runtime/stat.h @@ -44,9 +44,6 @@ enum Stat { StatUnlockedFailedImage = FORTRAN_RUNTIME_STAT_UNLOCKED_FAILED_IMAGE, // Additional "processor-defined" STAT= values - StatInvalidArgumentNumber = FORTRAN_RUNTIME_STAT_INVALID_ARG_NUMBER, - StatMissingArgument = FORTRAN_RUNTIME_STAT_MISSING_ARG, - StatValueTooShort = FORTRAN_RUNTIME_STAT_VALUE_TOO_SHORT, }; const char *StatErrorString(int); diff --git a/flang/test/Runtime/no-cpp-dep.c b/flang/test/Runtime/no-cpp-dep.c index 0f99ccc..9970e09 100644 --- a/flang/test/Runtime/no-cpp-dep.c +++ b/flang/test/Runtime/no-cpp-dep.c @@ -22,15 +22,12 @@ double RTNAME(CpuTime)(); void RTNAME(ProgramStart)(int, const char *[], const char *[]); int32_t RTNAME(ArgumentCount)(); -int32_t RTNAME(ArgumentValue)( - int32_t, const struct Descriptor *, const struct Descriptor *); int64_t RTNAME(ArgumentLength)(int32_t); int main() { double x = RTNAME(CpuTime)(); RTNAME(ProgramStart)(0, 0, 0); int32_t c = RTNAME(ArgumentCount)(); - int32_t v = RTNAME(ArgumentValue)(0, 0, 0); int32_t l = RTNAME(ArgumentLength)(0); return x + c + l; } diff --git a/flang/unittests/Runtime/CommandTest.cpp b/flang/unittests/Runtime/CommandTest.cpp index a13addd7..3841bbe 100644 --- a/flang/unittests/Runtime/CommandTest.cpp +++ b/flang/unittests/Runtime/CommandTest.cpp @@ -7,69 +7,17 @@ //===----------------------------------------------------------------------===// #include "flang/Runtime/command.h" -#include "gmock/gmock.h" #include "gtest/gtest.h" #include "flang/Runtime/descriptor.h" #include "flang/Runtime/main.h" using namespace Fortran::runtime; -template -static OwningPtr CreateEmptyCharDescriptor() { - OwningPtr descriptor{Descriptor::Create( - sizeof(char), n, 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 { - EXPECT_EQ(std::strncmp(value->OffsetElement(), expected.c_str(), - value->ElementBytes()), - 0); - } - - void CheckArgumentValue(int n, const char *argv) const { - OwningPtr value{CreateEmptyCharDescriptor()}; - ASSERT_NE(value, nullptr); - - std::string expected{GetPaddedStr(argv, value->ElementBytes())}; - - EXPECT_EQ(RTNAME(ArgumentValue)(n, value.get(), nullptr), 0); - CheckDescriptorEqStr(value.get(), expected); - } - - void CheckMissingArgumentValue(int n, const char *errStr = nullptr) const { - OwningPtr value{CreateEmptyCharDescriptor()}; - ASSERT_NE(value, nullptr); - - OwningPtr 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); - } - } }; static const char *commandOnlyArgv[]{"aProgram"}; @@ -86,10 +34,6 @@ TEST_F(ZeroArguments, ArgumentLength) { EXPECT_EQ(0, RTNAME(ArgumentLength)(1)); } -TEST_F(ZeroArguments, ArgumentValue) { - CheckArgumentValue(0, commandOnlyArgv[0]); -} - static const char *oneArgArgv[]{"aProgram", "anArgumentOfLength20"}; class OneArgument : public CommandFixture { protected: @@ -105,11 +49,6 @@ TEST_F(OneArgument, ArgumentLength) { EXPECT_EQ(0, RTNAME(ArgumentLength)(2)); } -TEST_F(OneArgument, ArgumentValue) { - CheckArgumentValue(0, oneArgArgv[0]); - CheckArgumentValue(1, oneArgArgv[1]); -} - static const char *severalArgsArgv[]{ "aProgram", "16-char-long-arg", "", "-22-character-long-arg", "o"}; class SeveralArguments : public CommandFixture { @@ -132,46 +71,3 @@ TEST_F(SeveralArguments, ArgumentLength) { EXPECT_EQ(1, RTNAME(ArgumentLength)(4)); EXPECT_EQ(0, RTNAME(ArgumentLength)(5)); } - -TEST_F(SeveralArguments, ArgumentValue) { - CheckArgumentValue(0, severalArgsArgv[0]); - CheckArgumentValue(1, severalArgsArgv[1]); - CheckArgumentValue(3, severalArgsArgv[3]); - CheckArgumentValue(4, severalArgsArgv[4]); -} - -TEST_F(SeveralArguments, NoArgumentValue) { - // Make sure we don't crash if the 'value' and 'error' parameters aren't - // passed. - EXPECT_EQ(RTNAME(ArgumentValue)(2, nullptr, nullptr), 0); - EXPECT_GT(RTNAME(ArgumentValue)(-1, nullptr, nullptr), 0); -} - -TEST_F(SeveralArguments, MissingArguments) { - CheckMissingArgumentValue(-1, "Invalid argument number"); - CheckMissingArgumentValue(2, "Missing argument"); - CheckMissingArgumentValue(5, "Invalid argument number"); - CheckMissingArgumentValue(5); -} - -TEST_F(SeveralArguments, ValueTooShort) { - OwningPtr tooShort{CreateEmptyCharDescriptor<15>()}; - ASSERT_NE(tooShort, nullptr); - EXPECT_EQ(RTNAME(ArgumentValue)(1, tooShort.get(), nullptr), -1); - CheckDescriptorEqStr(tooShort.get(), severalArgsArgv[1]); - - OwningPtr 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) { - OwningPtr errMsg{CreateEmptyCharDescriptor<3>()}; - EXPECT_GT(RTNAME(ArgumentValue)(-1, nullptr, errMsg.get()), 0); - CheckDescriptorEqStr(errMsg.get(), "Inv"); -} -- 2.7.4