cerc-io/solidity
15
0
Fork 0
mirror of https://github.com/ethereum/solidity synced 2023-10-03 13:03:40 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Properly resolve virtual modifiers

Browse Source
This commit is contained in:
Marenz 2021-12-30 18:16:29 +01:00
parent b8ee17340b
commit e969aed780
11 changed files with 109 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
Changelog.md
View File

@ -9,6 +9,7 @@ Compiler Features:
Bugfixes:
* Control Flow Graph: Perform proper virtual lookup for modifiers for uninitialized variable and unreachable code analysis.
Solc-Js:

33
libsolidity/analysis/ControlFlowBuilder.cpp
View File

@ -25,19 +25,21 @@ using namespace solidity::langutil;
using namespace solidity::frontend;
using namespace std;
ControlFlowBuilder::ControlFlowBuilder(CFG::NodeContainer& _nodeContainer, FunctionFlow const& _functionFlow):
ControlFlowBuilder::ControlFlowBuilder(CFG::NodeContainer& _nodeContainer, FunctionFlow const& _functionFlow, ContractDefinition const* _contract):
m_nodeContainer(_nodeContainer),
m_currentNode(_functionFlow.entry),
m_returnNode(_functionFlow.exit),
m_revertNode(_functionFlow.revert),
m_transactionReturnNode(_functionFlow.transactionReturn)
m_transactionReturnNode(_functionFlow.transactionReturn),
m_contract(_contract)
{
}
unique_ptr<FunctionFlow> ControlFlowBuilder::createFunctionFlow(
CFG::NodeContainer& _nodeContainer,
FunctionDefinition const& _function
FunctionDefinition const& _function,
ContractDefinition const* _contract
)
{
auto functionFlow = make_unique<FunctionFlow>();
@ -45,7 +47,7 @@ unique_ptr<FunctionFlow> ControlFlowBuilder::createFunctionFlow(
functionFlow->exit = _nodeContainer.newNode();
functionFlow->revert = _nodeContainer.newNode();
functionFlow->transactionReturn = _nodeContainer.newNode();
ControlFlowBuilder builder(_nodeContainer, *functionFlow);
ControlFlowBuilder builder(_nodeContainer, *functionFlow, _contract);
builder.appendControlFlow(_function);
return functionFlow;
@ -297,7 +299,8 @@ bool ControlFlowBuilder::visit(FunctionCall const& _functionCall)
_functionCall.expression().accept(*this);
ASTNode::listAccept(_functionCall.arguments(), *this);
m_currentNode->functionCalls.emplace_back(&_functionCall);
solAssert(!m_currentNode->functionCall);
m_currentNode->functionCall = &_functionCall;
auto nextNode = newLabel();
@ -321,8 +324,20 @@ bool ControlFlowBuilder::visit(ModifierInvocation const& _modifierInvocation)
auto modifierDefinition = dynamic_cast<ModifierDefinition const*>(
_modifierInvocation.name().annotation().referencedDeclaration
);
if (!modifierDefinition) return false;
if (!modifierDefinition->isImplemented()) return false;
if (!modifierDefinition)
return false;
VirtualLookup const& requiredLookup = *_modifierInvocation.name().annotation().requiredLookup;
if (requiredLookup == VirtualLookup::Virtual)
modifierDefinition = &modifierDefinition->resolveVirtual(*m_contract);
else
solAssert(requiredLookup == VirtualLookup::Static);
if (!modifierDefinition->isImplemented())
return false;
solAssert(!!m_returnNode, "");
m_placeholderEntry = newLabel();
@ -355,8 +370,8 @@ bool ControlFlowBuilder::visit(FunctionDefinition const& _functionDefinition)
}
for (auto const& modifier: _functionDefinition.modifiers())
appendControlFlow(*modifier);
for (auto const& modifierInvocation: _functionDefinition.modifiers())
appendControlFlow(*modifierInvocation);
appendControlFlow(_functionDefinition.body());

8
libsolidity/analysis/ControlFlowBuilder.h
View File

@ -37,13 +37,15 @@ class ControlFlowBuilder: private ASTConstVisitor, private yul::ASTWalker
public:
static std::unique_ptr<FunctionFlow> createFunctionFlow(
CFG::NodeContainer& _nodeContainer,
FunctionDefinition const& _function
FunctionDefinition const& _function,
ContractDefinition const* _contract = nullptr
);
private:
explicit ControlFlowBuilder(
CFG::NodeContainer& _nodeContainer,
FunctionFlow const& _functionFlow
FunctionFlow const& _functionFlow,
ContractDefinition const* _contract = nullptr
);
// Visits for constructing the control flow.
@ -158,6 +160,8 @@ private:
CFGNode* m_revertNode = nullptr;
CFGNode* m_transactionReturnNode = nullptr;
ContractDefinition const* m_contract = nullptr;
/// The current jump destination of break Statements.
CFGNode* m_breakJump = nullptr;
/// The current jump destination of continue Statements.

2
libsolidity/analysis/ControlFlowGraph.cpp
View File

@ -44,7 +44,7 @@ bool CFG::visit(ContractDefinition const& _contract)
for (FunctionDefinition const* function: contract->definedFunctions())
if (function->isImplemented())
m_functionControlFlow[{&_contract, function}] =
ControlFlowBuilder::createFunctionFlow(m_nodeContainer, *function);
ControlFlowBuilder::createFunctionFlow(m_nodeContainer, *function, &_contract);
return true;
}

