diff --git a/Changelog.md b/Changelog.md index f09fb0355c19..72461f021077 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,9 +1,14 @@ +### 0.5.16 (2020-01-02) + +Bugfixes: + * Yul Optimizer: Fix bug in redundant assignment remover in combination with break and continue statements. + + ### 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 5d94131a19c8..bbb9e93fa180 100644 --- a/docs/bugs.json +++ b/docs/bugs.json @@ -1,4 +1,15 @@ [ + { + "name": "YulOptimizerRedundantAssignmentBreakContinue0.5", + "summary": "The Yul optimizer can remove essential assignments to variables declared inside for loops when Yul's continue or break statement is used. You are unlikely to be affected if you do not use inline assembly with for loops and continue and break statements.", + "description": "The Yul optimizer has a stage that removes assignments to variables that are overwritten again or are not used in all following control-flow branches. This logic incorrectly removes such assignments to variables declared inside a for loop if they can be removed in a control-flow branch that ends with ``break`` or ``continue`` even though they cannot be removed in other control-flow branches. Variables declared outside of the respective for loop are not affected.", + "introduced": "0.5.8", + "fixed": "0.5.16", + "severity": "low", + "conditions": { + "yulOptimizer": true + } + }, { "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.", diff --git a/docs/bugs_by_version.json b/docs/bugs_by_version.json index 1a0e99749db3..a9ce0be92f7a 100644 --- a/docs/bugs_by_version.json +++ b/docs/bugs_by_version.json @@ -742,32 +742,46 @@ }, "0.5.10": { "bugs": [ + "YulOptimizerRedundantAssignmentBreakContinue0.5", "ABIEncoderV2CalldataStructsWithStaticallySizedAndDynamicallyEncodedMembers" ], "released": "2019-06-25" }, "0.5.11": { - "bugs": [], + "bugs": [ + "YulOptimizerRedundantAssignmentBreakContinue0.5" + ], "released": "2019-08-12" }, "0.5.12": { - "bugs": [], + "bugs": [ + "YulOptimizerRedundantAssignmentBreakContinue0.5" + ], "released": "2019-10-01" }, "0.5.13": { - "bugs": [], + "bugs": [ + "YulOptimizerRedundantAssignmentBreakContinue0.5" + ], "released": "2019-11-14" }, "0.5.14": { "bugs": [ + "YulOptimizerRedundantAssignmentBreakContinue0.5", "ABIEncoderV2LoopYulOptimizer" ], "released": "2019-12-09" }, "0.5.15": { - "bugs": [], + "bugs": [ + "YulOptimizerRedundantAssignmentBreakContinue0.5" + ], "released": "2019-12-17" }, + "0.5.16": { + "bugs": [], + "released": "2020-01-02" + }, "0.5.2": { "bugs": [ "SignedArrayStorageCopy", @@ -840,6 +854,7 @@ }, "0.5.8": { "bugs": [ + "YulOptimizerRedundantAssignmentBreakContinue0.5", "ABIEncoderV2CalldataStructsWithStaticallySizedAndDynamicallyEncodedMembers", "SignedArrayStorageCopy", "ABIEncoderV2StorageArrayWithMultiSlotElement", @@ -849,6 +864,7 @@ }, "0.5.9": { "bugs": [ + "YulOptimizerRedundantAssignmentBreakContinue0.5", "ABIEncoderV2CalldataStructsWithStaticallySizedAndDynamicallyEncodedMembers", "SignedArrayStorageCopy", "ABIEncoderV2StorageArrayWithMultiSlotElement" diff --git a/libyul/optimiser/RedundantAssignEliminator.cpp b/libyul/optimiser/RedundantAssignEliminator.cpp index 477814e221c5..3c619ad45600 100644 --- a/libyul/optimiser/RedundantAssignEliminator.cpp +++ b/libyul/optimiser/RedundantAssignEliminator.cpp @@ -268,29 +268,28 @@ void RedundantAssignEliminator::changeUndecidedTo(YulString _variable, Redundant void RedundantAssignEliminator::finalize(YulString _variable, RedundantAssignEliminator::State _finalState) { - finalize(m_assignments, _variable, _finalState); - for (auto& assignments: m_forLoopInfo.pendingBreakStmts) - finalize(assignments, _variable, _finalState); - for (auto& assignments: m_forLoopInfo.pendingContinueStmts) - finalize(assignments, _variable, _finalState); -} + std::map assignments; + joinMap(assignments, std::move(m_assignments[_variable]), State::join); + m_assignments.erase(_variable); -void RedundantAssignEliminator::finalize( - TrackedAssignments& _assignments, - YulString _variable, - RedundantAssignEliminator::State _finalState -) -{ - for (auto const& assignment: _assignments[_variable]) + for (auto& breakAssignments: m_forLoopInfo.pendingBreakStmts) + { + joinMap(assignments, std::move(breakAssignments[_variable]), State::join); + breakAssignments.erase(_variable); + } + for (auto& continueAssignments: m_forLoopInfo.pendingContinueStmts) + { + joinMap(assignments, std::move(continueAssignments[_variable]), State::join); + continueAssignments.erase(_variable); + } + + for (auto const& assignment: assignments) { State const state = assignment.second == State::Undecided ? _finalState : assignment.second; if (state == State::Unused && SideEffectsCollector{*m_dialect, *assignment.first->value}.movable()) - // TODO the only point where we actually need this - // to be a set is for the for loop m_pendingRemovals.insert(assignment.first); } - _assignments.erase(_variable); } void AssignmentRemover::operator()(Block& _block) diff --git a/libyul/optimiser/RedundantAssignEliminator.h b/libyul/optimiser/RedundantAssignEliminator.h index 65ddc19ae97f..0b68c10941e0 100644 --- a/libyul/optimiser/RedundantAssignEliminator.h +++ b/libyul/optimiser/RedundantAssignEliminator.h @@ -158,8 +158,6 @@ class RedundantAssignEliminator: public ASTWalker /// assignments to the final state. In this case, this also applies to pending /// break and continue TrackedAssignments. void finalize(YulString _variable, State _finalState); - /// Helper function for the above. - void finalize(TrackedAssignments& _assignments, YulString _variable, State _finalState); Dialect const* m_dialect; std::set m_declaredVariables; diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index bc03c2ef8cfc..dfe135db9c7a 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -14718,8 +14718,6 @@ BOOST_AUTO_TEST_CASE(event_wrong_abi_name) )"; compileAndRun(sourceCode, 0, "ClientReceipt", bytes()); compileAndRun(sourceCode, 0, "Test", bytes(), map{{"ClientReceipt", m_contractAddress}}); - u256 value(18); - u256 id(0x1234); callContractFunction("f()"); BOOST_REQUIRE_EQUAL(numLogs(), 1); diff --git a/test/libyul/yulOptimizerTests/redundantAssignEliminator/remove_break.yul b/test/libyul/yulOptimizerTests/redundantAssignEliminator/remove_break.yul new file mode 100644 index 000000000000..8e75d82edb7c --- /dev/null +++ b/test/libyul/yulOptimizerTests/redundantAssignEliminator/remove_break.yul @@ -0,0 +1,26 @@ +{ + let i := 0 + for {} lt(i, 2) { i := add(i, 1) } + { + let x + x := 1337 + if lt(i,1) { + x := 42 + break + } + mstore(0, x) + } +} +// ==== +// step: redundantAssignEliminator +// ---- +// { +// let i := 0 +// for { } lt(i, 2) { i := add(i, 1) } +// { +// let x +// x := 1337 +// if lt(i, 1) { break } +// mstore(0, x) +// } +// } diff --git a/test/libyul/yulOptimizerTests/redundantAssignEliminator/remove_continue.yul b/test/libyul/yulOptimizerTests/redundantAssignEliminator/remove_continue.yul new file mode 100644 index 000000000000..f5a29382b393 --- /dev/null +++ b/test/libyul/yulOptimizerTests/redundantAssignEliminator/remove_continue.yul @@ -0,0 +1,27 @@ +{ + let i := 0 + for {} lt(i, 2) { i := add(i, 1) } + { + let x + x := 1337 + if lt(i,1) { + x := 42 + continue + } + mstore(0, x) + } +} + +// ==== +// step: redundantAssignEliminator +// ---- +// { +// let i := 0 +// for { } lt(i, 2) { i := add(i, 1) } +// { +// let x +// x := 1337 +// if lt(i, 1) { continue } +// mstore(0, x) +// } +// }