Merge pull request #7866 from ethereum/considerInfiniteLoopsNonMovable

[Yul] Mark recursive functions and functions containing loops to be non-movable.

Changelog.md

@@ -20,6 +20,7 @@ Bugfixes:
  * SMTChecker: Fix internal error when using ``abi.decode``.
  * SMTChecker: Fix internal error when using arrays or mappings of functions.
  * SMTChecker: Fix internal error in array of structs type.
+ * Yul: Consider infinite loops and recursion to be not removable.
 
 
 

libyul/optimiser/CallGraphGenerator.cpp

@@ -29,7 +29,7 @@ using namespace std;
 using namespace dev;
 using namespace yul;
 
-map<YulString, set<YulString>> CallGraphGenerator::callGraph(Block const& _ast)
+CallGraph CallGraphGenerator::callGraph(Block const& _ast)
 {
 	CallGraphGenerator gen;
 	gen(_ast);
@@ -40,28 +40,33 @@ void CallGraphGenerator::operator()(FunctionalInstruction const& _functionalInst
 {
 	string name = dev::eth::instructionInfo(_functionalInstruction.instruction).name;
 	std::transform(name.begin(), name.end(), name.begin(), [](unsigned char _c) { return tolower(_c); });
-	m_callGraph[m_currentFunction].insert(YulString{name});
+	m_callGraph.functionCalls[m_currentFunction].insert(YulString{name});
 	ASTWalker::operator()(_functionalInstruction);
 }
 
 void CallGraphGenerator::operator()(FunctionCall const& _functionCall)
 {
-	m_callGraph[m_currentFunction].insert(_functionCall.functionName.name);
+	m_callGraph.functionCalls[m_currentFunction].insert(_functionCall.functionName.name);
 	ASTWalker::operator()(_functionCall);
 }
 
+void CallGraphGenerator::operator()(ForLoop const&)
+{
+	m_callGraph.functionsWithLoops.insert(m_currentFunction);
+}
+
 void CallGraphGenerator::operator()(FunctionDefinition const& _functionDefinition)
 {
 	YulString previousFunction = m_currentFunction;
 	m_currentFunction = _functionDefinition.name;
-	yulAssert(m_callGraph.count(m_currentFunction) == 0, "");
+	yulAssert(m_callGraph.functionCalls.count(m_currentFunction) == 0, "");
-	m_callGraph[m_currentFunction] = {};
+	m_callGraph.functionCalls[m_currentFunction] = {};
 	ASTWalker::operator()(_functionDefinition);
 	m_currentFunction = previousFunction;
 }
 
 CallGraphGenerator::CallGraphGenerator()
 {
-	m_callGraph[YulString{}] = {};
+	m_callGraph.functionCalls[YulString{}] = {};
 }
 

libyul/optimiser/CallGraphGenerator.h

@@ -31,25 +31,34 @@
 namespace yul
 {
 
+struct CallGraph
+{
+	std::map<YulString, std::set<YulString>> functionCalls;
+	std::set<YulString> functionsWithLoops;
+};
+
 /**
  * Specific AST walker that generates the call graph.
  *
+ * It also generates information about which functions contain for loops.
+ *
  * The outermost (non-function) context is denoted by the empty string.
  */
 class CallGraphGenerator: public ASTWalker
 {
 public:
-	static std::map<YulString, std::set<YulString>> callGraph(Block const& _ast);
+	static CallGraph callGraph(Block const& _ast);
 
 	using ASTWalker::operator();
 	void operator()(FunctionalInstruction const& _functionalInstruction) override;
 	void operator()(FunctionCall const& _functionCall) override;
+	void operator()(ForLoop const& _forLoop) override;
 	void operator()(FunctionDefinition const& _functionDefinition) override;
 
 private:
 	CallGraphGenerator();
 
-	std::map<YulString, std::set<YulString>> m_callGraph;
+	CallGraph m_callGraph;
 	/// The name of the function we are currently visiting during traversal.
 	YulString m_currentFunction;
 };

libyul/optimiser/Semantics.cpp

@@ -108,11 +108,44 @@ void MSizeFinder::operator()(FunctionCall const& _functionCall)
 
 map<YulString, SideEffects> SideEffectsPropagator::sideEffects(
 	Dialect const& _dialect,
-	map<YulString, std::set<YulString>> const& _directCallGraph
+	CallGraph const& _directCallGraph
 )
 {
+	// Any loop currently makes a function non-movable, because
+	// it could be a non-terminating loop.
+	// The same is true for any function part of a call cycle.
+	// In the future, we should refine that, because the property
+	// is actually a bit different from "not movable".
+
 	map<YulString, SideEffects> ret;
-	for (auto const& call: _directCallGraph)
+	for (auto const& function: _directCallGraph.functionsWithLoops)
+	{
+		ret[function].movable = false;
+		ret[function].sideEffectFree = false;
+		ret[function].sideEffectFreeIfNoMSize = false;
+	}
+
+	// Detect recursive functions.
+	for (auto const& call: _directCallGraph.functionCalls)
+	{
+		// TODO we could shortcut the search as soon as we find a
+		// function that has as bad side-effects as we can
+		// ever achieve via recursion.
+		auto search = [&](YulString const& _functionName, CycleDetector<YulString>& _cycleDetector, size_t) {
+			for (auto const& callee: _directCallGraph.functionCalls.at(_functionName))
+				if (!_dialect.builtin(callee))
+					if (_cycleDetector.run(callee))
+						return;
+		};
+		if (CycleDetector<YulString>(search).run(call.first))
+		{
+			ret[call.first].movable = false;
+			ret[call.first].sideEffectFree = false;
+			ret[call.first].sideEffectFreeIfNoMSize = false;
+		}
+	}
+
+	for (auto const& call: _directCallGraph.functionCalls)
 	{
 		YulString funName = call.first;
 		SideEffects sideEffects;
@@ -123,11 +156,15 @@ map<YulString, SideEffects> SideEffectsPropagator::sideEffects(
 				if (BuiltinFunction const* f = _dialect.builtin(_function))
 					sideEffects += f->sideEffects;
 				else
-					for (YulString callee: _directCallGraph.at(_function))
+				{
+					if (ret.count(_function))
+						sideEffects += ret[_function];
+					for (YulString callee: _directCallGraph.functionCalls.at(_function))
 						_addChild(callee);
+				}
 			}
 		);
-		ret[funName] = sideEffects;
+		ret[funName] += sideEffects;
 	}
 	return ret;
 }

libyul/optimiser/Semantics.h

@@ -22,6 +22,7 @@
 
 #include <libyul/optimiser/ASTWalker.h>
 #include <libyul/SideEffects.h>
+#include <libyul/optimiser/CallGraphGenerator.h>
 #include <libyul/AsmData.h>
 
 #include <set>
@@ -86,7 +87,7 @@ class SideEffectsPropagator
 public:
 	static std::map<YulString, SideEffects> sideEffects(
 		Dialect const& _dialect,
-		std::map<YulString, std::set<YulString>> const& _directCallGraph
+		CallGraph const& _directCallGraph
 	);
 };
 

test/libyul/FunctionSideEffects.cpp

@@ -87,7 +87,7 @@ TestCase::TestResult FunctionSideEffects::run(ostream& _stream, string const& _l
 
 	m_obtainedResult.clear();
 	for (auto const& fun: functionSideEffectsStr)
-		m_obtainedResult += fun.first + ": " + fun.second + "\n";
+		m_obtainedResult += fun.first + ":" + (fun.second.empty() ? "" : " ") + fun.second + "\n";
 
 	if (m_expectation != m_obtainedResult)
 	{

test/libyul/functionSideEffects/cyclic_graph.yul

@@ -5,6 +5,6 @@
 }
 // ----
 // : movable, sideEffectFree, sideEffectFreeIfNoMSize
-// a: movable, sideEffectFree, sideEffectFreeIfNoMSize
+// a:
-// b: movable, sideEffectFree, sideEffectFreeIfNoMSize
+// b:
-// c: movable, sideEffectFree, sideEffectFreeIfNoMSize
+// c:

test/libyul/functionSideEffects/doubly_recursive_function.yul

@@ -4,5 +4,5 @@
 }
 // ----
 // : movable, sideEffectFree, sideEffectFreeIfNoMSize
-// a: movable, sideEffectFree, sideEffectFreeIfNoMSize
+// a:
-// b: movable, sideEffectFree, sideEffectFreeIfNoMSize
+// b:

test/libyul/functionSideEffects/recursive_function.yul

@@ -3,4 +3,4 @@
 }
 // ----
 // : movable, sideEffectFree, sideEffectFreeIfNoMSize
