Refactoring of the ControlFlowGraph and use for detecting all uninitialized storage accesses.

This commit is contained in:
Daniel Kirchner 2018-12-07 18:20:35 +01:00
parent 1476acb804
commit 788612d2ef
29 changed files with 470 additions and 371 deletions

View File

@ -1,6 +1,7 @@
### 0.5.2 (unreleased)
Language Features:
* Control Flow Graph: Detect every access to uninitialized storage pointers.
Compiler Features:

View File

@ -17,6 +17,7 @@
#include <libsolidity/analysis/ControlFlowAnalyzer.h>
#include <liblangutil/SourceLocation.h>
#include <boost/range/algorithm/sort.hpp>
using namespace std;
using namespace langutil;
@ -33,131 +34,112 @@ bool ControlFlowAnalyzer::visit(FunctionDefinition const& _function)
if (_function.isImplemented())
{
auto const& functionFlow = m_cfg.functionFlow(_function);
checkUnassignedStorageReturnValues(_function, functionFlow.entry, functionFlow.exit);
checkUninitializedAccess(functionFlow.entry, functionFlow.exit);
}
return false;
}
set<VariableDeclaration const*> ControlFlowAnalyzer::variablesAssignedInNode(CFGNode const *node)
void ControlFlowAnalyzer::checkUninitializedAccess(CFGNode const* _entry, CFGNode const* _exit) const
{
set<VariableDeclaration const*> result;
for (auto expression: node->block.expressions)
struct NodeInfo
{
if (auto const* assignment = dynamic_cast<Assignment const*>(expression))
set<VariableDeclaration const*> unassignedVariablesAtEntry;
set<VariableDeclaration const*> unassignedVariablesAtExit;
set<VariableOccurrence const*> uninitializedVariableAccesses;
/// Propagate the information from another node to this node.
/// To be used to propagate information from a node to its exit nodes.
/// Returns true, if new variables were added and thus the current node has
/// to be traversed again.
bool propagateFrom(NodeInfo const& _entryNode)
{
stack<Expression const*> expressions;
expressions.push(&assignment->leftHandSide());
while (!expressions.empty())
{
Expression const* expression = expressions.top();
expressions.pop();
if (auto const *tuple = dynamic_cast<TupleExpression const*>(expression))
for (auto const& component: tuple->components())
expressions.push(component.get());
else if (auto const* identifier = dynamic_cast<Identifier const*>(expression))
if (auto const* variableDeclaration = dynamic_cast<VariableDeclaration const*>(
identifier->annotation().referencedDeclaration
))
result.insert(variableDeclaration);
}
size_t previousUnassignedVariablesAtEntry = unassignedVariablesAtEntry.size();
size_t previousUninitializedVariableAccessess = uninitializedVariableAccesses.size();
unassignedVariablesAtEntry += _entryNode.unassignedVariablesAtExit;
uninitializedVariableAccesses += _entryNode.uninitializedVariableAccesses;
return
unassignedVariablesAtEntry.size() > previousUnassignedVariablesAtEntry ||
uninitializedVariableAccesses.size() > previousUninitializedVariableAccessess
;
}
}
return result;
}
};
map<CFGNode const*, NodeInfo> nodeInfos;
set<CFGNode const*> nodesToTraverse;
nodesToTraverse.insert(_entry);
void ControlFlowAnalyzer::checkUnassignedStorageReturnValues(
FunctionDefinition const& _function,
CFGNode const* _functionEntry,
CFGNode const* _functionExit
) const
{
if (_function.returnParameterList()->parameters().empty())
return;
map<CFGNode const*, set<VariableDeclaration const*>> unassigned;
{
auto& unassignedAtFunctionEntry = unassigned[_functionEntry];
for (auto const& returnParameter: _function.returnParameterList()->parameters())
if (
returnParameter->type()->dataStoredIn(DataLocation::Storage) ||
returnParameter->type()->category() == Type::Category::Mapping
)
unassignedAtFunctionEntry.insert(returnParameter.get());
}
stack<CFGNode const*> nodesToTraverse;
nodesToTraverse.push(_functionEntry);
// walk all paths from entry with maximal set of unassigned return values
// Walk all paths starting from the nodes in ``nodesToTraverse`` until ``NodeInfo::propagateFrom``
// returns false for all exits, i.e. until all paths have been walked with maximal sets of unassigned
// variables and accesses.
while (!nodesToTraverse.empty())
{
auto node = nodesToTraverse.top();
nodesToTraverse.pop();
CFGNode const* currentNode = *nodesToTraverse.begin();
nodesToTraverse.erase(nodesToTraverse.begin());
auto& unassignedAtNode = unassigned[node];
if (node->block.returnStatement != nullptr)
if (node->block.returnStatement->expression())
unassignedAtNode.clear();
if (!unassignedAtNode.empty())
auto& nodeInfo = nodeInfos[currentNode];
auto unassignedVariables = nodeInfo.unassignedVariablesAtEntry;
for (auto const& variableOccurrence: currentNode->variableOccurrences)
{
// kill all return values to which a value is assigned
for (auto const* variableDeclaration: variablesAssignedInNode(node))
unassignedAtNode.erase(variableDeclaration);
// kill all return values referenced in inline assembly
// a reference is enough, checking whether there actually was an assignment might be overkill
for (auto assembly: node->block.inlineAssemblyStatements)
for (auto const& ref: assembly->annotation().externalReferences)
if (auto variableDeclaration = dynamic_cast<VariableDeclaration const*>(ref.second.declaration))
unassignedAtNode.erase(variableDeclaration);
switch (variableOccurrence.kind())
{
case VariableOccurrence::Kind::Assignment:
unassignedVariables.erase(&variableOccurrence.declaration());
break;
case VariableOccurrence::Kind::InlineAssembly:
// We consider all variables referenced in inline assembly as accessed.
// So far any reference is enough, but we might want to actually analyze
// the control flow in the assembly at some point.
case VariableOccurrence::Kind::Access:
case VariableOccurrence::Kind::Return:
if (unassignedVariables.count(&variableOccurrence.declaration()))
{
if (variableOccurrence.declaration().type()->dataStoredIn(DataLocation::Storage))
// Merely store the unassigned access. We do not generate an error right away, since this
// path might still always revert. It is only an error if this is propagated to the exit
// node of the function (i.e. there is a path with an uninitialized access).
nodeInfo.uninitializedVariableAccesses.insert(&variableOccurrence);
}
break;
case VariableOccurrence::Kind::Declaration:
unassignedVariables.insert(&variableOccurrence.declaration());
break;
}
}
nodeInfo.unassignedVariablesAtExit = std::move(unassignedVariables);
for (auto const& exit: node->exits)
{
auto& unassignedAtExit = unassigned[exit];
auto oldSize = unassignedAtExit.size();
unassignedAtExit.insert(unassignedAtNode.begin(), unassignedAtNode.end());
// (re)traverse an exit, if we are on a path with new unassigned return values to consider
// this will terminate, since there is only a finite number of unassigned return values
if (unassignedAtExit.size() > oldSize)
nodesToTraverse.push(exit);
}
// Propagate changes to all exits and queue them for traversal, if needed.
for (auto const& exit: currentNode->exits)
if (nodeInfos[exit].propagateFrom(nodeInfo))
nodesToTraverse.insert(exit);
}
if (!unassigned[_functionExit].empty())
auto const& exitInfo = nodeInfos[_exit];
if (!exitInfo.uninitializedVariableAccesses.empty())
{
vector<VariableDeclaration const*> unassignedOrdered(
unassigned[_functionExit].begin(),
unassigned[_functionExit].end()
);
sort(
unassignedOrdered.begin(),
unassignedOrdered.end(),
[](VariableDeclaration const* lhs, VariableDeclaration const* rhs) -> bool {
return lhs->id() < rhs->id();
vector<VariableOccurrence const*> uninitializedAccessesOrdered(
exitInfo.uninitializedVariableAccesses.begin(),
exitInfo.uninitializedVariableAccesses.end()
);
boost::range::sort(
uninitializedAccessesOrdered,
[](VariableOccurrence const* lhs, VariableOccurrence const* rhs) -> bool
{
return *lhs < *rhs;
}
);
for (auto const* returnVal: unassignedOrdered)
for (auto const* variableOccurrence: uninitializedAccessesOrdered)
{
SecondarySourceLocation ssl;
for (CFGNode* lastNodeBeforeExit: _functionExit->entries)
if (unassigned[lastNodeBeforeExit].count(returnVal))
{
if (!!lastNodeBeforeExit->block.returnStatement)
ssl.append("Problematic return:", lastNodeBeforeExit->block.returnStatement->location());
else
ssl.append("Problematic end of function:", _function.location());
}
if (variableOccurrence->occurrence())
ssl.append("The variable was declared here.", variableOccurrence->declaration().location());
m_errorReporter.typeError(
returnVal->location(),
variableOccurrence->occurrence() ?
variableOccurrence->occurrence()->location() :
variableOccurrence->declaration().location(),
ssl,
"This variable is of storage pointer type and might be returned without assignment and "
"could be used uninitialized. Assign the variable (potentially from itself) "
"to fix this error."
string("This variable is of storage pointer type and can be ") +
(variableOccurrence->kind() == VariableOccurrence::Kind::Return ? "returned" : "accessed") +
" without prior assignment."
);
}
}

View File

@ -37,12 +37,8 @@ public:
bool visit(FunctionDefinition const& _function) override;
private:
static std::set<VariableDeclaration const*> variablesAssignedInNode(CFGNode const *node);
void checkUnassignedStorageReturnValues(
FunctionDefinition const& _function,
CFGNode const* _functionEntry,
CFGNode const* _functionExit
) const;
/// Checks for uninitialized variable accesses in the control flow between @param _entry and @param _exit.
void checkUninitializedAccess(CFGNode const* _entry, CFGNode const* _exit) const;
CFG const& m_cfg;
langutil::ErrorReporter& m_errorReporter;

View File

@ -22,7 +22,10 @@ using namespace solidity;
using namespace std;
ControlFlowBuilder::ControlFlowBuilder(CFG::NodeContainer& _nodeContainer, FunctionFlow const& _functionFlow):
m_nodeContainer(_nodeContainer), m_currentFunctionFlow(_functionFlow), m_currentNode(_functionFlow.entry)
m_nodeContainer(_nodeContainer),
m_currentNode(_functionFlow.entry),
m_returnNode(_functionFlow.exit),
m_revertNode(_functionFlow.revert)
{
}
@ -37,28 +40,10 @@ unique_ptr<FunctionFlow> ControlFlowBuilder::createFunctionFlow(
functionFlow->revert = _nodeContainer.newNode();
ControlFlowBuilder builder(_nodeContainer, *functionFlow);
builder.appendControlFlow(_function);
connect(builder.m_currentNode, functionFlow->exit);
return functionFlow;
}
unique_ptr<ModifierFlow> ControlFlowBuilder::createModifierFlow(
CFG::NodeContainer& _nodeContainer,
ModifierDefinition const& _modifier
)
{
auto modifierFlow = unique_ptr<ModifierFlow>(new ModifierFlow());
modifierFlow->entry = _nodeContainer.newNode();
modifierFlow->exit = _nodeContainer.newNode();
modifierFlow->revert = _nodeContainer.newNode();
modifierFlow->placeholderEntry = _nodeContainer.newNode();
modifierFlow->placeholderExit = _nodeContainer.newNode();
ControlFlowBuilder builder(_nodeContainer, *modifierFlow);
builder.appendControlFlow(_modifier);
connect(builder.m_currentNode, modifierFlow->exit);
return modifierFlow;
}
bool ControlFlowBuilder::visit(BinaryOperation const& _operation)
{
solAssert(!!m_currentNode, "");
@ -219,64 +204,24 @@ bool ControlFlowBuilder::visit(Continue const&)
bool ControlFlowBuilder::visit(Throw const&)
{
solAssert(!!m_currentNode, "");
solAssert(!!m_currentFunctionFlow.revert, "");
connect(m_currentNode, m_currentFunctionFlow.revert);
solAssert(!!m_revertNode, "");
connect(m_currentNode, m_revertNode);
m_currentNode = newLabel();
return false;
}
bool ControlFlowBuilder::visit(Block const&)
{
solAssert(!!m_currentNode, "");
createLabelHere();
return true;
}
void ControlFlowBuilder::endVisit(Block const&)
{
solAssert(!!m_currentNode, "");
createLabelHere();
}
bool ControlFlowBuilder::visit(Return const& _return)
{
solAssert(!!m_currentNode, "");
solAssert(!!m_currentFunctionFlow.exit, "");
solAssert(!m_currentNode->block.returnStatement, "");
m_currentNode->block.returnStatement = &_return;
connect(m_currentNode, m_currentFunctionFlow.exit);
m_currentNode = newLabel();
return true;
}
bool ControlFlowBuilder::visit(PlaceholderStatement const&)
{
solAssert(!!m_currentNode, "");
auto modifierFlow = dynamic_cast<ModifierFlow const*>(&m_currentFunctionFlow);
solAssert(!!modifierFlow, "");
connect(m_currentNode, modifierFlow->placeholderEntry);
solAssert(!!m_placeholderEntry, "");
solAssert(!!m_placeholderExit, "");
connect(m_currentNode, m_placeholderEntry);
m_currentNode = newLabel();
connect(modifierFlow->placeholderExit, m_currentNode);
connect(m_placeholderExit, m_currentNode);
return false;
}
bool ControlFlowBuilder::visitNode(ASTNode const& node)
{
solAssert(!!m_currentNode, "");
if (auto const* expression = dynamic_cast<Expression const*>(&node))
m_currentNode->block.expressions.emplace_back(expression);
else if (auto const* variableDeclaration = dynamic_cast<VariableDeclaration const*>(&node))
m_currentNode->block.variableDeclarations.emplace_back(variableDeclaration);
else if (auto const* assembly = dynamic_cast<InlineAssembly const*>(&node))
m_currentNode->block.inlineAssemblyStatements.emplace_back(assembly);
return true;
}
bool ControlFlowBuilder::visit(FunctionCall const& _functionCall)
{
solAssert(!!m_currentNode, "");
@ -286,19 +231,19 @@ bool ControlFlowBuilder::visit(FunctionCall const& _functionCall)
switch (functionType->kind())
{
case FunctionType::Kind::Revert:
solAssert(!!m_currentFunctionFlow.revert, "");
solAssert(!!m_revertNode, "");
_functionCall.expression().accept(*this);
ASTNode::listAccept(_functionCall.arguments(), *this);
connect(m_currentNode, m_currentFunctionFlow.revert);
connect(m_currentNode, m_revertNode);
m_currentNode = newLabel();
return false;
case FunctionType::Kind::Require:
case FunctionType::Kind::Assert:
{
solAssert(!!m_currentFunctionFlow.revert, "");
solAssert(!!m_revertNode, "");
_functionCall.expression().accept(*this);
ASTNode::listAccept(_functionCall.arguments(), *this);
connect(m_currentNode, m_currentFunctionFlow.revert);
connect(m_currentNode, m_revertNode);
auto nextNode = newLabel();
connect(m_currentNode, nextNode);
m_currentNode = nextNode;
@ -310,6 +255,183 @@ bool ControlFlowBuilder::visit(FunctionCall const& _functionCall)
return ASTConstVisitor::visit(_functionCall);
}
bool ControlFlowBuilder::visit(ModifierInvocation const& _modifierInvocation)
{
if (auto arguments = _modifierInvocation.arguments())
for (auto& argument: *arguments)
appendControlFlow(*argument);
auto modifierDefinition = dynamic_cast<ModifierDefinition const*>(
_modifierInvocation.name()->annotation().referencedDeclaration
);
if (!modifierDefinition) return false;
solAssert(!!modifierDefinition, "");
solAssert(!!m_returnNode, "");
m_placeholderEntry = newLabel();
m_placeholderExit = newLabel();
appendControlFlow(*modifierDefinition);
connect(m_currentNode, m_returnNode);
m_currentNode = m_placeholderEntry;
m_returnNode = m_placeholderExit;
m_placeholderEntry = nullptr;
m_placeholderExit = nullptr;
return false;
}
bool ControlFlowBuilder::visit(FunctionDefinition const& _functionDefinition)
{
for (auto const& parameter: _functionDefinition.parameters())
appendControlFlow(*parameter);
for (auto const& returnParameter: _functionDefinition.returnParameters())
{
appendControlFlow(*returnParameter);
m_returnNode->variableOccurrences.emplace_back(
*returnParameter,
VariableOccurrence::Kind::Return,
nullptr
);
}
for (auto const& modifier: _functionDefinition.modifiers())
appendControlFlow(*modifier);
appendControlFlow(_functionDefinition.body());
connect(m_currentNode, m_returnNode);
m_currentNode = nullptr;
return false;
}
bool ControlFlowBuilder::visit(Return const& _return)
{
solAssert(!!m_currentNode, "");
solAssert(!!m_returnNode, "");
if (_return.expression())
{
appendControlFlow(*_return.expression());
// Returns with return expression are considered to be assignments to the return parameters.
for (auto returnParameter: _return.annotation().functionReturnParameters->parameters())
m_currentNode->variableOccurrences.emplace_back(
*returnParameter,
VariableOccurrence::Kind::Assignment,
&_return
);
}
connect(m_currentNode, m_returnNode);
m_currentNode = newLabel();
return true;
}
bool ControlFlowBuilder::visit(FunctionTypeName const&)
{
// Do not visit the parameters and return values of a function type name.
// We do not want to consider them as variable declarations for the control flow graph.
return false;
}
bool ControlFlowBuilder::visit(InlineAssembly const& _inlineAssembly)
{
solAssert(!!m_currentNode, "");
for (auto const& ref: _inlineAssembly.annotation().externalReferences)
{
if (auto variableDeclaration = dynamic_cast<VariableDeclaration const*>(ref.second.declaration))
m_currentNode->variableOccurrences.emplace_back(
*variableDeclaration,
VariableOccurrence::Kind::InlineAssembly,
&_inlineAssembly
);
}
return true;
}
bool ControlFlowBuilder::visit(VariableDeclaration const& _variableDeclaration)
{
solAssert(!!m_currentNode, "");
m_currentNode->variableOccurrences.emplace_back(
_variableDeclaration,
VariableOccurrence::Kind::Declaration,
nullptr
);
// Handle declaration with immediate assignment.
if (_variableDeclaration.value())
m_currentNode->variableOccurrences.emplace_back(
_variableDeclaration,
VariableOccurrence::Kind::Assignment,
_variableDeclaration.value().get()
);
// Function arguments are considered to be immediately assigned as well (they are "externally assigned").
else if (_variableDeclaration.isCallableParameter() && !_variableDeclaration.isReturnParameter())
m_currentNode->variableOccurrences.emplace_back(
_variableDeclaration,
VariableOccurrence::Kind::Assignment,
nullptr
);
return true;
}
bool ControlFlowBuilder::visit(VariableDeclarationStatement const& _variableDeclarationStatement)
{
solAssert(!!m_currentNode, "");
for (auto const& var: _variableDeclarationStatement.declarations())
if (var)
var->accept(*this);
if (_variableDeclarationStatement.initialValue())
{
_variableDeclarationStatement.initialValue()->accept(*this);
for (size_t i = 0; i < _variableDeclarationStatement.declarations().size(); i++)
if (auto const& var = _variableDeclarationStatement.declarations()[i])
{
auto expression = _variableDeclarationStatement.initialValue();
if (auto tupleExpression = dynamic_cast<TupleExpression const*>(expression))
if (tupleExpression->components().size() > 1)
{
solAssert(tupleExpression->components().size() > i, "");
expression = tupleExpression->components()[i].get();
}
while (auto tupleExpression = dynamic_cast<TupleExpression const*>(expression))
if (tupleExpression->components().size() == 1)
expression = tupleExpression->components().front().get();
else
break;
m_currentNode->variableOccurrences.emplace_back(
*var,
VariableOccurrence::Kind::Assignment,
expression
);
}
}
return false;
}
bool ControlFlowBuilder::visit(Identifier const& _identifier)
{
solAssert(!!m_currentNode, "");
if (auto const* variableDeclaration = dynamic_cast<VariableDeclaration const*>(_identifier.annotation().referencedDeclaration))
m_currentNode->variableOccurrences.emplace_back(
*variableDeclaration,
static_cast<Expression const&>(_identifier).annotation().lValueRequested ?
VariableOccurrence::Kind::Assignment :
VariableOccurrence::Kind::Access,
&_identifier
);
return true;
}
void ControlFlowBuilder::appendControlFlow(ASTNode const& _node)
{
_node.accept(*this);

View File

@ -38,14 +38,11 @@ public:
CFG::NodeContainer& _nodeContainer,
FunctionDefinition const& _function
);
static std::unique_ptr<ModifierFlow> createModifierFlow(
CFG::NodeContainer& _nodeContainer,
ModifierDefinition const& _modifier
);
private:
explicit ControlFlowBuilder(CFG::NodeContainer& _nodeContainer, FunctionFlow const& _functionFlow);
// Visits for constructing the control flow.
bool visit(BinaryOperation const& _operation) override;
bool visit(Conditional const& _conditional) override;
bool visit(IfStatement const& _ifStatement) override;
@ -54,12 +51,20 @@ private:
bool visit(Break const&) override;
bool visit(Continue const&) override;
bool visit(Throw const&) override;
bool visit(Block const&) override;
void endVisit(Block const&) override;
bool visit(Return const& _return) override;
bool visit(PlaceholderStatement const&) override;
bool visit(FunctionCall const& _functionCall) override;
bool visit(ModifierInvocation const& _modifierInvocation) override;
// Visits for constructing the control flow as well as filling variable occurrences.
bool visit(FunctionDefinition const& _functionDefinition) override;
bool visit(Return const& _return) override;
// Visits for filling variable occurrences.
bool visit(FunctionTypeName const& _functionTypeName) override;
bool visit(InlineAssembly const& _inlineAssembly) override;
bool visit(VariableDeclaration const& _variableDeclaration) override;
bool visit(VariableDeclarationStatement const& _variableDeclarationStatement) override;
bool visit(Identifier const& _identifier) override;
/// Appends the control flow of @a _node to the current control flow.
void appendControlFlow(ASTNode const& _node);
@ -73,9 +78,6 @@ private:
static void connect(CFGNode* _from, CFGNode* _to);
protected:
bool visitNode(ASTNode const& node) override;
private:
/// Splits the control flow starting at the current node into n paths.
@ -114,17 +116,18 @@ private:
CFG::NodeContainer& m_nodeContainer;
/// The control flow of the function that is currently parsed.
/// Note: this can also be a ModifierFlow
FunctionFlow const& m_currentFunctionFlow;
CFGNode* m_currentNode = nullptr;
CFGNode* m_returnNode = nullptr;
CFGNode* m_revertNode = nullptr;
/// The current jump destination of break Statements.
CFGNode* m_breakJump = nullptr;
/// The current jump destination of continue Statements.
CFGNode* m_continueJump = nullptr;
CFGNode* m_placeholderEntry = nullptr;
CFGNode* m_placeholderExit = nullptr;
/// Helper class that replaces the break and continue jump destinations for the
/// current scope and restores the originals at the end of the scope.
class BreakContinueScope

View File

@ -29,20 +29,14 @@ using namespace dev::solidity;
bool CFG::constructFlow(ASTNode const& _astRoot)
{
_astRoot.accept(*this);
applyModifiers();
return Error::containsOnlyWarnings(m_errorReporter.errors());
}
bool CFG::visit(ModifierDefinition const& _modifier)
{
m_modifierControlFlow[&_modifier] = ControlFlowBuilder::createModifierFlow(m_nodeContainer, _modifier);
return false;
}
bool CFG::visit(FunctionDefinition const& _function)
{
m_functionControlFlow[&_function] = ControlFlowBuilder::createFunctionFlow(m_nodeContainer, _function);
if (_function.isImplemented())
m_functionControlFlow[&_function] = ControlFlowBuilder::createFunctionFlow(m_nodeContainer, _function);
return false;
}
@ -57,81 +51,3 @@ CFGNode* CFG::NodeContainer::newNode()
m_nodes.emplace_back(new CFGNode());
return m_nodes.back().get();
}
void CFG::applyModifiers()
{
for (auto const& function: m_functionControlFlow)
{
for (auto const& modifierInvocation: boost::adaptors::reverse(function.first->modifiers()))
{
if (auto modifierDefinition = dynamic_cast<ModifierDefinition const*>(
modifierInvocation->name()->annotation().referencedDeclaration
))
{
solAssert(m_modifierControlFlow.count(modifierDefinition), "");
applyModifierFlowToFunctionFlow(*m_modifierControlFlow[modifierDefinition], function.second.get());
}
}
}
}
void CFG::applyModifierFlowToFunctionFlow(
ModifierFlow const& _modifierFlow,
FunctionFlow* _functionFlow
)
{
solAssert(!!_functionFlow, "");
map<CFGNode*, CFGNode*> copySrcToCopyDst;
// inherit the revert node of the function
copySrcToCopyDst[_modifierFlow.revert] = _functionFlow->revert;
// replace the placeholder nodes by the function entry and exit
copySrcToCopyDst[_modifierFlow.placeholderEntry] = _functionFlow->entry;
copySrcToCopyDst[_modifierFlow.placeholderExit] = _functionFlow->exit;
stack<CFGNode*> nodesToCopy;
nodesToCopy.push(_modifierFlow.entry);
// map the modifier entry to a new node that will become the new function entry
copySrcToCopyDst[_modifierFlow.entry] = m_nodeContainer.newNode();
while (!nodesToCopy.empty())
{
CFGNode* copySrcNode = nodesToCopy.top();
nodesToCopy.pop();
solAssert(copySrcToCopyDst.count(copySrcNode), "");
CFGNode* copyDstNode = copySrcToCopyDst[copySrcNode];
copyDstNode->block = copySrcNode->block;
for (auto const& entry: copySrcNode->entries)
{
if (!copySrcToCopyDst.count(entry))
{
copySrcToCopyDst[entry] = m_nodeContainer.newNode();
nodesToCopy.push(entry);
}
copyDstNode->entries.emplace_back(copySrcToCopyDst[entry]);
}
for (auto const& exit: copySrcNode->exits)
{
if (!copySrcToCopyDst.count(exit))
{
copySrcToCopyDst[exit] = m_nodeContainer.newNode();
nodesToCopy.push(exit);
}
copyDstNode->exits.emplace_back(copySrcToCopyDst[exit]);
}
}
// if the modifier control flow never reached its exit node,
// we need to create a new (disconnected) exit node now
if (!copySrcToCopyDst.count(_modifierFlow.exit))
copySrcToCopyDst[_modifierFlow.exit] = m_nodeContainer.newNode();
_functionFlow->entry = copySrcToCopyDst[_modifierFlow.entry];
_functionFlow->exit = copySrcToCopyDst[_modifierFlow.exit];
}

View File

@ -31,25 +31,57 @@ namespace dev
namespace solidity
{
/** Basic Control Flow Block.
* Basic block of control flow. Consists of a set of (unordered) AST nodes
* for which control flow is always linear. A basic control flow block
* encompasses at most one scope. Reverts are considered to break the control
* flow.
* @todo Handle function calls correctly. So far function calls are not considered
* to change the control flow.
*/
struct ControlFlowBlock
/** Occurrence of a variable in a block of control flow.
* Stores the declaration of the referenced variable, the
* kind of the occurrence and possibly the node at which
* it occurred.
*/
class VariableOccurrence
{
/// All variable declarations inside this control flow block.
std::vector<VariableDeclaration const*> variableDeclarations;
/// All expressions inside this control flow block (this includes all subexpressions!).
std::vector<Expression const*> expressions;
/// All inline assembly statements inside in this control flow block.
std::vector<InlineAssembly const*> inlineAssemblyStatements;
/// If control flow returns in this node, the return statement is stored in returnStatement,
/// otherwise returnStatement is nullptr.
Return const* returnStatement = nullptr;
public:
enum class Kind
{
Declaration,
Access,
Return,
Assignment,
InlineAssembly
};
VariableOccurrence(VariableDeclaration const& _declaration, Kind _kind, ASTNode const* _occurrence):
m_declaration(_declaration), m_occurrenceKind(_kind), m_occurrence(_occurrence)
{
}
/// Defines a deterministic order on variable occurrences.
bool operator<(VariableOccurrence const& _rhs) const
{
if (m_occurrence && _rhs.m_occurrence)
{
if (m_occurrence->id() < _rhs.m_occurrence->id()) return true;
if (_rhs.m_occurrence->id() < m_occurrence->id()) return false;
}
else if (_rhs.m_occurrence)
return true;
else if (m_occurrence)
return false;
using KindCompareType = std::underlying_type<VariableOccurrence::Kind>::type;
return
std::make_pair(m_declaration.id(), static_cast<KindCompareType>(m_occurrenceKind)) <
std::make_pair(_rhs.m_declaration.id(), static_cast<KindCompareType>(_rhs.m_occurrenceKind))
;
}
VariableDeclaration const& declaration() const { return m_declaration; }
Kind kind() const { return m_occurrenceKind; };
ASTNode const* occurrence() const { return m_occurrence; }
private:
/// Declaration of the occurring variable.
VariableDeclaration const& m_declaration;
/// Kind of occurrence.
Kind m_occurrenceKind = Kind::Access;
/// AST node at which the variable occurred, if available (may be nullptr).
ASTNode const* m_occurrence = nullptr;
};
/** Node of the Control Flow Graph.
@ -64,8 +96,8 @@ struct CFGNode
/// Exit nodes. All CFG nodes to which control flow may continue after this node.
std::vector<CFGNode*> exits;
/// Control flow in the node.
ControlFlowBlock block;
/// Variable occurrences in the node.
std::vector<VariableOccurrence> variableOccurrences;
};
/** Describes the control flow of a function. */
@ -85,19 +117,6 @@ struct FunctionFlow
CFGNode* revert = nullptr;
};
/** Describes the control flow of a modifier.
* Every placeholder breaks the control flow. The node preceding the
* placeholder is assigned placeholderEntry as exit and the node
* following the placeholder is assigned placeholderExit as entry.
*/
struct ModifierFlow: FunctionFlow
{
/// Control flow leading towards a placeholder exit in placeholderEntry.
CFGNode* placeholderEntry = nullptr;
/// Control flow coming from a placeholder enter from placeholderExit.
CFGNode* placeholderExit = nullptr;
};
class CFG: private ASTConstVisitor
{
public:
@ -105,7 +124,6 @@ public:
bool constructFlow(ASTNode const& _astRoot);
bool visit(ModifierDefinition const& _modifier) override;
bool visit(FunctionDefinition const& _function) override;
FunctionFlow const& functionFlow(FunctionDefinition const& _function) const;
@ -118,20 +136,6 @@ public:
std::vector<std::unique_ptr<CFGNode>> m_nodes;
};
private:
/// Initially the control flow for all functions *ignoring* modifiers and for
/// all modifiers is constructed. Afterwards the control flow of functions
/// is adjusted by applying all modifiers.
void applyModifiers();
/// Creates a copy of the modifier flow @a _modifierFlow, while replacing the
/// placeholder entry and exit with the function entry and exit, as well as
/// replacing the modifier revert node with the function's revert node.
/// The resulting control flow is the new function flow with the modifier applied.
/// @a _functionFlow is updated in-place.
void applyModifierFlowToFunctionFlow(
ModifierFlow const& _modifierFlow,
FunctionFlow* _functionFlow
);
langutil::ErrorReporter& m_errorReporter;
@ -141,7 +145,6 @@ private:
NodeContainer m_nodeContainer;
std::map<FunctionDefinition const*, std::unique_ptr<FunctionFlow>> m_functionControlFlow;
std::map<ModifierDefinition const*, std::unique_ptr<ModifierFlow>> m_modifierControlFlow;
};
}

View File

@ -2,4 +2,4 @@ contract C {
function f() internal pure returns (mapping(uint=>uint) storage r) { }
}
// ----
// TypeError: (53-82): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error.
// TypeError: (53-82): This variable is of storage pointer type and can be returned without prior assignment.

View File

@ -2,4 +2,4 @@ contract C {
function f() internal pure returns (mapping(uint=>uint) storage) {}
}
// ----
// TypeError: (53-80): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error.
// TypeError: (53-80): This variable is of storage pointer type and can be returned without prior assignment.

View File

@ -7,4 +7,4 @@ contract C {
}
}
// ----
// TypeError: (87-96): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error.
// TypeError: (87-96): This variable is of storage pointer type and can be returned without prior assignment.

View File

@ -1,26 +0,0 @@
contract C {
struct S { bool f; }
S s;
function f() internal returns (S storage c) {
assembly {
sstore(c_slot, sload(s_slot))
}
}
function g(bool flag) internal returns (S storage c) {
// control flow in assembly will not be analyzed for now,
// so this will not issue an error
assembly {
if flag {
sstore(c_slot, sload(s_slot))
}
}
}
function h() internal returns (S storage c) {
// any reference from assembly will be sufficient for now,
// so this will not issue an error
assembly {
sstore(s_slot, sload(c_slot))
}
}
}
// ----

View File

@ -45,8 +45,8 @@ contract C {
}
}
// ----
// TypeError: (87-98): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error.
// TypeError: (223-234): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error.
// TypeError: (440-451): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error.
// TypeError: (654-665): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error.
// TypeError: (871-882): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error.
// TypeError: (87-98): This variable is of storage pointer type and can be returned without prior assignment.
// TypeError: (223-234): This variable is of storage pointer type and can be returned without prior assignment.
// TypeError: (440-451): This variable is of storage pointer type and can be returned without prior assignment.
// TypeError: (654-665): This variable is of storage pointer type and can be returned without prior assignment.
// TypeError: (871-882): This variable is of storage pointer type and can be returned without prior assignment.

View File

@ -12,5 +12,5 @@ contract C {
}
}
// ----
// TypeError: (87-98): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error.
// TypeError: (182-193): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error.
// TypeError: (87-98): This variable is of storage pointer type and can be returned without prior assignment.
// TypeError: (182-193): This variable is of storage pointer type and can be returned without prior assignment.

