[llvm] r364253 - DataExtractor: use decodeSLEB128 to implement getSLEB128

[David Blaikie via llvm-commits]

Ah, thanks for pointing that out (I'm sure I ignored it/got lost in the
usual bot noise) - fixed in r364461

On Wed, Jun 26, 2019 at 10:03 AM Kostya Serebryany <kcc at google.com> wrote:

> Hi David,
>
> Looks like the ubsan bot is unhappy:
>
> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/33102/steps/check-llvm%20ubsan/logs/stdio
>
> llvm/include/llvm/Support/LEB128.h:179:36: runtime error: left shift of 127 by 63 places cannot be represented in type 'int64_t' (aka 'long')
>     #0 0x18803fc in llvm::decodeSLEB128(unsigned char const*, unsigned int*, unsigned char const*, char const**) /b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/Support/LEB128.h:179:36
>     #1 0x1a4cb23 in llvm::DataExtractor::getSLEB128(unsigned int*) const /b/sanitizer-x86_64-linux-fast/build/llvm/lib/Support/DataExtractor.cpp:167:20
>     #2 0x18ba814 in llvm::DWARFFormValue::skipValue(llvm::dwarf::Form, llvm::DataExtractor, unsigned int*, llvm::dwarf::FormParams) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp:180:21
>     #3 0x18fb1ae in llvm::DWARFDebugInfoEntry::extractFast(llvm::DWARFUnit const&, unsigned int*, llvm::DWARFDataExtractor const&, unsigned int, unsigned int) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/DebugInfo/DWARF/DWARFDebugInfoEntry.cpp:60:17
>
>
> On Mon, Jun 24, 2019 at 4:45 PM David Blaikie via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: dblaikie
>> Date: Mon Jun 24 16:45:18 2019
>> New Revision: 364253
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=364253&view=rev
>> Log:
>> DataExtractor: use decodeSLEB128 to implement getSLEB128
>>
>> Should've been NFC, but turns out DataExtractor had better test coverage
>> for decoding SLEB128 than the decodeSLEB128 did - revealing a couple of
>> bugs (one in the error handling, another in sign extension). So fixed
>> those to get the DataExtractor tests passing again.
>>
>> Modified:
>>     llvm/trunk/include/llvm/Support/LEB128.h
>>     llvm/trunk/lib/Support/DataExtractor.cpp
>>
>> Modified: llvm/trunk/include/llvm/Support/LEB128.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/LEB128.h?rev=364253&r1=364252&r2=364253&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/Support/LEB128.h (original)
>> +++ llvm/trunk/include/llvm/Support/LEB128.h Mon Jun 24 16:45:18 2019
>> @@ -165,6 +165,8 @@ inline int64_t decodeSLEB128(const uint8
>>    int64_t Value = 0;
>>    unsigned Shift = 0;
>>    uint8_t Byte;
>> +  if (error)
>> +    *error = nullptr;
>>    do {
>>      if (end && p == end) {
>>        if (error)
>> @@ -177,8 +179,8 @@ inline int64_t decodeSLEB128(const uint8
>>      Value |= (int64_t(Byte & 0x7f) << Shift);
>>      Shift += 7;
>>    } while (Byte >= 128);
>> -  // Sign extend negative numbers.
>> -  if (Byte & 0x40)
>> +  // Sign extend negative numbers if needed.
>> +  if (Shift < 64 && (Byte & 0x40))
>>      Value |= (-1ULL) << Shift;
>>    if (n)
>>      *n = (unsigned)(p - orig_p);
>>
>> Modified: llvm/trunk/lib/Support/DataExtractor.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/DataExtractor.cpp?rev=364253&r1=364252&r2=364253&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Support/DataExtractor.cpp (original)
>> +++ llvm/trunk/lib/Support/DataExtractor.cpp Mon Jun 24 16:45:18 2019
>> @@ -160,26 +160,15 @@ uint64_t DataExtractor::getULEB128(uint3
>>  }
>>
>>  int64_t DataExtractor::getSLEB128(uint32_t *offset_ptr) const {
>> -  int64_t result = 0;
>> -  if (Data.empty())
>> -    return 0;
>> -
>> -  unsigned shift = 0;
>> -  uint32_t offset = *offset_ptr;
>> -  uint8_t byte = 0;
>> +  assert(*offset_ptr <= Data.size());
>>
>> -  while (isValidOffset(offset)) {
>> -    byte = Data[offset++];
>> -    result |= uint64_t(byte & 0x7f) << shift;
>> -    shift += 7;
>> -    if ((byte & 0x80) == 0) {
>> -      // Sign bit of byte is 2nd high order bit (0x40)
>> -      if (shift < 64 && (byte & 0x40))
>> -        result |= -(1ULL << shift);
>> -
>> -      *offset_ptr = offset;
>> -      return result;
>> -    }
>> -  }
>> -  return 0;
>> +  const char *error;
>> +  unsigned bytes_read;
>> +  int64_t result = decodeSLEB128(
>> +      reinterpret_cast<const uint8_t *>(Data.data() + *offset_ptr),
>> &bytes_read,
>> +      reinterpret_cast<const uint8_t *>(Data.data() + Data.size()),
>> &error);
>> +  if (error)
>> +    return 0;
>> +  *offset_ptr += bytes_read;
>> +  return result;
>>  }
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190626/21294106/attachment.html>