From 7ca5ed70e619a2fe21d7a454a0d5e82e08a0084b Mon Sep 17 00:00:00 2001
From: "Daniel R. Carvalho" <odanrc@yahoo.com.br>
Date: Thu, 31 Dec 2020 11:52:06 -0300
Subject: [PATCH] base: Fix scientific number conversion in base/str

Previously converting scientific numbers to non-floating
numbers would yield incorrect results, because the
underlying conversion was using stoll and stoull, which
do not deal with scientific numbers properly. Therefore,
converting to_number("8.234e+08", val) would yield
val=8, which is blatantly wrong (expected 823400000).

This was fixed by searching for "e" within the string to
be converted.

To make sure all double and scientific number conversions
are correct, a few extra tests were added.

Change-Id: I6a9599d8473909d274326b6f8c268e3603044ab4
Signed-off-by: Daniel R. Carvalho <odanrc@yahoo.com.br>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/38775
Reviewed-by: Bobby R. Bruce <bbruce@ucdavis.edu>
Maintainer: Bobby R. Bruce <bbruce@ucdavis.edu>
Tested-by: kokoro <noreply+kokoro@google.com>
---
 src/base/str.hh      | 12 +++++++++
 src/base/str.test.cc | 62 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+)

diff --git a/src/base/str.hh b/src/base/str.hh
index 40fd780fdd..d91761d148 100644
--- a/src/base/str.hh
+++ b/src/base/str.hh
@@ -112,6 +112,10 @@ typename std::enable_if_t<std::is_integral<T>::value &&
                           std::is_signed<T>::value, T>
 __to_number(const std::string &value)
 {
+    // Cannot parse scientific numbers
+    if (value.find('e') != std::string::npos) {
+        throw std::invalid_argument("Cannot convert scientific to integral");
+    }
     // start big and narrow it down if needed, determine the base dynamically
     long long r = std::stoll(value, nullptr, 0);
     if (r < std::numeric_limits<T>::lowest()
@@ -126,6 +130,10 @@ typename std::enable_if_t<std::is_integral<T>::value &&
                           !std::is_signed<T>::value, T>
 __to_number(const std::string &value)
 {
+    // Cannot parse scientific numbers
+    if (value.find('e') != std::string::npos) {
+        throw std::invalid_argument("Cannot convert scientific to integral");
+    }
     // start big and narrow it down if needed, determine the base dynamically
     unsigned long long r = std::stoull(value, nullptr, 0);
     if (r > std::numeric_limits<T>::max())
@@ -137,6 +145,10 @@ template <class T>
 typename std::enable_if_t<std::is_enum<T>::value, T>
 __to_number(const std::string &value)
 {
+    // Cannot parse scientific numbers
+    if (value.find('e') != std::string::npos) {
+        throw std::invalid_argument("Cannot convert scientific to integral");
+    }
     auto r = __to_number<typename std::underlying_type<T>::type>(value);
     return static_cast<T>(r);
 }
diff --git a/src/base/str.test.cc b/src/base/str.test.cc
index f0cd483ab8..d3cbc3db2a 100644
--- a/src/base/str.test.cc
+++ b/src/base/str.test.cc
@@ -285,6 +285,58 @@ TEST(StrTest, ToNumberUnsigned8BitIntNegative)
     EXPECT_FALSE(to_number(input, output));
 }
 
+/** Test that a double that can be converted to int is always rounded down. */
+TEST(StrTest, ToNumberUnsigned8BitIntRoundDown)
+{
+    uint8_t output;
+    std::string input_1 = "2.99";
+    EXPECT_TRUE(to_number(input_1, output));
+    EXPECT_EQ(2, output);
+
+    std::string input_2 = "3.99";
+    EXPECT_TRUE(to_number(input_2, output));
+    EXPECT_EQ(3, output);
+}
+
+/**
+ * Test that a double can still be converted to int as long as it is below
+ * the numerical limit + 1.
+ */
+TEST(StrTest, ToNumber8BitUnsignedLimit)
+{
+    uint8_t output;
+    std::string input = "255.99";
+    EXPECT_TRUE(to_number(input, output));
+    EXPECT_EQ(255, output);
+}
+
+/**
+ * Test that a double cannot be converted to int when it passes the numerical
+ * limit.
+ */
+TEST(StrTest, ToNumber8BitUnsignedOutOfRange)
+{
+    uint8_t output;
+    std::string input = "256.99";
+    EXPECT_FALSE(to_number(input, output));
+}
+
+/** Test that a scientific number cannot be converted to int. */
+TEST(StrTest, ToNumberUnsignedScientific)
+{
+    uint32_t output;
+    std::string input = "8.234e+08";
+    EXPECT_FALSE(to_number(input, output));
+}
+
+/** Test that a negative scientific number cannot be converted to int. */
+TEST(StrTest, ToNumberIntScientificNegative)
+{
+    int32_t output;
+    std::string input = "-8.234e+08";
+    EXPECT_FALSE(to_number(input, output));
+}
+
 TEST(StrTest, ToNumber64BitInt)
 {
     int64_t output;
@@ -379,6 +431,16 @@ TEST(StrTest, ToNumberDoubleNegative)
     EXPECT_EQ(expected_output, output);
 }
 
+/** Test that a scientific number is converted properly to double. */
+TEST(StrTest, ToNumberScientific)
+{
+    double output;
+    std::string input = "8.234e+08";
+    double expected_output = 823400000;
+    EXPECT_TRUE(to_number(input, output));
+    EXPECT_EQ(expected_output, output);
+}
+
 /*
  * The "to_bool" function takes a string, "true" or "false"
  * (case-insenstive), and sets the second argument to the bool equivilent.