Merge pull request #8034 from ethereum/release_0515

chriseth · web-flow · commit 6a57276fdc9a · 2019-12-17T17:27:41.000+01:00
Backport yul loop fix into 0.5.
diff --git 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.14")
+set(PROJECT_VERSION "0.5.15")
 project(solidity VERSION ${PROJECT_VERSION} LANGUAGES C CXX)
 
 include(TestBigEndian)
diff --git a/Changelog.md b/Changelog.md
@@ -1,3 +1,9 @@
+### 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
@@ -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
@@ -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
@@ -50,9 +50,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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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))
+// }

Original file line number	Diff line number	Diff line change
`@@ -50,9 +50,10 @@ void CallGraphGenerator::operator()(FunctionCall const& _functionCall)`
`50`	`50`	`ASTWalker::operator()(_functionCall);`
`51`	`51`	`}`
`52`	`52`
`53`		`-void CallGraphGenerator::operator()(ForLoop const&)`
	`53`	`+void CallGraphGenerator::operator()(ForLoop const& _forLoop)`
`54`	`54`	`{`
`55`	`55`	`m_callGraph.functionsWithLoops.insert(m_currentFunction);`
	`56`	`+ ASTWalker::operator()(_forLoop);`
`56`	`57`	`}`
`57`	`58`
`58`	`59`	`void CallGraphGenerator::operator()(FunctionDefinition const& _functionDefinition)`