Hello Todd Lipcon, Tim Armstrong, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/12346

to look at the new patch set (#2).

Change subject: IMPALA-5031: `uint8_t & int` type is int
......................................................................

IMPALA-5031: `uint8_t & int` type is int

UBSAN finds the following in ParquetBoolDecoder.TestDecodeAndSkipping:

util/bit-stream-utils.inline.h:156:25: runtime error: left shift of 42 by 28 
places cannot be represented in type 'int'
    #0 BatchedBitReader::GetUleb128Int(unsigned int*) 
util/bit-stream-utils.inline.h:156:25
    #1 RleBatchDecoder<bool>::NextCounts() util/rle-encoding.h:778:40
    #2 RleBatchDecoder<bool>::NextNumRepeats() util/rle-encoding.h:622:28
    #3 RleBatchDecoder<bool>::GetValues(int, bool*) util/rle-encoding.h:858:27
    #4 bool ParquetBoolDecoder::DecodeValue<(parquet::Encoding::type)3>(bool*) 
exec/parquet/parquet-bool-decoder.h:85:24
    #5 TestSkipping(parquet::Encoding::type, unsigned char*, int, vector<bool> 
const&, int, int)::$_0::operator()() const 
exec/parquet/parquet-bool-decoder-test.cc:59
    #6 TestSkipping(parquet::Encoding::type, unsigned char*, int, vector<bool> 
const&, int, int) exec/parquet/parquet-bool-decoder-test.cc:69:221
    #7 ParquetBoolDecoder_TestDecodeAndSkipping_Test::TestBody() 
exec/parquet/parquet-bool-decoder-test.cc:85:5
    #9 testing::Test::Run() 
(/home/ubuntu/Impala/be/build/debug/exec/parquet/parquet-bool-decoder-test+0x6ee4f09)

The problem is the line

    *v |= (byte & 0x7F) << shift;

byte is an uint8_t and 0x7F is an int. The standard section
[expr.bit.and] then applies the "usual arithmetic conversions"
specified in [expr], which applies "if the type of the operand with
signed integer type can represent all of the values of the type of the
operand with unsigned integer type, the operand with unsigned integer
type shall be converted to the type of the operand with signed integer
type." That makes byte & 0x7F a signed integer type, and [expr.shift]
says that "if E1 has a signed type and non-negative value, and E1×2^E2
is representable in the corresponding unsigned type of the result
type, then that value, converted to the result type, is the resulting
value; otherwise, the behavior is undefined."

Change-Id: Ie6e0b956751090f3f8aadd6783b5e06e55e57abe
---
M be/src/util/bit-stream-utils.inline.h
1 file changed, 4 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/12346/2
--
To view, visit http://gerrit.cloudera.org:8080/12346
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie6e0b956751090f3f8aadd6783b5e06e55e57abe
Gerrit-Change-Number: 12346
Gerrit-PatchSet: 2
Gerrit-Owner: Jim Apple <jbapple-imp...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>

Reply via email to