Fix another UBSan test error from r248897 and follow on fix r249689
authorTeresa Johnson <tejohnson@google.com>
Thu, 8 Oct 2015 20:52:23 +0000 (20:52 +0000)
committerTeresa Johnson <tejohnson@google.com>
Thu, 8 Oct 2015 20:52:23 +0000 (20:52 +0000)
While here fix a few more issues with potential overflow and add
new tests for these cases. Ensured that test now passes with UBSan.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@249745 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/Support/Endian.h
unittests/Support/EndianTest.cpp

index 7680240deac81f8fbde40491d8dcec395e0dcf73..b98ff5ede77398c2bb7777d19fd20033d68d6bf4 100644 (file)
@@ -78,6 +78,9 @@ inline void write(void *memory, value_type value) {
          sizeof(value_type));
 }
 
+template <typename value_type>
+using make_unsigned_t = typename std::make_unsigned<value_type>::type;
+
 /// Read a value of a particular endianness from memory, for a location
 /// that starts at the given bit offset within the first byte.
 template <typename value_type, endianness endian, std::size_t alignment>
@@ -96,13 +99,15 @@ inline value_type readAtBitAlignment(const void *memory, uint64_t startBit) {
     val[1] = byte_swap<value_type, endian>(val[1]);
 
     // Shift bits from the lower value into place.
-    unsigned lowerVal = val[0] >> startBit;
+    make_unsigned_t<value_type> lowerVal = val[0] >> startBit;
     // Mask off upper bits after right shift in case of signed type.
-    unsigned numBitsFirstVal = (sizeof(value_type) * 8) - startBit;
-    lowerVal &= (1 << numBitsFirstVal) - 1;
+    make_unsigned_t<value_type> numBitsFirstVal =
+        (sizeof(value_type) * 8) - startBit;
+    lowerVal &= ((make_unsigned_t<value_type>)1 << numBitsFirstVal) - 1;
 
     // Get the bits from the upper value.
-    unsigned upperVal = val[1] & ((1 << startBit) - 1);
+    make_unsigned_t<value_type> upperVal =
+        val[1] & (((make_unsigned_t<value_type>)1 << startBit) - 1);
     // Shift them in to place.
     upperVal <<= numBitsFirstVal;
 
@@ -130,14 +135,15 @@ inline void writeAtBitAlignment(void *memory, value_type value,
 
     // Mask off any existing bits in the upper part of the lower value that
     // we want to replace.
-    val[0] &= (1 << startBit) - 1;
-    unsigned numBitsFirstVal = (sizeof(value_type) * 8) - startBit;
-    unsigned lowerVal = value;
+    val[0] &= ((make_unsigned_t<value_type>)1 << startBit) - 1;
+    make_unsigned_t<value_type> numBitsFirstVal =
+        (sizeof(value_type) * 8) - startBit;
+    make_unsigned_t<value_type> lowerVal = value;
     if (startBit > 0) {
       // Mask off the upper bits in the new value that are not going to go into
       // the lower value. This avoids a left shift of a negative value, which
       // is undefined behavior.
-      lowerVal &= ((1 << numBitsFirstVal) - 1);
+      lowerVal &= (((make_unsigned_t<value_type>)1 << numBitsFirstVal) - 1);
       // Now shift the new bits into place
       lowerVal <<= startBit;
     }
@@ -145,11 +151,11 @@ inline void writeAtBitAlignment(void *memory, value_type value,
 
     // Mask off any existing bits in the lower part of the upper value that
     // we want to replace.
-    val[1] &= ~((1 << startBit) - 1);
+    val[1] &= ~(((make_unsigned_t<value_type>)1 << startBit) - 1);
     // Next shift the bits that go into the upper value into position.
-    unsigned upperVal = value >> numBitsFirstVal;
+    make_unsigned_t<value_type> upperVal = value >> numBitsFirstVal;
     // Mask off upper bits after right shift in case of signed type.
-    upperVal &= (1 << startBit) - 1;
+    upperVal &= ((make_unsigned_t<value_type>)1 << startBit) - 1;
     val[1] |= upperVal;
 
     // Finally, rewrite values.
index ee236d0eec9dff4445dd259bd05702dc1dcea762..c2b5572e574e368085a82b07d3a0605791c1753a 100644 (file)
@@ -50,6 +50,23 @@ TEST(Endian, ReadBitAligned) {
       0x0f000000);
   EXPECT_EQ((endian::readAtBitAlignment<int, big, unaligned>(&bigval2[0], 4)),
             0x0f000000);
+  // Test to make sure left shift of start bit doesn't overflow.
+  EXPECT_EQ(
+      (endian::readAtBitAlignment<int, little, unaligned>(&littleval2[0], 1)),
+      0x78000000);
+  EXPECT_EQ((endian::readAtBitAlignment<int, big, unaligned>(&bigval2[0], 1)),
+            0x78000000);
+  // Test to make sure 64-bit int doesn't overflow.
+  unsigned char littleval3[] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xf0,
+                                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+  unsigned char bigval3[] = {0xf0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                             0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+  EXPECT_EQ((endian::readAtBitAlignment<int64_t, little, unaligned>(
+                &littleval3[0], 4)),
+            0x0f00000000000000);
+  EXPECT_EQ(
+      (endian::readAtBitAlignment<int64_t, big, unaligned>(&bigval3[0], 4)),
+      0x0f00000000000000);
 }
 
 TEST(Endian, WriteBitAligned) {
@@ -78,6 +95,73 @@ TEST(Endian, WriteBitAligned) {
   EXPECT_EQ(littleval[5], 0x00);
   EXPECT_EQ(littleval[6], 0x00);
   EXPECT_EQ(littleval[7], 0x00);
+
+  // This test makes sure 1<<31 doesn't overflow.
+  // Test to make sure left shift of start bit doesn't overflow.
+  unsigned char bigval2[8] = {0x00};
+  endian::writeAtBitAlignment<int32_t, big, unaligned>(bigval2, (int)0xffffffff,
+                                                       1);
+  EXPECT_EQ(bigval2[0], 0xff);
+  EXPECT_EQ(bigval2[1], 0xff);
+  EXPECT_EQ(bigval2[2], 0xff);
+  EXPECT_EQ(bigval2[3], 0xfe);
+  EXPECT_EQ(bigval2[4], 0x00);
+  EXPECT_EQ(bigval2[5], 0x00);
+  EXPECT_EQ(bigval2[6], 0x00);
+  EXPECT_EQ(bigval2[7], 0x01);
+
+  unsigned char littleval2[8] = {0x00};
+  endian::writeAtBitAlignment<int32_t, little, unaligned>(littleval2,
+                                                          (int)0xffffffff, 1);
+  EXPECT_EQ(littleval2[0], 0xfe);
+  EXPECT_EQ(littleval2[1], 0xff);
+  EXPECT_EQ(littleval2[2], 0xff);
+  EXPECT_EQ(littleval2[3], 0xff);
+  EXPECT_EQ(littleval2[4], 0x01);
+  EXPECT_EQ(littleval2[5], 0x00);
+  EXPECT_EQ(littleval2[6], 0x00);
+  EXPECT_EQ(littleval2[7], 0x00);
+
+  // Test to make sure 64-bit int doesn't overflow.
+  unsigned char bigval64[16] = {0x00};
+  endian::writeAtBitAlignment<int64_t, big, unaligned>(
+      bigval64, (int64_t)0xffffffffffffffff, 1);
+  EXPECT_EQ(bigval64[0], 0xff);
+  EXPECT_EQ(bigval64[1], 0xff);
+  EXPECT_EQ(bigval64[2], 0xff);
+  EXPECT_EQ(bigval64[3], 0xff);
+  EXPECT_EQ(bigval64[4], 0xff);
+  EXPECT_EQ(bigval64[5], 0xff);
+  EXPECT_EQ(bigval64[6], 0xff);
+  EXPECT_EQ(bigval64[7], 0xfe);
+  EXPECT_EQ(bigval64[8], 0x00);
+  EXPECT_EQ(bigval64[9], 0x00);
+  EXPECT_EQ(bigval64[10], 0x00);
+  EXPECT_EQ(bigval64[11], 0x00);
+  EXPECT_EQ(bigval64[12], 0x00);
+  EXPECT_EQ(bigval64[13], 0x00);
+  EXPECT_EQ(bigval64[14], 0x00);
+  EXPECT_EQ(bigval64[15], 0x01);
+
+  unsigned char littleval64[16] = {0x00};
+  endian::writeAtBitAlignment<int64_t, little, unaligned>(
+      littleval64, (int64_t)0xffffffffffffffff, 1);
+  EXPECT_EQ(littleval64[0], 0xfe);
+  EXPECT_EQ(littleval64[1], 0xff);
+  EXPECT_EQ(littleval64[2], 0xff);
+  EXPECT_EQ(littleval64[3], 0xff);
+  EXPECT_EQ(littleval64[4], 0xff);
+  EXPECT_EQ(littleval64[5], 0xff);
+  EXPECT_EQ(littleval64[6], 0xff);
+  EXPECT_EQ(littleval64[7], 0xff);
+  EXPECT_EQ(littleval64[8], 0x01);
+  EXPECT_EQ(littleval64[9], 0x00);
+  EXPECT_EQ(littleval64[10], 0x00);
+  EXPECT_EQ(littleval64[11], 0x00);
+  EXPECT_EQ(littleval64[12], 0x00);
+  EXPECT_EQ(littleval64[13], 0x00);
+  EXPECT_EQ(littleval64[14], 0x00);
+  EXPECT_EQ(littleval64[15], 0x00);
 }
 
 TEST(Endian, Write) {