Skip to content

Commit 84e30aa

Browse files
spuchinamikish
spuchin
and
amikish
authored
Fix interval multiplication overflow (#8188) (#8979)
Co-authored-by: amikish <111790456+amikish@users.noreply.github.com>
1 parent 49dcfaf commit 84e30aa

File tree

12 files changed

+68
-60
lines changed

12 files changed

+68
-60
lines changed

‎ydb/library/yql/minikql/invoke_builtins/mkql_builtins_mul.cpp

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -40,22 +40,23 @@ struct TNumMulInterval {
4040
const auto lv = static_cast<typename TOutput::TLayout>(left.template Get<typename TLeft::TLayout>());
4141
const auto rv = static_cast<typename TOutput::TLayout>(right.template Get<typename TRight::TLayout>());
4242
const auto ret = lv * rv;
43-
if (ret == 0) {
43+
if (rv == 0 || lv == 0) {
4444
return NUdf::TUnboxedValuePod(ret);
4545
}
46+
i64 i64Max = std::numeric_limits<i64>::max();
4647
if constexpr (std::is_same_v<ui64, typename TLeft::TLayout>) {
47-
if (left.Get<ui64>() > static_cast<ui64>(std::numeric_limits<i64>::max())) {
48+
if (left.Get<ui64>() >= static_cast<ui64>(i64Max)) {
4849
return NUdf::TUnboxedValuePod();
4950
}
5051
}
5152
if constexpr (std::is_same_v<ui64, typename TRight::TLayout>) {
52-
if (right.Get<ui64>() > static_cast<ui64>(std::numeric_limits<i64>::max())) {
53+
if (right.Get<ui64>() >= static_cast<ui64>(i64Max)) {
5354
return NUdf::TUnboxedValuePod();
5455
}
5556
}
56-
i64 lvAbs = (lv > 0) ? lv : -lv;
57-
i64 rvAbs = (rv > 0) ? rv : -rv;
58-
if (rvAbs != 0 && (std::numeric_limits<i64>::max() / rvAbs < lvAbs)) {
57+
auto div = i64Max / rv;
58+
auto divAbs = (div >= 0) ? div : -div;
59+
if ((lv >= 0) ? (lv > divAbs) : (lv < -divAbs)) {
5960
return NUdf::TUnboxedValuePod();
6061
}
6162
return IsBadInterval<TOutput>(ret) ? NUdf::TUnboxedValuePod() : NUdf::TUnboxedValuePod(ret);
@@ -68,39 +69,43 @@ struct TNumMulInterval {
6869
const auto bbMain = BasicBlock::Create(context, "bbMain", ctx.Func);
6970
const auto bbDone = BasicBlock::Create(context, "bbDone", ctx.Func);
7071
const auto resultType = Type::getInt128Ty(context);
71-
const auto result = PHINode::Create(resultType, 3, "result", bbDone);
72+
const auto result = PHINode::Create(resultType, 2, "result", bbDone);
7273

7374
const auto lv = GetterFor<typename TLeft::TLayout>(left, context, block);
7475
const auto lhs = StaticCast<typename TLeft::TLayout, i64>(lv, context, block);
7576
const auto rv = GetterFor<typename TRight::TLayout>(right, context, block);
7677
const auto rhs = StaticCast<typename TRight::TLayout, i64>(rv, context, block);
7778
const auto mul = BinaryOperator::CreateMul(lhs, rhs, "mul", block);
7879
const auto zero = ConstantInt::get(Type::getInt64Ty(context), 0);
79-
const auto mulZero = CmpInst::Create(Instruction::ICmp, ICmpInst::ICMP_EQ, mul, zero, "mulZero", block);
80+
const auto lhsZero = CmpInst::Create(Instruction::ICmp, ICmpInst::ICMP_EQ, lhs, zero, "lhsZero", block);
81+
const auto rhsZero = CmpInst::Create(Instruction::ICmp, ICmpInst::ICMP_EQ, rhs, zero, "rhsZero", block);
8082
const auto res = SetterFor<typename TOutput::TLayout>(mul, context, block);
8183

82-
BranchInst::Create(bbDone, bbMain, mulZero, block);
84+
BranchInst::Create(bbDone, bbMain, BinaryOperator::CreateOr(lhsZero, rhsZero, "mulZero", block), block);
8385
result->addIncoming(res, block);
8486

8587
block = bbMain;
8688

87-
const auto lhsAbs = SelectInst::Create(
88-
CmpInst::Create(Instruction::ICmp, ICmpInst::ICMP_SGT, lhs, zero, "lhsPos", block),
89-
lhs,
90-
BinaryOperator::CreateNeg(lhs, "lhsNeg", block),
91-
"lhsAbs", block);
92-
const auto rhsAbs = SelectInst::Create(
93-
CmpInst::Create(Instruction::ICmp, ICmpInst::ICMP_SGT, rhs, zero, "rhsPos", block),
94-
rhs,
95-
BinaryOperator::CreateNeg(rhs, "rhsNeg", block),
96-
"rhsAbs", block);
9789
const auto i64Max = ConstantInt::get(Type::getInt64Ty(context), std::numeric_limits<i64>::max());
98-
const auto div = BinaryOperator::CreateSDiv(i64Max, rhsAbs, "div", block);
99-
const auto mulOverflow = CmpInst::Create(Instruction::ICmp, ICmpInst::ICMP_SGT, lhsAbs, div, "mulOverflow", block);
90+
const auto div = BinaryOperator::CreateSDiv(i64Max, rhs, "div", block);
91+
const auto divAbs = SelectInst::Create(
92+
CmpInst::Create(Instruction::ICmp, ICmpInst::ICMP_SGE, div, zero, "divPos", block),
93+
div,
94+
BinaryOperator::CreateNeg(div, "divNeg", block),
95+
"divAbs", block);
96+
const auto divAbsNeg = BinaryOperator::CreateNeg(divAbs, "divAbsNeg", block);
97+
98+
const auto mulOverflow = SelectInst::Create(
99+
CmpInst::Create(Instruction::ICmp, ICmpInst::ICMP_SGE, lhs, zero, "lhsPos", block),
100+
CmpInst::Create(Instruction::ICmp, ICmpInst::ICMP_SGT, lhs, divAbs, "lhsDiv", block),
101+
CmpInst::Create(Instruction::ICmp, ICmpInst::ICMP_SLT, lhs, divAbsNeg, "lhsDivAbsNeg", block),
102+
"mulOverflow", block);
103+
100104
const auto i64Overflow = BinaryOperator::CreateOr(
101105
GenIsInt64Overflow<typename TLeft::TLayout>(lv, context, block),
102106
GenIsInt64Overflow<typename TRight::TLayout>(rv, context, block),
103107
"i64Overflow", block);
108+
104109
const auto bad = BinaryOperator::CreateOr(
105110
BinaryOperator::CreateOr(i64Overflow, mulOverflow, "overflow", block),
106111
GenIsBadInterval<TOutput>(mul, context, block),

‎ydb/library/yql/tests/sql/dq_file/part16/canondata/result.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -405,9 +405,9 @@
405405
],
406406
"test.test[bigdate-arithmetic-default.txt-Debug]": [
407407
{
408-
"checksum": "968be71411b4f41aa78165913bd74333",
409-
"size": 8688,
410-
"uri": "https://{canondata_backend}/1937367/518bbcf510ad7a43c5e77746bafd21ed0e3fdc6e/resource.tar.gz#test.test_bigdate-arithmetic-default.txt-Debug_/opt.yql_patched"
408+
"checksum": "1a5cecc5f3f3d734d4eef23e12f6725b",
409+
"size": 8887,
410+
"uri": "https://{canondata_backend}/1899731/2ec8224db091f2a7362c5e4ce595bc50329b8311/resource.tar.gz#test.test_bigdate-arithmetic-default.txt-Debug_/opt.yql_patched"
411411
}
412412
],
413413
"test.test[bigdate-arithmetic-default.txt-Plan]": [

‎ydb/library/yql/tests/sql/dq_file/part4/canondata/result.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -531,9 +531,9 @@
531531
],
532532
"test.test[bigdate-table_int_cast-default.txt-Debug]": [
533533
{
534-
"checksum": "1d672f23cc038dc42d14175121554c71",
534+
"checksum": "0f242de0900368c56d1b937487ced6cb",
535535
"size": 8617,
536-
"uri": "https://{canondata_backend}/1924537/907e79379e1e72f9d09545e57f65dee63f42dbfe/resource.tar.gz#test.test_bigdate-table_int_cast-default.txt-Debug_/opt.yql_patched"
536+
"uri": "https://{canondata_backend}/1809005/df0d5940a3b3a38ba468a035aba7ce54440f0891/resource.tar.gz#test.test_bigdate-table_int_cast-default.txt-Debug_/opt.yql_patched"
537537
}
538538
],
539539
"test.test[bigdate-table_int_cast-default.txt-Plan]": [

‎ydb/library/yql/tests/sql/dq_file/part6/canondata/result.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -457,9 +457,9 @@
457457
],
458458
"test.test[bigdate-table_arithmetic_mul_div-default.txt-Debug]": [
459459
{
460-
"checksum": "7d3363f2bc50bd3ecf9097b47926a9e4",
460+
"checksum": "11e7ab9520da2bde231d70dbb834e16a",
461461
"size": 9840,
462-
"uri": "https://{canondata_backend}/1936842/e2f5b27b418549665a04de58de4b4e487f33c292/resource.tar.gz#test.test_bigdate-table_arithmetic_mul_div-default.txt-Debug_/opt.yql_patched"
462+
"uri": "https://{canondata_backend}/1937424/022b4c4aaf443124c76bb3e388177d9b3de00044/resource.tar.gz#test.test_bigdate-table_arithmetic_mul_div-default.txt-Debug_/opt.yql_patched"
463463
}
464464
],
465465
"test.test[bigdate-table_arithmetic_mul_div-default.txt-Plan]": [

‎ydb/library/yql/tests/sql/hybrid_file/part2/canondata/result.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -435,9 +435,9 @@
435435
],
436436
"test.test[bigdate-table_int_cast-default.txt-Debug]": [
437437
{
438-
"checksum": "177fc2fac903bdde7623c6ad50195b2e",
439-
"size": 22146,
440-
"uri": "https://{canondata_backend}/1784826/45c8e56b489fcd70fc011cedb88a592ff2ca27ed/resource.tar.gz#test.test_bigdate-table_int_cast-default.txt-Debug_/opt.yql_patched"
438+
"checksum": "2255e5c0c361ed4e31289580bb6a402c",
439+
"size": 18588,
440+
"uri": "https://{canondata_backend}/1775059/b57c9040709a7b012953cf170d04a292adc8d3d3/resource.tar.gz#test.test_bigdate-table_int_cast-default.txt-Debug_/opt.yql_patched"
441441
}
442442
],
443443
"test.test[bigdate-table_int_cast-default.txt-Plan]": [

‎ydb/library/yql/tests/sql/hybrid_file/part7/canondata/result.json

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -477,9 +477,9 @@
477477
],
478478
"test.test[bigdate-arithmetic-default.txt-Debug]": [
479479
{
480-
"checksum": "74f20089d55fea37e0e7cdee17f0cbd8",
481-
"size": 8687,
482-
"uri": "https://{canondata_backend}/1784117/3885f0a76b64a32a48487f8866602d3fff1e416a/resource.tar.gz#test.test_bigdate-arithmetic-default.txt-Debug_/opt.yql_patched"
480+
"checksum": "c1939c3925f1c9e2d27b33692637bc61",
481+
"size": 8886,
482+
"uri": "https://{canondata_backend}/1889210/e26b9c7fc72b580fe82c1126f535456e73306c2c/resource.tar.gz#test.test_bigdate-arithmetic-default.txt-Debug_/opt.yql_patched"
483483
}
484484
],
485485
"test.test[bigdate-arithmetic-default.txt-Plan]": [
@@ -519,9 +519,9 @@
519519
],
520520
"test.test[bigdate-table_arithmetic_mul_div-default.txt-Debug]": [
521521
{
522-
"checksum": "48b58158564d3dd01c0d41b8dded6604",
523-
"size": 30153,
524-
"uri": "https://{canondata_backend}/1925842/7ff9f7f6f9fd763e965f539592431555505139a0/resource.tar.gz#test.test_bigdate-table_arithmetic_mul_div-default.txt-Debug_/opt.yql_patched"
522+
"checksum": "7f460bd132529e2aee95b0bcb5b608b7",
523+
"size": 28216,
524+
"uri": "https://{canondata_backend}/1817427/46729b354b9b15ea89f67bf14fefd2face8b402b/resource.tar.gz#test.test_bigdate-table_arithmetic_mul_div-default.txt-Debug_/opt.yql_patched"
525525
}
526526
],
527527
"test.test[bigdate-table_arithmetic_mul_div-default.txt-Plan]": [

‎ydb/library/yql/tests/sql/sql2yql/canondata/result.json

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2731,9 +2731,9 @@
27312731
],
27322732
"test_sql2yql.test[bigdate-arithmetic]": [
27332733
{
2734-
"checksum": "10d3641741062fdda397c48868d0bd18",
2735-
"size": 54577,
2736-
"uri": "https://{canondata_backend}/1784117/d56ae82ad9d30397a41490647be1bd2124718f98/resource.tar.gz#test_sql2yql.test_bigdate-arithmetic_/sql.yql"
2734+
"checksum": "dd71cf80379fd120f899f51a8d4185e0",
2735+
"size": 55200,
2736+
"uri": "https://{canondata_backend}/1784117/72de9d096244671bdc282e3e67e222f7d47084f5/resource.tar.gz#test_sql2yql.test_bigdate-arithmetic_/sql.yql"
27372737
}
27382738
],
27392739
"test_sql2yql.test[bigdate-bitcast_date32]": [
@@ -21995,9 +21995,9 @@
2199521995
],
2199621996
"test_sql_format.test[bigdate-arithmetic]": [
2199721997
{
21998-
"checksum": "74aed1e3957af10f2b714b9961faaa61",
21999-
"size": 7211,
22000-
"uri": "https://{canondata_backend}/1775319/65663ceaccc952bb64364e6408bfc54197bb33e2/resource.tar.gz#test_sql_format.test_bigdate-arithmetic_/formatted.sql"
21998+
"checksum": "9733896aaffd0f29c69369ba53a83908",
21999+
"size": 7322,
22000+
"uri": "https://{canondata_backend}/1784117/72de9d096244671bdc282e3e67e222f7d47084f5/resource.tar.gz#test_sql_format.test_bigdate-arithmetic_/formatted.sql"
2200122001
}
2200222002
],
2200322003
"test_sql_format.test[bigdate-bitcast_date32]": [

‎ydb/library/yql/tests/sql/suites/bigdate/Signed.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
{ "row"=-8; "i8"=-16; "i16"=-256; "i32"=-65536; "i64"=-4294967296; };
12
{ "row"=-7; "i8"=-128; "i16"=-32768; "i32"=-2147483648; "i64"=-9223372036854775808; };
23
{ "row"=-6; "i8"=-128; "i16"=-32768; "i32"=-2147483648; "i64"=-9223339708799999999; };
34
{ "row"=-5; "i8"=-128; "i16"=-32768; "i32"=-2147483648; "i64"=-4611669897600000001; };
@@ -13,3 +14,4 @@
1314
{ "row"=5; "i8"=127; "i16"=32767; "i32"=2147483647; "i64"=4611669811200000000; };
1415
{ "row"=6; "i8"=127; "i16"=32767; "i32"=2147483647; "i64"=9223339708799999999; };
1516
{ "row"=7; "i8"=127; "i16"=32767; "i32"=2147483647; "i64"=9223372036854775807; };
17+
{ "row"=8; "i8"=16; "i16"=256; "i32"=65536; "i64"=4294967296; };

‎ydb/library/yql/tests/sql/suites/bigdate/arithmetic.sql

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,4 +99,5 @@ select 0, -$interval64_max, -$interval64_min, -$interval64_zero
9999
, 8, $interval64_zero/$ui64_max, $interval64_zero/$i64_max, $interval64_plus1/$ui64_max, $interval64_plus1/$i64_max, $interval64_minus1/$ui64_max, $interval64_minus1/$i64_max
100100
, 9, $interval64_max/cast($interval64_max as int64), $interval64_min/cast($interval64_min as int64)
101101
, 10, abs($interval64_max), abs($interval64_min), abs($interval64_zero)
102+
, 11, cast(4294967296l as interval64) * 4294967296l, 4294967296ul * cast(4294967296l as interval64)
102103
;

‎ydb/library/yql/tests/sql/yt_native_file/part16/canondata/result.json

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -353,9 +353,9 @@
353353
],
354354
"test.test[bigdate-arithmetic-default.txt-Debug]": [
355355
{
356-
"checksum": "2559cb39ee0ad016c2eeba0175d90372",
357-
"size": 8617,
358-
"uri": "https://{canondata_backend}/1130705/85e00e1809c16d7a062c55ddef958687d825adb0/resource.tar.gz#test.test_bigdate-arithmetic-default.txt-Debug_/opt.yql"
356+
"checksum": "a8db511c5581bd3dac3da50a14581438",
357+
"size": 8816,
358+
"uri": "https://{canondata_backend}/1937424/bcd047e5237d8754d97b84f73ec58e1dfd3f242e/resource.tar.gz#test.test_bigdate-arithmetic-default.txt-Debug_/opt.yql"
359359
}
360360
],
361361
"test.test[bigdate-arithmetic-default.txt-Plan]": [
@@ -367,9 +367,9 @@
367367
],
368368
"test.test[bigdate-arithmetic-default.txt-Results]": [
369369
{
370-
"checksum": "78b14e7c48f2837c128e3175ccbdecb8",
371-
"size": 78678,
372-
"uri": "https://{canondata_backend}/1775059/3537a7096af92f6c747d78f769fafb259258445e/resource.tar.gz#test.test_bigdate-arithmetic-default.txt-Results_/results.txt"
370+
"checksum": "5c7490438490b2bc79701426ebfaa41a",
371+
"size": 79839,
372+
"uri": "https://{canondata_backend}/1937424/bcd047e5237d8754d97b84f73ec58e1dfd3f242e/resource.tar.gz#test.test_bigdate-arithmetic-default.txt-Results_/results.txt"
373373
}
374374
],
375375
"test.test[bigdate-compare_big_big-default.txt-Debug]": [

‎ydb/library/yql/tests/sql/yt_native_file/part4/canondata/result.json

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -441,9 +441,9 @@
441441
],
442442
"test.test[bigdate-table_int_cast-default.txt-Debug]": [
443443
{
444-
"checksum": "51e140cf0893afa5a725df8cc2c01834",
444+
"checksum": "66aa7125c203486f28ed2c642c5741f0",
445445
"size": 13465,
446-
"uri": "https://{canondata_backend}/1936842/bca567d011b535ed157a29d116aa0c03a876f699/resource.tar.gz#test.test_bigdate-table_int_cast-default.txt-Debug_/opt.yql"
446+
"uri": "https://{canondata_backend}/1946324/97f79e67e2ba571def52794d35b33cdebe8c85ed/resource.tar.gz#test.test_bigdate-table_int_cast-default.txt-Debug_/opt.yql"
447447
}
448448
],
449449
"test.test[bigdate-table_int_cast-default.txt-Plan]": [
@@ -455,9 +455,9 @@
455455
],
456456
"test.test[bigdate-table_int_cast-default.txt-Results]": [
457457
{
458-
"checksum": "1791e9dfc14aa122ae1f383b5651a4af",
459-
"size": 97973,
460-
"uri": "https://{canondata_backend}/1936997/9b8859f70925a58b024145127cca3e8e612258c0/resource.tar.gz#test.test_bigdate-table_int_cast-default.txt-Results_/results.txt"
458+
"checksum": "ec0c795efb2cb5df83406d85a55783b7",
459+
"size": 99849,
460+
"uri": "https://{canondata_backend}/1946324/97f79e67e2ba571def52794d35b33cdebe8c85ed/resource.tar.gz#test.test_bigdate-table_int_cast-default.txt-Results_/results.txt"
461461
}
462462
],
463463
"test.test[bigdate-tzstrliterals-default.txt-Debug]": [

‎ydb/library/yql/tests/sql/yt_native_file/part6/canondata/result.json

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -421,9 +421,9 @@
421421
],
422422
"test.test[bigdate-table_arithmetic_mul_div-default.txt-Debug]": [
423423
{
424-
"checksum": "7c0763701065ca8b3138feca9672ace3",
425-
"size": 24381,
426-
"uri": "https://{canondata_backend}/1925821/1cd5a00cb1561c8bd6ad6a05b5a3985def6ab335/resource.tar.gz#test.test_bigdate-table_arithmetic_mul_div-default.txt-Debug_/opt.yql"
424+
"checksum": "428a68b06c3b405e0087538758ca7e4c",
425+
"size": 24400,
426+
"uri": "https://{canondata_backend}/1917492/30b5bc2d6861ed60772eb6885e2fd9071a8898c0/resource.tar.gz#test.test_bigdate-table_arithmetic_mul_div-default.txt-Debug_/opt.yql"
427427
}
428428
],
429429
"test.test[bigdate-table_arithmetic_mul_div-default.txt-Plan]": [
@@ -435,9 +435,9 @@
435435
],
436436
"test.test[bigdate-table_arithmetic_mul_div-default.txt-Results]": [
437437
{
438-
"checksum": "af13590e677929ae5e2008ba94e8e3d5",
439-
"size": 662899,
440-
"uri": "https://{canondata_backend}/1924537/2f28b0eebe4156125fa9fe084e0ede26fda21bd7/resource.tar.gz#test.test_bigdate-table_arithmetic_mul_div-default.txt-Results_/results.txt"
438+
"checksum": "785fb2e27cc00e47a21316d75d79a7b5",
439+
"size": 662203,
440+
"uri": "https://{canondata_backend}/1917492/30b5bc2d6861ed60772eb6885e2fd9071a8898c0/resource.tar.gz#test.test_bigdate-table_arithmetic_mul_div-default.txt-Results_/results.txt"
441441
}
442442
],
443443
"test.test[bigdate-table_arithmetic_narrow-default.txt-Debug]": [

0 commit comments

Comments
 (0)