4
libsolidity/analysis/ControlFlowGraph.h
View File

@ -98,8 +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 calls done by this node
std::vector<FunctionCall const*> functionCalls;
/// Function call done by this node
FunctionCall const* functionCall = nullptr;
/// Variable occurrences in the node.
std::vector<VariableOccurrence> variableOccurrences;

74
libsolidity/analysis/ControlFlowRevertPruner.cpp
View File

@ -81,27 +81,27 @@ void ControlFlowRevertPruner::findRevertStates()
if (_node == functionFlow.exit)
foundExit = true;
for (auto const* functionCall: _node->functionCalls)
if (auto const* functionCall = _node->functionCall)
{
auto const* resolvedFunction = ASTNode::resolveFunctionCall(*functionCall, item.contract);
if (resolvedFunction == nullptr || !resolvedFunction->isImplemented())
continue;
CFG::FunctionContractTuple calledFunctionTuple{
findScopeContract(*resolvedFunction, item.contract),
resolvedFunction
};
switch (m_functions.at(calledFunctionTuple))
if (resolvedFunction && resolvedFunction->isImplemented())
{
case RevertState::Unknown:
wakeUp[calledFunctionTuple].insert(item);
foundUnknown = true;
return;
case RevertState::AllPathsRevert:
return;
case RevertState::HasNonRevertingPath:
break;
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;
}
}
}
@ -135,31 +135,29 @@ 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) {
for (auto const* functionCall: _node->functionCalls)
if (auto const* functionCall = _node->functionCall)
{
auto const* resolvedFunction = ASTNode::resolveFunctionCall(*functionCall, item.first.contract);
if (resolvedFunction == nullptr || !resolvedFunction->isImplemented())
continue;
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);
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)

6
test/libsolidity/syntaxTests/controlFlow/localStorageVariables/modifier_declaration_fine.sol
View File

@ -1,5 +1,5 @@
contract C {
modifier revertIfNoReturn() {
modifier alwaysRevert() {
_;
revert();
}
@ -9,10 +9,10 @@ contract C {
}
struct S { uint a; }
S s;
function f(bool flag) revertIfNoReturn() internal view {
function f(bool flag) alwaysRevert() internal view {
if (flag) s;
}
function g(bool flag) revertIfNoReturn() ifFlag(flag) internal view {
function g(bool flag) alwaysRevert() ifFlag(flag) internal view {
s;
}

12
test/libsolidity/syntaxTests/controlFlow/modifiers/modifier_different_functions.sol Normal file
View File

@ -0,0 +1,12 @@
contract A {
function f() mod internal returns (uint[] storage) {
revert();
}
function g() mod internal returns (uint[] storage) {
}
modifier mod() virtual {
_;
}
}
// ----
// TypeError 3464: (118-132): This variable is of storage pointer type and can be returned without prior assignment, which would lead to undefined behaviour.

17
test/libsolidity/syntaxTests/controlFlow/modifiers/modifier_override.sol Normal file
View File

@ -0,0 +1,17 @@
contract A {
function f() mod internal returns (uint[] storage) {
}
modifier mod() virtual {
revert();
_;
}
}
contract B is A {
modifier mod() override { _; }
function g() public {
f()[0] = 42;
}
}
// ----
// Warning 5740: (65-69): Unreachable code.
// TypeError 3464: (49-63): This variable is of storage pointer type and can be returned without prior assignment, which would lead to undefined behaviour.

8
test/libsolidity/syntaxTests/controlFlow/storageReturn/modifier_err.sol
View File

@ -1,5 +1,5 @@
contract C {
modifier revertIfNoReturn() {
modifier callAndRevert() {
_;
revert();
}
@ -13,10 +13,10 @@ contract C {
return s;
}
function g(bool flag) ifFlag(flag) revertIfNoReturn() internal view returns(S storage) {
function g(bool flag) ifFlag(flag) callAndRevert() internal view returns(S storage) {
return s;
}
}
// ----
// TypeError 3464: (249-258): This variable is of storage pointer type and can be returned without prior assignment, which would lead to undefined behaviour.
// TypeError 3464: (367-376): This variable is of storage pointer type and can be returned without prior assignment, which would lead to undefined behaviour.
// TypeError 3464: (246-255): This variable is of storage pointer type and can be returned without prior assignment, which would lead to undefined behaviour.
// TypeError 3464: (361-370): This variable is of storage pointer type and can be returned without prior assignment, which would lead to undefined behaviour.

6
test/libsolidity/syntaxTests/controlFlow/storageReturn/modifier_fine.sol
View File

@ -1,5 +1,5 @@
contract C {
modifier revertIfNoReturn() {
modifier callAndRevert() {
_;
revert();
}
@ -9,10 +9,10 @@ contract C {
}
struct S { uint a; }
S s;
function f(bool flag) revertIfNoReturn() internal view returns(S storage) {
function f(bool flag) callAndRevert() internal view returns(S storage) {
if (flag) return s;
}
function g(bool flag) revertIfNoReturn() ifFlag(flag) internal view returns(S storage) {
function g(bool flag) callAndRevert() ifFlag(flag) internal view returns(S storage) {
return s;
}