[llvm] r248897 - Add support for sub-byte aligned writes to lib/Support/Endian.h

Teresa Johnson via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 8 06:19:35 PDT 2015


I committed a fix at r249689. I didn't have a chance to validate via a
UBSan build, but will try to do so now. I did confirm that it
correctly avoids a negative value going into the left shift for that
test, however, so I anticipate that it will address the failure.

Thanks,
Teresa

On Wed, Oct 7, 2015 at 5:07 PM, Adrian Prantl <aprantl at apple.com> wrote:
>
>> On Oct 7, 2015, at 5:05 PM, Adrian Prantl <aprantl at apple.com> wrote:
>>
>> It’s a two-stage process. First the stage1 bot builds the compiler and the sanitizer-enabled runtime (you may be able to skip this step if you have one lying around) and then the second stage compiler runs came with '-DLLVM_USE_SANITIZER=Address;Undefined’ and a blacklist for certain C++ stdlib headers.
>>
>> At the top of this log you can see the full set of options passed to make:
>>
>> http://lab.llvm.org:8080/green/job/clang-stage2-cmake-RgSan_build/487/consoleFull
>>
>>
>> — adrian
>>
>>> On Oct 7, 2015, at 5:00 PM, Teresa Johnson <tejohnson at google.com> wrote:
>>>
>>> How do I get UBSan to run on the unittests? I don't see where the
>>> option is being added on the buildbot page, but maybe I missed it.
>
> The jenkins bots run three sub-jobs: Acquire, Build, Check. This is probably why you didn’t see it.
>
>>> Thanks,
>>> Teresa
>>>
>>> On Wed, Oct 7, 2015 at 4:18 PM, Teresa Johnson <tejohnson at google.com> wrote:
>>>> Yes, I added that function and see the problem. I will fix the UBSan
>>>> error by masking off the upper bits before the shift. It might be
>>>> tomorrow morning before I can fix it though as I am going offline
>>>> shortly. Will try to get it in today though.
>>>> Teresa
>>>>
>>>> On Wed, Oct 7, 2015 at 2:52 PM, Adrian Prantl <aprantl at apple.com> wrote:
>>>>> 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
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413
>>>
>>>
>>>
>>> --
>>> Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413
>>
>



-- 
Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413


More information about the llvm-commits mailing list