From bbeb093abf5048f134aca8d90b16c4a9d7f39679 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 2 Dec 2019 12:33:24 +0100 Subject: [PATCH] Detect loops in call graph generator. --- Changelog.md | 1 + libyul/optimiser/CallGraphGenerator.cpp | 17 ++++--- libyul/optimiser/CallGraphGenerator.h | 13 +++++- libyul/optimiser/Semantics.cpp | 45 +++++++++++++++++-- libyul/optimiser/Semantics.h | 3 +- .../functionSideEffects/cyclic_graph.yul | 6 +-- .../doubly_recursive_function.yul | 4 +- .../recursive_function.yul | 2 +- test/libyul/functionSideEffects/with_loop.yul | 9 ++++ .../fullSuite/abi_example1.yul | 19 ++++---- .../fullSuite/no_move_loop_orig.yul | 24 ++++++++++ .../loopInvariantCodeMotion/no_move_loop.yul | 29 ++++++++++++ .../no_move_recursive_function.yul | 26 +++++++++++ 13 files changed, 169 insertions(+), 29 deletions(-) create mode 100644 test/libyul/functionSideEffects/with_loop.yul create mode 100644 test/libyul/yulOptimizerTests/fullSuite/no_move_loop_orig.yul create mode 100644 test/libyul/yulOptimizerTests/loopInvariantCodeMotion/no_move_loop.yul create mode 100644 test/libyul/yulOptimizerTests/loopInvariantCodeMotion/no_move_recursive_function.yul diff --git a/Changelog.md b/Changelog.md index af170ed01..b95736856 100644 --- a/Changelog.md +++ b/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. diff --git a/libyul/optimiser/CallGraphGenerator.cpp b/libyul/optimiser/CallGraphGenerator.cpp index 0c11464ee..b1dc62f97 100644 --- a/libyul/optimiser/CallGraphGenerator.cpp +++ b/libyul/optimiser/CallGraphGenerator.cpp @@ -29,7 +29,7 @@ using namespace std; using namespace dev; using namespace yul; -map> 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, ""); - m_callGraph[m_currentFunction] = {}; + yulAssert(m_callGraph.functionCalls.count(m_currentFunction) == 0, ""); + m_callGraph.functionCalls[m_currentFunction] = {}; ASTWalker::operator()(_functionDefinition); m_currentFunction = previousFunction; } CallGraphGenerator::CallGraphGenerator() { - m_callGraph[YulString{}] = {}; + m_callGraph.functionCalls[YulString{}] = {}; } diff --git a/libyul/optimiser/CallGraphGenerator.h b/libyul/optimiser/CallGraphGenerator.h index 4dc420900..fb7e8d52c 100644 --- a/libyul/optimiser/CallGraphGenerator.h +++ b/libyul/optimiser/CallGraphGenerator.h @@ -31,25 +31,34 @@ namespace yul { +struct CallGraph +{ + std::map> functionCalls; + std::set 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> 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> m_callGraph; + CallGraph m_callGraph; /// The name of the function we are currently visiting during traversal. YulString m_currentFunction; }; diff --git a/libyul/optimiser/Semantics.cpp b/libyul/optimiser/Semantics.cpp index 62aaa0e0f..0d79e09c5 100644 --- a/libyul/optimiser/Semantics.cpp +++ b/libyul/optimiser/Semantics.cpp @@ -108,11 +108,44 @@ void MSizeFinder::operator()(FunctionCall const& _functionCall) map SideEffectsPropagator::sideEffects( Dialect const& _dialect, - map> 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 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& _cycleDetector, size_t) { + for (auto const& callee: _directCallGraph.functionCalls.at(_functionName)) + if (!_dialect.builtin(callee)) + if (_cycleDetector.run(callee)) + return; + }; + if (CycleDetector(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 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; } diff --git a/libyul/optimiser/Semantics.h b/libyul/optimiser/Semantics.h index 8953ab321..8c266910a 100644 --- a/libyul/optimiser/Semantics.h +++ b/libyul/optimiser/Semantics.h @@ -22,6 +22,7 @@ #include #include +#include #include #include @@ -86,7 +87,7 @@ class SideEffectsPropagator public: static std::map sideEffects( Dialect const& _dialect, - std::map> const& _directCallGraph + CallGraph const& _directCallGraph ); }; diff --git a/test/libyul/functionSideEffects/cyclic_graph.yul b/test/libyul/functionSideEffects/cyclic_graph.yul index 742d74087..0b5bb6d7f 100644 --- a/test/libyul/functionSideEffects/cyclic_graph.yul +++ b/test/libyul/functionSideEffects/cyclic_graph.yul @@ -5,6 +5,6 @@ } // ---- // : movable, sideEffectFree, sideEffectFreeIfNoMSize -// a: movable, sideEffectFree, sideEffectFreeIfNoMSize -// b: movable, sideEffectFree, sideEffectFreeIfNoMSize -// c: movable, sideEffectFree, sideEffectFreeIfNoMSize +// a: +// b: +// c: diff --git a/test/libyul/functionSideEffects/doubly_recursive_function.yul b/test/libyul/functionSideEffects/doubly_recursive_function.yul index cbc594f38..82a543b81 100644 --- a/test/libyul/functionSideEffects/doubly_recursive_function.yul +++ b/test/libyul/functionSideEffects/doubly_recursive_function.yul @@ -4,5 +4,5 @@ } // ---- // : movable, sideEffectFree, sideEffectFreeIfNoMSize -// a: movable, sideEffectFree, sideEffectFreeIfNoMSize -// b: movable, sideEffectFree, sideEffectFreeIfNoMSize +// a: +// b: diff --git a/test/libyul/functionSideEffects/recursive_function.yul b/test/libyul/functionSideEffects/recursive_function.yul index cacefefb4..05e474eb0 100644 --- a/test/libyul/functionSideEffects/recursive_function.yul +++ b/test/libyul/functionSideEffects/recursive_function.yul @@ -3,4 +3,4 @@ } // ---- // : movable, sideEffectFree, sideEffectFreeIfNoMSize -// a: movable, sideEffectFree, sideEffectFreeIfNoMSize +// a: diff --git a/test/libyul/functionSideEffects/with_loop.yul b/test/libyul/functionSideEffects/with_loop.yul new file mode 100644 index 000000000..a1deb26f5 --- /dev/null +++ b/test/libyul/functionSideEffects/with_loop.yul @@ -0,0 +1,9 @@ +{ + function f() -> x { x := g() } + function g() -> x { for {} 1 {} {} } + pop(f()) +} +// ---- +// : +// f: +// g: diff --git a/test/libyul/yulOptimizerTests/fullSuite/abi_example1.yul b/test/libyul/yulOptimizerTests/fullSuite/abi_example1.yul index 0d37cf1b9..ab1ebb10d 100644 --- a/test/libyul/yulOptimizerTests/fullSuite/abi_example1.yul +++ b/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, _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))) +// 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))) // sstore(value2, value3) // sstore(_1, pos) // } diff --git a/test/libyul/yulOptimizerTests/fullSuite/no_move_loop_orig.yul b/test/libyul/yulOptimizerTests/fullSuite/no_move_loop_orig.yul new file mode 100644 index 000000000..5562bd85e --- /dev/null +++ b/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 } } +// } +// } diff --git a/test/libyul/yulOptimizerTests/loopInvariantCodeMotion/no_move_loop.yul b/test/libyul/yulOptimizerTests/loopInvariantCodeMotion/no_move_loop.yul new file mode 100644 index 000000000..7f842bbdd --- /dev/null +++ b/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() +// } +// } diff --git a/test/libyul/yulOptimizerTests/loopInvariantCodeMotion/no_move_recursive_function.yul b/test/libyul/yulOptimizerTests/loopInvariantCodeMotion/no_move_recursive_function.yul new file mode 100644 index 000000000..5f3eeb7ac --- /dev/null +++ b/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() +// } +// }