Avoid UBSan report in arrayElement

This commit is contained in:
Alexey Milovidov 2021-01-21 11:10:31 +03:00
parent af4afff723
commit 1bb8cc5c9a
3 changed files with 65 additions and 7 deletions

View File

@ -231,7 +231,7 @@ struct ArrayElementNumImpl
if (builder) if (builder)
builder.update(j); builder.update(j);
} }
else if (index < 0 && static_cast<size_t>(-index) <= array_size) else if (index < 0 && -static_cast<size_t>(index) <= array_size)
{ {
size_t j = offsets[i] + index; size_t j = offsets[i] + index;
result[i] = data[j]; result[i] = data[j];
@ -329,7 +329,7 @@ struct ArrayElementStringImpl
TIndex index = indices[i]; TIndex index = indices[i];
if (index > 0 && static_cast<size_t>(index) <= array_size) if (index > 0 && static_cast<size_t>(index) <= array_size)
adjusted_index = index - 1; adjusted_index = index - 1;
else if (index < 0 && static_cast<size_t>(-index) <= array_size) else if (index < 0 && -static_cast<size_t>(index) <= array_size)
adjusted_index = array_size + index; adjusted_index = array_size + index;
else else
adjusted_index = array_size; /// means no element should be taken adjusted_index = array_size; /// means no element should be taken
@ -427,7 +427,7 @@ struct ArrayElementGenericImpl
if (builder) if (builder)
builder.update(j); builder.update(j);
} }
else if (index < 0 && static_cast<size_t>(-index) <= array_size) else if (index < 0 && -static_cast<size_t>(index) <= array_size)
{ {
size_t j = offsets[i] + index; size_t j = offsets[i] + index;
result.insertFrom(data, j); result.insertFrom(data, j);
@ -472,11 +472,24 @@ ColumnPtr FunctionArrayElement::executeNumberConst(
auto col_res = ColumnVector<DataType>::create(); auto col_res = ColumnVector<DataType>::create();
if (index.getType() == Field::Types::UInt64) if (index.getType() == Field::Types::UInt64)
{
ArrayElementNumImpl<DataType>::template vectorConst<false>( ArrayElementNumImpl<DataType>::template vectorConst<false>(
col_nested->getData(), col_array->getOffsets(), safeGet<UInt64>(index) - 1, col_res->getData(), builder); col_nested->getData(), col_array->getOffsets(), safeGet<UInt64>(index) - 1, col_res->getData(), builder);
}
else if (index.getType() == Field::Types::Int64) else if (index.getType() == Field::Types::Int64)
{
/// Cast to UInt64 before negation allows to avoid undefined behaviour for negation of the most negative number.
/// NOTE: this would be undefined behaviour in C++ sense, but nevertheless, compiler cannot see it on user provided data,
/// and generates the code that we want on supported CPU architectures (overflow in sense of two's complement arithmetic).
/// This is only needed to avoid UBSan report.
/// Negative array indices work this way:
/// arr[-1] is the element at offset 0 from the last
/// arr[-2] is the element at offset 1 from the last and so on.
ArrayElementNumImpl<DataType>::template vectorConst<true>( ArrayElementNumImpl<DataType>::template vectorConst<true>(
col_nested->getData(), col_array->getOffsets(), -safeGet<Int64>(index) - 1, col_res->getData(), builder); col_nested->getData(), col_array->getOffsets(), -(UInt64(safeGet<Int64>(index)) + 1), col_res->getData(), builder);
}
else else
throw Exception("Illegal type of array index", ErrorCodes::LOGICAL_ERROR); throw Exception("Illegal type of array index", ErrorCodes::LOGICAL_ERROR);
@ -534,7 +547,7 @@ FunctionArrayElement::executeStringConst(const ColumnsWithTypeAndName & argument
col_nested->getChars(), col_nested->getChars(),
col_array->getOffsets(), col_array->getOffsets(),
col_nested->getOffsets(), col_nested->getOffsets(),
-safeGet<Int64>(index) - 1, -(UInt64(safeGet<Int64>(index)) + 1),
col_res->getChars(), col_res->getChars(),
col_res->getOffsets(), col_res->getOffsets(),
builder); builder);
@ -588,7 +601,7 @@ ColumnPtr FunctionArrayElement::executeGenericConst(
col_nested, col_array->getOffsets(), safeGet<UInt64>(index) - 1, *col_res, builder); col_nested, col_array->getOffsets(), safeGet<UInt64>(index) - 1, *col_res, builder);
else if (index.getType() == Field::Types::Int64) else if (index.getType() == Field::Types::Int64)
ArrayElementGenericImpl::vectorConst<true>( ArrayElementGenericImpl::vectorConst<true>(
col_nested, col_array->getOffsets(), -safeGet<Int64>(index) - 1, *col_res, builder); col_nested, col_array->getOffsets(), -(UInt64(safeGet<Int64>(index) + 1)), *col_res, builder);
else else
throw Exception("Illegal type of array index", ErrorCodes::LOGICAL_ERROR); throw Exception("Illegal type of array index", ErrorCodes::LOGICAL_ERROR);
@ -639,7 +652,7 @@ ColumnPtr FunctionArrayElement::executeConst(const ColumnsWithTypeAndName & argu
if (builder) if (builder)
builder.update(j); builder.update(j);
} }
else if (index < 0 && static_cast<size_t>(-index) <= array_size) else if (index < 0 && -static_cast<size_t>(index) <= array_size)
{ {
size_t j = array_size + index; size_t j = array_size + index;
res->insertFrom(array_elements, j); res->insertFrom(array_elements, j);

View File

@ -0,0 +1,26 @@
0
0
0
0
---
0
0
0
---
0
0
0
0
0
0
0
1
---
0
0
0
0
0
0
0
1

View File

@ -0,0 +1,19 @@
SELECT [number][10000000000] FROM numbers(1);
SELECT [number][-10000000000] FROM numbers(1);
SELECT [number][-0x8000000000000000] FROM numbers(1);
SELECT [number][0xFFFFFFFFFFFFFFFF] FROM numbers(1);
SELECT '---';
SELECT [materialize(1)][0xFFFFFFFFFFFFFFFF];
SELECT [materialize(1)][materialize(18446744073709551615)];
SELECT [materialize(1)][-0x8000000000000000];
SELECT '---';
SELECT [number][arrayJoin([-0x8000000000000000, -10000000000, 0, -1])] FROM numbers(2);
SELECT '---';
SELECT [number][arrayJoin([0xFFFFFFFFFFFFFFFF, 10000000000, 0, 1])] FROM numbers(2);