From 788612d2efef33aad711646a1ace9dfee6237730 Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Fri, 7 Dec 2018 18:20:35 +0100 Subject: Refactoring of the ControlFlowGraph and use for detecting all uninitialized storage accesses. --- Changelog.md | 1 + libsolidity/analysis/ControlFlowAnalyzer.cpp | 184 +++++++-------- libsolidity/analysis/ControlFlowAnalyzer.h | 8 +- libsolidity/analysis/ControlFlowBuilder.cpp | 262 +++++++++++++++------ libsolidity/analysis/ControlFlowBuilder.h | 31 +-- libsolidity/analysis/ControlFlowGraph.cpp | 88 +------ libsolidity/analysis/ControlFlowGraph.h | 101 ++++---- .../controlFlow/mappingReturn/named_err.sol | 2 +- .../controlFlow/mappingReturn/unnamed_err.sol | 2 +- .../controlFlow/storageReturn/assembly_err.sol | 2 +- .../controlFlow/storageReturn/assembly_fine.sol | 26 -- .../controlFlow/storageReturn/dowhile_err.sol | 10 +- .../controlFlow/storageReturn/for_err.sol | 4 +- .../controlFlow/storageReturn/if_err.sol | 4 +- .../controlFlow/storageReturn/modifier_err.sol | 4 +- .../storageReturn/short_circuit_err.sol | 6 +- .../controlFlow/storageReturn/ternary_err.sol | 4 +- .../controlFlow/storageReturn/while_err.sol | 2 +- .../uninitializedAccess/always_revert.sol | 8 + .../controlFlow/uninitializedAccess/assembly.sol | 9 + .../uninitializedAccess/functionType.sol | 8 + .../uninitializedAccess/modifier_order_fail.sol | 8 + .../uninitializedAccess/modifier_order_fine.sol | 6 + .../uninitializedAccess/modifier_post_access.sol | 13 + .../uninitializedAccess/modifier_pre_access.sol | 13 + .../controlFlow/uninitializedAccess/smoke.sol | 10 + .../controlFlow/uninitializedAccess/struct.sol | 11 + .../uninitializedAccess/unreachable.sol | 10 + .../library_function_with_data_location_fine.sol | 6 + 29 files changed, 471 insertions(+), 372 deletions(-) delete mode 100644 test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly_fine.sol create mode 100644 test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/always_revert.sol create mode 100644 test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/assembly.sol create mode 100644 test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/functionType.sol create mode 100644 test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/modifier_order_fail.sol create mode 100644 test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/modifier_order_fine.sol create mode 100644 test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/modifier_post_access.sol create mode 100644 test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/modifier_pre_access.sol create mode 100644 test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/smoke.sol create mode 100644 test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/struct.sol create mode 100644 test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/unreachable.sol 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 #include +#include 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 ControlFlowAnalyzer::variablesAssignedInNode(CFGNode const *node) +void ControlFlowAnalyzer::checkUninitializedAccess(CFGNode const* _entry, CFGNode const* _exit) const { - set result; - for (auto expression: node->block.expressions) + struct NodeInfo { - if (auto const* assignment = dynamic_cast(expression)) + set unassignedVariablesAtEntry; + set unassignedVariablesAtExit; + set 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 expressions; - expressions.push(&assignment->leftHandSide()); - while (!expressions.empty()) - { - Expression const* expression = expressions.top(); - expressions.pop(); - - if (auto const *tuple = dynamic_cast(expression)) - for (auto const& component: tuple->components()) - expressions.push(component.get()); - else if (auto const* identifier = dynamic_cast(expression)) - if (auto const* variableDeclaration = dynamic_cast( - 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> 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 nodesToTraverse; - nodesToTraverse.push(_functionEntry); - - // walk all paths from entry with maximal set of unassigned return values + }; + map nodeInfos; + set 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(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 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 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 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 ControlFlowBuilder::createFunctionFlow( functionFlow->revert = _nodeContainer.newNode(); ControlFlowBuilder builder(_nodeContainer, *functionFlow); builder.appendControlFlow(_function); - connect(builder.m_currentNode, functionFlow->exit); - return functionFlow; -} - -unique_ptr ControlFlowBuilder::createModifierFlow( - CFG::NodeContainer& _nodeContainer, - ModifierDefinition const& _modifier -) -{ - auto modifierFlow = unique_ptr(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(&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(&node)) - m_currentNode->block.expressions.emplace_back(expression); - else if (auto const* variableDeclaration = dynamic_cast(&node)) - m_currentNode->block.variableDeclarations.emplace_back(variableDeclaration); - else if (auto const* assembly = dynamic_cast(&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( + _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(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(expression)) + if (tupleExpression->components().size() > 1) + { + solAssert(tupleExpression->components().size() > i, ""); + expression = tupleExpression->components()[i].get(); + } + while (auto tupleExpression = dynamic_cast(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(_identifier.annotation().referencedDeclaration)) + m_currentNode->variableOccurrences.emplace_back( + *variableDeclaration, + static_cast(_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 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( - 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 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 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 variableDeclarations; - /// All expressions inside this control flow block (this includes all subexpressions!). - std::vector expressions; - /// All inline assembly statements inside in this control flow block. - std::vector 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::type; + return + std::make_pair(m_declaration.id(), static_cast(m_occurrenceKind)) < + std::make_pair(_rhs.m_declaration.id(), static_cast(_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 exits; - /// Control flow in the node. - ControlFlowBlock block; + /// Variable occurrences in the node. + std::vector 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> 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> m_functionControlFlow; - std::map> 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. -- cgit