[llvm] r248897 - Add support for sub-byte aligned writes to lib/Support/Endian.h
Adrian Prantl via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 7 14:52:10 PDT 2015
Hi Teresa,
I’m still working on confirming it, but it looks quite plausible that this commit caused the following UBSan test failure on lab.llvm.org:
http://lab.llvm.org:8080/green/job/clang-stage2-cmake-RgSan_check/477/consoleFull#-1567746458254eaf0-7326-4999-85b0-388101f2d404
FAIL: LLVM-Unit :: Support/SupportTests/Endian.WriteBitAligned (23480 of 23746)
******************** TEST 'LLVM-Unit :: Support/SupportTests/Endian.WriteBitAligned' FAILED ********************
Note: Google Test filter = Endian.WriteBitAligned
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from Endian
[ RUN ] Endian.WriteBitAligned
/Users/buildslave/jenkins/sharedspace/clang-stage2-cmake-RgSan at 2/llvm/include/llvm/Support/Endian.h:135:21: runtime error: left shift of negative value -21846
SUMMARY: AddressSanitizer: undefined-behavior /Users/buildslave/jenkins/sharedspace/clang-stage2-cmake-RgSan at 2/llvm/include/llvm/Support/Endian.h:135:21 in
0 SupportTests 0x00000001009c01c3 llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 339
1 SupportTests 0x00000001009beaa5 llvm::sys::RunSignalHandlers() + 213
2 SupportTests 0x00000001009c143f SignalHandler(int) + 655
3 libsystem_platform.dylib 0x00007fff8b72ef1a _sigtramp + 26
4 libclang_rt.asan_osx_dynamic.dylib 0x0000000101c0678d getTypeCacheHashTableBucket(unsigned long)::__ubsan_vptr_hash_set + 532925
5 libsystem_c.dylib 0x00007fff853459b3 abort + 129
6 libclang_rt.asan_osx_dynamic.dylib 0x0000000100ee5f56 __sanitizer::Abort() + 6
-- adrian
> On Sep 30, 2015, at 6:20 AM, Teresa Johnson via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>
> Author: tejohnson
> Date: Wed Sep 30 08:20:37 2015
> New Revision: 248897
>
> URL: http://llvm.org/viewvc/llvm-project?rev=248897&view=rev
> Log:
> Add support for sub-byte aligned writes to lib/Support/Endian.h
>
> Summary:
> As per Duncan's review for D12536, I extracted the sub-byte bit aligned
> reading and writing code into lib/Support, and generalized it. Added calls from
> BackpatchWord. Also added unittests.
>
> Reviewers: dexonsmith
>
> Subscribers: llvm-commits
>
> Differential Revision: http://reviews.llvm.org/D13189
>
> Modified:
> llvm/trunk/include/llvm/Bitcode/BitstreamWriter.h
> llvm/trunk/include/llvm/Support/Endian.h
> llvm/trunk/unittests/Support/EndianTest.cpp
>
> Modified: llvm/trunk/include/llvm/Bitcode/BitstreamWriter.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Bitcode/BitstreamWriter.h?rev=248897&r1=248896&r2=248897&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Bitcode/BitstreamWriter.h (original)
> +++ llvm/trunk/include/llvm/Bitcode/BitstreamWriter.h Wed Sep 30 08:20:37 2015
> @@ -102,18 +102,13 @@ public:
> /// Backpatch a 32-bit word in the output at the given bit offset
> /// with the specified value.
> void BackpatchWord(uint64_t BitNo, unsigned NewWord) {
> + using namespace llvm::support;
> unsigned ByteNo = BitNo / 8;
> - if ((BitNo & 7) == 0) {
> - // Already 8-bit aligned
> - support::endian::write32le(&Out[ByteNo], NewWord);
> - } else {
> - uint64_t CurDWord = support::endian::read64le(&Out[ByteNo]);
> - unsigned StartBit = BitNo & 7;
> - // Currently expect to backpatch 0-value placeholders.
> - assert(((CurDWord >> StartBit) & 0xffffffff) == 0);
> - CurDWord |= NewWord << StartBit;
> - support::endian::write64le(&Out[ByteNo], CurDWord);
> - }
> + assert((!endian::readAtBitAlignment<uint32_t, little, unaligned>(
> + &Out[ByteNo], BitNo & 7)) &&
> + "Expected to be patching over 0-value placeholders");
> + endian::writeAtBitAlignment<uint32_t, little, unaligned>(
> + &Out[ByteNo], NewWord, BitNo & 7);
> }
>
> void Emit(uint32_t Val, unsigned NumBits) {
>
> Modified: llvm/trunk/include/llvm/Support/Endian.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/Endian.h?rev=248897&r1=248896&r2=248897&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Support/Endian.h (original)
> +++ llvm/trunk/include/llvm/Support/Endian.h Wed Sep 30 08:20:37 2015
> @@ -77,6 +77,81 @@ inline void write(void *memory, value_ty
> &value,
> sizeof(value_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>
> +inline value_type readAtBitAlignment(const void *memory, uint64_t startBit) {
> + assert(startBit < 8);
> + if (startBit == 0)
> + return read<value_type, endian, alignment>(memory);
> + else {
> + // Read two values and compose the result from them.
> + value_type val[2];
> + memcpy(&val[0],
> + LLVM_ASSUME_ALIGNED(
> + memory, (detail::PickAlignment<value_type, alignment>::value)),
> + sizeof(value_type) * 2);
> + val[0] = byte_swap<value_type, endian>(val[0]);
> + val[1] = byte_swap<value_type, endian>(val[1]);
> +
> + // Shift bits from the lower value into place.
> + unsigned 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;
> +
> + // Get the bits from the upper value.
> + unsigned upperVal = val[1] & ((1 << startBit) - 1);
> + // Shift them in to place.
> + upperVal <<= numBitsFirstVal;
> +
> + return lowerVal | upperVal;
> + }
> +}
> +
> +/// Write a value to memory with a particular endianness, for a location
> +/// that starts at the given bit offset within the first byte.
> +template <typename value_type, endianness endian, std::size_t alignment>
> +inline void writeAtBitAlignment(void *memory, value_type value,
> + uint64_t startBit) {
> + assert(startBit < 8);
> + if (startBit == 0)
> + write<value_type, endian, alignment>(memory, value);
> + else {
> + // Read two values and shift the result into them.
> + value_type val[2];
> + memcpy(&val[0],
> + LLVM_ASSUME_ALIGNED(
> + memory, (detail::PickAlignment<value_type, alignment>::value)),
> + sizeof(value_type) * 2);
> + val[0] = byte_swap<value_type, endian>(val[0]);
> + val[1] = byte_swap<value_type, endian>(val[1]);
> +
> + // Mask off any existing bits in the upper part of the lower value that
> + // we want to replace.
> + val[0] &= (1 << startBit) - 1;
> + // Now shift in the new bits
> + val[0] |= value << startBit;
> +
> + // Mask off any existing bits in the lower part of the upper value that
> + // we want to replace.
> + val[1] &= ~((1 << startBit) - 1);
> + // Next shift the bits that go into the upper value into position.
> + unsigned numBitsFirstVal = (sizeof(value_type) * 8) - startBit;
> + unsigned upperVal = value >> numBitsFirstVal;
> + // Mask off upper bits after right shift in case of signed type.
> + upperVal &= (1 << startBit) - 1;
> + val[1] |= upperVal;
> +
> + // Finally, rewrite values.
> + val[0] = byte_swap<value_type, endian>(val[0]);
> + val[1] = byte_swap<value_type, endian>(val[1]);
> + memcpy(LLVM_ASSUME_ALIGNED(
> + memory, (detail::PickAlignment<value_type, alignment>::value)),
> + &val[0], sizeof(value_type) * 2);
> + }
> +}
> } // end namespace endian
>
> namespace detail {
>
> Modified: llvm/trunk/unittests/Support/EndianTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/EndianTest.cpp?rev=248897&r1=248896&r2=248897&view=diff
> ==============================================================================
> --- llvm/trunk/unittests/Support/EndianTest.cpp (original)
> +++ llvm/trunk/unittests/Support/EndianTest.cpp Wed Sep 30 08:20:37 2015
> @@ -32,6 +32,54 @@ TEST(Endian, Read) {
> (endian::read<int32_t, little, unaligned>(littleval + 1)));
> }
>
> +TEST(Endian, ReadBitAligned) {
> + // Simple test to make sure we properly pull out the 0x0 word.
> + unsigned char littleval[] = {0x3f, 0x00, 0x00, 0x00, 0xc0, 0xff, 0xff, 0xff};
> + unsigned char bigval[] = {0x00, 0x00, 0x00, 0x3f, 0xff, 0xff, 0xff, 0xc0};
> + EXPECT_EQ(
> + (endian::readAtBitAlignment<int, little, unaligned>(&littleval[0], 6)),
> + 0x0);
> + EXPECT_EQ((endian::readAtBitAlignment<int, big, unaligned>(&bigval[0], 6)),
> + 0x0);
> + // Test to make sure that signed right shift of 0xf0000000 is masked
> + // properly.
> + unsigned char littleval2[] = {0x00, 0x00, 0x00, 0xf0, 0x00, 0x00, 0x00, 0x00};
> + unsigned char bigval2[] = {0xf0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
> + EXPECT_EQ(
> + (endian::readAtBitAlignment<int, little, unaligned>(&littleval2[0], 4)),
> + 0x0f000000);
> + EXPECT_EQ((endian::readAtBitAlignment<int, big, unaligned>(&bigval2[0], 4)),
> + 0x0f000000);
> +}
> +
> +TEST(Endian, WriteBitAligned) {
> + // This test ensures that signed right shift of 0xffffaa is masked
> + // properly.
> + unsigned char bigval[8] = {0x00};
> + endian::writeAtBitAlignment<int32_t, big, unaligned>(bigval, (int)0xffffaaaa,
> + 4);
> + EXPECT_EQ(bigval[0], 0xff);
> + EXPECT_EQ(bigval[1], 0xfa);
> + EXPECT_EQ(bigval[2], 0xaa);
> + EXPECT_EQ(bigval[3], 0xa0);
> + EXPECT_EQ(bigval[4], 0x00);
> + EXPECT_EQ(bigval[5], 0x00);
> + EXPECT_EQ(bigval[6], 0x00);
> + EXPECT_EQ(bigval[7], 0x0f);
> +
> + unsigned char littleval[8] = {0x00};
> + endian::writeAtBitAlignment<int32_t, little, unaligned>(littleval,
> + (int)0xffffaaaa, 4);
> + EXPECT_EQ(littleval[0], 0xa0);
> + EXPECT_EQ(littleval[1], 0xaa);
> + EXPECT_EQ(littleval[2], 0xfa);
> + EXPECT_EQ(littleval[3], 0xff);
> + EXPECT_EQ(littleval[4], 0x0f);
> + EXPECT_EQ(littleval[5], 0x00);
> + EXPECT_EQ(littleval[6], 0x00);
> + EXPECT_EQ(littleval[7], 0x00);
> +}
> +
> TEST(Endian, Write) {
> unsigned char data[5];
> endian::write<int32_t, big, unaligned>(data, -1362446643);
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list