Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libc] Fix improper initialization of StorageType #75610

Merged
merged 1 commit into from
Dec 15, 2023

Conversation

gchatelet
Copy link
Contributor

StorageType may be a BigInt under the hood. Initializing it with -1 does not yields the maximum value.

`StorageType` may be a `BigInt` under the hood. Initializing it with `-1` does not yields the maximum value.
@gchatelet gchatelet requested a review from lntue December 15, 2023 14:03
@llvmbot llvmbot added the libc label Dec 15, 2023
@llvmbot
Copy link

llvmbot commented Dec 15, 2023

@llvm/pr-subscribers-libc

Author: Guillaume Chatelet (gchatelet)

Changes

StorageType may be a BigInt under the hood. Initializing it with -1 does not yields the maximum value.


Full diff: https://github.com/llvm/llvm-project/pull/75610.diff

23 Files Affected:

  • (modified) libc/test/UnitTest/FPMatcher.h (+5)
  • (modified) libc/test/src/math/CeilTest.h (+1-1)
  • (modified) libc/test/src/math/CopySignTest.h (+1-1)
  • (modified) libc/test/src/math/FAbsTest.h (+1-1)
  • (modified) libc/test/src/math/FDimTest.h (+4-2)
  • (modified) libc/test/src/math/FMaxTest.h (+2-2)
  • (modified) libc/test/src/math/FMinTest.h (+2-2)
  • (modified) libc/test/src/math/FloorTest.h (+1-1)
  • (modified) libc/test/src/math/FrexpTest.h (+1-1)
  • (modified) libc/test/src/math/LogbTest.h (+1-1)
  • (modified) libc/test/src/math/ModfTest.h (+1-1)
  • (modified) libc/test/src/math/RoundTest.h (+1-1)
  • (modified) libc/test/src/math/SqrtTest.h (+1-1)
  • (modified) libc/test/src/math/TruncTest.h (+1-1)
  • (modified) libc/test/src/math/cos_test.cpp (+1-1)
  • (modified) libc/test/src/math/sin_test.cpp (+1-1)
  • (modified) libc/test/src/math/smoke/CopySignTest.h (+1-1)
  • (modified) libc/test/src/math/smoke/FDimTest.h (+4-2)
  • (modified) libc/test/src/math/smoke/FMaxTest.h (+2-2)
  • (modified) libc/test/src/math/smoke/FMinTest.h (+2-2)
  • (modified) libc/test/src/math/smoke/LogbTest.h (+1-1)
  • (modified) libc/test/src/math/smoke/ModfTest.h (+1-1)
  • (modified) libc/test/src/math/tan_test.cpp (+1-1)
diff --git a/libc/test/UnitTest/FPMatcher.h b/libc/test/UnitTest/FPMatcher.h
index a3b776493fe2c4..003de7eb72ac8e 100644
--- a/libc/test/UnitTest/FPMatcher.h
+++ b/libc/test/UnitTest/FPMatcher.h
@@ -9,6 +9,7 @@
 #ifndef LLVM_LIBC_UTILS_UNITTEST_FPMATCHER_H
 #define LLVM_LIBC_UTILS_UNITTEST_FPMATCHER_H
 
+#include "src/__support/CPP/type_traits.h"
 #include "src/__support/FPUtil/FEnvImpl.h"
 #include "src/__support/FPUtil/FPBits.h"
 #include "src/__support/FPUtil/fpbits_str.h"
