[libc-commits] [libc] 360b41c - [libc] Fix undefined behavior in UInt<>::shift_right.

[Tue Ly via libc-commits]

Author: Tue Ly
Date: 2022-12-07T18:38:08-05:00
New Revision: 360b41c7ba7f09e375e27b674ba44e96f3d1f055

URL: https://github.com/llvm/llvm-project/commit/360b41c7ba7f09e375e27b674ba44e96f3d1f055
DIFF: https://github.com/llvm/llvm-project/commit/360b41c7ba7f09e375e27b674ba44e96f3d1f055.diff

LOG: [libc] Fix undefined behavior in UInt<>::shift_right.

Fix undefined behavior of left-shifting uint64_t by 64 in
`UInt<>::shift_right` implementation.

Reviewed By: michaelrj, sivachandra

Differential Revision: https://reviews.llvm.org/D139566

Added: 
    

Modified: 
    libc/src/__support/UInt.h
    libc/test/src/__support/uint_test.cpp

Removed: 
    


################################################################################
diff  --git a/libc/src/__support/UInt.h b/libc/src/__support/UInt.h
index 28bd86a93d160..6068ab6decfd9 100644
--- a/libc/src/__support/UInt.h
+++ b/libc/src/__support/UInt.h
@@ -386,18 +386,39 @@ template <size_t Bits> struct UInt {
   }
 
   constexpr void shift_right(size_t s) {
+#ifdef __SIZEOF_INT128__
+    if constexpr (Bits == 128) {
+      // Use builtin 128 bits if available;
+      if (s >= 128) {
+        val[0] = 0;
+        val[1] = 0;
+        return;
+      }
+      __uint128_t tmp = __uint128_t(val[0]) + (__uint128_t(val[1]) << 64);
+      tmp >>= s;
+      val[0] = uint64_t(tmp);
+      val[1] = uint64_t(tmp >> 64);
+      return;
+    }
+#endif // __SIZEOF_INT128__
+
     const size_t drop = s / 64;  // Number of words to drop
     const size_t shift = s % 64; // Bit shift in the remaining words.
 
     size_t i = 0;
 
     if (drop < WordCount) {
-      size_t j = drop;
-      for (; j < WordCount - 1; ++i, ++j) {
-        val[i] = (val[j] >> shift) | (val[j + 1] << (64 - shift));
+      if (shift > 0) {
+        for (size_t j = drop; j < WordCount - 1; ++i, ++j) {
+          val[i] = (val[j] >> shift) | (val[j + 1] << (64 - shift));
+        }
+        val[i] = val[WordCount - 1] >> shift;
+        ++i;
+      } else {
+        for (size_t j = drop; j < WordCount; ++i, ++j) {
+          val[i] = val[j];
+        }
       }
-      val[i] = val[j] >> shift;
-      ++i;
     }
 
     for (; i < WordCount; ++i) {

diff  --git a/libc/test/src/__support/uint_test.cpp b/libc/test/src/__support/uint_test.cpp
index 2da633ecb267b..3ee20470f2821 100644
--- a/libc/test/src/__support/uint_test.cpp
+++ b/libc/test/src/__support/uint_test.cpp
@@ -352,6 +352,17 @@ TEST(LlvmLibcUIntClassTest, ShiftRightTests) {
   LL_UInt128 result6({0, 0});
   EXPECT_EQ((val2 >> 128), result6);
   EXPECT_EQ((val2 >> 256), result6);
+
+  LL_UInt128 v1({0x1111222233334444, 0xaaaabbbbccccdddd});
+  LL_UInt128 r1({0xaaaabbbbccccdddd, 0});
+  EXPECT_EQ((v1 >> 64), r1);
+
+  LL_UInt192 v2({0x1111222233334444, 0x5555666677778888, 0xaaaabbbbccccdddd});
+  LL_UInt192 r2({0x5555666677778888, 0xaaaabbbbccccdddd, 0});
+  LL_UInt192 r3({0xaaaabbbbccccdddd, 0, 0});
+  EXPECT_EQ((v2 >> 64), r2);
+  EXPECT_EQ((v2 >> 128), r3);
+  EXPECT_EQ((r2 >> 64), r3);
 }
 
 TEST(LlvmLibcUIntClassTest, AndTests) {
@@ -502,11 +513,8 @@ TEST(LlvmLibcUIntClassTest, FullMulTests) {
   } while (0)
 
 TEST(LlvmLibcUIntClassTest, QuickMulHiTests) {
-  // TODO(lntue): Investigate / Analyze the error bounds for other rounding
-  // modes.  It the error bounds seems to be able to reach to WordCount instead
-  // of WordCount - 1 in the CI environment.
-  TEST_QUICK_MUL_HI(128, 2);
-  TEST_QUICK_MUL_HI(192, 3);
-  TEST_QUICK_MUL_HI(256, 4);
-  TEST_QUICK_MUL_HI(512, 8);
+  TEST_QUICK_MUL_HI(128, 1);
+  TEST_QUICK_MUL_HI(192, 2);
+  TEST_QUICK_MUL_HI(256, 3);
+  TEST_QUICK_MUL_HI(512, 7);
 }