Fix error checking in AtofPrecise. Add more test cases.

microsoft · Apr 26, 2021 · 4a658e0 · 4a658e0
1 parent 9c30899
commit 4a658e0
Show file tree

Hide file tree

Showing 2 changed files with 63 additions and 0 deletions.
diff --git a/include/LightGBM/utils/common.h b/include/LightGBM/utils/common.h
@@ -341,6 +341,7 @@ inline static const char* AtofPrecise(const char* p, double* out) {
 
   // Rare path: Not in RFC 7159 format. Possible "inf", "nan", etc. Fallback to standard library:
   char* end2;
+  errno = 0;  // This is Required before calling strtod.
   *out = std::strtod(p, &end2);  // strtod is locale aware.
   if (end2 == p) {
     Log::Fatal("no conversion to double for: %s", p);

diff --git a/tests/cpp_tests/test_common.cpp b/tests/cpp_tests/test_common.cpp
@@ -9,6 +9,10 @@
 #include "../include/LightGBM/utils/common.h"
 
 
+// This is a basic test for floating number parsing.
+// Most of the test cases come from:
+// https://github.com/dmlc/xgboost/blob/master/tests/cpp/common/test_charconv.cc
+// https://github.com/Alexhuszagh/rust-lexical/blob/master/data/test-parse-unittests/strtod_tests.toml
 class AtofPreciseTest : public testing::Test {
  public:
   struct AtofTestCase {
@@ -27,6 +31,15 @@ class AtofPreciseTest : public testing::Test {
     }
     return got;
   }
+
+  static double Int64Bits2Double(uint64_t v) {
+    union {
+      uint64_t i;
+      double d;
+    } conv;
+    conv.i = v;
+    return conv.d;
+  }
 };
 
 TEST_F(AtofPreciseTest, Basic) {
@@ -53,6 +66,55 @@ TEST_F(AtofPreciseTest, Basic) {
   }
 }
 
+TEST_F(AtofPreciseTest, CornerCases) {
+  AtofTestCase test_cases[] = {
+      { "1e-400", 0.0 },
+      { "2.4703282292062326e-324", 0.0 },
+      { "4.9406564584124654e-324", Int64Bits2Double(0x0000000000000001LU) },
+      { "8.44291197326099e-309", Int64Bits2Double(0x0006123400000001LU) },
+      // FLT_MAX
+      { "3.40282346638528859811704183484516925440e38",
+        static_cast<double>(std::numeric_limits<float>::max()) },
+      // FLT_MIN
+      { "1.1754943508222875079687365372222456778186655567720875215087517062784172594547271728515625e-38",
+        static_cast<double>(std::numeric_limits<float>::min()) },
+      // DBL_MAX (1 + (1 - 2^-52)) * 2^1023 = (2^53 - 1) * 2^971
+      { "17976931348623157081452742373170435679807056752584499659891747680315"
+        "72607800285387605895586327668781715404589535143824642343213268894641"
+        "82768467546703537516986049910576551282076245490090389328944075868508"
+        "45513394230458323690322294816580855933212334827479782620414472316873"
+        "8177180919299881250404026184124858368", std::numeric_limits<double>::max() },
+      { "1.7976931348623158e+308", std::numeric_limits<double>::max() },
+      // Add 1 got inf.
+      { "1.7976931348623159e+308", std::numeric_limits<double>::infinity() },
+      // 2^971 * (2^53 - 1 + 1/2) : the smallest number resolving to inf
+      {"179769313486231580793728971405303415079934132710037826936173778980444"
+       "968292764750946649017977587207096330286416692887910946555547851940402"
+       "630657488671505820681908902000708383676273854845817711531764475730270"
+       "069855571366959622842914819860834936475292719074168444365510704342711"
+       "559699508093042880177904174497792", std::numeric_limits<double>::infinity() },
+      // Near DBL_MIN
+      { "2.2250738585072009e-308", Int64Bits2Double(0x000fffffffffffffLU) },
+      // DBL_MIN 2^-1022
+      { "2.2250738585072012e-308", std::numeric_limits<double>::min() },
+      { "2.2250738585072014e-308", std::numeric_limits<double>::min() },
+  };
+
+  for (auto const& test : test_cases) {
+    TestAtofPrecise(test.data, test.expected);
+  }
+}
+
+TEST_F(AtofPreciseTest, UnderOverFlow) {
+  double got = 0;
+  ASSERT_THROW(LightGBM::Common::AtofPrecise("1e+400", &got),  std::runtime_error);
+}
+
+TEST_F(AtofPreciseTest, ErrorInput) {
+  double got = 0;
+  ASSERT_THROW(LightGBM::Common::AtofPrecise("x1", &got),  std::runtime_error);
+}
+
 TEST_F(AtofPreciseTest, NaN) {
   AtofTestCase test_cases[] = {
       { "nan", std::numeric_limits<double>::quiet_NaN() },