@@ -62,6 +63,8 @@ template <TestCond C, typename T> FPMatcher<T, C> getMatcher(T expectedValue) {
 template <typename T> struct FPTest : public Test {
   using FPBits = LIBC_NAMESPACE::fputil::FPBits<T>;
   using StorageType = typename FPBits::StorageType;
+  static constexpr StorageType STORAGE_MAX =
+      LIBC_NAMESPACE::cpp::numeric_limits<StorageType>::max();
   static constexpr T zero = FPBits::zero();
   static constexpr T neg_zero = FPBits::neg_zero();
   static constexpr T aNaN = FPBits::build_quiet_nan(1);
@@ -88,6 +91,8 @@ template <typename T> struct FPTest : public Test {
 #define DECLARE_SPECIAL_CONSTANTS(T)                                           \
   using FPBits = LIBC_NAMESPACE::fputil::FPBits<T>;                            \
   using StorageType = typename FPBits::StorageType;                            \
+  static constexpr StorageType STORAGE_MAX =                                   \
+      LIBC_NAMESPACE::cpp::numeric_limits<StorageType>::max();                 \
   const T zero = FPBits::zero();                                               \
   const T neg_zero = FPBits::neg_zero();                                       \
   const T aNaN = FPBits::build_quiet_nan(1);                                   \
diff --git a/libc/test/src/math/CeilTest.h b/libc/test/src/math/CeilTest.h
index c8b10e86479e83..67b33cb14110bc 100644
--- a/libc/test/src/math/CeilTest.h
+++ b/libc/test/src/math/CeilTest.h
@@ -65,7 +65,7 @@ template <typename T> class CeilTest : public LIBC_NAMESPACE::testing::Test {
 
   void testRange(CeilFunc func) {
     constexpr StorageType COUNT = 100'000;
-    constexpr StorageType STEP = StorageType(-1) / COUNT;
+    constexpr StorageType STEP = STORAGE_MAX / COUNT;
     for (StorageType i = 0, v = 0; i <= COUNT; ++i, v += STEP) {
       T x = T(FPBits(v));
       if (isnan(x) || isinf(x))
diff --git a/libc/test/src/math/CopySignTest.h b/libc/test/src/math/CopySignTest.h
index fe0b5855c5be2f..c916416a2e5b91 100644
--- a/libc/test/src/math/CopySignTest.h
+++ b/libc/test/src/math/CopySignTest.h
@@ -35,7 +35,7 @@ class CopySignTest : public LIBC_NAMESPACE::testing::Test {
 
   void testRange(CopySignFunc func) {
     constexpr StorageType COUNT = 100'000;
-    constexpr StorageType STEP = StorageType(-1) / COUNT;
+    constexpr StorageType STEP = STORAGE_MAX / COUNT;
     for (StorageType i = 0, v = 0; i <= COUNT; ++i, v += STEP) {
       T x = T(FPBits(v));
       if (isnan(x) || isinf(x))
diff --git a/libc/test/src/math/FAbsTest.h b/libc/test/src/math/FAbsTest.h
index e4009d2a3df5f3..b2b146167e6252 100644
--- a/libc/test/src/math/FAbsTest.h
+++ b/libc/test/src/math/FAbsTest.h
@@ -33,7 +33,7 @@ template <typename T> class FAbsTest : public LIBC_NAMESPACE::testing::Test {
 
   void testRange(FabsFunc func) {
     constexpr StorageType COUNT = 100'000;
-    constexpr StorageType STEP = StorageType(-1) / COUNT;
+    constexpr StorageType STEP = STORAGE_MAX / COUNT;
     for (StorageType i = 0, v = 0; i <= COUNT; ++i, v += STEP) {
       T x = T(FPBits(v));
       if (isnan(x) || isinf(x))
diff --git a/libc/test/src/math/FDimTest.h b/libc/test/src/math/FDimTest.h
index 5bca63604b0f55..e926640abc90e0 100644
--- a/libc/test/src/math/FDimTest.h
+++ b/libc/test/src/math/FDimTest.h
@@ -53,9 +53,11 @@ class FDimTestTemplate : public LIBC_NAMESPACE::testing::Test {
   }
 
   void test_in_range(FuncPtr func) {
+    constexpr StorageType STORAGE_MAX =
+        LIBC_NAMESPACE::cpp::numeric_limits<StorageType>::max();
     constexpr StorageType COUNT = 100'001;
-    constexpr StorageType STEP = StorageType(-1) / COUNT;
-    for (StorageType i = 0, v = 0, w = StorageType(-1); i <= COUNT;
+    constexpr StorageType STEP = STORAGE_MAX / COUNT;
+    for (StorageType i = 0, v = 0, w = STORAGE_MAX; i <= COUNT;
          ++i, v += STEP, w -= STEP) {
       T x = T(FPBits(v)), y = T(FPBits(w));
       if (isnan(x) || isinf(x))
diff --git a/libc/test/src/math/FMaxTest.h b/libc/test/src/math/FMaxTest.h
index c62d15c53d1e94..1a1d2a77268d1c 100644
--- a/libc/test/src/math/FMaxTest.h
+++ b/libc/test/src/math/FMaxTest.h
@@ -56,8 +56,8 @@ template <typename T> class FMaxTest : public LIBC_NAMESPACE::testing::Test {
 
   void testRange(FMaxFunc func) {
     constexpr StorageType COUNT = 100'001;
-    constexpr StorageType STEP = StorageType(-1) / COUNT;
-    for (StorageType i = 0, v = 0, w = StorageType(-1); i <= COUNT;
+    constexpr StorageType STEP = STORAGE_MAX / COUNT;
+    for (StorageType i = 0, v = 0, w = STORAGE_MAX; i <= COUNT;
          ++i, v += STEP, w -= STEP) {
       T x = T(FPBits(v)), y = T(FPBits(w));
       if (isnan(x) || isinf(x))
diff --git a/libc/test/src/math/FMinTest.h b/libc/test/src/math/FMinTest.h
index ac37d14825c620..742bb5cdd1021b 100644
--- a/libc/test/src/math/FMinTest.h
+++ b/libc/test/src/math/FMinTest.h
@@ -56,8 +56,8 @@ template <typename T> class FMinTest : public LIBC_NAMESPACE::testing::Test {
 
   void testRange(FMinFunc func) {
     constexpr StorageType COUNT = 100'001;
-    constexpr StorageType STEP = StorageType(-1) / COUNT;
-    for (StorageType i = 0, v = 0, w = StorageType(-1); i <= COUNT;
+    constexpr StorageType STEP = STORAGE_MAX / COUNT;
+    for (StorageType i = 0, v = 0, w = STORAGE_MAX; i <= COUNT;
          ++i, v += STEP, w -= STEP) {
       T x = T(FPBits(v)), y = T(FPBits(w));
       if (isnan(x) || isinf(x))
diff --git a/libc/test/src/math/FloorTest.h b/libc/test/src/math/FloorTest.h
index 95fc7313f843ef..c1b05c5d130834 100644
--- a/libc/test/src/math/FloorTest.h
+++ b/libc/test/src/math/FloorTest.h
@@ -65,7 +65,7 @@ template <typename T> class FloorTest : public LIBC_NAMESPACE::testing::Test {
 
   void testRange(FloorFunc func) {
     constexpr StorageType COUNT = 100'000;
-    constexpr StorageType STEP = StorageType(-1) / COUNT;
+    constexpr StorageType STEP = STORAGE_MAX / COUNT;
     for (StorageType i = 0, v = 0; i <= COUNT; ++i, v += STEP) {
       T x = T(FPBits(v));
       if (isnan(x) || isinf(x))
diff --git a/libc/test/src/math/FrexpTest.h b/libc/test/src/math/FrexpTest.h
index 8fbb4bf1f0b638..0e66bd5804d58c 100644
--- a/libc/test/src/math/FrexpTest.h
+++ b/libc/test/src/math/FrexpTest.h
@@ -95,7 +95,7 @@ template <typename T> class FrexpTest : public LIBC_NAMESPACE::testing::Test {
   void testRange(FrexpFunc func) {
     using StorageType = typename FPBits::StorageType;
     constexpr StorageType COUNT = 100'000;
-    constexpr StorageType STEP = StorageType(-1) / COUNT;
+    constexpr StorageType STEP = STORAGE_MAX / COUNT;
     for (StorageType i = 0, v = 0; i <= COUNT; ++i, v += STEP) {
       T x = static_cast<T>(FPBits(v));
       if (isnan(x) || isinf(x) || x == 0.0l)
diff --git a/libc/test/src/math/LogbTest.h b/libc/test/src/math/LogbTest.h
index be330e861efb24..2049c8ffe950ee 100644
--- a/libc/test/src/math/LogbTest.h
+++ b/libc/test/src/math/LogbTest.h
@@ -74,7 +74,7 @@ template <typename T> class LogbTest : public LIBC_NAMESPACE::testing::Test {
   void testRange(LogbFunc func) {
     using StorageType = typename FPBits::StorageType;
     constexpr StorageType COUNT = 100'000;
-    constexpr StorageType STEP = StorageType(-1) / COUNT;
+    constexpr StorageType STEP = STORAGE_MAX / COUNT;
     for (StorageType i = 0, v = 0; i <= COUNT; ++i, v += STEP) {
       T x = static_cast<T>(FPBits(v));
       if (isnan(x) || isinf(x) || x == 0.0l)
diff --git a/libc/test/src/math/ModfTest.h b/libc/test/src/math/ModfTest.h
index f9dbeb99b7dbd2..f5196a74fa7425 100644
--- a/libc/test/src/math/ModfTest.h
+++ b/libc/test/src/math/ModfTest.h
@@ -85,7 +85,7 @@ template <typename T> class ModfTest : public LIBC_NAMESPACE::testing::Test {
 
   void testRange(ModfFunc func) {
     constexpr StorageType COUNT = 100'000;
-    constexpr StorageType STEP = StorageType(-1) / COUNT;
+    constexpr StorageType STEP = STORAGE_MAX / COUNT;
     for (StorageType i = 0, v = 0; i <= COUNT; ++i, v += STEP) {
       T x = T(FPBits(v));
       if (isnan(x) || isinf(x) || x == T(0.0))
diff --git a/libc/test/src/math/RoundTest.h b/libc/test/src/math/RoundTest.h
index a0e7085ffc9eaa..3ec46159c65d95 100644
--- a/libc/test/src/math/RoundTest.h
+++ b/libc/test/src/math/RoundTest.h
@@ -65,7 +65,7 @@ template <typename T> class RoundTest : public LIBC_NAMESPACE::testing::Test {
 
   void testRange(RoundFunc func) {
     constexpr StorageType COUNT = 100'000;
-    constexpr StorageType STEP = StorageType(-1) / COUNT;
+    constexpr StorageType STEP = STORAGE_MAX / COUNT;
     for (StorageType i = 0, v = 0; i <= COUNT; ++i, v += STEP) {
       T x = T(FPBits(v));
       if (isnan(x) || isinf(x))
diff --git a/libc/test/src/math/SqrtTest.h b/libc/test/src/math/SqrtTest.h
index e5a4e5fc562b3f..2cfe401c5542a3 100644
--- a/libc/test/src/math/SqrtTest.h
+++ b/libc/test/src/math/SqrtTest.h
@@ -56,7 +56,7 @@ template <typename T> class SqrtTest : public LIBC_NAMESPACE::testing::Test {
 
   void test_normal_range(SqrtFunc func) {
     constexpr StorageType COUNT = 200'001;
-    constexpr StorageType STEP = StorageType(-1) / COUNT;
+    constexpr StorageType STEP = STORAGE_MAX / COUNT;
     for (StorageType i = 0, v = 0; i <= COUNT; ++i, v += STEP) {
       T x = LIBC_NAMESPACE::cpp::bit_cast<T>(v);
       if (isnan(x) || (x < 0)) {
diff --git a/libc/test/src/math/TruncTest.h b/libc/test/src/math/TruncTest.h
index c353ac0e914739..2be40790258ef7 100644
--- a/libc/test/src/math/TruncTest.h
+++ b/libc/test/src/math/TruncTest.h
@@ -65,7 +65,7 @@ template <typename T> class TruncTest : public LIBC_NAMESPACE::testing::Test {
 
   void testRange(TruncFunc func) {
     constexpr StorageType COUNT = 100'000;
-    constexpr StorageType STEP = StorageType(-1) / COUNT;
+    constexpr StorageType STEP = STORAGE_MAX / COUNT;
     for (StorageType i = 0, v = 0; i <= COUNT; ++i, v += STEP) {
       T x = T(FPBits(v));
       if (isnan(x) || isinf(x))
diff --git a/libc/test/src/math/cos_test.cpp b/libc/test/src/math/cos_test.cpp
index add4e2b39f3ca5..a4c332bc7fb52b 100644
--- a/libc/test/src/math/cos_test.cpp
+++ b/libc/test/src/math/cos_test.cpp
@@ -20,7 +20,7 @@ namespace mpfr = LIBC_NAMESPACE::testing::mpfr;
 TEST_F(LlvmLibcCosTest, Range) {
   static constexpr double _2pi = 6.283185307179586;
   constexpr StorageType COUNT = 100'000;
-  constexpr StorageType STEP = StorageType(-1) / COUNT;
+  constexpr StorageType STEP = STORAGE_MAX / COUNT;
   for (StorageType i = 0, v = 0; i <= COUNT; ++i, v += STEP) {
     double x = double(FPBits(v));
     // TODO: Expand the range of testing after range reduction is implemented.
diff --git a/libc/test/src/math/sin_test.cpp b/libc/test/src/math/sin_test.cpp
index 8bd8f52c90ee02..cd9eeaff598fea 100644
--- a/libc/test/src/math/sin_test.cpp
+++ b/libc/test/src/math/sin_test.cpp
@@ -21,7 +21,7 @@ namespace mpfr = LIBC_NAMESPACE::testing::mpfr;
 TEST_F(LlvmLibcSinTest, Range) {
   static constexpr double _2pi = 6.283185307179586;
   constexpr StorageType COUNT = 100'000;
-  constexpr StorageType STEP = StorageType(-1) / COUNT;
+  constexpr StorageType STEP = STORAGE_MAX / COUNT;
   for (StorageType i = 0, v = 0; i <= COUNT; ++i, v += STEP) {
     double x = double(FPBits(v));
     // TODO: Expand the range of testing after range reduction is implemented.
diff --git a/libc/test/src/math/smoke/CopySignTest.h b/libc/test/src/math/smoke/CopySignTest.h
index cb82d59783ac1b..1108a45ae673e5 100644
--- a/libc/test/src/math/smoke/CopySignTest.h
+++ b/libc/test/src/math/smoke/CopySignTest.h
@@ -32,7 +32,7 @@ class CopySignTest : public LIBC_NAMESPACE::testing::Test {
 
   void testRange(CopySignFunc func) {
     constexpr StorageType COUNT = 100'000;
-    constexpr StorageType STEP = StorageType(-1) / COUNT;
+    constexpr StorageType STEP = STORAGE_MAX / COUNT;
     for (StorageType i = 0, v = 0; i <= COUNT; ++i, v += STEP) {
       FPBits x_bits = FPBits(v);
       T x = T(v);
diff --git a/libc/test/src/math/smoke/FDimTest.h b/libc/test/src/math/smoke/FDimTest.h
index 5bca63604b0f55..e926640abc90e0 100644
--- a/libc/test/src/math/smoke/FDimTest.h
+++ b/libc/test/src/math/smoke/FDimTest.h
@@ -53,9 +53,11 @@ class FDimTestTemplate : public LIBC_NAMESPACE::testing::Test {
   }
 
   void test_in_range(FuncPtr func) {
+    constexpr StorageType STORAGE_MAX =
+        LIBC_NAMESPACE::cpp::numeric_limits<StorageType>::max();
     constexpr StorageType COUNT = 100'001;
-    constexpr StorageType STEP = StorageType(-1) / COUNT;
-    for (StorageType i = 0, v = 0, w = StorageType(-1); i <= COUNT;
+    constexpr StorageType STEP = STORAGE_MAX / COUNT;
+    for (StorageType i = 0, v = 0, w = STORAGE_MAX; i <= COUNT;
          ++i, v += STEP, w -= STEP) {
       T x = T(FPBits(v)), y = T(FPBits(w));
       if (isnan(x) || isinf(x))
diff --git a/libc/test/src/math/smoke/FMaxTest.h b/libc/test/src/math/smoke/FMaxTest.h
index 8de21a7ba2585b..372d9c5571d319 100644
--- a/libc/test/src/math/smoke/FMaxTest.h
+++ b/libc/test/src/math/smoke/FMaxTest.h
@@ -53,8 +53,8 @@ template <typename T> class FMaxTest : public LIBC_NAMESPACE::testing::Test {
 
   void testRange(FMaxFunc func) {
     constexpr StorageType COUNT = 100'001;
-    constexpr StorageType STEP = StorageType(-1) / COUNT;
-    for (StorageType i = 0, v = 0, w = StorageType(-1); i <= COUNT;
+    constexpr StorageType STEP = STORAGE_MAX / COUNT;
+    for (StorageType i = 0, v = 0, w = STORAGE_MAX; i <= COUNT;
          ++i, v += STEP, w -= STEP) {
       T x = T(FPBits(v)), y = T(FPBits(w));
       if (isnan(x) || isinf(x))
diff --git a/libc/test/src/math/smoke/FMinTest.h b/libc/test/src/math/smoke/FMinTest.h
index a6e3c78d85caff..a51f30803ba82a 100644
--- a/libc/test/src/math/smoke/FMinTest.h
+++ b/libc/test/src/math/smoke/FMinTest.h
@@ -53,8 +53,8 @@ template <typename T> class FMinTest : public LIBC_NAMESPACE::testing::Test {
 
   void testRange(FMinFunc func) {
     constexpr StorageType COUNT = 100'001;
-    constexpr StorageType STEP = StorageType(-1) / COUNT;
-    for (StorageType i = 0, v = 0, w = StorageType(-1); i <= COUNT;
+    constexpr StorageType STEP = STORAGE_MAX / COUNT;
+    for (StorageType i = 0, v = 0, w = STORAGE_MAX; i <= COUNT;
          ++i, v += STEP, w -= STEP) {
       T x = T(FPBits(v)), y = T(FPBits(w));
       if (isnan(x) || isinf(x))
diff --git a/libc/test/src/math/smoke/LogbTest.h b/libc/test/src/math/smoke/LogbTest.h
index 1359bf8c81313e..a0a01a885c1041 100644
--- a/libc/test/src/math/smoke/LogbTest.h
+++ b/libc/test/src/math/smoke/LogbTest.h
@@ -71,7 +71,7 @@ template <typename T> class LogbTest : public LIBC_NAMESPACE::testing::Test {
   void testRange(LogbFunc func) {
     using StorageType = typename FPBits::StorageType;
     constexpr StorageType COUNT = 100'000;
-    constexpr StorageType STEP = StorageType(-1) / COUNT;
+    constexpr StorageType STEP = STORAGE_MAX / COUNT;
     for (StorageType i = 0, v = 0; i <= COUNT; ++i, v += STEP) {
       T x = static_cast<T>(FPBits(v));
       if (isnan(x) || isinf(x) || x == 0.0l)
diff --git a/libc/test/src/math/smoke/ModfTest.h b/libc/test/src/math/smoke/ModfTest.h
index b0a2aed9df57d2..ea7e36b67d11fa 100644
--- a/libc/test/src/math/smoke/ModfTest.h
+++ b/libc/test/src/math/smoke/ModfTest.h
@@ -82,7 +82,7 @@ template <typename T> class ModfTest : public LIBC_NAMESPACE::testing::Test {
 
   void testRange(ModfFunc func) {
     constexpr StorageType COUNT = 100'000;
-    constexpr StorageType STEP = StorageType(-1) / COUNT;
+    constexpr StorageType STEP = STORAGE_MAX / COUNT;
     for (StorageType i = 0, v = 0; i <= COUNT; ++i, v += STEP) {
       T x = T(FPBits(v));
       if (isnan(x) || isinf(x) || x == T(0.0))
diff --git a/libc/test/src/math/tan_test.cpp b/libc/test/src/math/tan_test.cpp
index ab88e8eb7199dd..8fae425f96f8f2 100644
--- a/libc/test/src/math/tan_test.cpp
+++ b/libc/test/src/math/tan_test.cpp
@@ -20,7 +20,7 @@ namespace mpfr = LIBC_NAMESPACE::testing::mpfr;
 TEST_F(LlvmLibcTanTest, Range) {
   static constexpr double _2pi = 6.283185307179586;
   constexpr StorageType COUNT = 100'000;
-  constexpr StorageType STEP = StorageType(-1) / COUNT;
+  constexpr StorageType STEP = STORAGE_MAX / COUNT;
   for (StorageType i = 0, v = 0; i <= COUNT; ++i, v += STEP) {
     double x = double(FPBits(v));
     // TODO: Expand the range of testing after range reduction is implemented.

@gchatelet gchatelet merged commit 57fcc23 into llvm:main Dec 15, 2023
5 checks passed
@gchatelet gchatelet deleted the fix_improper_init_of_storage_type branch December 15, 2023 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants