From 1e50fc61e46eb81c607cb9d649c93decfd99cbad Mon Sep 17 00:00:00 2001 From: chriseth Date: Sun, 29 Dec 2019 14:21:23 +0100 Subject: [PATCH 1/6] Fix redundant assignment removal in combination with break / continue. --- .../optimiser/RedundantAssignEliminator.cpp | 31 +++++++++---------- libyul/optimiser/RedundantAssignEliminator.h | 2 -- 2 files changed, 15 insertions(+), 18 deletions(-) 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; From 92c3511f47158ecaf1bba717eded84410f55132f Mon Sep 17 00:00:00 2001 From: chriseth Date: Sun, 29 Dec 2019 15:16:54 +0100 Subject: [PATCH 2/6] Tests. --- .../remove_break.yul | 26 ++++++++++++++++++ .../remove_continue.yul | 27 +++++++++++++++++++ 2 files changed, 53 insertions(+) create mode 100644 test/libyul/yulOptimizerTests/redundantAssignEliminator/remove_break.yul create mode 100644 test/libyul/yulOptimizerTests/redundantAssignEliminator/remove_continue.yul 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) +// } +// } From 1c2096a360f1590e41cd535524201d1f7f346297 Mon Sep 17 00:00:00 2001 From: chriseth Date: Sun, 29 Dec 2019 15:14:43 +0100 Subject: [PATCH 3/6] Changelog entry. --- Changelog.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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: From 8328f826a1c3aca400c463b58cd7f43d71f87f97 Mon Sep 17 00:00:00 2001 From: chriseth Date: Sun, 29 Dec 2019 15:51:27 +0100 Subject: [PATCH 4/6] Bug list entry. --- docs/bugs.json | 11 +++++++++++ docs/bugs_by_version.json | 24 ++++++++++++++++++++---- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/docs/bugs.json b/docs/bugs.json index 5d94131a19c8..52b062c8a4f9 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 remove assignments to variables that are overwritten again or are not used in all following control-flow branches. This logic incorrectly removes such assignments to varibales 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" From 026f7c37af8e75b7ae7cbebd8e30eba032b6ef0f Mon Sep 17 00:00:00 2001 From: Leonardo Alt Date: Thu, 2 Jan 2020 12:54:52 +0100 Subject: [PATCH 5/6] Review 1 --- docs/bugs.json | 2 +- test/libsolidity/SolidityEndToEndTest.cpp | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/docs/bugs.json b/docs/bugs.json index 52b062c8a4f9..bbb9e93fa180 100644 --- a/docs/bugs.json +++ b/docs/bugs.json @@ -2,7 +2,7 @@ { "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 remove assignments to variables that are overwritten again or are not used in all following control-flow branches. This logic incorrectly removes such assignments to varibales 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.", + "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", 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); From 9c3226ce7558bfa639ca06ddd7214ae9bf4e1fc9 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 2 Jan 2020 19:52:34 +0100 Subject: [PATCH 6/6] Set version to 0.5.16. --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 10b830a6ca5b..0c84c6d15d34 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -10,7 +10,7 @@ include(EthPolicy) eth_policy() # project name and version should be set after cmake_policy CMP0048 -set(PROJECT_VERSION "0.5.15") +set(PROJECT_VERSION "0.5.16") project(solidity VERSION ${PROJECT_VERSION} LANGUAGES C CXX) include(TestBigEndian)