View File

@ -14,5 +14,5 @@ contract C {
}
}
// ----
// TypeError: (96-107): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error.
// TypeError: (186-197): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error.
// TypeError: (96-107): This variable is of storage pointer type and can be returned without prior assignment.
// TypeError: (186-197): This variable is of storage pointer type and can be returned without prior assignment.

View File

@ -18,5 +18,5 @@ contract C {
}
}
// ----
// TypeError: (249-258): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error.
// TypeError: (367-376): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error.
// TypeError: (249-258): This variable is of storage pointer type and can be returned without prior assignment.
// TypeError: (367-376): This variable is of storage pointer type and can be returned without prior assignment.

View File

@ -13,6 +13,6 @@ contract C {
}
}
// ----
// TypeError: (87-98): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error.
// TypeError: (176-187): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error.
// TypeError: (264-275): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error.
// TypeError: (87-98): This variable is of storage pointer type and can be returned without prior assignment.
// TypeError: (176-187): This variable is of storage pointer type and can be returned without prior assignment.
// TypeError: (264-275): This variable is of storage pointer type and can be returned without prior assignment.

View File

@ -9,5 +9,5 @@ contract C {
}
}
// ----
// TypeError: (96-107): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error.
// TypeError: (200-211): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error.
// TypeError: (96-107): This variable is of storage pointer type and can be returned without prior assignment.
// TypeError: (200-211): This variable is of storage pointer type and can be returned without prior assignment.

