aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDaniel Kirchner <daniel@ekpyron.org>2018-12-08 01:20:35 +0800
committerDaniel Kirchner <daniel@ekpyron.org>2018-12-12 11:20:53 +0800
commit788612d2efef33aad711646a1ace9dfee6237730 (patch)
treeea4f403f1bf1bdfe027afa2f3401fdb450feb30a
parent1476acb8045033a9a3d2e1a1d13c5aaa8ed6942c (diff)
downloaddexon-solidity-788612d2efef33aad711646a1ace9dfee6237730.tar.gz
dexon-solidity-788612d2efef33aad711646a1ace9dfee6237730.tar.zst
dexon-solidity-788612d2efef33aad711646a1ace9dfee6237730.zip
Refactoring of the ControlFlowGraph and use for detecting all uninitialized storage accesses.
-rw-r--r--Changelog.md1
-rw-r--r--libsolidity/analysis/ControlFlowAnalyzer.cpp184
-rw-r--r--libsolidity/analysis/ControlFlowAnalyzer.h8
-rw-r--r--libsolidity/analysis/ControlFlowBuilder.cpp262
-rw-r--r--libsolidity/analysis/ControlFlowBuilder.h31
-rw-r--r--libsolidity/analysis/ControlFlowGraph.cpp88
-rw-r--r--libsolidity/analysis/ControlFlowGraph.h101
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/mappingReturn/named_err.sol2
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/mappingReturn/unnamed_err.sol2
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly_err.sol2
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly_fine.sol26
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/dowhile_err.sol10
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/for_err.sol4
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/if_err.sol4
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/modifier_err.sol4
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/short_circuit_err.sol6
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/ternary_err.sol4
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/while_err.sol2
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/always_revert.sol8
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/assembly.sol9
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/functionType.sol8
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/modifier_order_fail.sol8
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/modifier_order_fine.sol6
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/modifier_post_access.sol13
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/modifier_pre_access.sol13
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/smoke.sol10
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/struct.sol11
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/unreachable.sol10
-rw-r--r--test/libsolidity/syntaxTests/dataLocations/libraries/library_function_with_data_location_fine.sol6
29 files changed, 471 insertions, 372 deletions
diff --git a/Changelog.md b/Changelog.md
index 5bbf5af6..7e60ff38 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -1,6 +1,7 @@
### 0.5.2 (unreleased)
Language Features:
+ * Control Flow Graph: Detect every access to uninitialized storage pointers.
Compiler Features:
diff --git a/libsolidity/analysis/ControlFlowAnalyzer.cpp b/libsolidity/analysis/ControlFlowAnalyzer.cpp
index fe58f0aa..a7e1ed97 100644
--- a/libsolidity/analysis/ControlFlowAnalyzer.cpp
+++ b/libsolidity/analysis/ControlFlowAnalyzer.cpp
@@ -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;
-}
-
-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
+ };
+ map<CFGNode const*, NodeInfo> nodeInfos;
+ set<CFGNode const*> nodesToTraverse;
+ nodesToTraverse.insert(_entry);
+
+ // 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();
-
- auto& unassignedAtNode = unassigned[node];
+ CFGNode const* currentNode = *nodesToTraverse.begin();
+ nodesToTraverse.erase(nodesToTraverse.begin());
- 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."
);
}
}
diff --git a/libsolidity/analysis/ControlFlowAnalyzer.h b/libsolidity/analysis/ControlFlowAnalyzer.h
index 411d57ff..859d826a 100644
--- a/libsolidity/analysis/ControlFlowAnalyzer.h
+++ b/libsolidity/analysis/ControlFlowAnalyzer.h
@@ -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;
diff --git a/libsolidity/analysis/ControlFlowBuilder.cpp b/libsolidity/analysis/ControlFlowBuilder.cpp
index 5bd39da3..3dab8b16 100644
--- a/libsolidity/analysis/ControlFlowBuilder.cpp
+++ b/libsolidity/analysis/ControlFlowBuilder.cpp
@@ -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,26 +40,8 @@ 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;
+ return functionFlow;
}
bool ControlFlowBuilder::visit(BinaryOperation const& _operation)
@@ -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);
diff --git a/libsolidity/analysis/ControlFlowBuilder.h b/libsolidity/analysis/ControlFlowBuilder.h
index 40605e00..f196e5fc 100644
--- a/libsolidity/analysis/ControlFlowBuilder.h
+++ b/libsolidity/analysis/ControlFlowBuilder.h
@@ -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
diff --git a/libsolidity/analysis/ControlFlowGraph.cpp b/libsolidity/analysis/ControlFlowGraph.cpp
index b8860158..a7cc9508 100644
--- a/libsolidity/analysis/ControlFlowGraph.cpp
+++ b/libsolidity/analysis/ControlFlowGraph.cpp
@@ -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];
-}
diff --git a/libsolidity/analysis/ControlFlowGraph.h b/libsolidity/analysis/ControlFlowGraph.h
index 8fe9fe8e..db8e1565 100644
--- a/libsolidity/analysis/ControlFlowGraph.h
+++ b/libsolidity/analysis/ControlFlowGraph.h
@@ -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;
};
}
diff --git a/test/libsolidity/syntaxTests/controlFlow/mappingReturn/named_err.sol b/test/libsolidity/syntaxTests/controlFlow/mappingReturn/named_err.sol
index 35420b6d..dbd3db08 100644
--- a/test/libsolidity/syntaxTests/controlFlow/mappingReturn/named_err.sol
+++ b/test/libsolidity/syntaxTests/controlFlow/mappingReturn/named_err.sol
@@ -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.
diff --git a/test/libsolidity/syntaxTests/controlFlow/mappingReturn/unnamed_err.sol b/test/libsolidity/syntaxTests/controlFlow/mappingReturn/unnamed_err.sol
index 52a8b3d7..476e799b 100644
--- a/test/libsolidity/syntaxTests/controlFlow/mappingReturn/unnamed_err.sol
+++ b/test/libsolidity/syntaxTests/controlFlow/mappingReturn/unnamed_err.sol
@@ -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.
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly_err.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly_err.sol
index cad9b8e8..a6df889d 100644
--- a/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly_err.sol
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly_err.sol
@@ -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.
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly_fine.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly_fine.sol
deleted file mode 100644
index 0d3db856..00000000
--- a/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly_fine.sol
+++ /dev/null
@@ -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))
- }
- }
-}
-// ----
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/dowhile_err.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/dowhile_err.sol
index eb574c96..b868d61d 100644
--- a/test/libsolidity/syntaxTests/controlFlow/storageReturn/dowhile_err.sol
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/dowhile_err.sol
@@ -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.
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/for_err.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/for_err.sol
index 9aa580a4..d2dec9c1 100644
--- a/test/libsolidity/syntaxTests/controlFlow/storageReturn/for_err.sol
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/for_err.sol
@@ -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.
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/if_err.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/if_err.sol
index f3e55318..12ec31e4 100644
--- a/test/libsolidity/syntaxTests/controlFlow/storageReturn/if_err.sol
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/if_err.sol
@@ -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.
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/modifier_err.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/modifier_err.sol
index 42342979..ee1e08fd 100644
--- a/test/libsolidity/syntaxTests/controlFlow/storageReturn/modifier_err.sol
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/modifier_err.sol
@@ -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.
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/short_circuit_err.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/short_circuit_err.sol
index d0ad8245..e3579628 100644
--- a/test/libsolidity/syntaxTests/controlFlow/storageReturn/short_circuit_err.sol
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/short_circuit_err.sol
@@ -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.
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/ternary_err.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/ternary_err.sol
index 6d10287b..d5ad73c5 100644
--- a/test/libsolidity/syntaxTests/controlFlow/storageReturn/ternary_err.sol
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/ternary_err.sol
@@ -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.
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/while_err.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/while_err.sol
index e7b4fae7..aabb76dd 100644
--- a/test/libsolidity/syntaxTests/controlFlow/storageReturn/while_err.sol
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/while_err.sol
@@ -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.
diff --git a/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/always_revert.sol b/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/always_revert.sol
new file mode 100644
index 00000000..96767402
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/always_revert.sol
@@ -0,0 +1,8 @@
+contract C {
+ function f() internal view returns(uint[] storage a)
+ {
+ uint b = a[0];
+ revert();
+ b;
+ }
+} \ No newline at end of file
diff --git a/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/assembly.sol b/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/assembly.sol
new file mode 100644
index 00000000..bfcbbef5
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/assembly.sol
@@ -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.
diff --git a/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/functionType.sol b/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/functionType.sol
new file mode 100644
index 00000000..1d683eff
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/functionType.sol
@@ -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))
+ {
+ }
+}
diff --git a/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/modifier_order_fail.sol b/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/modifier_order_fail.sol
new file mode 100644
index 00000000..90d228fa
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/modifier_order_fail.sol
@@ -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.
diff --git a/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/modifier_order_fine.sol b/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/modifier_order_fine.sol
new file mode 100644
index 00000000..af133929
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/modifier_order_fine.sol
@@ -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) {}
+} \ No newline at end of file
diff --git a/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/modifier_post_access.sol b/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/modifier_post_access.sol
new file mode 100644
index 00000000..b9f08615
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/modifier_post_access.sol
@@ -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.
diff --git a/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/modifier_pre_access.sol b/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/modifier_pre_access.sol
new file mode 100644
index 00000000..81618aec
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/modifier_pre_access.sol
@@ -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.
diff --git a/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/smoke.sol b/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/smoke.sol
new file mode 100644
index 00000000..41ced51d
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/smoke.sol
@@ -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.
diff --git a/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/struct.sol b/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/struct.sol
new file mode 100644
index 00000000..b4f2be5d
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/struct.sol
@@ -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.
diff --git a/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/unreachable.sol b/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/unreachable.sol
new file mode 100644
index 00000000..b941ad34
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/unreachable.sol
@@ -0,0 +1,10 @@
+contract C {
+ uint[] s;
+ function f() internal returns (uint[] storage a)
+ {
+ revert();
+ a[0] = 0;
+ a = s;
+ }
+}
+// ----
diff --git a/test/libsolidity/syntaxTests/dataLocations/libraries/library_function_with_data_location_fine.sol b/test/libsolidity/syntaxTests/dataLocations/libraries/library_function_with_data_location_fine.sol
index 7a276f95..95d4e02a 100644
--- a/test/libsolidity/syntaxTests/dataLocations/libraries/library_function_with_data_location_fine.sol
+++ b/test/libsolidity/syntaxTests/dataLocations/libraries/library_function_with_data_location_fine.sol
@@ -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.