cerc-io/solidity
16
0
Fork 0
mirror of https://github.com/ethereum/solidity synced 2023-10-03 13:03:40 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

FullInliner: Fix order of arguments of inlined functions

Browse Source
This commit is contained in:
Kamil Śliwak 2023-07-11 12:13:00 +02:00
parent a07f6c443a
commit 5e1e0e7752
12 changed files with 106 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
Changelog.md
View File

@ -1,5 +1,9 @@
### 0.8.21 (unreleased)
Important Bugfixes:
* Yul Optimizer: Fix ``FullInliner`` step (``i``) not preserving the evaluation order of arguments passed into inlined functions in code that is not in expression-split form (i.e. when using a custom optimizer sequence in which the step not preceded by ``ExpressionSplitter`` (``x``)).
Language Features:
* Allow qualified access to events from other contracts.
* Relax restrictions on initialization of immutable variables. Reads and writes may now happen at any point at construction time outside of functions and modifiers. Explicit initialization is no longer mandatory.

13
docs/bugs.json
View File

@ -1,4 +1,17 @@
[
{
"uid": "SOL-2023-2",
"name": "FullInlinerNonExpressionSplitArgumentEvaluationOrder",
"summary": "Optimizer sequences containing FullInliner do not preserve the evaluation order of arguments of inlined function calls in code that is not in expression-split form.",
"description": "Function call arguments in Yul are evaluated right to left. This order matters when the argument expressions have side-effects, and changing it may change contract behavior. FullInliner is an optimizer step that can replace a function call with the body of that function. The transformation involves assigning argument expressions to temporary variables, which imposes an explicit evaluation order. FullInliner was written with the assumption that this order does not necessarily have to match usual argument evaluation order because the argument expressions have no side-effects. In most circumstances this assumption is true because the default optimization step sequence contains the ExpressionSplitter step. ExpressionSplitter ensures that the code is in *expression-split form*, which means that function calls cannot appear nested inside expressions, and all function call arguments have to be variables. The assumption is, however, not guaranteed to be true in general. Version 0.6.7 introduced a setting allowing users to specify an arbitrary optimization step sequence, making it possible for the FullInliner to actually encounter argument expressions with side-effects, which can result in behavior differences between optimized and unoptimized bytecode. Contracts compiled without optimization or with the default optimization sequence are not affected. To trigger the bug the user has to explicitly choose compiler settings that contain a sequence with FullInliner step not preceded by ExpressionSplitter.",
"link": "https://blog.soliditylang.org/2023/07/19/full-inliner-non-expression-split-argument-evaluation-order-bug/",
"introduced": "0.6.7",
"fixed": "0.8.21",
"severity": "low",
"conditions": {
"yulOptimizer": true
}
},
{
"uid": "SOL-2022-7",
"name": "StorageWriteRemovalBeforeConditionalTermination",

46
docs/bugs_by_version.json
View File

@ -1422,6 +1422,7 @@
},
"0.6.10": {
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder",
"AbiReencodingHeadOverflowWithStaticArrayCleanup",
"DirtyBytesArrayToStorage",
"DataLocationChangeInInternalOverride",
@ -1436,6 +1437,7 @@
},
"0.6.11": {
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder",
"AbiReencodingHeadOverflowWithStaticArrayCleanup",
"DirtyBytesArrayToStorage",
"DataLocationChangeInInternalOverride",
@ -1450,6 +1452,7 @@
},
"0.6.12": {
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder",
"AbiReencodingHeadOverflowWithStaticArrayCleanup",
"DirtyBytesArrayToStorage",
"DataLocationChangeInInternalOverride",
@ -1548,6 +1551,7 @@
},
"0.6.7": {
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder",
"AbiReencodingHeadOverflowWithStaticArrayCleanup",
"DirtyBytesArrayToStorage",
"NestedCalldataArrayAbiReencodingSizeValidation",
@ -1564,6 +1568,7 @@
},
"0.6.8": {
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder",
"AbiReencodingHeadOverflowWithStaticArrayCleanup",
"DirtyBytesArrayToStorage",
"NestedCalldataArrayAbiReencodingSizeValidation",
@ -1577,6 +1582,7 @@
},
"0.6.9": {
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder",
"AbiReencodingHeadOverflowWithStaticArrayCleanup",
"DirtyBytesArrayToStorage",
"DataLocationChangeInInternalOverride",
@ -1592,6 +1598,7 @@
},
"0.7.0": {
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder",
"AbiReencodingHeadOverflowWithStaticArrayCleanup",
"DirtyBytesArrayToStorage",
"DataLocationChangeInInternalOverride",
@ -1606,6 +1613,7 @@
},
"0.7.1": {
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder",
"AbiReencodingHeadOverflowWithStaticArrayCleanup",
"DirtyBytesArrayToStorage",
"DataLocationChangeInInternalOverride",
@ -1621,6 +1629,7 @@
},
"0.7.2": {
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder",
"AbiReencodingHeadOverflowWithStaticArrayCleanup",
"DirtyBytesArrayToStorage",
"DataLocationChangeInInternalOverride",
@ -1635,6 +1644,7 @@
},
"0.7.3": {
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder",
"AbiReencodingHeadOverflowWithStaticArrayCleanup",
"DirtyBytesArrayToStorage",
"DataLocationChangeInInternalOverride",
@ -1648,6 +1658,7 @@
},
"0.7.4": {
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder",
"AbiReencodingHeadOverflowWithStaticArrayCleanup",
"DirtyBytesArrayToStorage",
"DataLocationChangeInInternalOverride",
@ -1660,6 +1671,7 @@
},
"0.7.5": {
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder",
"AbiReencodingHeadOverflowWithStaticArrayCleanup",
"DirtyBytesArrayToStorage",
"DataLocationChangeInInternalOverride",
@ -1672,6 +1684,7 @@
},
"0.7.6": {
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder",
"AbiReencodingHeadOverflowWithStaticArrayCleanup",
"DirtyBytesArrayToStorage",
"DataLocationChangeInInternalOverride",
@ -1684,6 +1697,7 @@
},
"0.8.0": {
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder",
"AbiReencodingHeadOverflowWithStaticArrayCleanup",
"DirtyBytesArrayToStorage",
"DataLocationChangeInInternalOverride",
@ -1696,6 +1710,7 @@
},
"0.8.1": {
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder",
"AbiReencodingHeadOverflowWithStaticArrayCleanup",
"DirtyBytesArrayToStorage",
"DataLocationChangeInInternalOverride",
@ -1708,6 +1723,7 @@
},
"0.8.10": {
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder",
"AbiReencodingHeadOverflowWithStaticArrayCleanup",
"DirtyBytesArrayToStorage",
"DataLocationChangeInInternalOverride",
@ -1717,6 +1733,7 @@
},
"0.8.11": {
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder",
"AbiReencodingHeadOverflowWithStaticArrayCleanup",
"DirtyBytesArrayToStorage",
"DataLocationChangeInInternalOverride",
@ -1727,6 +1744,7 @@
},
"0.8.12": {
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder",
"AbiReencodingHeadOverflowWithStaticArrayCleanup",
"DirtyBytesArrayToStorage",
"DataLocationChangeInInternalOverride",
@ -1737,6 +1755,7 @@
},
"0.8.13": {
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder",
"StorageWriteRemovalBeforeConditionalTermination",
"AbiReencodingHeadOverflowWithStaticArrayCleanup",
"DirtyBytesArrayToStorage",
@ -1748,6 +1767,7 @@
},
"0.8.14": {
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder",
"StorageWriteRemovalBeforeConditionalTermination",
"AbiReencodingHeadOverflowWithStaticArrayCleanup",
"DirtyBytesArrayToStorage",
@ -1757,6 +1777,7 @@
},
"0.8.15": {
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder",
"StorageWriteRemovalBeforeConditionalTermination",
"AbiReencodingHeadOverflowWithStaticArrayCleanup"
],
@ -1764,24 +1785,32 @@
},
"0.8.16": {
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder",
"StorageWriteRemovalBeforeConditionalTermination"
],
"released": "2022-08-08"
},
"0.8.17": {
"bugs": [],
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder"
],
"released": "2022-09-08"
},
"0.8.18": {
"bugs": [],
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder"
],
"released": "2023-02-01"
},
"0.8.19": {
"bugs": [],
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder"
],
"released": "2023-02-22"
},
"0.8.2": {
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder",
"AbiReencodingHeadOverflowWithStaticArrayCleanup",
"DirtyBytesArrayToStorage",
"DataLocationChangeInInternalOverride",
@ -1793,11 +1822,14 @@
"released": "2021-03-02"
},
"0.8.20": {
"bugs": [],
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder"
],
"released": "2023-05-10"
},
"0.8.3": {
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder",
"AbiReencodingHeadOverflowWithStaticArrayCleanup",
"DirtyBytesArrayToStorage",
"DataLocationChangeInInternalOverride",
@ -1809,6 +1841,7 @@
},
"0.8.4": {
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder",
"AbiReencodingHeadOverflowWithStaticArrayCleanup",
"DirtyBytesArrayToStorage",
"DataLocationChangeInInternalOverride",
@ -1819,6 +1852,7 @@
},
"0.8.5": {
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder",
"AbiReencodingHeadOverflowWithStaticArrayCleanup",
"DirtyBytesArrayToStorage",
"DataLocationChangeInInternalOverride",
@ -1829,6 +1863,7 @@
},
"0.8.6": {
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder",
"AbiReencodingHeadOverflowWithStaticArrayCleanup",
"DirtyBytesArrayToStorage",
"DataLocationChangeInInternalOverride",
@ -1839,6 +1874,7 @@
},
"0.8.7": {
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder",
"AbiReencodingHeadOverflowWithStaticArrayCleanup",
"DirtyBytesArrayToStorage",
"DataLocationChangeInInternalOverride",
@ -1849,6 +1885,7 @@
},
"0.8.8": {
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder",
"AbiReencodingHeadOverflowWithStaticArrayCleanup",
"DirtyBytesArrayToStorage",
"DataLocationChangeInInternalOverride",
@ -1860,6 +1897,7 @@
},
"0.8.9": {
"bugs": [
"FullInlinerNonExpressionSplitArgumentEvaluationOrder",
"AbiReencodingHeadOverflowWithStaticArrayCleanup",
"DirtyBytesArrayToStorage",
"DataLocationChangeInInternalOverride",

2
docs/internals/optimizer.rst
View File

@ -570,7 +570,7 @@ It is not applied to loop iteration-condition, because the loop control flow doe
this "outlining" of the inner expressions in all cases. We can sidestep this limitation by applying
:ref:`for-loop-condition-into-body` to move the iteration condition into loop body.
The final program should be in a form such that (with the exception of loop conditions)
The final program should be in an *expression-split form*, where (with the exception of loop conditions)
function calls cannot appear nested inside expressions
and all function call arguments have to be variables.

6
libyul/optimiser/FullInliner.cpp
View File

@ -38,6 +38,8 @@
#include <libsolutil/Visitor.h>
#include <range/v3/action/remove.hpp>
#include <range/v3/view/reverse.hpp>
#include <range/v3/view/zip.hpp>
using namespace std;
using namespace solidity;
@ -305,8 +307,8 @@ vector<Statement> InlineModifier::performInline(Statement& _statement, FunctionC
newStatements.emplace_back(std::move(varDecl));
};
for (size_t i = 0; i < _funCall.arguments.size(); ++i)
newVariable(function->parameters[i], &_funCall.arguments[i]);
for (auto&& [parameter, argument]: ranges::views::zip(function->parameters, _funCall.arguments) | ranges::views::reverse)
newVariable(parameter, &argument);
for (auto const& var: function->returnVariables)
newVariable(var, nullptr);

2
libyul/optimiser/FullInliner.h
View File

@ -58,8 +58,8 @@ class NameCollector;
*
* function f(a, b) -> c { ... }
*
* let f_a := x
* let f_b := y
* let f_a := x
* let f_c
* code of f, with replacements: a -> f_a, b -> f_b, c -> f_c
* let z := f_c

5
test/libyul/yulOptimizerTests/fullInliner/call_arguments_with_side_effects.yul
View File

@ -16,8 +16,9 @@
// let _1 := ret_7
// let ret_1_10 := 0
// return(0, 0)
// let a_13 := ret_1_10
// let b_14 := _1
// let _2 := ret_1_10
// let b_13 := _1
// let a_14 := _2
// }
// function fun_revert() -> ret
// { revert(0, 0) }

20
test/libyul/yulOptimizerTests/fullInliner/call_arguments_without_side_effects.yul
View File

@ -24,25 +24,27 @@
// {
// let _1 := 333
// let _2 := 222
// let a_13 := 111
// let _3 := 111
// let c_13 := _1
// let b_14 := _2
// let c_15 := _1
// let a_15 := _3
// let x := 111
// let y := 222
// let z := 333
// let a_16 := x
// let c_16 := 333
// let b_17 := y
// let c_18 := z
// let a_18 := x
// let _5 := calldataload(333)
// let _7 := sload(222)
// let a_19 := mload(111)
// let _9 := mload(111)
// let c_19 := _5
// let b_20 := _7
// let c_21 := _5
// let a_21 := _9
// let a_1 := 222
// let _11 := mload(333)
// let a_22 := 111
// let _12 := 111
// let c_22 := _11
// let b_23 := a_1
// let c_24 := _11
// let a_24 := _12
// }
// function empty(a, b, c)
// { }

9
test/libyul/yulOptimizerTests/fullInliner/move_up_rightwards_argument.yul
View File

@ -13,12 +13,13 @@
// let _2 := mload(5)
// let _4 := mload(4)
// let _6 := mload(3)
// let a_13 := mload(2)
// let _8 := mload(2)
// let c_13 := _4
// let b_14 := _6
// let c_15 := _4
// let a_15 := _8
// let x_16 := 0
// x_16 := add(a_13, b_14)
// x_16 := mul(x_16, c_15)
// x_16 := add(a_15, b_14)
// x_16 := mul(x_16, c_13)
// let _10 := add(x_16, _2)
// let y := add(mload(1), _10)
// }

9
test/libyul/yulOptimizerTests/fullInliner/multi_fun.yul
View File

@ -12,13 +12,14 @@
// let a_8 := 3
// let x_9 := 0
// x_9 := add(a_8, a_8)
// let b_10 := x_9
// let c_11 := _1
// let _3 := x_9
// let c_10 := _1
// let b_11 := _3
// let y_12 := 0
// let a_6_13 := b_10
// let a_6_13 := b_11
// let x_7_14 := 0
// x_7_14 := add(a_6_13, a_6_13)
// y_12 := mul(mload(c_11), x_7_14)
// y_12 := mul(mload(c_10), x_7_14)
// let y_1 := y_12
// }
// function f(a) -> x

10
test/libyul/yulOptimizerTests/fullInlinerWithoutSplitter/call_arguments_with_side_effects.yul
View File

@ -11,12 +11,12 @@
//
// {
// {
// let ret_1_3 := 0
// return(0, 0)
// let a_1 := ret_1_3
// let ret_4 := 0
// let ret_3 := 0
// revert(0, 0)
// let b_2 := ret_4
// let b_1 := ret_3
// let ret_1_4 := 0
// return(0, 0)
// let a_2 := ret_1_4
// }
// function fun_revert() -> ret
// { revert(0, 0) }

24
test/libyul/yulOptimizerTests/fullInlinerWithoutSplitter/call_arguments_without_side_effects.yul
View File

@ -22,22 +22,22 @@
//
// {
// {
// let a_2 := 111
// let b_3 := 222
// let c_4 := 333
// let c_1 := 333
// let b_2 := 222
// let a_3 := 111
// let x := 111
// let y := 222
// let z := 333
// let a_5 := x
// let b_6 := y
// let c_7 := z
// let a_8 := mload(111)
// let b_9 := sload(222)
// let c_10 := calldataload(333)
// let c_4 := z
// let b_5 := y
// let a_6 := x
// let c_7 := calldataload(333)
// let b_8 := sload(222)
// let a_9 := mload(111)
// let a_1 := 222
// let a_11 := 111
// let b_12 := a_1
// let c_13 := mload(333)
// let c_10 := mload(333)
// let b_11 := a_1
// let a_12 := 111
// }
// function empty(a, b, c)
// { }