From fbe11815177ac1d221eb99695b1d931f7a1f29d6 Mon Sep 17 00:00:00 2001 From: wechman Date: Tue, 30 Aug 2022 09:46:55 +0200 Subject: [PATCH] CFGNode: For function calls store a pointer to a resolved function definition rather than the FunctionCall AST node --- libsolidity/analysis/ControlFlowBuilder.cpp | 5 +- libsolidity/analysis/ControlFlowBuilder.h | 4 +- libsolidity/analysis/ControlFlowGraph.cpp | 6 +- libsolidity/analysis/ControlFlowGraph.h | 5 +- .../analysis/ControlFlowRevertPruner.cpp | 76 +++++++++---------- 5 files changed, 46 insertions(+), 50 deletions(-) diff --git a/libsolidity/analysis/ControlFlowBuilder.cpp b/libsolidity/analysis/ControlFlowBuilder.cpp index 1d1e33f50..3794dcf6f 100644 --- a/libsolidity/analysis/ControlFlowBuilder.cpp +++ b/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); diff --git a/libsolidity/analysis/ControlFlowBuilder.h b/libsolidity/analysis/ControlFlowBuilder.h index a150262f1..91ca0eefa 100644 --- a/libsolidity/analysis/ControlFlowBuilder.h +++ b/libsolidity/analysis/ControlFlowBuilder.h @@ -38,14 +38,14 @@ public: static std::unique_ptr 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. diff --git a/libsolidity/analysis/ControlFlowGraph.cpp b/libsolidity/analysis/ControlFlowGraph.cpp index ca36b421c..71acb8a67 100644 --- a/libsolidity/analysis/ControlFlowGraph.cpp +++ b/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; } diff --git a/libsolidity/analysis/ControlFlowGraph.h b/libsolidity/analysis/ControlFlowGraph.h index 7383783fd..4ded67268 100644 --- a/libsolidity/analysis/ControlFlowGraph.h +++ b/libsolidity/analysis/ControlFlowGraph.h @@ -98,9 +98,8 @@ struct CFGNode std::vector entries; /// Exit nodes. All CFG nodes to which control flow may continue after this node. std::vector 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 variableOccurrences; // Source location of this control flow block. diff --git a/libsolidity/analysis/ControlFlowRevertPruner.cpp b/libsolidity/analysis/ControlFlowRevertPruner.cpp index ae9c18a13..c96ad2b03 100644 --- a/libsolidity/analysis/ControlFlowRevertPruner.cpp +++ b/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{{functionFlow.entry}}.run( [&](CFGNode* _node, auto&& _addChild) { - if (auto const* functionCall = _node->functionCall) - { - auto const* resolvedFunction = ASTNode::resolveFunctionCall(*functionCall, item.first.contract); + 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); - 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; - } - } + _node->exits = {functionFlow.revert}; + functionFlow.revert->entries.push_back(_node); + return; + default: + break; + } for (CFGNode* exit: _node->exits) _addChild(exit);