-// a: movable, sideEffectFree, sideEffectFreeIfNoMSize
+// a:

test/libyul/functionSideEffects/with_loop.yul

@@ -0,0 +1,9 @@
+{
+  function f() -> x { x := g() }
+  function g() -> x { for {} 1 {} {} }
+  pop(f())
+}
+// ----
+// :
+// f:
+// g:

test/libyul/yulOptimizerTests/fullSuite/abi_example1.yul

@@ -477,16 +477,15 @@
 //             pos := add(pos, 0x60)
 //         }
 //         let _3 := mload(64)
-//         let _4 := mload(0x20)
+//         if slt(sub(_3, length), 128) { revert(_1, _1) }
-//         if slt(sub(_3, _4), 128) { revert(_1, _1) }
+//         let offset := calldataload(add(length, 64))
-//         let offset := calldataload(add(_4, 64))
+//         let _4 := 0xffffffffffffffff
-//         let _5 := 0xffffffffffffffff
+//         if gt(offset, _4) { revert(_1, _1) }
-//         if gt(offset, _5) { revert(_1, _1) }
+//         let value2 := abi_decode_t_array$_t_uint256_$dyn_memory_ptr(add(length, offset), _3)
-//         let value2 := abi_decode_t_array$_t_uint256_$dyn_memory_ptr(add(_4, offset), _3)
+//         let offset_1 := calldataload(add(length, 0x60))
-//         let offset_1 := calldataload(add(_4, 0x60))
+//         if gt(offset_1, _4) { revert(_1, _1) }
-//         if gt(offset_1, _5) { revert(_1, _1) }
+//         let value3 := abi_decode_t_array$_t_array$_t_uint256_$2_memory_$dyn_memory_ptr(add(length, offset_1), _3)
-//         let value3 := abi_decode_t_array$_t_array$_t_uint256_$2_memory_$dyn_memory_ptr(add(_4, offset_1), _3)
+//         sstore(calldataload(length), calldataload(add(length, 0x20)))
-//         sstore(calldataload(_4), calldataload(add(_4, 0x20)))
 //         sstore(value2, value3)
 //         sstore(_1, pos)
 //     }

