aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorchriseth <chris@ethereum.org>2018-05-15 03:05:50 +0800
committerGitHub <noreply@github.com>2018-05-15 03:05:50 +0800
commit7f97995e11ebfbbd76df4d792a358793ae5f9cc4 (patch)
tree1a9af2c7a182d8633a2b2c104bd52fa3acac3b8c
parent8f17f7219aec4586961af41b9bdd1804b3d59aa5 (diff)
parentd7d71a14dfa7ab6f51842c086c25b9e7a2d9b19a (diff)
downloaddexon-solidity-7f97995e11ebfbbd76df4d792a358793ae5f9cc4.tar
dexon-solidity-7f97995e11ebfbbd76df4d792a358793ae5f9cc4.tar.gz
dexon-solidity-7f97995e11ebfbbd76df4d792a358793ae5f9cc4.tar.bz2
dexon-solidity-7f97995e11ebfbbd76df4d792a358793ae5f9cc4.tar.lz
dexon-solidity-7f97995e11ebfbbd76df4d792a358793ae5f9cc4.tar.xz
dexon-solidity-7f97995e11ebfbbd76df4d792a358793ae5f9cc4.tar.zst
dexon-solidity-7f97995e11ebfbbd76df4d792a358793ae5f9cc4.zip
Merge pull request #4057 from ethereum/cfg
Control flow graph for uninitialized storage return detection.
-rw-r--r--Changelog.md2
-rw-r--r--libsolidity/analysis/ControlFlowAnalyzer.cpp156
-rw-r--r--libsolidity/analysis/ControlFlowAnalyzer.h52
-rw-r--r--libsolidity/analysis/ControlFlowBuilder.cpp370
-rw-r--r--libsolidity/analysis/ControlFlowBuilder.h143
-rw-r--r--libsolidity/analysis/ControlFlowGraph.cpp136
-rw-r--r--libsolidity/analysis/ControlFlowGraph.h148
-rw-r--r--libsolidity/interface/CompilerStack.cpp18
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly_fine.sol26
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly_warn.sol10
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/default_location.sol19
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/dowhile_fine.sol36
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/dowhile_warn.sol35
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/emptyReturn_fine.sol6
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/emptyReturn_warn.sol15
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/for_fine.sol13
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/for_warn.sol16
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/if_fine.sol29
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/if_warn.sol18
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/modifier_fine.sol20
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/modifier_warn.sol22
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/revert_fine.sol12
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/short_circuit_fine.sol11
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/short_circuit_warn.sol18
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/smoke.sol10
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/ternary_fine.sol14
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/ternary_warn.sol13
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/throw_fine.sol9
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/tuple_fine.sol12
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/while_fine.sol19
-rw-r--r--test/libsolidity/syntaxTests/controlFlow/storageReturn/while_warn.sol11
31 files changed, 1419 insertions, 0 deletions
diff --git a/Changelog.md b/Changelog.md
index dfa24a25..6f6f672c 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -4,6 +4,8 @@ Features:
* Remove deprecated ``constant`` as function state modifier from documentation and tests (but still leave it as a valid feature).
* Build System: Update internal dependency of jsoncpp to 1.8.4, which introduces more strictness and reduces memory usage.
* Code Generator: Use native shift instructions on target Constantinople.
+ * Control Flow Graph: Add Control Flow Graph as analysis structure.
+ * Control Flow Graph: Warn about returning uninitialized storage pointers.
* Gas Estimator: Only explore paths with higher gas costs. This reduces accuracy but greatly improves the speed of gas estimation.
* Optimizer: Remove unnecessary masking of the result of known short instructions (``ADDRESS``, ``CALLER``, ``ORIGIN`` and ``COINBASE``).
* Parser: Display nicer error messages by showing the actual tokens and not internal names.
diff --git a/libsolidity/analysis/ControlFlowAnalyzer.cpp b/libsolidity/analysis/ControlFlowAnalyzer.cpp
new file mode 100644
index 00000000..6edf7986
--- /dev/null
+++ b/libsolidity/analysis/ControlFlowAnalyzer.cpp
@@ -0,0 +1,156 @@
+/*
+ This file is part of solidity.
+
+ solidity is free software: you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation, either version 3 of the License, or
+ (at your option) any later version.
+
+ solidity is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with solidity. If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include <libsolidity/analysis/ControlFlowAnalyzer.h>
+
+using namespace std;
+using namespace dev::solidity;
+
+bool ControlFlowAnalyzer::analyze(ASTNode const& _astRoot)
+{
+ _astRoot.accept(*this);
+ return Error::containsOnlyWarnings(m_errorReporter.errors());
+}
+
+bool ControlFlowAnalyzer::visit(FunctionDefinition const& _function)
+{
+ auto const& functionFlow = m_cfg.functionFlow(_function);
+ checkUnassignedStorageReturnValues(_function, functionFlow.entry, functionFlow.exit);
+ return false;
+}
+
+set<VariableDeclaration const*> ControlFlowAnalyzer::variablesAssignedInNode(CFGNode const *node)
+{
+ set<VariableDeclaration const*> result;
+ for (auto expression: node->block.expressions)
+ {
+ if (auto const* assignment = dynamic_cast<Assignment const*>(expression))
+ {
+ 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);
+ }
+ }
+ }
+ 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))
+ unassignedAtFunctionEntry.insert(returnParameter.get());
+ }
+
+ stack<CFGNode const*> nodesToTraverse;
+ nodesToTraverse.push(_functionEntry);
+
+ // walk all paths from entry with maximal set of unassigned return values
+ while (!nodesToTraverse.empty())
+ {
+ auto node = nodesToTraverse.top();
+ nodesToTraverse.pop();
+
+ auto& unassignedAtNode = unassigned[node];
+
+ if (node->block.returnStatement != nullptr)
+ if (node->block.returnStatement->expression())
+ unassignedAtNode.clear();
+ if (!unassignedAtNode.empty())
+ {
+ // 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);
+ }
+
+ 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);
+ }
+ }
+
+ if (!unassigned[_functionExit].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();
+ }
+ );
+ for (auto const* returnVal: unassignedOrdered)
+ {
+ 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());
+ }
+
+ m_errorReporter.warning(
+ returnVal->location(),
+ "This variable is of storage pointer type and might be returned without assignment. "
+ "This can cause storage corruption. Assign the variable (potentially from itself) "
+ "to remove this warning.",
+ ssl
+ );
+ }
+ }
+}
diff --git a/libsolidity/analysis/ControlFlowAnalyzer.h b/libsolidity/analysis/ControlFlowAnalyzer.h
new file mode 100644
index 00000000..43e13fb6
--- /dev/null
+++ b/libsolidity/analysis/ControlFlowAnalyzer.h
@@ -0,0 +1,52 @@
+/*
+ This file is part of solidity.
+
+ solidity is free software: you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation, either version 3 of the License, or
+ (at your option) any later version.
+
+ solidity is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with solidity. If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#pragma once
+
+#include <libsolidity/analysis/ControlFlowGraph.h>
+
+#include <set>
+
+namespace dev
+{
+namespace solidity
+{
+
+class ControlFlowAnalyzer: private ASTConstVisitor
+{
+public:
+ explicit ControlFlowAnalyzer(CFG const& _cfg, ErrorReporter& _errorReporter):
+ m_cfg(_cfg), m_errorReporter(_errorReporter) {}
+
+ bool analyze(ASTNode const& _astRoot);
+
+ virtual 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;
+
+ CFG const& m_cfg;
+ ErrorReporter& m_errorReporter;
+};
+
+}
+}
diff --git a/libsolidity/analysis/ControlFlowBuilder.cpp b/libsolidity/analysis/ControlFlowBuilder.cpp
new file mode 100644
index 00000000..35d7687c
--- /dev/null
+++ b/libsolidity/analysis/ControlFlowBuilder.cpp
@@ -0,0 +1,370 @@
+/*
+ This file is part of solidity.
+
+ solidity is free software: you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation, either version 3 of the License, or
+ (at your option) any later version.
+
+ solidity is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with solidity. If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include <libsolidity/analysis/ControlFlowBuilder.h>
+
+using namespace dev;
+using namespace solidity;
+using namespace std;
+
+ControlFlowBuilder::ControlFlowBuilder(CFG::NodeContainer& _nodeContainer, FunctionFlow const& _functionFlow):
+ m_nodeContainer(_nodeContainer), m_currentFunctionFlow(_functionFlow), m_currentNode(_functionFlow.entry)
+{
+}
+
+unique_ptr<FunctionFlow> ControlFlowBuilder::createFunctionFlow(
+ CFG::NodeContainer& _nodeContainer,
+ FunctionDefinition const& _function
+)
+{
+ auto functionFlow = unique_ptr<FunctionFlow>(new FunctionFlow());
+ functionFlow->entry = _nodeContainer.newNode();
+ functionFlow->exit = _nodeContainer.newNode();
+ functionFlow->revert = _nodeContainer.newNode();
+ ControlFlowBuilder builder(_nodeContainer, *functionFlow);
+ builder.appendControlFlow(_function);
+ connect(builder.m_currentNode, functionFlow->exit);
+ return functionFlow;
+}
+
+
+unique_ptr<ModifierFlow> ControlFlowBuilder::createModifierFlow(
+ CFG::NodeContainer& _nodeContainer,
+ ModifierDefinition const& _modifier
+)
+{
+ auto modifierFlow = unique_ptr<ModifierFlow>(new ModifierFlow());
+ modifierFlow->entry = _nodeContainer.newNode();
+ modifierFlow->exit = _nodeContainer.newNode();
+ modifierFlow->revert = _nodeContainer.newNode();
+ modifierFlow->placeholderEntry = _nodeContainer.newNode();
+ modifierFlow->placeholderExit = _nodeContainer.newNode();
+ ControlFlowBuilder builder(_nodeContainer, *modifierFlow);
+ builder.appendControlFlow(_modifier);
+ connect(builder.m_currentNode, modifierFlow->exit);
+ return modifierFlow;
+}
+
+bool ControlFlowBuilder::visit(BinaryOperation const& _operation)
+{
+ solAssert(!!m_currentNode, "");
+
+ switch(_operation.getOperator())
+ {
+ case Token::Or:
+ case Token::And:
+ {
+ appendControlFlow(_operation.leftExpression());
+
+ auto nodes = splitFlow<2>();
+ nodes[0] = createFlow(nodes[0], _operation.rightExpression());
+ mergeFlow(nodes, nodes[1]);
+
+ return false;
+ }
+ default:
+ break;
+ }
+ return ASTConstVisitor::visit(_operation);
+}
+
+bool ControlFlowBuilder::visit(Conditional const& _conditional)
+{
+ solAssert(!!m_currentNode, "");
+
+ _conditional.condition().accept(*this);
+
+ auto nodes = splitFlow<2>();
+
+ nodes[0] = createFlow(nodes[0], _conditional.trueExpression());
+ nodes[1] = createFlow(nodes[1], _conditional.falseExpression());
+
+ mergeFlow(nodes);
+
+ return false;
+}
+
+bool ControlFlowBuilder::visit(IfStatement const& _ifStatement)
+{
+ solAssert(!!m_currentNode, "");
+
+ _ifStatement.condition().accept(*this);
+
+ auto nodes = splitFlow<2>();
+ nodes[0] = createFlow(nodes[0], _ifStatement.trueStatement());
+
+ if (_ifStatement.falseStatement())
+ {
+ nodes[1] = createFlow(nodes[1], *_ifStatement.falseStatement());
+ mergeFlow(nodes);
+ }
+ else
+ mergeFlow(nodes, nodes[1]);
+
+ return false;
+}
+
+bool ControlFlowBuilder::visit(ForStatement const& _forStatement)
+{
+ solAssert(!!m_currentNode, "");
+
+ if (_forStatement.initializationExpression())
+ _forStatement.initializationExpression()->accept(*this);
+
+ auto condition = createLabelHere();
+
+ if (_forStatement.condition())
+ appendControlFlow(*_forStatement.condition());
+
+ auto loopExpression = newLabel();
+ auto nodes = splitFlow<2>();
+ auto afterFor = nodes[1];
+ m_currentNode = nodes[0];
+
+ {
+ BreakContinueScope scope(*this, afterFor, loopExpression);
+ appendControlFlow(_forStatement.body());
+ }
+
+ placeAndConnectLabel(loopExpression);
+
+ if (auto expression = _forStatement.loopExpression())
+ appendControlFlow(*expression);
+
+ connect(m_currentNode, condition);
+ m_currentNode = afterFor;
+
+ return false;
+}
+
+bool ControlFlowBuilder::visit(WhileStatement const& _whileStatement)
+{
+ solAssert(!!m_currentNode, "");
+
+ if (_whileStatement.isDoWhile())
+ {
+ auto afterWhile = newLabel();
+ auto whileBody = createLabelHere();
+
+ {
+ // Note that "continue" in this case currently indeed jumps to whileBody
+ // and not to the condition. This is inconsistent with JavaScript and C and
+ // therefore a bug. This will be fixed in the future (planned for 0.5.0)
+ // and the Control Flow Graph will have to be adjusted accordingly.
+ BreakContinueScope scope(*this, afterWhile, whileBody);
+ appendControlFlow(_whileStatement.body());
+ }
+ appendControlFlow(_whileStatement.condition());
+
+ connect(m_currentNode, whileBody);
+ placeAndConnectLabel(afterWhile);
+ }
+ else
+ {
+ auto whileCondition = createLabelHere();
+
+ appendControlFlow(_whileStatement.condition());
+
+ auto nodes = splitFlow<2>();
+
+ auto whileBody = nodes[0];
+ auto afterWhile = nodes[1];
+
+ m_currentNode = whileBody;
+ {
+ BreakContinueScope scope(*this, afterWhile, whileCondition);
+ appendControlFlow(_whileStatement.body());
+ }
+
+ connect(m_currentNode, whileCondition);
+
+ m_currentNode = afterWhile;
+ }
+
+
+ return false;
+}
+
+bool ControlFlowBuilder::visit(Break const&)
+{
+ solAssert(!!m_currentNode, "");
+ solAssert(!!m_breakJump, "");
+ connect(m_currentNode, m_breakJump);
+ m_currentNode = newLabel();
+ return false;
+}
+
+bool ControlFlowBuilder::visit(Continue const&)
+{
+ solAssert(!!m_currentNode, "");
+ solAssert(!!m_continueJump, "");
+ connect(m_currentNode, m_continueJump);
+ m_currentNode = newLabel();
+ return false;
+}
+
+bool ControlFlowBuilder::visit(Throw const&)
+{
+ solAssert(!!m_currentNode, "");
+ solAssert(!!m_currentFunctionFlow.revert, "");
+ connect(m_currentNode, m_currentFunctionFlow.revert);
+ 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);
+
+ m_currentNode = newLabel();
+
+ connect(modifierFlow->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, "");
+ solAssert(!!_functionCall.expression().annotation().type, "");
+
+ if (auto functionType = dynamic_pointer_cast<FunctionType const>(_functionCall.expression().annotation().type))
+ switch (functionType->kind())
+ {
+ case FunctionType::Kind::Revert:
+ solAssert(!!m_currentFunctionFlow.revert, "");
+ _functionCall.expression().accept(*this);
+ ASTNode::listAccept(_functionCall.arguments(), *this);
+ connect(m_currentNode, m_currentFunctionFlow.revert);
+ m_currentNode = newLabel();
+ return false;
+ case FunctionType::Kind::Require:
+ case FunctionType::Kind::Assert:
+ {
+ solAssert(!!m_currentFunctionFlow.revert, "");
+ _functionCall.expression().accept(*this);
+ ASTNode::listAccept(_functionCall.arguments(), *this);
+ connect(m_currentNode, m_currentFunctionFlow.revert);
+ auto nextNode = newLabel();
+ connect(m_currentNode, nextNode);
+ m_currentNode = nextNode;
+ return false;
+ }
+ default:
+ break;
+ }
+ return ASTConstVisitor::visit(_functionCall);
+}
+
+void ControlFlowBuilder::appendControlFlow(ASTNode const& _node)
+{
+ _node.accept(*this);
+}
+
+CFGNode* ControlFlowBuilder::createFlow(CFGNode* _entry, ASTNode const& _node)
+{
+ auto oldCurrentNode = m_currentNode;
+ m_currentNode = _entry;
+ appendControlFlow(_node);
+ auto endNode = m_currentNode;
+ m_currentNode = oldCurrentNode;
+ return endNode;
+}
+
+void ControlFlowBuilder::connect(CFGNode* _from, CFGNode* _to)
+{
+ solAssert(_from, "");
+ solAssert(_to, "");
+ _from->exits.push_back(_to);
+ _to->entries.push_back(_from);
+}
+
+CFGNode* ControlFlowBuilder::newLabel()
+{
+ return m_nodeContainer.newNode();
+}
+
+CFGNode* ControlFlowBuilder::createLabelHere()
+{
+ auto label = m_nodeContainer.newNode();
+ connect(m_currentNode, label);
+ m_currentNode = label;
+ return label;
+}
+
+void ControlFlowBuilder::placeAndConnectLabel(CFGNode* _node)
+{
+ connect(m_currentNode, _node);
+ m_currentNode = _node;
+}
+
+ControlFlowBuilder::BreakContinueScope::BreakContinueScope(
+ ControlFlowBuilder& _parser,
+ CFGNode* _breakJump,
+ CFGNode* _continueJump
+): m_parser(_parser), m_origBreakJump(_parser.m_breakJump), m_origContinueJump(_parser.m_continueJump)
+{
+ m_parser.m_breakJump = _breakJump;
+ m_parser.m_continueJump = _continueJump;
+}
+
+ControlFlowBuilder::BreakContinueScope::~BreakContinueScope()
+{
+ m_parser.m_breakJump = m_origBreakJump;
+ m_parser.m_continueJump = m_origContinueJump;
+}
diff --git a/libsolidity/analysis/ControlFlowBuilder.h b/libsolidity/analysis/ControlFlowBuilder.h
new file mode 100644
index 00000000..e9d96e5f
--- /dev/null
+++ b/libsolidity/analysis/ControlFlowBuilder.h
@@ -0,0 +1,143 @@
+/*
+ This file is part of solidity.
+
+ solidity is free software: you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation, either version 3 of the License, or
+ (at your option) any later version.
+
+ solidity is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with solidity. If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#pragma once
+
+#include <libsolidity/analysis/ControlFlowGraph.h>
+#include <libsolidity/ast/AST.h>
+#include <libsolidity/ast/ASTVisitor.h>
+
+#include <array>
+#include <memory>
+
+namespace dev {
+namespace solidity {
+
+/** Helper class that builds the control flow of a function or modifier.
+ * Modifiers are not yet applied to the functions. This is done in a second
+ * step in the CFG class.
+ */
+class ControlFlowBuilder: private ASTConstVisitor
+{
+public:
+ static std::unique_ptr<FunctionFlow> createFunctionFlow(
+ 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);
+
+ virtual bool visit(BinaryOperation const& _operation) override;
+ virtual bool visit(Conditional const& _conditional) override;
+ virtual bool visit(IfStatement const& _ifStatement) override;
+ virtual bool visit(ForStatement const& _forStatement) override;
+ virtual bool visit(WhileStatement const& _whileStatement) override;
+ virtual bool visit(Break const&) override;
+ virtual bool visit(Continue const&) override;
+ virtual bool visit(Throw const&) override;
+ virtual bool visit(Block const&) override;
+ virtual void endVisit(Block const&) override;
+ virtual bool visit(Return const& _return) override;
+ virtual bool visit(PlaceholderStatement const&) override;
+ virtual bool visit(FunctionCall const& _functionCall) override;
+
+
+ /// Appends the control flow of @a _node to the current control flow.
+ void appendControlFlow(ASTNode const& _node);
+
+ /// Starts at @a _entry and parses the control flow of @a _node.
+ /// @returns The node at which the parsed control flow ends.
+ /// m_currentNode is not affected (it is saved and restored).
+ CFGNode* createFlow(CFGNode* _entry, ASTNode const& _node);
+
+ /// Creates an arc from @a _from to @a _to.
+ static void connect(CFGNode* _from, CFGNode* _to);
+
+
+protected:
+ virtual bool visitNode(ASTNode const& node) override;
+
+private:
+
+ /// Splits the control flow starting at the current node into n paths.
+ /// m_currentNode is set to nullptr and has to be set manually or
+ /// using mergeFlow later.
+ template<size_t n>
+ std::array<CFGNode*, n> splitFlow()
+ {
+ std::array<CFGNode*, n> result;
+ for (auto& node: result)
+ {
+ node = m_nodeContainer.newNode();
+ connect(m_currentNode, node);
+ }
+ m_currentNode = nullptr;
+ return result;
+ }
+
+ /// Merges the control flow of @a _nodes to @a _endNode.
+ /// If @a _endNode is nullptr, a new node is creates and used as end node.
+ /// Sets the merge destination as current node.
+ /// Note: @a _endNode may be one of the nodes in @a _nodes.
+ template<size_t n>
+ void mergeFlow(std::array<CFGNode*, n> const& _nodes, CFGNode* _endNode = nullptr)
+ {
+ CFGNode* mergeDestination = (_endNode == nullptr) ? m_nodeContainer.newNode() : _endNode;
+ for (auto& node: _nodes)
+ if (node != mergeDestination)
+ connect(node, mergeDestination);
+ m_currentNode = mergeDestination;
+ }
+
+ CFGNode* newLabel();
+ CFGNode* createLabelHere();
+ void placeAndConnectLabel(CFGNode *_node);
+
+ 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;
+
+ /// The current jump destination of break Statements.
+ CFGNode* m_breakJump = nullptr;
+ /// The current jump destination of continue Statements.
+ CFGNode* m_continueJump = 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
+ {
+ public:
+ BreakContinueScope(ControlFlowBuilder& _parser, CFGNode* _breakJump, CFGNode* _continueJump);
+ ~BreakContinueScope();
+ private:
+ ControlFlowBuilder& m_parser;
+ CFGNode* m_origBreakJump;
+ CFGNode* m_origContinueJump;
+ };
+};
+
+}
+}
diff --git a/libsolidity/analysis/ControlFlowGraph.cpp b/libsolidity/analysis/ControlFlowGraph.cpp
new file mode 100644
index 00000000..9b3da0eb
--- /dev/null
+++ b/libsolidity/analysis/ControlFlowGraph.cpp
@@ -0,0 +1,136 @@
+/*
+ This file is part of solidity.
+
+ solidity is free software: you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation, either version 3 of the License, or
+ (at your option) any later version.
+
+ solidity is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with solidity. If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include <libsolidity/analysis/ControlFlowGraph.h>
+#include <libsolidity/analysis/ControlFlowBuilder.h>
+
+#include <boost/range/adaptor/reversed.hpp>
+
+#include <algorithm>
+
+using namespace std;
+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);
+ return false;
+}
+
+FunctionFlow const& CFG::functionFlow(FunctionDefinition const& _function) const
+{
+ solAssert(m_functionControlFlow.count(&_function), "");
+ return *m_functionControlFlow.find(&_function)->second;
+}
+
+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];
+} \ No newline at end of file
diff --git a/libsolidity/analysis/ControlFlowGraph.h b/libsolidity/analysis/ControlFlowGraph.h
new file mode 100644
index 00000000..c646e4f1
--- /dev/null
+++ b/libsolidity/analysis/ControlFlowGraph.h
@@ -0,0 +1,148 @@
+/*
+ This file is part of solidity.
+
+ solidity is free software: you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation, either version 3 of the License, or
+ (at your option) any later version.
+
+ solidity is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with solidity. If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#pragma once
+
+#include <libsolidity/ast/AST.h>
+#include <libsolidity/ast/ASTVisitor.h>
+#include <libsolidity/interface/ErrorReporter.h>
+
+#include <map>
+#include <memory>
+#include <stack>
+#include <vector>
+
+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
+{
+ /// 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;
+};
+
+/** Node of the Control Flow Graph.
+ * The control flow is a directed graph connecting control flow blocks.
+ * An arc between two nodes indicates that the control flow can possibly
+ * move from its start node to its end node during execution.
+ */
+struct CFGNode
+{
+ /// Entry nodes. All CFG nodes from which control flow may move into this node.
+ std::vector<CFGNode*> entries;
+ /// 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;
+};
+
+/** Describes the control flow of a function. */
+struct FunctionFlow
+{
+ virtual ~FunctionFlow() {}
+ /// Entry node. Control flow of the function starts here.
+ /// This node is empty and does not have any entries.
+ CFGNode* entry = nullptr;
+ /// Exit node. All non-reverting control flow of the function ends here.
+ /// This node is empty and does not have any exits, but may have multiple entries
+ /// (e.g. all return statements of the function).
+ CFGNode* exit = nullptr;
+ /// Revert node. Control flow of the function in case of revert.
+ /// This node is empty does not have any exits, but may have multiple entries
+ /// (e.g. all assert, require, revert and throw statements).
+ 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:
+ explicit CFG(ErrorReporter& _errorReporter): m_errorReporter(_errorReporter) {}
+
+ bool constructFlow(ASTNode const& _astRoot);
+
+ virtual bool visit(ModifierDefinition const& _modifier) override;
+ virtual bool visit(FunctionDefinition const& _function) override;
+
+ FunctionFlow const& functionFlow(FunctionDefinition const& _function) const;
+
+ class NodeContainer
+ {
+ public:
+ CFGNode* newNode();
+ private:
+ 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
+ );
+
+ ErrorReporter& m_errorReporter;
+
+ /// Node container.
+ /// All nodes allocated during the construction of the control flow graph
+ /// are owned by the CFG class and stored in this container.
+ 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/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp
index 4ff14aa2..47dc30cf 100644
--- a/libsolidity/interface/CompilerStack.cpp
+++ b/libsolidity/interface/CompilerStack.cpp
@@ -29,6 +29,8 @@
#include <libsolidity/ast/AST.h>
#include <libsolidity/parsing/Scanner.h>
#include <libsolidity/parsing/Parser.h>
+#include <libsolidity/analysis/ControlFlowAnalyzer.h>
+#include <libsolidity/analysis/ControlFlowGraph.h>
#include <libsolidity/analysis/GlobalContext.h>
#include <libsolidity/analysis/NameAndTypeResolver.h>
#include <libsolidity/analysis/TypeChecker.h>
@@ -224,6 +226,22 @@ bool CompilerStack::analyze()
if (noErrors)
{
+ CFG cfg(m_errorReporter);
+ for (Source const* source: m_sourceOrder)
+ if (!cfg.constructFlow(*source->ast))
+ noErrors = false;
+
+ if (noErrors)
+ {
+ ControlFlowAnalyzer controlFlowAnalyzer(cfg, m_errorReporter);
+ for (Source const* source: m_sourceOrder)
+ if (!controlFlowAnalyzer.analyze(*source->ast))
+ noErrors = false;
+ }
+ }
+
+ if (noErrors)
+ {
StaticAnalyzer staticAnalyzer(m_errorReporter);
for (Source const* source: m_sourceOrder)
if (!staticAnalyzer.analyze(*source->ast))
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly_fine.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly_fine.sol
new file mode 100644
index 00000000..65902cc8
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly_fine.sol
@@ -0,0 +1,26 @@
+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 a warning
+ 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 a warning
+ assembly {
+ sstore(s_slot, sload(c_slot))
+ }
+ }
+}
+// ----
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly_warn.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly_warn.sol
new file mode 100644
index 00000000..09c13847
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly_warn.sol
@@ -0,0 +1,10 @@
+contract C {
+ struct S { bool f; }
+ S s;
+ function f() internal pure returns (S storage) {
+ assembly {
+ }
+ }
+}
+// ----
+// Warning: (87-88): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/default_location.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/default_location.sol
new file mode 100644
index 00000000..9a42192d
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/default_location.sol
@@ -0,0 +1,19 @@
+contract C {
+ struct S { bool f; }
+ S s;
+ function f() internal view returns (S c) {
+ c = s;
+ }
+ function g() internal view returns (S) {
+ return s;
+ }
+ function h() internal pure returns (S) {
+ }
+ function i(bool flag) internal view returns (S c) {
+ if (flag) c = s;
+ }
+ function j(bool flag) internal view returns (S) {
+ if (flag) return s;
+ }
+}
+// ----
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/dowhile_fine.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/dowhile_fine.sol
new file mode 100644
index 00000000..6520672c
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/dowhile_fine.sol
@@ -0,0 +1,36 @@
+contract C {
+ struct S { bool f; }
+ S s;
+ function f() internal view returns (S storage c) {
+ do {} while((c = s).f);
+ }
+ function g() internal view returns (S storage c) {
+ do { c = s; } while(false);
+ }
+ function h() internal view returns (S storage c) {
+ c = s;
+ do {} while(false);
+ }
+ function i() internal view returns (S storage c) {
+ do {} while(false);
+ c = s;
+ }
+ function j() internal view returns (S storage c) {
+ do {
+ c = s;
+ break;
+ } while(false);
+ }
+ function k() internal view returns (S storage c) {
+ do {
+ if (s.f) {
+ continue;
+ break;
+ }
+ else {
+ c = s;
+ }
+ } while(false);
+ }
+}
+// ----
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/dowhile_warn.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/dowhile_warn.sol
new file mode 100644
index 00000000..f1a92e9c
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/dowhile_warn.sol
@@ -0,0 +1,35 @@
+contract C {
+ struct S { bool f; }
+ S s;
+ function f() internal view returns (S storage c) {
+ do {
+ break;
+ c = s;
+ } while(false);
+ }
+ function g() internal view returns (S storage c) {
+ do {
+ if (s.f) {
+ continue;
+ c = s;
+ }
+ else {
+ }
+ } while(false);
+ }
+ function h() internal view returns (S storage c) {
+ do {
+ if (s.f) {
+ break;
+ continue;
+ }
+ else {
+ c = s;
+ }
+ } while(false);
+ }
+}
+// ----
+// Warning: (87-98): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
+// Warning: (223-234): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
+// Warning: (440-451): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/emptyReturn_fine.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/emptyReturn_fine.sol
new file mode 100644
index 00000000..3a0a30ea
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/emptyReturn_fine.sol
@@ -0,0 +1,6 @@
+contract C {
+ struct S { bool f; }
+ S s;
+ function f() internal view returns (S storage c, S storage d) { c = s; d = s; return; }
+}
+// ----
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/emptyReturn_warn.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/emptyReturn_warn.sol
new file mode 100644
index 00000000..0a5b2fbf
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/emptyReturn_warn.sol
@@ -0,0 +1,15 @@
+contract C {
+ struct S { bool f; }
+ S s;
+ function f() internal pure returns (S storage) { return; }
+ function g() internal view returns (S storage c, S storage) { c = s; return; }
+ function h() internal view returns (S storage, S storage d) { d = s; return; }
+ function i() internal pure returns (S storage, S storage) { return; }
+ function j() internal view returns (S storage, S storage) { return (s,s); }
+}
+// ----
+// Warning: (87-88): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
+// Warning: (163-164): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
+// Warning: (233-234): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
+// Warning: (316-317): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
+// Warning: (327-328): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/for_fine.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/for_fine.sol
new file mode 100644
index 00000000..aa82cb9a
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/for_fine.sol
@@ -0,0 +1,13 @@
+contract C {
+ struct S { bool f; }
+ S s;
+ function f() internal view returns (S storage c) {
+ for(c = s;;) {
+ }
+ }
+ function g() internal view returns (S storage c) {
+ for(; (c = s).f;) {
+ }
+ }
+}
+// ----
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/for_warn.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/for_warn.sol
new file mode 100644
index 00000000..ba9a2440
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/for_warn.sol
@@ -0,0 +1,16 @@
+contract C {
+ struct S { bool f; }
+ S s;
+ function f() internal view returns (S storage c) {
+ for(;; c = s) {
+ }
+ }
+ function g() internal view returns (S storage c) {
+ for(;;) {
+ c = s;
+ }
+ }
+}
+// ----
+// Warning: (87-98): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
+// Warning: (182-193): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/if_fine.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/if_fine.sol
new file mode 100644
index 00000000..b809e95d
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/if_fine.sol
@@ -0,0 +1,29 @@
+contract C {
+ struct S { bool f; }
+ S s;
+ function f(bool flag) internal view returns (S storage c) {
+ if (flag) c = s;
+ else c = s;
+ }
+ function g(bool flag) internal view returns (S storage c) {
+ if (flag) c = s;
+ else { c = s; }
+ }
+ function h(bool flag) internal view returns (S storage c) {
+ if (flag) c = s;
+ else
+ {
+ if (!flag) c = s;
+ else c = s;
+ }
+ }
+ function i() internal view returns (S storage c) {
+ if ((c = s).f) {
+ }
+ }
+ function j() internal view returns (S storage c) {
+ if ((c = s).f && !(c = s).f) {
+ }
+ }
+}
+// ----
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/if_warn.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/if_warn.sol
new file mode 100644
index 00000000..c257c252
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/if_warn.sol
@@ -0,0 +1,18 @@
+contract C {
+ struct S { bool f; }
+ S s;
+ function f(bool flag) internal view returns (S storage c) {
+ if (flag) c = s;
+ }
+ function g(bool flag) internal returns (S storage c) {
+ if (flag) c = s;
+ else
+ {
+ if (!flag) c = s;
+ else s.f = true;
+ }
+ }
+}
+// ----
+// Warning: (96-107): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
+// Warning: (186-197): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/modifier_fine.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/modifier_fine.sol
new file mode 100644
index 00000000..ee37f6d6
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/modifier_fine.sol
@@ -0,0 +1,20 @@
+contract C {
+ modifier revertIfNoReturn() {
+ _;
+ revert();
+ }
+ modifier ifFlag(bool flag) {
+ if (flag)
+ _;
+ }
+ struct S { uint a; }
+ S s;
+ function f(bool flag) revertIfNoReturn() internal view returns(S storage) {
+ if (flag) return s;
+ }
+ function g(bool flag) revertIfNoReturn() ifFlag(flag) internal view returns(S storage) {
+ return s;
+ }
+
+}
+// ----
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/modifier_warn.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/modifier_warn.sol
new file mode 100644
index 00000000..50c6dd99
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/modifier_warn.sol
@@ -0,0 +1,22 @@
+contract C {
+ modifier revertIfNoReturn() {
+ _;
+ revert();
+ }
+ modifier ifFlag(bool flag) {
+ if (flag)
+ _;
+ }
+ struct S { uint a; }
+ S s;
+ function f(bool flag) ifFlag(flag) internal view returns(S storage) {
+ return s;
+ }
+
+ function g(bool flag) ifFlag(flag) revertIfNoReturn() internal view returns(S storage) {
+ return s;
+ }
+}
+// ----
+// Warning: (249-250): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
+// Warning: (367-368): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/revert_fine.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/revert_fine.sol
new file mode 100644
index 00000000..022f2d1c
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/revert_fine.sol
@@ -0,0 +1,12 @@
+contract C {
+ struct S { bool f; }
+ S s;
+ function f() internal pure returns (S storage) {
+ revert();
+ }
+ function g(bool flag) internal view returns (S storage c) {
+ if (flag) c = s;
+ else revert();
+ }
+}
+// ----
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/short_circuit_fine.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/short_circuit_fine.sol
new file mode 100644
index 00000000..699849c0
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/short_circuit_fine.sol
@@ -0,0 +1,11 @@
+contract C {
+ struct S { bool f; }
+ S s;
+ function f() internal view returns (S storage c) {
+ (c = s).f && false;
+ }
+ function g() internal view returns (S storage c) {
+ (c = s).f || true;
+ }
+}
+// ----
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/short_circuit_warn.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/short_circuit_warn.sol
new file mode 100644
index 00000000..9f660f11
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/short_circuit_warn.sol
@@ -0,0 +1,18 @@
+contract C {
+ struct S { bool f; }
+ S s;
+ function f() internal view returns (S storage c) {
+ false && (c = s).f;
+ }
+ function g() internal view returns (S storage c) {
+ true || (c = s).f;
+ }
+ function h() internal view returns (S storage c) {
+ // expect warning, although this is always fine
+ true && (false || (c = s).f);
+ }
+}
+// ----
+// Warning: (87-98): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
+// Warning: (176-187): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
+// Warning: (264-275): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/smoke.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/smoke.sol
new file mode 100644
index 00000000..beeadbe4
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/smoke.sol
@@ -0,0 +1,10 @@
+contract C {
+ struct S { bool f; }
+ S s;
+ function f() internal pure {}
+ function g() internal view returns (S storage) { return s; }
+ function h() internal view returns (S storage c) { return s; }
+ function i() internal view returns (S storage c) { c = s; }
+ function j() internal view returns (S storage c) { (c) = s; }
+}
+// ----
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/ternary_fine.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/ternary_fine.sol
new file mode 100644
index 00000000..ee3869bd
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/ternary_fine.sol
@@ -0,0 +1,14 @@
+contract C {
+ struct S { bool f; }
+ S s;
+ function f(bool flag) internal view returns (S storage c) {
+ flag ? c = s : c = s;
+ }
+ function g(bool flag) internal view returns (S storage c) {
+ flag ? c = s : (c = s);
+ }
+ function h(bool flag) internal view returns (S storage c) {
+ flag ? (c = s).f : (c = s).f;
+ }
+}
+// ----
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/ternary_warn.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/ternary_warn.sol
new file mode 100644
index 00000000..57561fbb
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/ternary_warn.sol
@@ -0,0 +1,13 @@
+contract C {
+ struct S { bool f; }
+ S s;
+ function f(bool flag) internal view returns (S storage c) {
+ flag ? (c = s).f : false;
+ }
+ function g(bool flag) internal view returns (S storage c) {
+ flag ? false : (c = s).f;
+ }
+}
+// ----
+// Warning: (96-107): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
+// Warning: (200-211): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/throw_fine.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/throw_fine.sol
new file mode 100644
index 00000000..4cecc27c
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/throw_fine.sol
@@ -0,0 +1,9 @@
+contract C {
+ struct S { bool f; }
+ S s;
+ function f() internal pure returns (S storage) {
+ throw;
+ }
+}
+// ----
+// Warning: (108-113): "throw" is deprecated in favour of "revert()", "require()" and "assert()".
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/tuple_fine.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/tuple_fine.sol
new file mode 100644
index 00000000..0b171560
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/tuple_fine.sol
@@ -0,0 +1,12 @@
+contract C {
+ struct S { bool f; }
+ S s;
+ function f() internal view returns (S storage, uint) {
+ return (s,2);
+ }
+ function g() internal view returns (S storage c) {
+ uint a;
+ (c, a) = f();
+ }
+}
+// ----
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/while_fine.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/while_fine.sol
new file mode 100644
index 00000000..71543422
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/while_fine.sol
@@ -0,0 +1,19 @@
+contract C {
+ struct S { bool f; }
+ S s;
+ function f() internal view returns (S storage c) {
+ while((c = s).f) {
+ }
+ }
+ function g() internal view returns (S storage c) {
+ c = s;
+ while(false) {
+ }
+ }
+ function h() internal view returns (S storage c) {
+ while(false) {
+ }
+ c = s;
+ }
+}
+// ----
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/while_warn.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/while_warn.sol
new file mode 100644
index 00000000..26db892f
--- /dev/null
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/while_warn.sol
@@ -0,0 +1,11 @@
+contract C {
+ struct S { bool f; }
+ S s;
+ function f() internal view returns (S storage c) {
+ while(false) {
+ c = s;
+ }
+ }
+}
+// ----
+// Warning: (87-98): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.