diff --git a/Changelog.md b/Changelog.md index e0d9a9fef..857809e55 100644 --- a/Changelog.md +++ b/Changelog.md @@ -45,6 +45,11 @@ Compiler Features: * ABIEncoderV2: Do not warn about enabled ABIEncoderV2 anymore (the pragma is still needed, though). +### 0.5.15 (2019-12-17) + +Bugfixes: + * Yul Optimizer: Fix incorrect redundant load optimization crossing user-defined functions that contain for-loops with memory / storage writes. + ### 0.5.14 (2019-12-09) Language Features: diff --git a/docs/bugs.json b/docs/bugs.json index 6c34eb44e..5d94131a1 100644 --- a/docs/bugs.json +++ b/docs/bugs.json @@ -1,4 +1,17 @@ [ + { + "name": "ABIEncoderV2LoopYulOptimizer", + "summary": "If both the experimental ABIEncoderV2 and the experimental Yul optimizer are activated, one component of the Yul optimizer may reuse data in memory that has been changed in the meantime.", + "description": "The Yul optimizer incorrectly replaces ``mload`` and ``sload`` calls with values that have been previously written to the load location (and potentially changed in the meantime) if all of the following conditions are met: (1) there is a matching ``mstore`` or ``sstore`` call before; (2) the contents of memory or storage is only changed in a function that is called (directly or indirectly) in between the first store and the load call; (3) called function contains a for loop where the same memory location is changed in the condition or the post or body block. When used in Solidity mode, this can only happen if the experimental ABIEncoderV2 is activated and the experimental Yul optimizer has been activated manually in addition to the regular optimizer in the compiler settings.", + "introduced": "0.5.14", + "fixed": "0.5.15", + "severity": "low", + "conditions": { + "ABIEncoderV2": true, + "optimizer": true, + "yulOptimizer": true + } + }, { "name": "ABIEncoderV2CalldataStructsWithStaticallySizedAndDynamicallyEncodedMembers", "summary": "Reading from calldata structs that contain dynamically encoded, but statically-sized members can result in incorrect values.", diff --git a/docs/bugs_by_version.json b/docs/bugs_by_version.json index c412be830..1a0e99749 100644 --- a/docs/bugs_by_version.json +++ b/docs/bugs_by_version.json @@ -759,9 +759,15 @@ "released": "2019-11-14" }, "0.5.14": { - "bugs": [], + "bugs": [ + "ABIEncoderV2LoopYulOptimizer" + ], "released": "2019-12-09" }, + "0.5.15": { + "bugs": [], + "released": "2019-12-17" + }, "0.5.2": { "bugs": [ "SignedArrayStorageCopy", diff --git a/libyul/optimiser/CallGraphGenerator.cpp b/libyul/optimiser/CallGraphGenerator.cpp index 49673a153..6d0a0e54a 100644 --- a/libyul/optimiser/CallGraphGenerator.cpp +++ b/libyul/optimiser/CallGraphGenerator.cpp @@ -42,9 +42,10 @@ void CallGraphGenerator::operator()(FunctionCall const& _functionCall) ASTWalker::operator()(_functionCall); } -void CallGraphGenerator::operator()(ForLoop const&) +void CallGraphGenerator::operator()(ForLoop const& _forLoop) { m_callGraph.functionsWithLoops.insert(m_currentFunction); + ASTWalker::operator()(_forLoop); } void CallGraphGenerator::operator()(FunctionDefinition const& _functionDefinition) diff --git a/test/libyul/functionSideEffects/mload_in_function.yul b/test/libyul/functionSideEffects/mload_in_function.yul new file mode 100644 index 000000000..170535926 --- /dev/null +++ b/test/libyul/functionSideEffects/mload_in_function.yul @@ -0,0 +1,11 @@ +{ + function foo(x) { + for {} x { x := mload(0) mstore(0, 0)} {} + } + mstore(0, 1337) + foo(42) + sstore(0, mload(0)) +} +// ---- +// : invalidatesStorage, invalidatesMemory +// foo: invalidatesMemory diff --git a/test/libyul/yulOptimizerTests/fullSuite/abi_example1.yul b/test/libyul/yulOptimizerTests/fullSuite/abi_example1.yul index ab1ebb10d..0d37cf1b9 100644 --- a/test/libyul/yulOptimizerTests/fullSuite/abi_example1.yul +++ b/test/libyul/yulOptimizerTests/fullSuite/abi_example1.yul @@ -477,15 +477,16 @@ // pos := add(pos, 0x60) // } // let _3 := mload(64) -// if slt(sub(_3, length), 128) { revert(_1, _1) } -// let offset := calldataload(add(length, 64)) -// let _4 := 0xffffffffffffffff -// if gt(offset, _4) { revert(_1, _1) } -// let value2 := abi_decode_t_array$_t_uint256_$dyn_memory_ptr(add(length, offset), _3) -// let offset_1 := calldataload(add(length, 0x60)) -// if gt(offset_1, _4) { revert(_1, _1) } -// let value3 := abi_decode_t_array$_t_array$_t_uint256_$2_memory_$dyn_memory_ptr(add(length, offset_1), _3) -// sstore(calldataload(length), calldataload(add(length, 0x20))) +// let _4 := mload(0x20) +// if slt(sub(_3, _4), 128) { revert(_1, _1) } +// let offset := calldataload(add(_4, 64)) +// let _5 := 0xffffffffffffffff +// if gt(offset, _5) { revert(_1, _1) } +// let value2 := abi_decode_t_array$_t_uint256_$dyn_memory_ptr(add(_4, offset), _3) +// let offset_1 := calldataload(add(_4, 0x60)) +// if gt(offset_1, _5) { revert(_1, _1) } +// let value3 := abi_decode_t_array$_t_array$_t_uint256_$2_memory_$dyn_memory_ptr(add(_4, offset_1), _3) +// sstore(calldataload(_4), calldataload(add(_4, 0x20))) // sstore(value2, value3) // sstore(_1, pos) // } diff --git a/test/libyul/yulOptimizerTests/loadResolver/mload_in_function.yul b/test/libyul/yulOptimizerTests/loadResolver/mload_in_function.yul new file mode 100644 index 000000000..9bb8261dd --- /dev/null +++ b/test/libyul/yulOptimizerTests/loadResolver/mload_in_function.yul @@ -0,0 +1,29 @@ +{ + function foo(x) { + for {} x { x := mload(0) mstore(0, 0)} {} + } + mstore(0, 1337) + foo(42) + sstore(0, mload(0)) +} +// ==== +// step: loadResolver +// ---- +// { +// function foo(x) +// { +// for { } +// x +// { +// let _1 := 0 +// x := mload(_1) +// mstore(_1, _1) +// } +// { } +// } +// let _4 := 1337 +// let _5 := 0 +// mstore(_5, _4) +// foo(42) +// sstore(_5, mload(_5)) +// } diff --git a/test/libyul/yulOptimizerTests/loadResolver/mstore_in_function_loop_body.yul b/test/libyul/yulOptimizerTests/loadResolver/mstore_in_function_loop_body.yul new file mode 100644 index 000000000..2de873942 --- /dev/null +++ b/test/libyul/yulOptimizerTests/loadResolver/mstore_in_function_loop_body.yul @@ -0,0 +1,35 @@ +{ + function userNot(x) -> y { + y := iszero(x) + } + + function funcWithLoop(x) { + for {} userNot(x) { mstore(0, 0) } {} + } + + mstore(0, 1337) + funcWithLoop(42) + sstore(0, mload(0)) +} +// ==== +// step: loadResolver +// ---- +// { +// function userNot(x) -> y +// { y := iszero(x) } +// function funcWithLoop(x_1) +// { +// for { } +// userNot(x_1) +// { +// let _1 := 0 +// mstore(_1, _1) +// } +// { } +// } +// let _3 := 1337 +// let _4 := 0 +// mstore(_4, _3) +// funcWithLoop(42) +// sstore(_4, mload(_4)) +// } diff --git a/test/libyul/yulOptimizerTests/loadResolver/mstore_in_function_loop_init.yul b/test/libyul/yulOptimizerTests/loadResolver/mstore_in_function_loop_init.yul new file mode 100644 index 000000000..1e06cf8df --- /dev/null +++ b/test/libyul/yulOptimizerTests/loadResolver/mstore_in_function_loop_init.yul @@ -0,0 +1,32 @@ +{ + function userNot(x) -> y { + y := iszero(x) + } + + function funcWithLoop(x) { + for { mstore(0, 0) } userNot(x) {} {} + } + + mstore(0, 1337) + funcWithLoop(42) + sstore(0, mload(0)) +} +// ==== +// step: loadResolver +// ---- +// { +// function userNot(x) -> y +// { y := iszero(x) } +// function funcWithLoop(x_1) +// { +// let _1 := 0 +// mstore(_1, _1) +// for { } userNot(x_1) { } +// { } +// } +// let _3 := 1337 +// let _4 := 0 +// mstore(_4, _3) +// funcWithLoop(42) +// sstore(_4, mload(_4)) +// }