From d96ee5bedd4ea16dc8c5198a0859ac3d81257872 Mon Sep 17 00:00:00 2001 From: Teresa Johnson Date: Thu, 8 Oct 2015 20:52:23 +0000 Subject: [PATCH] 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. llvm-svn: 249745 --- include/llvm/Support/Endian.h | 28 ++++++----- unittests/Support/EndianTest.cpp | 84 ++++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+), 11 deletions(-) diff --git a/include/llvm/Support/Endian.h b/include/llvm/Support/Endian.h index 7680240deac..b98ff5ede77 100644 --- a/include/llvm/Support/Endian.h +++ b/include/llvm/Support/Endian.h @@ -78,6 +78,9 @@ inline void write(void *memory, value_type value) { sizeof(value_type)); } +template +using make_unsigned_t = typename std::make_unsigned::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 @@ -96,13 +99,15 @@ inline value_type readAtBitAlignment(const void *memory, uint64_t startBit) { val[1] = byte_swap(val[1]); // Shift bits from the lower value into place. - unsigned lowerVal = val[0] >> startBit; + make_unsigned_t 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 numBitsFirstVal = + (sizeof(value_type) * 8) - startBit; + lowerVal &= ((make_unsigned_t)1 << numBitsFirstVal) - 1; // Get the bits from the upper value. - unsigned upperVal = val[1] & ((1 << startBit) - 1); + make_unsigned_t upperVal = + val[1] & (((make_unsigned_t)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)1 << startBit) - 1; + make_unsigned_t numBitsFirstVal = + (sizeof(value_type) * 8) - startBit; + make_unsigned_t 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)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)1 << startBit) - 1); // Next shift the bits that go into the upper value into position. - unsigned upperVal = value >> numBitsFirstVal; + make_unsigned_t upperVal = value >> numBitsFirstVal; // Mask off upper bits after right shift in case of signed type. - upperVal &= (1 << startBit) - 1; + upperVal &= ((make_unsigned_t)1 << startBit) - 1; val[1] |= upperVal; // Finally, rewrite values. diff --git a/unittests/Support/EndianTest.cpp b/unittests/Support/EndianTest.cpp index ee236d0eec9..c2b5572e574 100644 --- a/unittests/Support/EndianTest.cpp +++ b/unittests/Support/EndianTest.cpp @@ -50,6 +50,23 @@ TEST(Endian, ReadBitAligned) { 0x0f000000); EXPECT_EQ((endian::readAtBitAlignment(&bigval2[0], 4)), 0x0f000000); + // Test to make sure left shift of start bit doesn't overflow. + EXPECT_EQ( + (endian::readAtBitAlignment(&littleval2[0], 1)), + 0x78000000); + EXPECT_EQ((endian::readAtBitAlignment(&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( + &littleval3[0], 4)), + 0x0f00000000000000); + EXPECT_EQ( + (endian::readAtBitAlignment(&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(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(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( + 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( + 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) {