[llvm] r249745 - Fix another UBSan test error from r248897 and follow on fix r249689

Teresa Johnson via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 8 13:52:23 PDT 2015


Author: tejohnson
Date: Thu Oct  8 15:52:23 2015
New Revision: 249745

URL: http://llvm.org/viewvc/llvm-project?rev=249745&view=rev
Log:
Fix another UBSan test error from r248897 and follow on fix r249689

While here fix a few more issues with potential overflow and add
new tests for these cases. Ensured that test now passes with UBSan.

Modified:
    llvm/trunk/include/llvm/Support/Endian.h
    llvm/trunk/unittests/Support/EndianTest.cpp

Modified: llvm/trunk/include/llvm/Support/Endian.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/Endian.h?rev=249745&r1=249744&r2=249745&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/Endian.h (original)
+++ llvm/trunk/include/llvm/Support/Endian.h Thu Oct  8 15:52:23 2015
@@ -78,6 +78,9 @@ inline void write(void *memory, value_ty
          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(con
     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 *me
 
     // 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 *me
 
     // 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.

Modified: llvm/trunk/unittests/Support/EndianTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/EndianTest.cpp?rev=249745&r1=249744&r2=249745&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/EndianTest.cpp (original)
+++ llvm/trunk/unittests/Support/EndianTest.cpp Thu Oct  8 15:52:23 2015
@@ -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) {




More information about the llvm-commits mailing list