Skip to content

Commit b70e064

Browse files
chrisethchriseth
authored
Merge pull request #13130 from ethereum/check-overflow-after-add-sub-operations
Check overflow after add sub operations
2 parents 2397f09 + 4fd5c11 commit b70e064

File tree

49 files changed

+429
-413
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+429
-413
lines changed

‎Changelog.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ Compiler Features:
88
* Yul IR Code Generation: Improved copy routines for arrays with packed storage layout.
99
* Yul Optimizer: Add rule to convert `mod(mul(X, Y), A)` into `mulmod(X, Y, A)`, if `A` is a power of two.
1010
* Yul Optimizer: Add rule to convert `mod(add(X, Y), A)` into `addmod(X, Y, A)`, if `A` is a power of two.
11-
11+
* Code Generator: More efficient code for checked addition and subtraction.
1212

1313
Bugfixes:
1414
* Commandline Interface: Disallow the following options outside of the compiler mode: ``--via-ir``,``--metadata-literal``, ``--metadata-hash``, ``--model-checker-show-unproved``, ``--model-checker-div-mod-no-slacks``, ``--model-checker-engine``, ``--model-checker-invariants``, ``--model-checker-solvers``, ``--model-checker-timeout``, ``--model-checker-contracts``, ``--model-checker-targets``.

‎libsolidity/codegen/YulUtilFunctions.cpp

Lines changed: 40 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -615,25 +615,34 @@ string YulUtilFunctions::divide32CeilFunction()
615615
string YulUtilFunctions::overflowCheckedIntAddFunction(IntegerType const& _type)
616616
{
617617
string functionName = "checked_add_" + _type.identifier();
618-
// TODO: Consider to add a special case for unsigned 256-bit integers
619-
// and use the following instead:
620-
// sum := add(x, y) if lt(sum, x) { <panic>() }
621618
return m_functionCollector.createFunction(functionName, [&]() {
622619
return
623620
Whiskers(R"(
624621
function <functionName>(x, y) -> sum {
625622
x := <cleanupFunction>(x)
626623
y := <cleanupFunction>(y)
624+
sum := add(x, y)
627625
<?signed>
628-
// overflow, if x >= 0 and y > (maxValue - x)
629-
if and(iszero(slt(x, 0)), sgt(y, sub(<maxValue>, x))) { <panic>() }
630-
// underflow, if x < 0 and y < (minValue - x)
631-
if and(slt(x, 0), slt(y, sub(<minValue>, x))) { <panic>() }
626+
<?256bit>
627+
// overflow, if x >= 0 and sum < y
628+
// underflow, if x < 0 and sum >= y
629+
if or(
630+
and(iszero(slt(x, 0)), slt(sum, y)),
631+
and(slt(x, 0), iszero(slt(sum, y)))
632+
) { <panic>() }
633+
<!256bit>
634+
if or(
635+
sgt(sum, <maxValue>),
636+
slt(sum, <minValue>)
637+
) { <panic>() }
638+
</256bit>
632639
<!signed>
633-
// overflow, if x > (maxValue - y)
634-
if gt(x, sub(<maxValue>, y)) { <panic>() }
640+
<?256bit>
641+
if gt(x, sum) { <panic>() }
642+
<!256bit>
643+
if gt(sum, <maxValue>) { <panic>() }
644+
</256bit>
635645
</signed>
636-
sum := add(x, y)
637646
}
638647
)")
639648
("functionName", functionName)
@@ -642,6 +651,7 @@ string YulUtilFunctions::overflowCheckedIntAddFunction(IntegerType const& _type)
642651
("minValue", toCompactHexWithPrefix(u256(_type.minValue())))
643652
("cleanupFunction", cleanupFunction(_type))
644653
("panic", panicFunction(PanicCode::UnderOverflow))
654+
("256bit", _type.numBits() == 256)
645655
.render();
646656
});
647657
}
@@ -795,15 +805,28 @@ string YulUtilFunctions::overflowCheckedIntSubFunction(IntegerType const& _type)
795805
function <functionName>(x, y) -> diff {
796806
x := <cleanupFunction>(x)
797807
y := <cleanupFunction>(y)
808+
diff := sub(x, y)
798809
<?signed>
799-
// underflow, if y >= 0 and x < (minValue + y)
800-
if and(iszero(slt(y, 0)), slt(x, add(<minValue>, y))) { <panic>() }
801-
// overflow, if y < 0 and x > (maxValue + y)
802-
if and(slt(y, 0), sgt(x, add(<maxValue>, y))) { <panic>() }
810+
<?256bit>
811+
// underflow, if y >= 0 and diff > x
812+
// overflow, if y < 0 and diff < x
813+
if or(
814+
and(iszero(slt(y, 0)), sgt(diff, x)),
815+
and(slt(y, 0), slt(diff, x))
816+
) { <panic>() }
817+
<!256bit>
818+
if or(
819+
slt(diff, <minValue>),
820+
sgt(diff, <maxValue>)
821+
) { <panic>() }
822+
</256bit>
803823
<!signed>
804-
if lt(x, y) { <panic>() }
824+
<?256bit>
825+
if gt(diff, x) { <panic>() }
826+
<!256bit>
827+
if gt(diff, <maxValue>) { <panic>() }
828+
</256bit>
805829
</signed>
806-
diff := sub(x, y)
807830
}
808831
)")
809832
("functionName", functionName)
@@ -812,6 +835,7 @@ string YulUtilFunctions::overflowCheckedIntSubFunction(IntegerType const& _type)
812835
("minValue", toCompactHexWithPrefix(u256(_type.minValue())))
813836
("cleanupFunction", cleanupFunction(_type))
814837
("panic", panicFunction(PanicCode::UnderOverflow))
838+
("256bit", _type.numBits() == 256)
815839
.render();
816840
});
817841
}

