From 5112bd6b6e10b27b81aa83cfdbe3588973a6f1f5 Mon Sep 17 00:00:00 2001
From: Diana Picus <diana.picus@linaro.org>
Date: Tue, 27 Apr 2021 08:56:14 +0000
Subject: [PATCH] [flang] Fix a bug in the character runtime

The number of bytes copied in CopyAndPad should depend on the size of
the type being copied, not on its shift value (which in the case of char
is 0, leading to no bytes at all being copied).

Add unit tests for CharacterMin and CharacterMax, which exercise this
code path.

Differential Revision: https://reviews.llvm.org/D101355
---
 flang/runtime/character.cpp                    |  4 +-
 flang/unittests/RuntimeGTest/CharacterTest.cpp | 71 +++++++++++++++++++++++++-
 2 files changed, 72 insertions(+), 3 deletions(-)

diff --git a/flang/runtime/character.cpp b/flang/runtime/character.cpp
index 3b383bb..ed8e5f4 100644
--- a/flang/runtime/character.cpp
+++ b/flang/runtime/character.cpp
@@ -456,9 +456,9 @@ static void CopyAndPad(
       to[j] = static_cast<TO>(' ');
     }
   } else if (toChars <= fromChars) {
-    std::memcpy(to, from, toChars * shift<TO>);
+    std::memcpy(to, from, toChars * sizeof(TO));
   } else {
-    std::memcpy(to, from, fromChars * shift<TO>);
+    std::memcpy(to, from, fromChars * sizeof(TO));
     for (std::size_t j{fromChars}; j < toChars; ++j) {
       to[j] = static_cast<TO>(' ');
     }
diff --git a/flang/unittests/RuntimeGTest/CharacterTest.cpp b/flang/unittests/RuntimeGTest/CharacterTest.cpp
index 6afba32..458a7fd 100644
--- a/flang/unittests/RuntimeGTest/CharacterTest.cpp
+++ b/flang/unittests/RuntimeGTest/CharacterTest.cpp
@@ -10,6 +10,7 @@
 // in Fortran.
 
 #include "../../runtime/character.h"
+#include "../../runtime/descriptor.h"
 #include "gtest/gtest.h"
 #include <cstring>
 #include <functional>
@@ -134,11 +135,79 @@ TYPED_TEST(CharacterComparisonTests, CompareCharacters) {
     std::memcpy(buf[0], x, xBytes);
     std::memcpy(buf[1], y, yBytes);
     ASSERT_EQ(cmp, expect) << "compare '" << x << "'(" << xBytes << ") to '"
-                           << y << "'(" << yBytes << "), got " << cmp
+                           << y << "'(" << yBytes << "'), got " << cmp
                            << ", should be " << expect << '\n';
   }
 }
 
+// Test MIN() and MAX()
+struct ExtremumTestCase {
+  const char *x, *y, *expect;
+};
+
+template <typename CHAR>
+void RunExtremumTests(const char *which,
+    std::function<void(Descriptor &, const Descriptor &, const char *, int)>
+        function,
+    const std::vector<ExtremumTestCase> &testCases) {
+
+  // Helper for creating, allocating and filling up a descriptor with data from
+  // a raw character literal, converted to the CHAR type used by the test.
+  auto CreateDescriptor = [](const char *raw) -> OwningPtr<Descriptor> {
+    std::size_t length{std::strlen(raw)};
+    std::basic_string<CHAR> converted{raw, raw + length};
+
+    OwningPtr<Descriptor> descriptor{Descriptor::Create(
+        sizeof(CHAR), length, nullptr, 0, nullptr, CFI_attribute_allocatable)};
+    if (descriptor->Allocate() != 0) {
+      return nullptr;
+    }
+
+    std::copy(
+        converted.begin(), converted.end(), descriptor->OffsetElement<CHAR>());
+
+    return descriptor;
+  };
+
+  for (const auto &t : testCases) {
+    OwningPtr<Descriptor> x = CreateDescriptor(t.x);
+    OwningPtr<Descriptor> y = CreateDescriptor(t.y);
+
+    ASSERT_NE(x, nullptr);
+    ASSERT_TRUE(x->IsAllocated());
+    ASSERT_NE(y, nullptr);
+    ASSERT_TRUE(y->IsAllocated());
+    function(*x, *y, nullptr, 0);
+
+    std::basic_string<CHAR> got{
+        x->OffsetElement<CHAR>(), x->ElementBytes() / sizeof(CHAR)};
+    std::basic_string<CHAR> expect{t.expect, t.expect + std::strlen(t.expect)};
+    EXPECT_EQ(expect, got) << which << "('" << t.x << "','" << t.y
+                           << "') for CHARACTER(kind=" << sizeof(CHAR) << ")";
+  }
+}
+
+template <typename CHAR> struct ExtremumTests : public ::testing::Test {};
+TYPED_TEST_CASE(ExtremumTests, CharacterTypes);
+
+TYPED_TEST(ExtremumTests, MinTests) {
+  static std::vector<ExtremumTestCase> tests{
+      {"a", "z", "a"},
+      {"zaaa", "aa", "aa  "},
+      {"aaz", "aaaaa", "aaaaa"},
+  };
+  RunExtremumTests<TypeParam>("MIN", RTNAME(CharacterMin), tests);
+}
+
+TYPED_TEST(ExtremumTests, MaxTests) {
+  static std::vector<ExtremumTestCase> tests{
+      {"a", "z", "z"},
+      {"zaa", "aaaaa", "zaa  "},
+      {"aaaaa", "aazaa", "aazaa"},
+  };
+  RunExtremumTests<TypeParam>("MAX", RTNAME(CharacterMax), tests);
+}
+
 // Test search functions INDEX(), SCAN(), and VERIFY()
 
 template <typename CHAR>
-- 
2.7.4