From a33da17300f919b2e730b911bd20813e84d41523 Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Mon, 5 Sep 2022 11:31:09 +0200 Subject: [PATCH] Bugfix and tests. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Kamil Śliwak --- libyul/optimiser/UnusedStoreEliminator.cpp | 5 +- .../unused_store_storage_removal_bug.sol | 15 +++++ ...nditionally_terminating_function_call.yul} | 3 +- ...ing_function_call_complex_control_flow.yul | 60 +++++++++++++++++++ ...nally_terminating_function_call_revert.yul | 27 +++++++++ ...onditionally_terminating_function_call.yul | 33 ++++++++++ ...onditionally_terminating_function_call.yul | 23 +++++++ 7 files changed, 164 insertions(+), 2 deletions(-) create mode 100644 test/libsolidity/semanticTests/unused_store_storage_removal_bug.sol rename test/libyul/yulOptimizerTests/unusedStoreEliminator/{function_side_effects_3.yul => store_before_conditionally_terminating_function_call.yul} (79%) create mode 100644 test/libyul/yulOptimizerTests/unusedStoreEliminator/store_before_conditionally_terminating_function_call_complex_control_flow.yul create mode 100644 test/libyul/yulOptimizerTests/unusedStoreEliminator/store_before_conditionally_terminating_function_call_revert.yul create mode 100644 test/libyul/yulOptimizerTests/unusedStoreEliminator/store_before_indirectly_conditionally_terminating_function_call.yul create mode 100644 test/libyul/yulOptimizerTests/unusedStoreEliminator/store_before_unconditionally_terminating_function_call.yul diff --git a/libyul/optimiser/UnusedStoreEliminator.cpp b/libyul/optimiser/UnusedStoreEliminator.cpp index 754c02ac5..2e96be2e7 100644 --- a/libyul/optimiser/UnusedStoreEliminator.cpp +++ b/libyul/optimiser/UnusedStoreEliminator.cpp @@ -105,10 +105,13 @@ void UnusedStoreEliminator::operator()(FunctionCall const& _functionCall) else sideEffects = m_controlFlowSideEffects.at(_functionCall.functionName.name); + if (sideEffects.canTerminate) + changeUndecidedTo(State::Used, Location::Storage); if (!sideEffects.canContinue) { changeUndecidedTo(State::Unused, Location::Memory); - changeUndecidedTo(sideEffects.canTerminate ? State::Used : State::Unused, Location::Storage); + if (!sideEffects.canTerminate) + changeUndecidedTo(State::Unused, Location::Storage); } } diff --git a/test/libsolidity/semanticTests/unused_store_storage_removal_bug.sol b/test/libsolidity/semanticTests/unused_store_storage_removal_bug.sol new file mode 100644 index 000000000..87d4ef429 --- /dev/null +++ b/test/libsolidity/semanticTests/unused_store_storage_removal_bug.sol @@ -0,0 +1,15 @@ +contract C { + uint public x; + function f() public { + x = 1; // This write used to be removed by the Yul optimizer due to the StorageWriteRemovalBeforeConditionalTermination bug. + g(); + x = 2; + } + function g() internal { + if (msg.data.length > 4) return; + assembly { return(0, 0) } + } +} +// ---- +// f() -> +// x() -> 1 diff --git a/test/libyul/yulOptimizerTests/unusedStoreEliminator/function_side_effects_3.yul b/test/libyul/yulOptimizerTests/unusedStoreEliminator/store_before_conditionally_terminating_function_call.yul similarity index 79% rename from test/libyul/yulOptimizerTests/unusedStoreEliminator/function_side_effects_3.yul rename to test/libyul/yulOptimizerTests/unusedStoreEliminator/store_before_conditionally_terminating_function_call.yul index a66345423..44fbb7011 100644 --- a/test/libyul/yulOptimizerTests/unusedStoreEliminator/function_side_effects_3.yul +++ b/test/libyul/yulOptimizerTests/unusedStoreEliminator/store_before_conditionally_terminating_function_call.yul @@ -5,7 +5,7 @@ } let x := 0 let y := 1 - sstore(x, y) + sstore(x, y) // used to be removed due to the StorageWriteRemovalBeforeConditionalTermination conditionallyStop() sstore(x, y) } @@ -16,6 +16,7 @@ // { // let x := 0 // let y := 1 +// sstore(x, y) // conditionallyStop() // sstore(x, y) // } diff --git a/test/libyul/yulOptimizerTests/unusedStoreEliminator/store_before_conditionally_terminating_function_call_complex_control_flow.yul b/test/libyul/yulOptimizerTests/unusedStoreEliminator/store_before_conditionally_terminating_function_call_complex_control_flow.yul new file mode 100644 index 000000000..dd88f4973 --- /dev/null +++ b/test/libyul/yulOptimizerTests/unusedStoreEliminator/store_before_conditionally_terminating_function_call_complex_control_flow.yul @@ -0,0 +1,60 @@ +{ + function conditionallyStop() { + if calldataload(0) { leave } + return(0, 0) + } + function g() { + let a := 0 + let b := 1 + sstore(a, b) + } + let x := 0 + let y := 1 + let z := 2 + switch calldataload(64) + case 0 { + sstore(z, x) + g() + } + default { + sstore(x, z) // used to be removed due to the StorageWriteRemovalBeforeConditionalTermination + } + conditionallyStop() + switch calldataload(32) + case 0 { + revert(0, 0) + } + default { + sstore(x, z) + } +} +// ---- +// step: unusedStoreEliminator +// +// { +// { +// let x := 0 +// let y := 1 +// let z := 2 +// switch calldataload(64) +// case 0 { +// sstore(z, x) +// g() +// } +// default { sstore(x, z) } +// conditionallyStop() +// switch calldataload(32) +// case 0 { revert(0, 0) } +// default { sstore(x, z) } +// } +// function conditionallyStop() +// { +// if calldataload(0) { leave } +// return(0, 0) +// } +// function g() +// { +// let a := 0 +// sstore(a, 1) +// } +// } diff --git a/test/libyul/yulOptimizerTests/unusedStoreEliminator/store_before_conditionally_terminating_function_call_revert.yul b/test/libyul/yulOptimizerTests/unusedStoreEliminator/store_before_conditionally_terminating_function_call_revert.yul new file mode 100644 index 000000000..fc45bb64c --- /dev/null +++ b/test/libyul/yulOptimizerTests/unusedStoreEliminator/store_before_conditionally_terminating_function_call_revert.yul @@ -0,0 +1,27 @@ +{ + function conditionallyStop() { + if calldataload(0) { leave } + return(0, 0) + } + let x := 0 + let y := 1 + sstore(x, y) // used to be removed due to the to the StorageWriteRemovalBeforeConditionalTermination bug + conditionallyStop() + revert(0,0) +} +// ---- +// step: unusedStoreEliminator +// +// { +// { +// let x := 0 +// sstore(x, 1) +// conditionallyStop() +// revert(0, 0) +// } +// function conditionallyStop() +// { +// if calldataload(0) { leave } +// return(0, 0) +// } +// } diff --git a/test/libyul/yulOptimizerTests/unusedStoreEliminator/store_before_indirectly_conditionally_terminating_function_call.yul b/test/libyul/yulOptimizerTests/unusedStoreEliminator/store_before_indirectly_conditionally_terminating_function_call.yul new file mode 100644 index 000000000..1d15078eb --- /dev/null +++ b/test/libyul/yulOptimizerTests/unusedStoreEliminator/store_before_indirectly_conditionally_terminating_function_call.yul @@ -0,0 +1,33 @@ +{ + function conditionallyStop() { + if calldataload(0) { leave } + returnEmpty() + } + function returnEmpty() { + return(0, 0) + } + let x := 0 + let y := 1 + sstore(x, y) // used to be removed due to a bug + conditionallyStop() + sstore(x, y) +} +// ---- +// step: unusedStoreEliminator +// +// { +// { +// let x := 0 +// let y := 1 +// sstore(x, y) +// conditionallyStop() +// sstore(x, y) +// } +// function conditionallyStop() +// { +// if calldataload(0) { leave } +// returnEmpty() +// } +// function returnEmpty() +// { return(0, 0) } +// } diff --git a/test/libyul/yulOptimizerTests/unusedStoreEliminator/store_before_unconditionally_terminating_function_call.yul b/test/libyul/yulOptimizerTests/unusedStoreEliminator/store_before_unconditionally_terminating_function_call.yul new file mode 100644 index 000000000..c104e4d9b --- /dev/null +++ b/test/libyul/yulOptimizerTests/unusedStoreEliminator/store_before_unconditionally_terminating_function_call.yul @@ -0,0 +1,23 @@ +{ + function neverStop() { + if calldataload(0) { leave } // prevent inlining + } + let x := 0 + let y := 1 + sstore(x, y) // should be removed + neverStop() + sstore(x, y) +} +// ---- +// step: unusedStoreEliminator +// +// { +// { +// let x := 0 +// let y := 1 +// neverStop() +// sstore(x, y) +// } +// function neverStop() +// { if calldataload(0) { leave } } +// }