View File

@ -8,4 +8,4 @@ contract C {
}
}
// ----
// TypeError: (87-98): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error.
// TypeError: (87-98): This variable is of storage pointer type and can be returned without prior assignment.

View File

@ -0,0 +1,8 @@
contract C {
function f() internal view returns(uint[] storage a)
{
uint b = a[0];
revert();
b;
}
}

View File

@ -0,0 +1,9 @@
contract C {
uint[] r;
function f() internal view returns (uint[] storage s) {
assembly { pop(s_slot) }
s = r;
}
}
// ----
// TypeError: (92-126): This variable is of storage pointer type and can be accessed without prior assignment.

View File

@ -0,0 +1,8 @@
contract C {
// Make sure function parameters and return values are not considered
// for uninitialized return detection in the control flow analysis.
function f(function(uint[] storage) internal returns (uint[] storage)) internal pure
returns (function(uint[] storage) internal returns (uint[] storage))
{
}
}

View File

@ -0,0 +1,8 @@
contract C {
modifier m1(uint[] storage a) { _; }
modifier m2(uint[] storage a) { _; }
uint[] s;
function f() m1(b) m2(b = s) internal view returns (uint[] storage b) {}
}
// ----
// TypeError: (129-130): This variable is of storage pointer type and can be accessed without prior assignment.

