Merge pull request #12226 from ethereum/nondisambiguatedsideeffects

Control flow side effects on non-disambiguated source.

libyul/CMakeLists.txt

@@ -40,6 +40,8 @@ add_library(yul
 	Dialect.cpp
 	Dialect.h
 	Exceptions.h
+	FunctionReferenceResolver.cpp
+	FunctionReferenceResolver.h
 	Object.cpp
 	Object.h
 	ObjectParser.cpp

libyul/ControlFlowSideEffectsCollector.cpp

@@ -20,6 +20,7 @@
 
 #include <libyul/AST.h>
 #include <libyul/Dialect.h>
+#include <libyul/FunctionReferenceResolver.h>
 
 #include <libsolutil/Common.h>
 #include <libsolutil/CommonData.h>
@@ -35,16 +36,15 @@ using namespace solidity::yul;
 
 ControlFlowBuilder::ControlFlowBuilder(Block const& _ast)
 {
-	for (auto const& statement: _ast.statements)
+	m_currentNode = newNode();
-		if (auto const* function = get_if<FunctionDefinition>(&statement))
+	(*this)(_ast);
-			(*this)(*function);
 }
 
 void ControlFlowBuilder::operator()(FunctionCall const& _functionCall)
 {
 	walkVector(_functionCall.arguments | ranges::views::reverse);
 	newConnectedNode();
-	m_currentNode->functionCall = _functionCall.functionName.name;
+	m_currentNode->functionCall = &_functionCall;
 }
 
 void ControlFlowBuilder::operator()(If const& _if)
@@ -78,7 +78,9 @@ void ControlFlowBuilder::operator()(Switch const& _switch)
 void ControlFlowBuilder::operator()(FunctionDefinition const& _function)
 {
 	ScopedSaveAndRestore currentNode(m_currentNode, nullptr);
-	yulAssert(!m_leave && !m_break && !m_continue, "Function hoister has not been used.");
+	ScopedSaveAndRestore leave(m_leave, nullptr);
+	ScopedSaveAndRestore _break(m_break, nullptr);
+	ScopedSaveAndRestore _continue(m_continue, nullptr);
 
 	FunctionFlow flow;
 	flow.exit = newNode();
@@ -90,7 +92,7 @@ void ControlFlowBuilder::operator()(FunctionDefinition const& _function)
 
 	m_currentNode->successors.emplace_back(flow.exit);
 
-	m_functionFlows[_function.name] = move(flow);
+	m_functionFlows[&_function] = move(flow);
 
 	m_leave = nullptr;
 }
@@ -164,14 +166,17 @@ ControlFlowSideEffectsCollector::ControlFlowSideEffectsCollector(
 	Block const& _ast
 ):
 	m_dialect(_dialect),
-	m_cfgBuilder(_ast)
+	m_cfgBuilder(_ast),
+	m_functionReferences(FunctionReferenceResolver{_ast}.references())
 {
-	for (auto&& [name, flow]: m_cfgBuilder.functionFlows())
+	for (auto&& [function, flow]: m_cfgBuilder.functionFlows())
 	{
 		yulAssert(!flow.entry->functionCall);
-		m_processedNodes[name] = {};
+		yulAssert(function);
-		m_pendingNodes[name].push_front(flow.entry);
+		m_processedNodes[function] = {};
-		m_functionSideEffects[name] = {false, false, false};
+		m_pendingNodes[function].push_front(flow.entry);
+		m_functionSideEffects[function] = {false, false, false};
+		m_functionCalls[function] = {};
 	}
 
 	// Process functions while we have progress. For now, we are only interested
@@ -180,8 +185,8 @@ ControlFlowSideEffectsCollector::ControlFlowSideEffectsCollector(
 	while (progress)
 	{
 		progress = false;
-		for (auto const& functionName: m_pendingNodes | ranges::views::keys)
+		for (FunctionDefinition const* function: m_pendingNodes | ranges::views::keys)
-			if (processFunction(functionName))
+			if (processFunction(*function))
 				progress = true;
 	}
 
@@ -190,57 +195,64 @@ ControlFlowSideEffectsCollector::ControlFlowSideEffectsCollector(
 	// If we have not set `canContinue` by now, the function's exit
 	// is not reachable.
 
-	for (auto&& [functionName, calls]: m_functionCalls)
+	// Now it is sufficient to handle the reachable function calls (`m_functionCalls`),
+	// we do not have to consider the control-flow graph anymore.
+	for (auto&& [function, calls]: m_functionCalls)
 	{
-		ControlFlowSideEffects& sideEffects = m_functionSideEffects[functionName];
+		yulAssert(function);
-		auto _visit = [&, visited = std::set<YulString>{}](YulString _function, auto&& _recurse) mutable {
+		ControlFlowSideEffects& functionSideEffects = m_functionSideEffects[function];
-			if (sideEffects.canTerminate && sideEffects.canRevert)
+		auto _visit = [&, visited = std::set<FunctionDefinition const*>{}](FunctionDefinition const& _function, auto&& _recurse) mutable {
+			// Worst side-effects already, stop searching.
+			if (functionSideEffects.canTerminate && functionSideEffects.canRevert)
 				return;
-			if (!visited.insert(_function).second)
+			if (!visited.insert(&_function).second)
 				return;
 
-			ControlFlowSideEffects const* calledSideEffects = nullptr;
+			for (FunctionCall const* call: m_functionCalls.at(&_function))
-			if (BuiltinFunction const* f = _dialect.builtin(_function))
+			{
-				calledSideEffects = &f->controlFlowSideEffects;
+				ControlFlowSideEffects const& calledSideEffects = sideEffects(*call);
-			else
+				if (calledSideEffects.canTerminate)
-				calledSideEffects = &m_functionSideEffects.at(_function);
+					functionSideEffects.canTerminate = true;
+				if (calledSideEffects.canRevert)
+					functionSideEffects.canRevert = true;
 
-			if (calledSideEffects->canTerminate)
+				if (m_functionReferences.count(call))
-				sideEffects.canTerminate = true;
+					_recurse(*m_functionReferences.at(call), _recurse);
-			if (calledSideEffects->canRevert)
+			}
-				sideEffects.canRevert = true;
-
-			set<YulString> emptySet;
-			for (YulString callee: util::valueOrDefault(m_functionCalls, _function, emptySet))
-				_recurse(callee, _recurse);
 		};
-		for (auto const& call: calls)
+		_visit(*function, _visit);
-			_visit(call, _visit);
 	}
-
 }
 
-bool ControlFlowSideEffectsCollector::processFunction(YulString _name)
+map<YulString, ControlFlowSideEffects> ControlFlowSideEffectsCollector::functionSideEffectsNamed() const
+{
+	map<YulString, ControlFlowSideEffects> result;
+	for (auto&& [function, sideEffects]: m_functionSideEffects)
+		yulAssert(result.insert({function->name, sideEffects}).second);
+	return result;
+}
+
+bool ControlFlowSideEffectsCollector::processFunction(FunctionDefinition const& _function)
 {
 	bool progress = false;
-	while (ControlFlowNode const* node = nextProcessableNode(_name))
+	while (ControlFlowNode const* node = nextProcessableNode(_function))
 	{
-		if (node == m_cfgBuilder.functionFlows().at(_name).exit)
+		if (node == m_cfgBuilder.functionFlows().at(&_function).exit)
 		{
-			m_functionSideEffects[_name].canContinue = true;
+			m_functionSideEffects[&_function].canContinue = true;
 			return true;
 		}
 		for (ControlFlowNode const* s: node->successors)
-			recordReachabilityAndQueue(_name, s);
+			recordReachabilityAndQueue(_function, s);
 
 		progress = true;
 	}
 	return progress;
 }
 
-ControlFlowNode const* ControlFlowSideEffectsCollector::nextProcessableNode(YulString _functionName)
+ControlFlowNode const* ControlFlowSideEffectsCollector::nextProcessableNode(FunctionDefinition const& _function)
 {
-	std::list<ControlFlowNode const*>& nodes = m_pendingNodes[_functionName];
+	std::list<ControlFlowNode const*>& nodes = m_pendingNodes[&_function];
 	auto it = ranges::find_if(nodes, [this](ControlFlowNode const* _node) {
 		return !_node->functionCall || sideEffects(*_node->functionCall).canContinue;
 	});
@@ -252,22 +264,22 @@ ControlFlowNode const* ControlFlowSideEffectsCollector::nextProcessableNode(YulS
 	return node;
 }
 
-ControlFlowSideEffects const& ControlFlowSideEffectsCollector::sideEffects(YulString _functionName) const
+ControlFlowSideEffects const& ControlFlowSideEffectsCollector::sideEffects(FunctionCall const& _call) const
 {
-	if (auto const* builtin = m_dialect.builtin(_functionName))
+	if (auto const* builtin = m_dialect.builtin(_call.functionName.name))
 		return builtin->controlFlowSideEffects;
 	else
-		return m_functionSideEffects.at(_functionName);
+		return m_functionSideEffects.at(m_functionReferences.at(&_call));
 }
 
 void ControlFlowSideEffectsCollector::recordReachabilityAndQueue(
-	YulString _functionName,
+	FunctionDefinition const& _function,
 	ControlFlowNode const* _node
 )
 {
 	if (_node->functionCall)
-		m_functionCalls[_functionName].insert(*_node->functionCall);
+		m_functionCalls[&_function].insert(_node->functionCall);
-	if (m_processedNodes[_functionName].insert(_node).second)
+	if (m_processedNodes[&_function].insert(_node).second)
-		m_pendingNodes.at(_functionName).push_front(_node);
+		m_pendingNodes.at(&_function).push_front(_node);
 }
 

libyul/ControlFlowSideEffectsCollector.h

@@ -34,8 +34,8 @@ struct Dialect;
 struct ControlFlowNode
 {
 	std::vector<ControlFlowNode const*> successors;
-	/// Name of the called function if the node calls a function.
+	/// Function call AST node, if present.
-	std::optional<YulString> functionCall;
+	FunctionCall const* functionCall = nullptr;
 };
 
 /**
@@ -56,7 +56,7 @@ public:
 	/// Computes the control-flows of all function defined in the block.
 	/// Assumes the functions are hoisted to the topmost block.
 	explicit ControlFlowBuilder(Block const& _ast);
-	std::map<YulString, FunctionFlow> const& functionFlows() const { return m_functionFlows; }
+	std::map<FunctionDefinition const*, FunctionFlow> const& functionFlows() const { return m_functionFlows; }
 
 private:
 	using ASTWalker::operator();
@@ -79,12 +79,14 @@ private:
 	ControlFlowNode const* m_break = nullptr;
 	ControlFlowNode const* m_continue = nullptr;
 
-	std::map<YulString, FunctionFlow> m_functionFlows;
+	std::map<FunctionDefinition const*, FunctionFlow> m_functionFlows;
 };
 
 
 /**
- * Requires: Disambiguator, Function Hoister.
+ * Computes control-flow side-effects for user-defined functions.
+ * Source does not have to be disambiguated, unless you want the side-effects
+ * based on function names.
  */
 class ControlFlowSideEffectsCollector
 {
@@ -94,36 +96,43 @@ public:
 		Block const& _ast
 	);
 
-	std::map<YulString, ControlFlowSideEffects> const& functionSideEffects() const
+	std::map<FunctionDefinition const*, ControlFlowSideEffects> const& functionSideEffects() const
 	{
 		return m_functionSideEffects;
 	}
+	/// Returns the side effects by function name, requires unique function names.
+	std::map<YulString, ControlFlowSideEffects> functionSideEffectsNamed() const;
 private:
 
 	/// @returns false if nothing could be processed.
-	bool processFunction(YulString _name);
+	bool processFunction(FunctionDefinition const& _function);
 
 	/// @returns the next pending node of the function that is not
 	/// a function call to a function that might not continue.
 	/// De-queues the node or returns nullptr if no such node is found.
-	ControlFlowNode const* nextProcessableNode(YulString _functionName);
+	ControlFlowNode const* nextProcessableNode(FunctionDefinition const& _function);
 
 	/// @returns the side-effects of either a builtin call or a user defined function
 	/// call (as far as already computed).
-	ControlFlowSideEffects const& sideEffects(YulString _functionName) const;
+	ControlFlowSideEffects const& sideEffects(FunctionCall const& _call) const;
 
 	/// Queues the given node to be processed (if not already visited)
 	/// and if it is a function call, records that `_functionName` calls
 	/// `*_node->functionCall`.
-	void recordReachabilityAndQueue(YulString _functionName, ControlFlowNode const* _node);
+	void recordReachabilityAndQueue(FunctionDefinition const& _function, ControlFlowNode const* _node);
 
 	Dialect const& m_dialect;
 	ControlFlowBuilder m_cfgBuilder;
-	std::map<YulString, ControlFlowSideEffects> m_functionSideEffects;
+	/// Function references, but only for calls to user-defined functions.
-	std::map<YulString, std::list<ControlFlowNode const*>> m_pendingNodes;
+	std::map<FunctionCall const*, FunctionDefinition const*> m_functionReferences;
-	std::map<YulString, std::set<ControlFlowNode const*>> m_processedNodes;
+	/// Side effects of user-defined functions, is being constructod.
-	/// `x` is in `m_functionCalls[y]` if a direct call to `x` is reachable inside `y`
+	std::map<FunctionDefinition const*, ControlFlowSideEffects> m_functionSideEffects;
-	std::map<YulString, std::set<YulString>> m_functionCalls;
+	/// Control flow nodes still to process, per function.
+	std::map<FunctionDefinition const*, std::list<ControlFlowNode const*>> m_pendingNodes;
+	/// Control flow nodes already processed, per function.
+	std::map<FunctionDefinition const*, std::set<ControlFlowNode const*>> m_processedNodes;
+	/// Set of reachable function calls nodes in each function (including calls to builtins).
+	std::map<FunctionDefinition const*, std::set<FunctionCall const*>> m_functionCalls;
 };
 
 

libyul/FunctionReferenceResolver.cpp

@@ -0,0 +1,60 @@
+/*
+	This file is part of solidity.
+
+	solidity is free software: you can redistribute it and/or modify
+	it under the terms of the GNU General Public License as published by
+	the Free Software Foundation, either version 3 of the License, or
+	(at your option) any later version.
+
+	solidity is distributed in the hope that it will be useful,
+	but WITHOUT ANY WARRANTY; without even the implied warranty of
+	MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+	GNU General Public License for more details.
+
+	You should have received a copy of the GNU General Public License
+	along with solidity.  If not, see <http://www.gnu.org/licenses/>.
+*/
+// SPDX-License-Identifier: GPL-3.0
+
+#include <libyul/FunctionReferenceResolver.h>
+
+#include <libyul/AST.h>
+#include <libsolutil/CommonData.h>
+
+#include <range/v3/view/reverse.hpp>
+
+using namespace std;
+using namespace solidity::yul;
+using namespace solidity::util;
+
+FunctionReferenceResolver::FunctionReferenceResolver(Block const& _ast)
+{
+	(*this)(_ast);
+	yulAssert(m_scopes.empty());
+}
+
+void FunctionReferenceResolver::operator()(FunctionCall const& _functionCall)
+{
+	for (auto&& scope: m_scopes | ranges::views::reverse)
+		if (FunctionDefinition const** function = util::valueOrNullptr(scope, _functionCall.functionName.name))
+		{
+			m_functionReferences[&_functionCall] = *function;
+			break;
+		}
+
+	// If we did not find anything, it was a builtin call.
+
+	ASTWalker::operator()(_functionCall);
+}
+
+void FunctionReferenceResolver::operator()(Block const& _block)
+{
+	m_scopes.emplace_back();
+	for (auto const& statement: _block.statements)
+		if (auto const* function = get_if<FunctionDefinition>(&statement))
+			m_scopes.back()[function->name] = function;
+
+	ASTWalker::operator()(_block);
+
+	m_scopes.pop_back();
+}

libyul/FunctionReferenceResolver.h

@@ -0,0 +1,48 @@
+/*
+	This file is part of solidity.
+
+	solidity is free software: you can redistribute it and/or modify
+	it under the terms of the GNU General Public License as published by
+	the Free Software Foundation, either version 3 of the License, or
+	(at your option) any later version.
+
+	solidity is distributed in the hope that it will be useful,
+	but WITHOUT ANY WARRANTY; without even the implied warranty of
+	MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+	GNU General Public License for more details.
+
+	You should have received a copy of the GNU General Public License
+	along with solidity.  If not, see <http://www.gnu.org/licenses/>.
+*/
+// SPDX-License-Identifier: GPL-3.0
+
+#pragma once
+
+#include <libyul/optimiser/ASTWalker.h>
+
+namespace solidity::yul
+{
+
+/**
+ * Resolves references to user-defined functions in function calls.
+ * Assumes the code is correct, i.e. does not check for references to be valid or unique.
+ *
+ * Be careful not to iterate over the result - it is not deterministic.
+ */
+class FunctionReferenceResolver: private ASTWalker
+{
+public:
+	explicit FunctionReferenceResolver(Block const& _ast);
+	std::map<FunctionCall const*, FunctionDefinition const*> const& references() const { return m_functionReferences; }
+
+private:
+	using ASTWalker::operator();
+	void operator()(FunctionCall const& _functionCall) override;
+	void operator()(Block const& _block) override;
+
+	std::map<FunctionCall const*, FunctionDefinition const*> m_functionReferences;
+	std::vector<std::map<YulString, FunctionDefinition const*>> m_scopes;
+};
+
+
+}

libyul/optimiser/ConditionalSimplifier.cpp

@@ -29,8 +29,10 @@ using namespace solidity::util;
 
 void ConditionalSimplifier::run(OptimiserStepContext& _context, Block& _ast)
 {
-	ControlFlowSideEffectsCollector sideEffects(_context.dialect, _ast);
+	ConditionalSimplifier{
-	ConditionalSimplifier{_context.dialect, sideEffects.functionSideEffects()}(_ast);
+		_context.dialect,
+		ControlFlowSideEffectsCollector{_context.dialect, _ast}.functionSideEffectsNamed()
+	}(_ast);
 }
 
 void ConditionalSimplifier::operator()(Switch& _switch)

libyul/optimiser/ConditionalSimplifier.h

@@ -62,12 +62,12 @@ public:
 private:
 	explicit ConditionalSimplifier(
 		Dialect const& _dialect,
-		std::map<YulString, ControlFlowSideEffects> const& _sideEffects
+		std::map<YulString, ControlFlowSideEffects> _sideEffects
 	):
-		m_dialect(_dialect), m_functionSideEffects(_sideEffects)
+		m_dialect(_dialect), m_functionSideEffects(move(_sideEffects))
 	{}
 	Dialect const& m_dialect;
-	std::map<YulString, ControlFlowSideEffects> const& m_functionSideEffects;
+	std::map<YulString, ControlFlowSideEffects> m_functionSideEffects;
 };
 
 }

libyul/optimiser/ConditionalUnsimplifier.cpp

@@ -30,8 +30,10 @@ using namespace solidity::util;
 
 void ConditionalUnsimplifier::run(OptimiserStepContext& _context, Block& _ast)
 {
-	ControlFlowSideEffectsCollector sideEffects(_context.dialect, _ast);
+	ConditionalUnsimplifier{
-	ConditionalUnsimplifier{_context.dialect, sideEffects.functionSideEffects()}(_ast);
+		_context.dialect,
+		ControlFlowSideEffectsCollector{_context.dialect, _ast}.functionSideEffectsNamed()
+	}(_ast);
 }
 
 void ConditionalUnsimplifier::operator()(Switch& _switch)

libyul/optimiser/DeadCodeEliminator.cpp

@@ -40,7 +40,7 @@ void DeadCodeEliminator::run(OptimiserStepContext& _context, Block& _ast)
 	ControlFlowSideEffectsCollector sideEffects(_context.dialect, _ast);
 	DeadCodeEliminator{
 		_context.dialect,
-		sideEffects.functionSideEffects()
+		sideEffects.functionSideEffectsNamed()
 	}(_ast);
 }
 

libyul/optimiser/DeadCodeEliminator.h

@@ -23,6 +23,7 @@
 
 #include <libyul/optimiser/ASTWalker.h>
 #include <libyul/YulString.h>
+#include <libyul/ControlFlowSideEffects.h>
 
 #include <map>
 #include <set>
@@ -31,7 +32,6 @@ namespace solidity::yul
 {
 struct Dialect;
 struct OptimiserStepContext;
-struct ControlFlowSideEffects;
 
 /**
  * Optimisation stage that removes unreachable code
@@ -62,11 +62,11 @@ public:
 private:
 	DeadCodeEliminator(
 		Dialect const& _dialect,
-		std::map<YulString, ControlFlowSideEffects> const& _sideEffects
+		std::map<YulString, ControlFlowSideEffects> _sideEffects
-	): m_dialect(_dialect), m_functionSideEffects(_sideEffects) {}
+	): m_dialect(_dialect), m_functionSideEffects(move(_sideEffects)) {}
 
 	Dialect const& m_dialect;
-	std::map<YulString, ControlFlowSideEffects> const& m_functionSideEffects;
+	std::map<YulString, ControlFlowSideEffects> m_functionSideEffects;
 };
 
 }

test/libyul/ControlFlowSideEffectsTest.cpp

@@ -22,6 +22,7 @@
 #include <test/libyul/Common.h>
 
 #include <libyul/Object.h>
+#include <libyul/AST.h>
 #include <libyul/ControlFlowSideEffects.h>
 #include <libyul/ControlFlowSideEffectsCollector.h>
 #include <libyul/backends/evm/EVMDialect.h>
@@ -61,19 +62,15 @@ TestCase::TestResult ControlFlowSideEffectsTest::run(ostream& _stream, string co
 	if (!obj.code)
 		BOOST_THROW_EXCEPTION(runtime_error("Parsing input failed."));
 
-	std::map<YulString, ControlFlowSideEffects> sideEffects =
+	ControlFlowSideEffectsCollector sideEffects(
-		ControlFlowSideEffectsCollector(
+		EVMDialect::strictAssemblyForEVMObjects(langutil::EVMVersion()),
-			EVMDialect::strictAssemblyForEVMObjects(langutil::EVMVersion()),
+		*obj.code
-			*obj.code
+	);
-		).functionSideEffects();
-
-	std::map<std::string, std::string> controlFlowSideEffectsStr;
-	for (auto&& [fun, effects]: sideEffects)
-		controlFlowSideEffectsStr[fun.str()] = toString(effects);
-
 	m_obtainedResult.clear();
-	for (auto&& [functionName, effect]: controlFlowSideEffectsStr)
+	forEach<FunctionDefinition const>(*obj.code, [&](FunctionDefinition const& _fun) {
-		m_obtainedResult += functionName + (effect.empty() ? ":" : ": " + effect) + "\n";
+		string effectStr = toString(sideEffects.functionSideEffects().at(&_fun));
+		m_obtainedResult += _fun.name.str() + (effectStr.empty() ? ":" : ": " + effectStr) + "\n";
+	});
 
 	return checkResult(_stream, _linePrefix, _formatted);
 }

test/libyul/controlFlowSideEffects/nondisambiguated.yul

@@ -0,0 +1,23 @@
+{
+    function a() {
+        {
+            function b() { if calldataload(0) { return(0, 0) } }
+            b()
+        }
+        {
+            function b() { revert(0, 0) }
+            b()
+        }
+    }
+    {
+        function b() {
+            leave
+            revert(0, 0)
+        }
+    }
+}
+// ----
+// a: can terminate, can revert
+// b: can terminate, can continue
+// b: can revert
+// b: can continue

test/libyul/controlFlowSideEffects/recursion.yul

@@ -31,7 +31,7 @@
 // b: can revert
 // c:
 // d:
-// reg: can continue
 // x:
 // y:
 // z:
+// reg: can continue