diff options
61 files changed, 1599 insertions, 151 deletions
diff --git a/.travis.yml b/.travis.yml index 3640d2e2..8487deef 100644 --- a/.travis.yml +++ b/.travis.yml @@ -173,7 +173,7 @@ before_script: && scripts/create_source_tarball.sh) script: - - test $SOLC_EMSCRIPTEN != On || (scripts/test_emscripten.sh) + - test $SOLC_EMSCRIPTEN != On -o $SOLC_TESTS != On || (scripts/test_emscripten.sh) - test $SOLC_TESTS != On || (cd $TRAVIS_BUILD_DIR && scripts/tests.sh) - test $SOLC_STOREBYTECODE != On || (cd $TRAVIS_BUILD_DIR && scripts/bytecodecompare/storebytecode.sh) diff --git a/Changelog.md b/Changelog.md index 94613589..ce0adfd7 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,8 +1,11 @@ ### 0.4.24 (unreleased) 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. @@ -19,8 +19,6 @@ defaults: - run_tests: &run_tests name: Tests command: scripts/tests.sh --junit_report test_results - environment: - TERM: dumb - solc_artifact: &solc_artifact path: build/solc/solc destination: solc @@ -36,6 +34,8 @@ jobs: build_emscripten: docker: - image: trzeci/emscripten:sdk-tag-1.37.21-64bit + environment: + TERM: xterm steps: - checkout - restore_cache: @@ -68,6 +68,8 @@ jobs: test_emscripten_solcjs: docker: - image: trzeci/emscripten:sdk-tag-1.37.21-64bit + environment: + TERM: xterm steps: - checkout - attach_workspace: @@ -92,6 +94,8 @@ jobs: test_emscripten_external: docker: - image: trzeci/emscripten:sdk-tag-1.37.21-64bit + environment: + TERM: xterm steps: - checkout - attach_workspace: @@ -116,6 +120,8 @@ jobs: build_x86_linux: docker: - image: buildpack-deps:artful + environment: + TERM: xterm steps: - checkout - run: @@ -131,6 +137,8 @@ jobs: build_x86_mac: macos: xcode: "9.0" + environment: + TERM: xterm steps: - checkout - run: @@ -150,6 +158,8 @@ jobs: test_x86_linux: docker: - image: buildpack-deps:artful + environment: + TERM: xterm steps: - checkout - attach_workspace: @@ -167,6 +177,8 @@ jobs: test_x86_mac: macos: xcode: "9.0" + environment: + TERM: xterm steps: - checkout - attach_workspace: diff --git a/docs/conf.py b/docs/conf.py index 3bbee671..7e107f2a 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -22,7 +22,8 @@ import re # documentation root, use os.path.abspath to make it absolute, like shown here. def setup(sphinx): - sys.path.insert(0, os.path.abspath('./utils')) + thisdir = os.path.dirname(os.path.realpath(__file__)) + sys.path.insert(0, thisdir + '/utils') from SolidityLexer import SolidityLexer sphinx.add_lexer('Solidity', SolidityLexer()) diff --git a/docs/contracts.rst b/docs/contracts.rst index 3576cd7b..487e80ae 100644 --- a/docs/contracts.rst +++ b/docs/contracts.rst @@ -473,7 +473,7 @@ The following statements are considered modifying the state: } .. note:: - ``constant`` on functions is an alias to ``view``, but this is deprecated and is planned to be dropped in version 0.5.0. + ``constant`` on functions is an alias to ``view``, but this is deprecated and will be dropped in version 0.5.0. .. note:: Getter methods are marked ``view``. diff --git a/docs/installing-solidity.rst b/docs/installing-solidity.rst index cba30ed3..05ee0748 100644 --- a/docs/installing-solidity.rst +++ b/docs/installing-solidity.rst @@ -35,7 +35,7 @@ npm / Node.js ============= Use `npm` for a convenient and portable way to install `solcjs`, a Solidity compiler. The -`solcjs` program has less features than all options further down this page. Our +`solcjs` program has fewer features than all options further down this page. Our :ref:`commandline-compiler` documentation assumes you are using the full-featured compiler, `solc`. So if you install `solcjs` from `npm` then you will stop reading the documentation here and then continue to `solc-js <https://github.com/ethereum/solc-js>`_. diff --git a/docs/introduction-to-smart-contracts.rst b/docs/introduction-to-smart-contracts.rst index 84b1fff8..d1789c44 100644 --- a/docs/introduction-to-smart-contracts.rst +++ b/docs/introduction-to-smart-contracts.rst @@ -25,7 +25,7 @@ Storage storedData = x; } - function get() public constant returns (uint) { + function get() public view returns (uint) { return storedData; } } diff --git a/docs/julia.rst b/docs/julia.rst index 078bc55b..bc918117 100644 --- a/docs/julia.rst +++ b/docs/julia.rst @@ -306,12 +306,20 @@ Type Conversion Functions JULIA has no support for implicit type conversion and therefore functions exists to provide explicit conversion. When converting a larger type to a shorter type a runtime exception can occur in case of an overflow. -The following type conversion functions must be available: -- ``u32tobool(x:u32) -> y:bool`` -- ``booltou32(x:bool) -> y:u32`` -- ``u32tou64(x:u32) -> y:u64`` -- ``u64tou32(x:u64) -> y:u32`` -- etc. (TBD) +Truncating conversions are supported between the following types: + - ``bool`` + - ``u32`` + - ``u64`` + - ``u256`` + - ``s256`` + +For each of these a type conversion function exists having the prototype in the form of ``<input_type>to<output_type>(x:<input_type>) -> y:<output_type>``, +such as ``u32tobool(x:u32) -> y:bool``, ``u256tou32(x:u256) -> y:u32`` or ``s256tou256(x:s256) -> y:u256``. + +.. note:: + + ``u32tobool(x:u32) -> y:bool`` can be implemented as ``y := not(iszerou256(x))`` and + ``booltou32(x:bool) -> y:u32`` can be implemented as ``switch x case true:bool { y := 1:u32 } case false:bool { y := 0:u32 }`` Low-level Functions ------------------- @@ -319,6 +327,16 @@ Low-level Functions The following functions must be available: +---------------------------------------------------------------------------------------------------------------+ +| *Logic* | ++---------------------------------------------+-----------------------------------------------------------------+ +| not(x:bool) -> z:bool | logical not | ++---------------------------------------------+-----------------------------------------------------------------+ +| and(x:bool, y:bool) -> z:bool | logical and | ++---------------------------------------------+-----------------------------------------------------------------+ +| or(x:bool, y:bool) -> z:bool | logical or | ++---------------------------------------------+-----------------------------------------------------------------+ +| xor(x:bool, y:bool) -> z:bool | xor | ++---------------------------------------------+-----------------------------------------------------------------+ | *Arithmetics* | +---------------------------------------------+-----------------------------------------------------------------+ | addu256(x:u256, y:u256) -> z:u256 | x + y | @@ -343,15 +361,19 @@ The following functions must be available: +---------------------------------------------+-----------------------------------------------------------------+ | mulmodu256(x:u256, y:u256, m:u256) -> z:u256| (x * y) % m with arbitrary precision arithmetics | +---------------------------------------------+-----------------------------------------------------------------+ -| ltu256(x:u256, y:u256) -> z:bool | 1 if x < y, 0 otherwise | +| ltu256(x:u256, y:u256) -> z:bool | true if x < y, false otherwise | +---------------------------------------------+-----------------------------------------------------------------+ -| gtu256(x:u256, y:u256) -> z:bool | 1 if x > y, 0 otherwise | +| gtu256(x:u256, y:u256) -> z:bool | true if x > y, false otherwise | +---------------------------------------------+-----------------------------------------------------------------+ -| sltu256(x:s256, y:s256) -> z:bool | 1 if x < y, 0 otherwise, for signed numbers in two's complement | +| sltu256(x:s256, y:s256) -> z:bool | true if x < y, false otherwise | +| | (for signed numbers in two's complement) | +---------------------------------------------+-----------------------------------------------------------------+ -| sgtu256(x:s256, y:s256) -> z:bool | 1 if x > y, 0 otherwise, for signed numbers in two's complement | +| sgtu256(x:s256, y:s256) -> z:bool | true if x > y, false otherwise | +| | (for signed numbers in two's complement) | +---------------------------------------------+-----------------------------------------------------------------+ -| equ256(x:u256, y:u256) -> z:bool | 1 if x == y, 0 otherwise | +| equ256(x:u256, y:u256) -> z:bool | true if x == y, false otherwise | ++---------------------------------------------+-----------------------------------------------------------------+ +| iszerou256(x:u256) -> z:bool | true if x == 0, false otherwise | +---------------------------------------------+-----------------------------------------------------------------+ | notu256(x:u256) -> z:u256 | ~x, every bit of x is negated | +---------------------------------------------+-----------------------------------------------------------------+ @@ -405,10 +427,6 @@ The following functions must be available: | insize:u256, out:u256, | but also keep ``caller`` | | outsize:u256) -> r:u256 | and ``callvalue`` | +---------------------------------------------+-----------------------------------------------------------------+ -| stop() | stop execution, identical to return(0,0) | -| | Perhaps it would make sense retiring this as it equals to | -| | return(0,0). It can be an optimisation by the EVM backend. | -+---------------------------------------------+-----------------------------------------------------------------+ | abort() | abort (equals to invalid instruction on EVM) | +---------------------------------------------+-----------------------------------------------------------------+ | return(p:u256, s:u256) | end execution, return data mem[p..(p+s)) | @@ -473,6 +491,8 @@ The following functions must be available: +---------------------------------------------+-----------------------------------------------------------------+ | *Others* | +---------------------------------------------+-----------------------------------------------------------------+ +| discard(unused:bool) | discard value | ++---------------------------------------------+-----------------------------------------------------------------+ | discardu256(unused:u256) | discard value | +---------------------------------------------+-----------------------------------------------------------------+ | splitu256tou64(x:u256) -> (x1:u64, x2:u64, | split u256 to four u64's | @@ -481,7 +501,7 @@ The following functions must be available: | combineu64tou256(x1:u64, x2:u64, x3:u64, | combine four u64's into a single u256 | | x4:u64) -> (x:u256) | | +---------------------------------------------+-----------------------------------------------------------------+ -| sha3(p:u256, s:u256) -> v:u256 | keccak(mem[p...(p+s))) | +| keccak256(p:u256, s:u256) -> v:u256 | keccak(mem[p...(p+s))) | +---------------------------------------------+-----------------------------------------------------------------+ Backends diff --git a/docs/requirements.txt b/docs/requirements.txt new file mode 100644 index 00000000..0607b1ef --- /dev/null +++ b/docs/requirements.txt @@ -0,0 +1 @@ +sphinx_rtd_theme>=0.3.1 diff --git a/docs/security-considerations.rst b/docs/security-considerations.rst index 3e1c3a12..4133edb1 100644 --- a/docs/security-considerations.rst +++ b/docs/security-considerations.rst @@ -120,7 +120,7 @@ Gas Limit and Loops Loops that do not have a fixed number of iterations, for example, loops that depend on storage values, have to be used carefully: Due to the block gas limit, transactions can only consume a certain amount of gas. Either explicitly or just due to normal operation, the number of iterations in a loop can grow beyond the block gas limit which can cause the complete -contract to be stalled at a certain point. This may not apply to ``constant`` functions that are only executed +contract to be stalled at a certain point. This may not apply to ``view`` functions that are only executed to read data from the blockchain. Still, such functions may be called by other contracts as part of on-chain operations and stall those. Please be explicit about such cases in the documentation of your contracts. diff --git a/docs/style-guide.rst b/docs/style-guide.rst index 0c58f3eb..6b28f2ab 100644 --- a/docs/style-guide.rst +++ b/docs/style-guide.rst @@ -269,7 +269,7 @@ Functions should be grouped according to their visibility and ordered: - internal - private -Within a grouping, place the ``constant`` functions last. +Within a grouping, place the ``view`` and ``pure`` functions last. Yes:: @@ -285,7 +285,10 @@ Yes:: // External functions // ... - // External functions that are constant + // External functions that are view + // ... + + // External functions that are pure // ... // Public functions diff --git a/libjulia/optimiser/FullInliner.cpp b/libjulia/optimiser/FullInliner.cpp index e78f4eb0..e8776e23 100644 --- a/libjulia/optimiser/FullInliner.cpp +++ b/libjulia/optimiser/FullInliner.cpp @@ -168,10 +168,10 @@ void InlineModifier::visit(Statement& _statement) // Replace pop(0) expression statemets (and others) by empty blocks. if (_statement.type() == typeid(ExpressionStatement)) { - ExpressionStatement& expSt = boost::get<ExpressionStatement&>(_statement); + ExpressionStatement& expSt = boost::get<ExpressionStatement>(_statement); if (expSt.expression.type() == typeid(FunctionalInstruction)) { - FunctionalInstruction& funInstr = boost::get<FunctionalInstruction&>(expSt.expression); + FunctionalInstruction& funInstr = boost::get<FunctionalInstruction>(expSt.expression); if (funInstr.instruction == solidity::Instruction::POP) if (MovableChecker(funInstr.arguments.at(0)).movable()) _statement = Block{expSt.location, {}}; diff --git a/libsolidity/CMakeLists.txt b/libsolidity/CMakeLists.txt index 97b01c83..0bdec4b4 100644 --- a/libsolidity/CMakeLists.txt +++ b/libsolidity/CMakeLists.txt @@ -28,7 +28,7 @@ else() endif() add_library(solidity ${sources} ${headers}) -target_link_libraries(solidity PUBLIC evmasm devcore) +target_link_libraries(solidity PUBLIC evmasm devcore ${Boost_FILESYSTEM_LIBRARY} ${Boost_SYSTEM_LIBRARY}) if (${Z3_FOUND}) target_link_libraries(solidity PUBLIC ${Z3_LIBRARY}) 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/inlineasm/AsmAnalysis.cpp b/libsolidity/inlineasm/AsmAnalysis.cpp index abf7ddf2..9f505889 100644 --- a/libsolidity/inlineasm/AsmAnalysis.cpp +++ b/libsolidity/inlineasm/AsmAnalysis.cpp @@ -54,6 +54,7 @@ bool AsmAnalyzer::analyze(Block const& _block) bool AsmAnalyzer::operator()(Label const& _label) { + solAssert(!_label.name.empty(), ""); checkLooseFeature( _label.location, "The use of labels is deprecated. Please use \"if\", \"switch\", \"for\" or function calls instead." @@ -107,6 +108,7 @@ bool AsmAnalyzer::operator()(assembly::Literal const& _literal) bool AsmAnalyzer::operator()(assembly::Identifier const& _identifier) { + solAssert(!_identifier.name.empty(), ""); size_t numErrorsBefore = m_errorReporter.errors().size(); bool success = true; if (m_currentScope->lookup(_identifier.name, Scope::Visitor( @@ -208,6 +210,7 @@ bool AsmAnalyzer::operator()(assembly::StackAssignment const& _assignment) bool AsmAnalyzer::operator()(assembly::Assignment const& _assignment) { + solAssert(_assignment.value, ""); int const expectedItems = _assignment.variableNames.size(); solAssert(expectedItems >= 1, ""); int const stackHeight = m_stackHeight; @@ -259,6 +262,7 @@ bool AsmAnalyzer::operator()(assembly::VariableDeclaration const& _varDecl) bool AsmAnalyzer::operator()(assembly::FunctionDefinition const& _funDef) { + solAssert(!_funDef.name.empty(), ""); Block const* virtualBlock = m_info.virtualBlocks.at(&_funDef).get(); solAssert(virtualBlock, ""); Scope& varScope = scope(virtualBlock); @@ -280,6 +284,7 @@ bool AsmAnalyzer::operator()(assembly::FunctionDefinition const& _funDef) bool AsmAnalyzer::operator()(assembly::FunctionCall const& _funCall) { + solAssert(!_funCall.functionName.name.empty(), ""); bool success = true; size_t arguments = 0; size_t returns = 0; @@ -349,6 +354,8 @@ bool AsmAnalyzer::operator()(If const& _if) bool AsmAnalyzer::operator()(Switch const& _switch) { + solAssert(_switch.expression, ""); + bool success = true; if (!expectExpression(*_switch.expression)) @@ -391,6 +398,8 @@ bool AsmAnalyzer::operator()(Switch const& _switch) bool AsmAnalyzer::operator()(assembly::ForLoop const& _for) { + solAssert(_for.condition, ""); + Scope* originalScope = m_currentScope; bool success = true; @@ -478,6 +487,7 @@ bool AsmAnalyzer::expectDeposit(int _deposit, int _oldHeight, SourceLocation con bool AsmAnalyzer::checkAssignment(assembly::Identifier const& _variable, size_t _valueSize) { + solAssert(!_variable.name.empty(), ""); bool success = true; size_t numErrorsBefore = m_errorReporter.errors().size(); size_t variableSize(-1); 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/lllc/CMakeLists.txt b/lllc/CMakeLists.txt index 5c480093..d6538ee2 100644 --- a/lllc/CMakeLists.txt +++ b/lllc/CMakeLists.txt @@ -1,5 +1,5 @@ add_executable(lllc main.cpp) -target_link_libraries(lllc PRIVATE lll) +target_link_libraries(lllc PRIVATE lll ${Boost_SYSTEM_LIBRARY}) if (INSTALL_LLLC) include(GNUInstallDirs) diff --git a/std/StandardToken.sol b/std/StandardToken.sol index ca0658f2..c2fc3a66 100644 --- a/std/StandardToken.sol +++ b/std/StandardToken.sol @@ -13,11 +13,11 @@ contract StandardToken is Token { balance[_initialOwner] = _supply; } - function balanceOf(address _account) constant public returns (uint) { + function balanceOf(address _account) view public returns (uint) { return balance[_account]; } - function totalSupply() constant public returns (uint) { + function totalSupply() view public returns (uint) { return supply; } @@ -53,7 +53,7 @@ contract StandardToken is Token { return true; } - function allowance(address _owner, address _spender) constant public returns (uint256) { + function allowance(address _owner, address _spender) view public returns (uint256) { return m_allowance[_owner][_spender]; } } diff --git a/std/Token.sol b/std/Token.sol index 4b4eb71e..7348a8f5 100644 --- a/std/Token.sol +++ b/std/Token.sol @@ -4,10 +4,10 @@ contract Token { event Transfer(address indexed _from, address indexed _to, uint256 _value); event Approval(address indexed _owner, address indexed _spender, uint256 _value); - function totalSupply() constant public returns (uint256 supply); - function balanceOf(address _owner) constant public returns (uint256 balance); + function totalSupply() view public returns (uint256 supply); + function balanceOf(address _owner) view public returns (uint256 balance); function transfer(address _to, uint256 _value) public returns (bool success); function transferFrom(address _from, address _to, uint256 _value) public returns (bool success); function approve(address _spender, uint256 _value) public returns (bool success); - function allowance(address _owner, address _spender) constant public returns (uint256 remaining); + function allowance(address _owner, address _spender) view public returns (uint256 remaining); } diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index dd82a05c..2db2aadd 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -2105,7 +2105,7 @@ BOOST_AUTO_TEST_CASE(packed_keccak256_complex_types) char const* sourceCode = R"( contract test { uint120[3] x; - function f() view returns (bytes32 hash1, bytes32 hash2, bytes32 hash3) { + function f() returns (bytes32 hash1, bytes32 hash2, bytes32 hash3) { uint120[] memory y = new uint120[](3); x[0] = y[0] = uint120(-2); x[1] = y[1] = uint120(-3); @@ -10917,9 +10917,9 @@ BOOST_AUTO_TEST_CASE(negative_stack_height) bool Aboolc; bool exists; } - function nredit(uint startindex) public constant returns(uint[500] CIDs, uint[500] dates, uint[500] RIDs, bool[500] Cboolas, uint[500] amounts){} - function return500InvoicesByDates(uint begindate, uint enddate, uint startindex) public constant returns(uint[500] AIDs, bool[500] Aboolas, uint[500] dates, bytes32[3][500] Abytesas, bytes32[3][500] bytesbs, bytes32[2][500] bytescs, uint[500] amounts, bool[500] Aboolbs, bool[500] Aboolcs){} - function return500PaymentsByDates(uint begindate, uint enddate, uint startindex) public constant returns(uint[500] BIDs, uint[500] dates, uint[500] RIDs, bool[500] Bboolas, bytes32[3][500] bytesbs,bytes32[2][500] bytescs, uint[500] amounts, bool[500] Bboolbs){} + function nredit(uint startindex) public pure returns(uint[500] CIDs, uint[500] dates, uint[500] RIDs, bool[500] Cboolas, uint[500] amounts){} + function return500InvoicesByDates(uint begindate, uint enddate, uint startindex) public view returns(uint[500] AIDs, bool[500] Aboolas, uint[500] dates, bytes32[3][500] Abytesas, bytes32[3][500] bytesbs, bytes32[2][500] bytescs, uint[500] amounts, bool[500] Aboolbs, bool[500] Aboolcs){} + function return500PaymentsByDates(uint begindate, uint enddate, uint startindex) public view returns(uint[500] BIDs, uint[500] dates, uint[500] RIDs, bool[500] Bboolas, bytes32[3][500] bytesbs,bytes32[2][500] bytescs, uint[500] amounts, bool[500] Bboolbs){} } )"; compileAndRun(sourceCode, 0, "C"); @@ -11105,13 +11105,13 @@ BOOST_AUTO_TEST_CASE(bare_call_invalid_address) char const* sourceCode = R"( contract C { /// Calling into non-existant account is successful (creates the account) - function f() external view returns (bool) { + function f() external returns (bool) { return address(0x4242).call(); } - function g() external view returns (bool) { + function g() external returns (bool) { return address(0x4242).callcode(); } - function h() external view returns (bool) { + function h() external returns (bool) { return address(0x4242).delegatecall(); } } @@ -11133,13 +11133,13 @@ BOOST_AUTO_TEST_CASE(delegatecall_return_value) function get() external view returns (uint) { return value; } - function get_delegated() external view returns (bool) { + function get_delegated() external returns (bool) { return this.delegatecall(bytes4(sha3("get()"))); } function assert0() external view { assert(value == 0); } - function assert0_delegated() external view returns (bool) { + function assert0_delegated() external returns (bool) { return this.delegatecall(bytes4(sha3("assert0()"))); } } @@ -11600,7 +11600,7 @@ BOOST_AUTO_TEST_CASE(abi_encode_v2) require(y[0] == "e"); } S s; - function f4() public view returns (bytes r) { + function f4() public returns (bytes r) { string memory x = "abc"; s.a = 7; s.b.push(2); diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 91fd1fff..5af67659 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -831,24 +831,6 @@ BOOST_AUTO_TEST_CASE(illegal_override_visibility) CHECK_ERROR(text, TypeError, "Overriding function visibility differs"); } -BOOST_AUTO_TEST_CASE(illegal_override_remove_constness) -{ - char const* text = R"( - contract B { function f() constant {} } - contract C is B { function f() public {} } - )"; - CHECK_ERROR(text, TypeError, "Overriding function changes state mutability from \"view\" to \"nonpayable\"."); -} - -BOOST_AUTO_TEST_CASE(illegal_override_add_constness) -{ - char const* text = R"( - contract B { function f() public {} } - contract C is B { function f() constant {} } - )"; - CHECK_ERROR(text, TypeError, "Overriding function changes state mutability from \"nonpayable\" to \"view\"."); -} - BOOST_AUTO_TEST_CASE(complex_inheritance) { char const* text = R"( @@ -993,19 +975,6 @@ BOOST_AUTO_TEST_CASE(private_state_variable) BOOST_CHECK_MESSAGE(function == nullptr, "Accessor function of an internal variable should not exist"); } -BOOST_AUTO_TEST_CASE(missing_state_variable) -{ - char const* text = R"( - contract Scope { - function getStateVar() constant public returns (uint stateVar) { - stateVar = Scope.stateVar; // should fail. - } - } - )"; - CHECK_ERROR(text, TypeError, "Member \"stateVar\" not found or not visible after argument-dependent lookup in type(contract Scope)"); -} - - BOOST_AUTO_TEST_CASE(base_class_state_variable_accessor) { // test for issue #1126 https://github.com/ethereum/cpp-ethereum/issues/1126 @@ -1119,17 +1088,6 @@ BOOST_AUTO_TEST_CASE(fallback_function_with_return_parameters) CHECK_ERROR(text, TypeError, "Fallback function cannot return values."); } -BOOST_AUTO_TEST_CASE(fallback_function_with_constant_modifier) -{ - char const* text = R"( - contract C { - uint x; - function() constant { x = 2; } - } - )"; - CHECK_ERROR(text, TypeError, "Fallback function must be payable or non-payable"); -} - BOOST_AUTO_TEST_CASE(fallback_function_twice) { char const* text = R"( @@ -2327,30 +2285,6 @@ BOOST_AUTO_TEST_CASE(constant_string_literal_disallows_assignment) CHECK_ERROR(text, TypeError, "Index access for string is not possible."); } -BOOST_AUTO_TEST_CASE(assign_constant_function_value_to_constant_0_4_x) -{ - char const* text = R"( - contract C { - function () constant returns (uint) x; - uint constant y = x(); - } - )"; - CHECK_WARNING(text, "Initial value for constant variable has to be compile-time constant."); -} - -BOOST_AUTO_TEST_CASE(assign_constant_function_value_to_constant) -{ - char const* text = R"( - pragma experimental "v0.5.0"; - - contract C { - function () constant returns (uint) x; - uint constant y = x(); - } - )"; - CHECK_ERROR(text, TypeError, "Initial value for constant variable has to be compile-time constant."); -} - BOOST_AUTO_TEST_CASE(assignment_to_const_var_involving_conversion) { char const* text = R"( diff --git a/test/libsolidity/SolidityParser.cpp b/test/libsolidity/SolidityParser.cpp index 77686b03..cbea8694 100644 --- a/test/libsolidity/SolidityParser.cpp +++ b/test/libsolidity/SolidityParser.cpp @@ -838,40 +838,6 @@ BOOST_AUTO_TEST_CASE(multiple_visibility_specifiers) CHECK_PARSE_ERROR(text, "Visibility already specified as \"private\"."); } -BOOST_AUTO_TEST_CASE(multiple_statemutability_specifiers) -{ - char const* text = R"( - contract c { - function f() payable payable {} - })"; - CHECK_PARSE_ERROR(text, "State mutability already specified as \"payable\"."); - text = R"( - contract c { - function f() constant constant {} - })"; - CHECK_PARSE_ERROR(text, "State mutability already specified as \"view\"."); - text = R"( - contract c { - function f() constant view {} - })"; - CHECK_PARSE_ERROR(text, "State mutability already specified as \"view\"."); - text = R"( - contract c { - function f() payable constant {} - })"; - CHECK_PARSE_ERROR(text, "State mutability already specified as \"payable\"."); - text = R"( - contract c { - function f() pure payable {} - })"; - CHECK_PARSE_ERROR(text, "State mutability already specified as \"pure\"."); - text = R"( - contract c { - function f() pure constant {} - })"; - CHECK_PARSE_ERROR(text, "State mutability already specified as \"pure\"."); -} - BOOST_AUTO_TEST_CASE(literal_constants_with_ether_subdenominations) { char const* text = R"( diff --git a/test/libsolidity/syntaxTests/constants/assign_constant_function_value.sol b/test/libsolidity/syntaxTests/constants/assign_constant_function_value.sol new file mode 100644 index 00000000..88e94e29 --- /dev/null +++ b/test/libsolidity/syntaxTests/constants/assign_constant_function_value.sol @@ -0,0 +1,6 @@ +contract C { + function () pure returns (uint) x; + uint constant y = x(); +} +// ---- +// Warning: (74-77): Initial value for constant variable has to be compile-time constant. This will fail to compile with the next breaking version change. diff --git a/test/libsolidity/syntaxTests/constants/assign_constant_function_value_050.sol b/test/libsolidity/syntaxTests/constants/assign_constant_function_value_050.sol new file mode 100644 index 00000000..2c92899d --- /dev/null +++ b/test/libsolidity/syntaxTests/constants/assign_constant_function_value_050.sol @@ -0,0 +1,8 @@ +pragma experimental "v0.5.0"; + +contract C { + function () pure returns (uint) x; + uint constant y = x(); +} +// ---- +// TypeError: (105-108): Initial value for constant variable has to be compile-time constant. 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. diff --git a/test/libsolidity/syntaxTests/fallback/pure_modifier.sol b/test/libsolidity/syntaxTests/fallback/pure_modifier.sol new file mode 100644 index 00000000..20d5b0ac --- /dev/null +++ b/test/libsolidity/syntaxTests/fallback/pure_modifier.sol @@ -0,0 +1,6 @@ +contract C { + uint x; + function() pure { x = 2; } +} +// ---- +// TypeError: (29-55): Fallback function must be payable or non-payable, but is "pure". diff --git a/test/libsolidity/syntaxTests/fallback/view_modifier.sol b/test/libsolidity/syntaxTests/fallback/view_modifier.sol new file mode 100644 index 00000000..44c5d204 --- /dev/null +++ b/test/libsolidity/syntaxTests/fallback/view_modifier.sol @@ -0,0 +1,6 @@ +contract C { + uint x; + function() view { x = 2; } +} +// ---- +// TypeError: (29-55): Fallback function must be payable or non-payable, but is "view". diff --git a/test/libsolidity/syntaxTests/inheritance/override/add_view.sol b/test/libsolidity/syntaxTests/inheritance/override/add_view.sol new file mode 100644 index 00000000..9973b23e --- /dev/null +++ b/test/libsolidity/syntaxTests/inheritance/override/add_view.sol @@ -0,0 +1,4 @@ +contract B { function f() public {} } +contract C is B { function f() view {} } +// ---- +// TypeError: (56-76): Overriding function changes state mutability from "nonpayable" to "view". diff --git a/test/libsolidity/syntaxTests/inheritance/override/remove_view.sol b/test/libsolidity/syntaxTests/inheritance/override/remove_view.sol new file mode 100644 index 00000000..e58f6b20 --- /dev/null +++ b/test/libsolidity/syntaxTests/inheritance/override/remove_view.sol @@ -0,0 +1,4 @@ +contract B { function f() view {} } +contract C is B { function f() public {} } +// ---- +// TypeError: (54-76): Overriding function changes state mutability from "view" to "nonpayable". diff --git a/test/libsolidity/syntaxTests/missing_state_variable.sol b/test/libsolidity/syntaxTests/missing_state_variable.sol new file mode 100644 index 00000000..02082a45 --- /dev/null +++ b/test/libsolidity/syntaxTests/missing_state_variable.sol @@ -0,0 +1,7 @@ +contract Scope { + function getStateVar() view public returns (uint stateVar) { + stateVar = Scope.stateVar; // should fail. + } +} +// ---- +// TypeError: (101-115): Member "stateVar" not found or not visible after argument-dependent lookup in type(contract Scope) diff --git a/test/libsolidity/syntaxTests/parsing/constant_state_modifier.sol b/test/libsolidity/syntaxTests/parsing/constant_state_modifier.sol new file mode 100644 index 00000000..da068351 --- /dev/null +++ b/test/libsolidity/syntaxTests/parsing/constant_state_modifier.sol @@ -0,0 +1,7 @@ +contract C { + uint s; + // this test should fail starting from 0.5.0 + function f() public constant returns (uint) { + return s; + } +} diff --git a/test/libsolidity/syntaxTests/parsing/empty_function.sol b/test/libsolidity/syntaxTests/parsing/empty_function.sol index 4f845189..218fd9a7 100644 --- a/test/libsolidity/syntaxTests/parsing/empty_function.sol +++ b/test/libsolidity/syntaxTests/parsing/empty_function.sol @@ -1,10 +1,10 @@ contract test { uint256 stateVar; - function functionName(bytes20 arg1, address addr) constant returns (int id) { } + function functionName(bytes20 arg1, address addr) view returns (int id) { } } // ---- -// Warning: (36-115): No visibility specified. Defaulting to "public". +// Warning: (36-111): No visibility specified. Defaulting to "public". // Warning: (58-70): Unused function parameter. Remove or comment out the variable name to silence this warning. // Warning: (72-84): Unused function parameter. Remove or comment out the variable name to silence this warning. -// Warning: (104-110): Unused function parameter. Remove or comment out the variable name to silence this warning. -// Warning: (36-115): Function state mutability can be restricted to pure +// Warning: (100-106): Unused function parameter. Remove or comment out the variable name to silence this warning. +// Warning: (36-111): Function state mutability can be restricted to pure diff --git a/test/libsolidity/syntaxTests/parsing/multiple_statemutability_specifiers.sol b/test/libsolidity/syntaxTests/parsing/multiple_statemutability_specifiers.sol new file mode 100644 index 00000000..a05bf452 --- /dev/null +++ b/test/libsolidity/syntaxTests/parsing/multiple_statemutability_specifiers.sol @@ -0,0 +1,33 @@ +contract c1 { + function f() payable payable {} +} +contract c2 { + function f() view view {} +} +contract c3 { + function f() pure pure {} +} +contract c4 { + function f() pure view {} +} +contract c5 { + function f() payable view {} +} +contract c6 { + function f() pure payable {} +} +contract c7 { + function f() pure constant {} +} +contract c8 { + function f() view constant {} +} +// ---- +// ParserError: (39-46): State mutability already specified as "payable". +// ParserError: (88-92): State mutability already specified as "view". +// ParserError: (134-138): State mutability already specified as "pure". +// ParserError: (180-184): State mutability already specified as "pure". +// ParserError: (229-233): State mutability already specified as "payable". +// ParserError: (275-282): State mutability already specified as "pure". +// ParserError: (324-332): State mutability already specified as "pure". +// ParserError: (374-382): State mutability already specified as "view". diff --git a/test/tools/CMakeLists.txt b/test/tools/CMakeLists.txt index febb0c26..11714017 100644 --- a/test/tools/CMakeLists.txt +++ b/test/tools/CMakeLists.txt @@ -1,5 +1,5 @@ add_executable(solfuzzer fuzzer.cpp) -target_link_libraries(solfuzzer PRIVATE libsolc evmasm ${Boost_PROGRAM_OPTIONS_LIBRARIES}) +target_link_libraries(solfuzzer PRIVATE libsolc evmasm ${Boost_PROGRAM_OPTIONS_LIBRARIES} ${Boost_SYSTEM_LIBRARIES}) add_executable(isoltest isoltest.cpp ../Options.cpp ../libsolidity/SyntaxTest.cpp ../libsolidity/AnalysisFramework.cpp) target_link_libraries(isoltest PRIVATE libsolc solidity evmasm ${Boost_PROGRAM_OPTIONS_LIBRARIES} ${Boost_UNIT_TEST_FRAMEWORK_LIBRARIES}) |