View File

@ -0,0 +1,6 @@
contract C {
modifier m1(uint[] storage a) { _; }
modifier m2(uint[] storage a) { _; }
uint[] s;
function f() m1(b = s) m2(b) internal view returns (uint[] storage b) {}
}

View File

@ -0,0 +1,13 @@
contract C {
uint[] s;
modifier mod(uint[] storage b) {
_;
b[0] = 0;
}
function f() mod(a) internal returns (uint[] storage a)
{
a = s;
}
}
// ----
// TypeError: (120-121): This variable is of storage pointer type and can be accessed without prior assignment.

View File

@ -0,0 +1,13 @@
contract C {
uint[] s;
modifier mod(uint[] storage b) {
b[0] = 0;
_;
}
function f() mod(a) internal returns (uint[] storage a)
{
a = s;
}
}
// ----
// TypeError: (120-121): This variable is of storage pointer type and can be accessed without prior assignment.

View File

@ -0,0 +1,10 @@
contract C {
uint[] s;
function f() internal returns (uint[] storage a)
{
a[0] = 0;
a = s;
}
}
// ----
// TypeError: (94-95): This variable is of storage pointer type and can be accessed without prior assignment.

View File

@ -0,0 +1,11 @@
contract C {
struct S { uint a; }
S s;
function f() internal returns (S storage r)
{
r.a = 0;
r = s;
}
}
// ----
// TypeError: (109-110): This variable is of storage pointer type and can be accessed without prior assignment.

View File

@ -0,0 +1,10 @@
contract C {
uint[] s;
function f() internal returns (uint[] storage a)
{
revert();
a[0] = 0;
a = s;
}
}
// ----

View File

@ -8,3 +8,9 @@ library L {
function i(uint[] calldata, uint[] storage) external pure returns (S storage x) {return x; }
}
// ----
// TypeError: (197-198): This variable is of storage pointer type and can be accessed without prior assignment.
// TypeError: (203-204): This variable is of storage pointer type and can be accessed without prior assignment.
// TypeError: (359-360): This variable is of storage pointer type and can be accessed without prior assignment.
// TypeError: (365-366): This variable is of storage pointer type and can be accessed without prior assignment.
// TypeError: (460-461): This variable is of storage pointer type and can be accessed without prior assignment.
// TypeError: (557-558): This variable is of storage pointer type and can be accessed without prior assignment.