FIx UB in APInt::ashr

[jroelofs]

Summary

i64 -1, whose sign bit is the 0th one, can't be left shifted without invoking UB.

[majnemer]

lib/Support/APInt.cpp

1047–1048

Isn't this SignExtend64(Val, BitWidth) ?

[jroelofs]

Use SignExtend64()

lib/Support/APInt.cpp

1047–1048

Why yes, it is.

[majnemer]

LGTM

[jroelofs]

r278280

lib/Support/APInt.cpp

   // Handle a degenerate case
   if (shiftAmt == 0)
     return *this;
 
   // Handle single word shifts with built-in ashr
   if (isSingleWord()) {
     if (shiftAmt == BitWidth)
       return APInt(BitWidth, 0); // undefined
-    else {
-      unsigned SignBit = APINT_BITS_PER_WORD - BitWidth;
-      return APInt(BitWidth,
-        (((int64_t(VAL) << SignBit) >> SignBit) >> shiftAmt));
-    }
+    return APInt(BitWidth, SignExtend64(VAL, BitWidth) >> shiftAmt);
   }
 
   // If all the bits were shifted out, the result is, technically, undefined.
   // We return -1 if it was negative, 0 otherwise. We check this early to avoid
   // issues in the algorithm below.
   if (shiftAmt == BitWidth) {
     if (isNegative())
       return APInt(BitWidth, -1ULL, true);
     else
       return APInt(BitWidth, 0);
   }

unittests/ADT/APIntTest.cpp

 // Test that APInt shift left works when bitwidth > 64 and shiftamt == 0
 TEST(APIntTest, ShiftLeftByZero) {
   APInt One = APInt::getNullValue(65) + 1;
   APInt Shl = One.shl(0);
   EXPECT_TRUE(Shl[0]);
   EXPECT_FALSE(Shl[1]);
 }
 
+TEST(APIntTest, i64_ArithmeticRightShiftNegative) {
+  const APInt neg_one(64, static_cast<uint64_t>(-1), true);
+  EXPECT_EQ(neg_one, neg_one.ashr(7));
+}
+
 TEST(APIntTest, i128_NegativeCount) {
   APInt Minus3(128, static_cast<uint64_t>(-3), true);
   EXPECT_EQ(126u, Minus3.countLeadingOnes());
   EXPECT_EQ(-3, Minus3.getSExtValue());
 
   APInt Minus1(128, static_cast<uint64_t>(-1), true);
   EXPECT_EQ(0u, Minus1.countLeadingZeros());
   EXPECT_EQ(128u, Minus1.countLeadingOnes());