‎test/cmdlineTests/asm_json/output

Lines changed: 58 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -1300,14 +1300,14 @@ EVM assembly:
13001300
},
13011301
{
13021302
"begin": 1211,
1303-
"end": 1516,
1303+
"end": 1402,
13041304
"name": "tag",
13051305
"source": 1,
13061306
"value": "10"
13071307
},
13081308
{
13091309
"begin": 1211,
1310-
"end": 1516,
1310+
"end": 1402,
13111311
"name": "JUMPDEST",
13121312
"source": 1
13131313
},
@@ -1423,159 +1423,146 @@ EVM assembly:
14231423
"source": 1
14241424
},
14251425
{
1426-
"begin": 1458,
1427-
"end": 1459,
1426+
"begin": 1347,
1427+
"end": 1348,
14281428
"name": "DUP3",
14291429
"source": 1
14301430
},
14311431
{
1432-
"begin": 1390,
1433-
"end": 1456,
1434-
"name": "PUSH",
1435-
"source": 1,
1436-
"value": "FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF"
1432+
"begin": 1344,
1433+
"end": 1345,
1434+
"name": "DUP3",
1435+
"source": 1
14371436
},
14381437
{
1439-
"begin": 1386,
1440-
"end": 1460,
1441-
"name": "SUB",
1438+
"begin": 1340,
1439+
"end": 1349,
1440+
"name": "ADD",
1441+
"source": 1
1442+
},
1443+
{
1444+
"begin": 1333,
1445+
"end": 1349,
1446+
"name": "SWAP1",
14421447
"source": 1
14431448
},
14441449
{
1445-
"begin": 1383,
1446-
"end": 1384,
1450+
"begin": 1333,
1451+
"end": 1349,
1452+
"name": "POP",
1453+
"source": 1
1454+
},
1455+
{
1456+
"begin": 1368,
1457+
"end": 1371,
1458+
"name": "DUP1",
1459+
"source": 1
1460+
},
1461+
{
1462+
"begin": 1365,
1463+
"end": 1366,
14471464
"name": "DUP3",
14481465
"source": 1
14491466
},
14501467
{
1451-
"begin": 1380,
1452-
"end": 1461,
1468+
"begin": 1362,
1469+
"end": 1372,
14531470
"name": "GT",
14541471
"source": 1
14551472
},
14561473
{
1457-
"begin": 1377,
1458-
"end": 1484,
1474+
"begin": 1359,
1475+
"end": 1395,
14591476
"name": "ISZERO",
14601477
"source": 1
14611478
},
14621479
{
1463-
"begin": 1377,
1464-
"end": 1484,
1480+
"begin": 1359,
1481+
"end": 1395,
14651482
"name": "PUSH [tag]",
14661483
"source": 1,
14671484
"value": "37"
14681485
},
14691486
{
1470-
"begin": 1377,
1471-
"end": 1484,
1487+
"begin": 1359,
1488+
"end": 1395,
14721489
"name": "JUMPI",
14731490
"source": 1
14741491
},
14751492
{
1476-
"begin": 1464,
1477-
"end": 1482,
1493+
"begin": 1375,
1494+
"end": 1393,
14781495
"name": "PUSH [tag]",
14791496
"source": 1,
14801497
"value": "38"
14811498
},
14821499
{
1483-
"begin": 1464,
1484-
"end": 1482,
1500+
"begin": 1375,
1501+
"end": 1393,
14851502
"name": "PUSH [tag]",
14861503
"source": 1,
14871504
"value": "18"
14881505
},
14891506
{
1490-
"begin": 1464,
1491-
"end": 1482,
1507+
"begin": 1375,
1508+
"end": 1393,
14921509
"jumpType": "[in]",
14931510
"name": "JUMP",
14941511
"source": 1
14951512
},
14961513
{
1497-
"begin": 1464,
1498-
"end": 1482,
1514+
"begin": 1375,
1515+
"end": 1393,
14991516
"name": "tag",
15001517
"source": 1,
15011518
"value": "38"
15021519
},
15031520
{
1504-
"begin": 1464,
1505-
"end": 1482,
1521+
"begin": 1375,
1522+
"end": 1393,
15061523
"name": "JUMPDEST",
15071524
"source": 1
15081525
},
15091526
{
1510-
"begin": 1377,
1511-
"end": 1484,
1527+
"begin": 1359,
1528+
"end": 1395,
15121529
"name": "tag",
15131530
"source": 1,
15141531
"value": "37"
15151532
},
15161533
{
1517-
"begin": 1377,
1518-
"end": 1484,
1534+
"begin": 1359,
1535+
"end": 1395,
15191536
"name": "JUMPDEST",
15201537
"source": 1
15211538
},
1522-
{
1523-
"begin": 1508,
1524-
"end": 1509,
1525-
"name": "DUP3",
1526-
"source": 1
1527-
},
1528-
{
1529-
"begin": 1505,
1530-
"end": 1506,
1531-
"name": "DUP3",
1532-
"source": 1
1533-
},
1534-
{
1535-
"begin": 1501,
1536-
"end": 1510,
1537-
"name": "ADD",
1538-
"source": 1
1539-
},
1540-
{
1541-
"begin": 1494,
1542-
"end": 1510,
1543-
"name": "SWAP1",
1544-
"source": 1
1545-
},
1546-
{
1547-
"begin": 1494,
1548-
"end": 1510,
1549-
"name": "POP",
1550-
"source": 1
1551-
},
15521539
{
15531540
"begin": 1211,
1554-
"end": 1516,
1541+
"end": 1402,
15551542
"name": "SWAP3",
15561543
"source": 1
15571544
},
15581545
{
15591546
"begin": 1211,
1560-
"end": 1516,
1547+
"end": 1402,
15611548
"name": "SWAP2",
15621549
"source": 1
15631550
},
15641551
{
15651552
"begin": 1211,
1566-
"end": 1516,
1553+
"end": 1402,
15671554
"name": "POP",
15681555
"source": 1
15691556
},
15701557
{
15711558
"begin": 1211,
1572-
"end": 1516,
1559+
"end": 1402,
15731560
"name": "POP",
15741561
"source": 1
15751562
},
15761563
{
15771564
"begin": 1211,
1578-
"end": 1516,
1565+
"end": 1402,
15791566
"jumpType": "[out]",
15801567
"name": "JUMP",
15811568
"source": 1

0 commit comments

Comments
 (0)