Bugfix and tests.

Co-authored-by: Kamil Śliwak <[email protected]>

Author: ekpyron

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);
 	}
 }
 

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

test/libyul/yulOptimizerTests/unusedStoreEliminator/function_side_effects_3.yul renamed to 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)
 //     }

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)
+//     }
+// }

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)
+//     }
+// }

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) }
+// }

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 } }
+// }