test/libyul/yulOptimizerTests/fullSuite/no_move_loop_orig.yul

@@ -0,0 +1,24 @@
+{
+  for {} msize() {
+    function foo_s_0() -> x_1 { for {} caller() {} {} }
+    // x_3 used to be a movable loop invariant because `foo_s_0()` used to be movable
+    let x_3 := foo_s_0()
+    mstore(192, x_3)
+  }
+  {}
+}
+// ====
+// step: fullSuite
+// ----
+// {
+//     {
+//         for { }
+//         1
+//         {
+//             for { } iszero(iszero(caller())) { }
+//             { }
+//             mstore(192, 0)
+//         }
+//         { if iszero(msize()) { break } }
+//     }
+// }

test/libyul/yulOptimizerTests/loopInvariantCodeMotion/no_move_loop.yul

@@ -0,0 +1,29 @@
+{
+  function f() -> x { x := g() }
+  function g() -> x { for {} 1 {} {} }
+
+  let b := 1
+  for { let a := 1 } iszero(eq(a, 10)) { a := add(a, 1) } {
+    let t := f()
+    let q := g()
+  }
+}
+// ====
+// step: loopInvariantCodeMotion
+// ----
+// {
+//     function f() -> x
+//     { x := g() }
+//     function g() -> x_1
+//     {
+//         for { } 1 { }
+//         { }
+//     }
+//     let b := 1
+//     let a := 1
+//     for { } iszero(eq(a, 10)) { a := add(a, 1) }
+//     {
+//         let t := f()
+//         let q := g()
+//     }
+// }

test/libyul/yulOptimizerTests/loopInvariantCodeMotion/no_move_recursive_function.yul

@@ -0,0 +1,26 @@
+{
+  function f() -> x { x := g() }
+  function g() -> x { x := g() }
+
+  let b := 1
+  for { let a := 1 } iszero(eq(a, 10)) { a := add(a, 1) } {
+    let t := f()
+    let q := g()
+  }
+}
+// ====
+// step: loopInvariantCodeMotion
+// ----
+// {
+//     function f() -> x
+//     { x := g() }
+//     function g() -> x_1
+//     { x_1 := g() }
+//     let b := 1
+//     let a := 1
+//     for { } iszero(eq(a, 10)) { a := add(a, 1) }
+//     {
+//         let t := f()
+//         let q := g()
+//     }
+// }