Merge pull request #13693 from ethereum/resolved-function-call-in-cfg

Resolved `FunctionDefinition` instead of `FunctionCall` in CFG

Author: chriseth

libsolidity/analysis/ControlFlowBuilder.cpp

@@ -300,8 +300,7 @@ bool ControlFlowBuilder::visit(FunctionCall const& _functionCall)
 				_functionCall.expression().accept(*this);
 				ASTNode::listAccept(_functionCall.arguments(), *this);
 
-				solAssert(!m_currentNode->functionCall);
-				m_currentNode->functionCall = &_functionCall;
+				m_currentNode->functionDefinition = ASTNode::resolveFunctionCall(_functionCall, m_contract);
 
 				auto nextNode = newLabel();
 
@@ -318,6 +317,8 @@ bool ControlFlowBuilder::visit(FunctionCall const& _functionCall)
 
 bool ControlFlowBuilder::visit(ModifierInvocation const& _modifierInvocation)
 {
+	solAssert(m_contract, "Free functions cannot have modifiers");
+
 	if (auto arguments = _modifierInvocation.arguments())
 		for (auto& argument: *arguments)
 			appendControlFlow(*argument);

libsolidity/analysis/ControlFlowBuilder.h

@@ -38,14 +38,14 @@ class ControlFlowBuilder: private ASTConstVisitor, private yul::ASTWalker
 	static std::unique_ptr<FunctionFlow> createFunctionFlow(
 		CFG::NodeContainer& _nodeContainer,
 		FunctionDefinition const& _function,
-		ContractDefinition const* _contract = nullptr
+		ContractDefinition const* _contract
 	);
 
 private:
 	explicit ControlFlowBuilder(
 		CFG::NodeContainer& _nodeContainer,
 		FunctionFlow const& _functionFlow,
-		ContractDefinition const* _contract = nullptr
+		ContractDefinition const* _contract
 	);
 
 	// Visits for constructing the control flow.

libsolidity/analysis/ControlFlowGraph.cpp

@@ -34,7 +34,11 @@ bool CFG::constructFlow(ASTNode const& _astRoot)
 bool CFG::visit(FunctionDefinition const& _function)
 {
 	if (_function.isImplemented() && _function.isFree())
-		m_functionControlFlow[{nullptr, &_function}] = ControlFlowBuilder::createFunctionFlow(m_nodeContainer, _function);
+		m_functionControlFlow[{nullptr, &_function}] = ControlFlowBuilder::createFunctionFlow(
+			m_nodeContainer,
+			_function,
+			nullptr /* _contract */
+		);
 	return false;
 }
 

libsolidity/analysis/ControlFlowGraph.h

@@ -98,9 +98,8 @@ struct CFGNode
 	std::vector<CFGNode*> entries;
 	/// Exit nodes. All CFG nodes to which control flow may continue after this node.
 	std::vector<CFGNode*> exits;
-	/// Function call done by this node
-	FunctionCall const* functionCall = nullptr;
-
+	/// Resolved definition of the function called by this node
+	FunctionDefinition const* functionDefinition = nullptr;
 	/// Variable occurrences in the node.
 	std::vector<VariableOccurrence> variableOccurrences;
 	// Source location of this control flow block.

libsolidity/analysis/ControlFlowRevertPruner.cpp

@@ -81,27 +81,23 @@ void ControlFlowRevertPruner::findRevertStates()
 				if (_node == functionFlow.exit)
 					foundExit = true;
 
-				if (auto const* functionCall = _node->functionCall)
+				auto const* resolvedFunction = _node->functionDefinition;
+				if (resolvedFunction && resolvedFunction->isImplemented())
 				{
-					auto const* resolvedFunction = ASTNode::resolveFunctionCall(*functionCall, item.contract);
-
-					if (resolvedFunction && resolvedFunction->isImplemented())
+					CFG::FunctionContractTuple calledFunctionTuple{
+						findScopeContract(*resolvedFunction, item.contract),
+						resolvedFunction
+					};
+					switch (m_functions.at(calledFunctionTuple))
 					{
-						CFG::FunctionContractTuple calledFunctionTuple{
-							findScopeContract(*resolvedFunction, item.contract),
-							resolvedFunction
-						};
-						switch (m_functions.at(calledFunctionTuple))
-						{
-							case RevertState::Unknown:
-								wakeUp[calledFunctionTuple].insert(item);
-								foundUnknown = true;
-								return;
-							case RevertState::AllPathsRevert:
-								return;
-							case RevertState::HasNonRevertingPath:
-								break;
-						}
+						case RevertState::Unknown:
+							wakeUp[calledFunctionTuple].insert(item);
+							foundUnknown = true;
+							return;
+						case RevertState::AllPathsRevert:
+							return;
+						case RevertState::HasNonRevertingPath:
+							break;
 					}
 				}
 
@@ -135,30 +131,26 @@ void ControlFlowRevertPruner::modifyFunctionFlows()
 		FunctionFlow const& functionFlow = m_cfg.functionFlow(*item.first.function, item.first.contract);
 		solidity::util::BreadthFirstSearch<CFGNode*>{{functionFlow.entry}}.run(
 			[&](CFGNode* _node, auto&& _addChild) {
-				if (auto const* functionCall = _node->functionCall)
-				{
-					auto const* resolvedFunction = ASTNode::resolveFunctionCall(*functionCall, item.first.contract);
-
-					if (resolvedFunction && resolvedFunction->isImplemented())
-						switch (m_functions.at({findScopeContract(*resolvedFunction, item.first.contract), resolvedFunction}))
-						{
-							case RevertState::Unknown:
-								[[fallthrough]];
-							case RevertState::AllPathsRevert:
-								// If the revert states of the functions do not
-								// change anymore, we treat all "unknown" states as
-								// "reverting", since they can only be caused by
-								// recursion.
-								for (CFGNode * node: _node->exits)
-									ranges::remove(node->entries, _node);
-
-								_node->exits = {functionFlow.revert};
-								functionFlow.revert->entries.push_back(_node);
-								return;
-							default:
-								break;
-						}
-				}
+				auto const* resolvedFunction = _node->functionDefinition;
+				if (resolvedFunction && resolvedFunction->isImplemented())
+					switch (m_functions.at({findScopeContract(*resolvedFunction, item.first.contract), resolvedFunction}))
+					{
+						case RevertState::Unknown:
+							[[fallthrough]];
+						case RevertState::AllPathsRevert:
+							// If the revert states of the functions do not
+							// change anymore, we treat all "unknown" states as
+							// "reverting", since they can only be caused by
+							// recursion.
+							for (CFGNode * node: _node->exits)
+								ranges::remove(node->entries, _node);
+
+							_node->exits = {functionFlow.revert};
+							functionFlow.revert->entries.push_back(_node);
+							return;
+						default:
+							break;
+					}
 
 				for (CFGNode* exit: _node->exits)
 					_addChild(exit);