diff options
85 files changed, 1986 insertions, 186 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/CMakeLists.txt b/CMakeLists.txt index 4ac56b43..0c05208f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -21,7 +21,7 @@ include(EthCcache) # Let's find our dependencies include(EthDependencies) include(jsoncpp) -include_directories(${JSONCPP_INCLUDE_DIR}) +include_directories(SYSTEM ${JSONCPP_INCLUDE_DIR}) find_package(Threads) diff --git a/Changelog.md b/Changelog.md index 9004dc29..72c27f8e 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,20 +1,26 @@ ### 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. + * General: Allow multiple variables to be declared as part of a tuple assignment, e.g. ``(uint a, uint b) = ...``. * 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. * Parser: Use the entire location of the token instead of only its starting position as source location for parser errors. + * SMT Checker: Support state variables of integer and bool type. * Type Checker: Deprecate the ``years`` unit denomination and raise a warning for it (or an error as experimental 0.5.0 feature). * Type Checker: Make literals (without explicit type casting) an error for tight packing as experimental 0.5.0 feature. * Type Checker: Warn about wildcard tuple assignments (this will turn into an error with version 0.5.0). Bugfixes: + * Code Generator: Fix ``revert`` with reason coming from a state or local string variable. * Type Checker: Show proper error when trying to ``emit`` a non-event. * Type Checker: Warn about empty tuple components (this will turn into an error with version 0.5.0). - + * Type Checker: The ABI encoding functions are pure and thus can be used for constants. ### 0.4.23 (2018-04-19) @@ -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/cmake/EthCompilerSettings.cmake b/cmake/EthCompilerSettings.cmake index c73536ad..683d1d2e 100644 --- a/cmake/EthCompilerSettings.cmake +++ b/cmake/EthCompilerSettings.cmake @@ -62,8 +62,9 @@ if (("${CMAKE_CXX_COMPILER_ID}" MATCHES "GNU") OR ("${CMAKE_CXX_COMPILER_ID}" MA # Additional Clang-specific compiler settings. elseif ("${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang") if ("${CMAKE_SYSTEM_NAME}" MATCHES "Darwin") - # Set stack size to 16MB - by default Apple's clang defines a stack size of 8MB, some tests require more. - set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,-stack_size -Wl,0x1000000") + # Set stack size to 32MB - by default Apple's clang defines a stack size of 8MB. + # Normally 16MB is enough to run all tests, but it will exceed the stack, if -DSANITIZE=address is used. + set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,-stack_size -Wl,0x2000000") endif() # Some Linux-specific Clang settings. We don't want these for OS X. 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/control-structures.rst b/docs/control-structures.rst index f18e1e10..7849d15a 100644 --- a/docs/control-structures.rst +++ b/docs/control-structures.rst @@ -272,9 +272,12 @@ Assignment Destructuring Assignments and Returning Multiple Values ------------------------------------------------------- -Solidity internally allows tuple types, i.e. a list of objects of potentially different types whose size is a constant at compile-time. Those tuples can be used to return multiple values at the same time and also assign them to multiple variables (or LValues in general) at the same time:: +Solidity internally allows tuple types, i.e. a list of objects of potentially different types whose size is a constant at compile-time. Those tuples can be used to return multiple values at the same time. +These can then either be assigned to newly declared variables or to pre-existing variables (or LValues in general): - pragma solidity ^0.4.16; +:: + + pragma solidity >0.4.23 <0.5.0; contract C { uint[] data; @@ -284,12 +287,8 @@ Solidity internally allows tuple types, i.e. a list of objects of potentially di } function g() public { - // Variables declared with type - uint x; - bool b; - uint y; - // Tuple values can be assigned to these pre-existing variables - (x, b, y) = f(); + // Variables declared with type and assigned from the returned tuple. + (uint x, bool b, uint y) = f(); // Common trick to swap values -- does not work for non-value storage types. (x, y) = (y, x); // Components can be left out (also for variable declarations). @@ -330,7 +329,9 @@ A variable declared anywhere within a function will be in scope for the *entire (this will change soon, see below). This happens because Solidity inherits its scoping rules from JavaScript. This is in contrast to many languages where variables are only scoped where they are declared until the end of the semantic block. -As a result, the following code is illegal and cause the compiler to throw an error, ``Identifier already declared``:: +As a result, the following code is illegal and cause the compiler to throw an error, ``Identifier already declared``: + +:: // This will not compile diff --git a/docs/frequently-asked-questions.rst b/docs/frequently-asked-questions.rst index 6a2fe685..ca5a1aee 100644 --- a/docs/frequently-asked-questions.rst +++ b/docs/frequently-asked-questions.rst @@ -203,7 +203,7 @@ situation. If you do not want to throw, you can return a pair:: - pragma solidity ^0.4.16; + pragma solidity >0.4.23 <0.5.0; contract C { uint[] counters; @@ -219,7 +219,7 @@ If you do not want to throw, you can return a pair:: } function checkCounter(uint index) public view { - var (counter, error) = getCounter(index); + (uint counter, bool error) = getCounter(index); if (error) { // ... } else { diff --git a/docs/grammar.txt b/docs/grammar.txt index 565db9a4..0dda4f49 100644 --- a/docs/grammar.txt +++ b/docs/grammar.txt @@ -78,7 +78,7 @@ Break = 'break' Return = 'return' Expression? Throw = 'throw' EmitStatement = 'emit' FunctionCall -VariableDefinition = ('var' IdentifierList | VariableDeclaration) ( '=' Expression )? +VariableDefinition = ('var' IdentifierList | VariableDeclaration | '(' VariableDeclaration? (',' VariableDeclaration? )* ')' ) ( '=' Expression )? IdentifierList = '(' ( Identifier? ',' )* Identifier? ')' // Precedence by order (see github.com/ethereum/solidity/pull/732) 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 2c99b91e..c9b73db2 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/solidity-by-example.rst b/docs/solidity-by-example.rst index f6038f7d..2b3d4b48 100644 --- a/docs/solidity-by-example.rst +++ b/docs/solidity-by-example.rst @@ -388,7 +388,7 @@ high or low invalid bids. :: - pragma solidity ^0.4.22; + pragma solidity >0.4.23 <0.5.0; contract BlindAuction { struct Bid { @@ -467,8 +467,8 @@ high or low invalid bids. uint refund; for (uint i = 0; i < length; i++) { - var bid = bids[msg.sender][i]; - var (value, fake, secret) = + Bid storage bid = bids[msg.sender][i]; + (uint value, bool fake, bytes32 secret) = (_values[i], _fake[i], _secret[i]); if (bid.blindedBid != keccak256(value, fake, secret)) { // Bid was not actually revealed. 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/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index a222bdf0..e8694e88 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -2093,6 +2093,9 @@ bool TypeChecker::visit(MemberAccess const& _memberAccess) if (auto tt = dynamic_cast<TypeType const*>(exprType.get())) if (tt->actualType()->category() == Type::Category::Enum) annotation.isPure = true; + if (auto magicType = dynamic_cast<MagicType const*>(exprType.get())) + if (magicType->kind() == MagicType::Kind::ABI) + annotation.isPure = true; return false; } diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index dc548538..60e3183c 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -2865,7 +2865,11 @@ bool FunctionType::isPure() const m_kind == Kind::RIPEMD160 || m_kind == Kind::AddMod || m_kind == Kind::MulMod || - m_kind == Kind::ObjectCreation; + m_kind == Kind::ObjectCreation || + m_kind == Kind::ABIEncode || + m_kind == Kind::ABIEncodePacked || + m_kind == Kind::ABIEncodeWithSelector || + m_kind == Kind::ABIEncodeWithSignature; } TypePointers FunctionType::parseElementaryTypeVector(strings const& _types) diff --git a/libsolidity/ast/Types.h b/libsolidity/ast/Types.h index 6defacfc..4884696d 100644 --- a/libsolidity/ast/Types.h +++ b/libsolidity/ast/Types.h @@ -1046,8 +1046,8 @@ public: return *m_declaration; } bool hasDeclaration() const { return !!m_declaration; } - /// @returns true if the result of this function only depends on its arguments - /// and it does not modify the state. + /// @returns true if the result of this function only depends on its arguments, + /// does not modify the state and is a compile-time constant. /// Currently, this will only return true for internal functions like keccak and ecrecover. bool isPure() const; bool isPayable() const { return m_stateMutability == StateMutability::Payable; } diff --git a/libsolidity/codegen/CompilerUtils.cpp b/libsolidity/codegen/CompilerUtils.cpp index a39e799c..d9f17263 100644 --- a/libsolidity/codegen/CompilerUtils.cpp +++ b/libsolidity/codegen/CompilerUtils.cpp @@ -89,7 +89,6 @@ void CompilerUtils::revertWithStringData(Type const& _argumentType) abiEncode({_argumentType.shared_from_this()}, {make_shared<ArrayType>(DataLocation::Memory, true)}); toSizeAfterFreeMemoryPointer(); m_context << Instruction::REVERT; - m_context.adjustStackOffset(_argumentType.sizeOnStack()); } unsigned CompilerUtils::loadFromMemory( diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp index a8222e21..4bcc1fa9 100644 --- a/libsolidity/codegen/ExpressionCompiler.cpp +++ b/libsolidity/codegen/ExpressionCompiler.cpp @@ -933,7 +933,11 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) // condition was not met, flag an error m_context.appendInvalid(); else if (arguments.size() > 1) + { utils().revertWithStringData(*arguments.at(1)->annotation().type); + // Here, the argument is consumed, but in the other branch, it is still there. + m_context.adjustStackOffset(arguments.at(1)->annotation().type->sizeOnStack()); + } else m_context.appendRevert(); // the success branch diff --git a/libsolidity/formal/SMTChecker.cpp b/libsolidity/formal/SMTChecker.cpp index c4dee22d..425c5c1e 100644 --- a/libsolidity/formal/SMTChecker.cpp +++ b/libsolidity/formal/SMTChecker.cpp @@ -58,6 +58,19 @@ void SMTChecker::analyze(SourceUnit const& _source) _source.accept(*this); } +bool SMTChecker::visit(ContractDefinition const& _contract) +{ + for (auto _var : _contract.stateVariables()) + if (_var->type()->isValueType()) + createVariable(*_var); + return true; +} + +void SMTChecker::endVisit(ContractDefinition const&) +{ + m_stateVariables.clear(); +} + void SMTChecker::endVisit(VariableDeclaration const& _varDecl) { if (_varDecl.isLocalVariable() && _varDecl.type()->isValueType() &&_varDecl.value()) @@ -72,13 +85,13 @@ bool SMTChecker::visit(FunctionDefinition const& _function) "Assertion checker does not yet support constructors and functions with modifiers." ); m_currentFunction = &_function; - // We only handle local variables, so we clear at the beginning of the function. - // If we add storage variables, those should be cleared differently. m_interface->reset(); m_variables.clear(); + m_variables.insert(m_stateVariables.begin(), m_stateVariables.end()); m_pathConditions.clear(); m_loopExecutionHappened = false; initializeLocalVariables(_function); + resetStateVariables(); return true; } @@ -586,6 +599,12 @@ void SMTChecker::checkCondition( expressionsToEvaluate.emplace_back(currentValue(*var)); expressionNames.push_back(var->name()); } + for (auto const& var: m_stateVariables) + if (knownVariable(*var.first)) + { + expressionsToEvaluate.emplace_back(currentValue(*var.first)); + expressionNames.push_back(var.first->name()); + } } smt::CheckResult result; vector<string> values; @@ -607,7 +626,8 @@ void SMTChecker::checkCondition( message << " for:\n"; solAssert(values.size() == expressionNames.size(), ""); for (size_t i = 0; i < values.size(); ++i) - message << " " << expressionNames.at(i) << " = " << values.at(i) << "\n"; + if (expressionsToEvaluate.at(i).name != values.at(i)) + message << " " << expressionNames.at(i) << " = " << values.at(i) << "\n"; } else message << "."; @@ -722,6 +742,15 @@ void SMTChecker::initializeLocalVariables(FunctionDefinition const& _function) setZeroValue(*retParam); } +void SMTChecker::resetStateVariables() +{ + for (auto const& variable: m_stateVariables) + { + newValue(*variable.first); + setUnknownValue(*variable.first); + } +} + void SMTChecker::resetVariables(vector<Declaration const*> _variables) { for (auto const* decl: _variables) @@ -752,7 +781,14 @@ bool SMTChecker::createVariable(VariableDeclaration const& _varDecl) if (SSAVariable::isSupportedType(_varDecl.type()->category())) { solAssert(m_variables.count(&_varDecl) == 0, ""); - m_variables.emplace(&_varDecl, SSAVariable(_varDecl, *m_interface)); + solAssert(m_stateVariables.count(&_varDecl) == 0, ""); + if (_varDecl.isLocalVariable()) + m_variables.emplace(&_varDecl, SSAVariable(_varDecl, *m_interface)); + else + { + solAssert(_varDecl.isStateVariable(), ""); + m_stateVariables.emplace(&_varDecl, SSAVariable(_varDecl, *m_interface)); + } return true; } else diff --git a/libsolidity/formal/SMTChecker.h b/libsolidity/formal/SMTChecker.h index fd54fb5c..50d40ab9 100644 --- a/libsolidity/formal/SMTChecker.h +++ b/libsolidity/formal/SMTChecker.h @@ -50,6 +50,8 @@ private: // because the order of expression evaluation is undefined // TODO: or just force a certain order, but people might have a different idea about that. + virtual bool visit(ContractDefinition const& _node) override; + virtual void endVisit(ContractDefinition const& _node) override; virtual void endVisit(VariableDeclaration const& _node) override; virtual bool visit(FunctionDefinition const& _node) override; virtual void endVisit(FunctionDefinition const& _node) override; @@ -111,6 +113,7 @@ private: smt::CheckResult checkSatisfiable(); void initializeLocalVariables(FunctionDefinition const& _function); + void resetStateVariables(); void resetVariables(std::vector<Declaration const*> _variables); /// Given two different branches and the touched variables, /// merge the touched variables into after-branch ite variables @@ -163,6 +166,7 @@ private: bool m_loopExecutionHappened = false; std::map<Expression const*, smt::Expression> m_expressions; std::map<Declaration const*, SSAVariable> m_variables; + std::map<Declaration const*, SSAVariable> m_stateVariables; std::vector<smt::Expression> m_pathConditions; ErrorReporter& m_errorReporter; diff --git a/libsolidity/formal/VariableUsage.cpp b/libsolidity/formal/VariableUsage.cpp index 4e96059d..c2dea844 100644 --- a/libsolidity/formal/VariableUsage.cpp +++ b/libsolidity/formal/VariableUsage.cpp @@ -33,7 +33,6 @@ VariableUsage::VariableUsage(ASTNode const& _node) solAssert(declaration, ""); if (VariableDeclaration const* varDecl = dynamic_cast<VariableDeclaration const*>(declaration)) if ( - varDecl->isLocalVariable() && identifier->annotation().lValueRequested && varDecl->annotation().type->isValueType() ) 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/libsolidity/parsing/Parser.cpp b/libsolidity/parsing/Parser.cpp index d1be13a5..e2e1eebc 100644 --- a/libsolidity/parsing/Parser.cpp +++ b/libsolidity/parsing/Parser.cpp @@ -54,6 +54,7 @@ public: template <class NodeType, typename... Args> ASTPointer<NodeType> createNode(Args&& ... _args) { + solAssert(m_location.sourceName, ""); if (m_location.end < 0) markEndPosition(); return make_shared<NodeType>(m_location, forward<Args>(_args)...); @@ -1086,15 +1087,79 @@ ASTPointer<Statement> Parser::parseSimpleStatement(ASTPointer<ASTString> const& LookAheadInfo statementType; IndexAccessedPath iap; - tie(statementType, iap) = tryParseIndexAccessedPath(); - switch (statementType) + if (m_scanner->currentToken() == Token::LParen) { - case LookAheadInfo::VariableDeclaration: - return parseVariableDeclarationStatement(_docString, typeNameFromIndexAccessStructure(iap)); - case LookAheadInfo::Expression: - return parseExpressionStatement(_docString, expressionFromIndexAccessStructure(iap)); - default: - solAssert(false, ""); + ASTNodeFactory nodeFactory(*this); + size_t emptyComponents = 0; + // First consume all empty components. + expectToken(Token::LParen); + while (m_scanner->currentToken() == Token::Comma) + { + m_scanner->next(); + emptyComponents++; + } + + // Now see whether we have a variable declaration or an expression. + tie(statementType, iap) = tryParseIndexAccessedPath(); + switch (statementType) + { + case LookAheadInfo::VariableDeclaration: + { + vector<ASTPointer<VariableDeclaration>> variables; + ASTPointer<Expression> value; + // We have already parsed something like `(,,,,a.b.c[2][3]` + VarDeclParserOptions options; + options.allowLocationSpecifier = true; + variables = vector<ASTPointer<VariableDeclaration>>(emptyComponents, nullptr); + variables.push_back(parseVariableDeclaration(options, typeNameFromIndexAccessStructure(iap))); + + while (m_scanner->currentToken() != Token::RParen) + { + expectToken(Token::Comma); + if (m_scanner->currentToken() == Token::Comma || m_scanner->currentToken() == Token::RParen) + variables.push_back(nullptr); + else + variables.push_back(parseVariableDeclaration(options)); + } + expectToken(Token::RParen); + expectToken(Token::Assign); + value = parseExpression(); + nodeFactory.setEndPositionFromNode(value); + return nodeFactory.createNode<VariableDeclarationStatement>(_docString, variables, value); + } + case LookAheadInfo::Expression: + { + // Complete parsing the expression in the current component. + vector<ASTPointer<Expression>> components(emptyComponents, nullptr); + components.push_back(parseExpression(expressionFromIndexAccessStructure(iap))); + while (m_scanner->currentToken() != Token::RParen) + { + expectToken(Token::Comma); + if (m_scanner->currentToken() == Token::Comma || m_scanner->currentToken() == Token::RParen) + components.push_back(ASTPointer<Expression>()); + else + components.push_back(parseExpression()); + } + nodeFactory.markEndPosition(); + expectToken(Token::RParen); + return parseExpressionStatement(_docString, nodeFactory.createNode<TupleExpression>(components, false)); + } + default: + solAssert(false, ""); + } + } + else + { + tie(statementType, iap) = tryParseIndexAccessedPath(); + switch (statementType) + { + case LookAheadInfo::VariableDeclaration: + return parseVariableDeclarationStatement(_docString, typeNameFromIndexAccessStructure(iap)); + case LookAheadInfo::Expression: + return parseExpressionStatement(_docString, expressionFromIndexAccessStructure(iap)); + default: + solAssert(false, ""); + } } } @@ -1144,6 +1209,9 @@ ASTPointer<VariableDeclarationStatement> Parser::parseVariableDeclarationStateme ASTPointer<TypeName> const& _lookAheadArrayType ) { + // This does not parse multi variable declaration statements starting directly with + // `(`, they are parsed in parseSimpleStatement, because they are hard to distinguish + // from tuple expressions. RecursionGuard recursionGuard(*this); ASTNodeFactory nodeFactory(*this); if (_lookAheadArrayType) 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/SMTChecker.cpp b/test/libsolidity/SMTChecker.cpp index beb933a4..71fdb906 100644 --- a/test/libsolidity/SMTChecker.cpp +++ b/test/libsolidity/SMTChecker.cpp @@ -467,6 +467,82 @@ BOOST_AUTO_TEST_CASE(bool_int_mixed) CHECK_SUCCESS_NO_WARNINGS(text); } +BOOST_AUTO_TEST_CASE(storage_value_vars) +{ + string text = R"( + contract C + { + address a; + bool b; + uint c; + function f(uint x) public { + if (x == 0) + { + a = 100; + b = true; + } + else + { + a = 200; + b = false; + } + assert(a > 0 && b); + } + } + )"; + CHECK_WARNING(text, "Assertion violation happens here"); + text = R"( + contract C + { + address a; + bool b; + uint c; + function f() public view { + assert(c > 0); + } + } + )"; + CHECK_WARNING(text, "Assertion violation happens here"); + text = R"( + contract C + { + function f(uint x) public { + if (x == 0) + { + a = 100; + b = true; + } + else + { + a = 200; + b = false; + } + assert(b == (a < 200)); + } + + function g() public view { + require(a < 100); + assert(c >= 0); + } + address a; + bool b; + uint c; + } + )"; + CHECK_SUCCESS_NO_WARNINGS(text); + text = R"( + contract C + { + function f() public view { + assert(c > 0); + } + uint c; + } + )"; + CHECK_WARNING(text, "Assertion violation happens here"); + +} + BOOST_AUTO_TEST_CASE(while_loop_simple) { // Check that variables are cleared diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 3a4a2dad..42f69099 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); @@ -7610,6 +7610,33 @@ BOOST_AUTO_TEST_CASE(multi_variable_declaration) ABI_CHECK(callContractFunction("f()", encodeArgs()), encodeArgs(true)); } +BOOST_AUTO_TEST_CASE(typed_multi_variable_declaration) +{ + char const* sourceCode = R"( + contract C { + struct S { uint x; } + S s; + function g() internal returns (uint, S storage, uint) { + s.x = 7; + return (1, s, 2); + } + function f() returns (bool) { + (uint x1, S storage y1, uint z1) = g(); + if (x1 != 1 || y1.x != 7 || z1 != 2) return false; + (, S storage y2,) = g(); + if (y2.x != 7) return false; + (uint x2,,) = g(); + if (x2 != 1) return false; + (,,uint z2) = g(); + if (z2 != 2) return false; + return true; + } + } + )"; + compileAndRun(sourceCode); + ABI_CHECK(callContractFunction("f()", encodeArgs()), encodeArgs(true)); +} + BOOST_AUTO_TEST_CASE(tuples) { char const* sourceCode = R"( @@ -10660,12 +10687,24 @@ BOOST_AUTO_TEST_CASE(revert_with_cause) { char const* sourceCode = R"( contract D { + string constant msg1 = "test1234567890123456789012345678901234567890"; + string msg2 = "test1234567890123456789012345678901234567890"; function f() public { revert("test123"); } function g() public { revert("test1234567890123456789012345678901234567890"); } + function h() public { + revert(msg1); + } + function i() public { + revert(msg2); + } + function j() public { + string memory msg3 = "test1234567890123456789012345678901234567890"; + revert(msg3); + } } contract C { D d = new D(); @@ -10686,6 +10725,15 @@ BOOST_AUTO_TEST_CASE(revert_with_cause) function g() public returns (bool, bytes) { return forward(address(d), msg.data); } + function h() public returns (bool, bytes) { + return forward(address(d), msg.data); + } + function i() public returns (bool, bytes) { + return forward(address(d), msg.data); + } + function j() public returns (bool, bytes) { + return forward(address(d), msg.data); + } } )"; compileAndRun(sourceCode, 0, "C"); @@ -10693,6 +10741,9 @@ BOOST_AUTO_TEST_CASE(revert_with_cause) bytes const errorSignature = bytes{0x08, 0xc3, 0x79, 0xa0}; ABI_CHECK(callContractFunction("f()"), haveReturndata ? encodeArgs(0, 0x40, 0x64) + errorSignature + encodeArgs(0x20, 7, "test123") + bytes(28, 0) : bytes()); ABI_CHECK(callContractFunction("g()"), haveReturndata ? encodeArgs(0, 0x40, 0x84) + errorSignature + encodeArgs(0x20, 44, "test1234567890123456789012345678901234567890") + bytes(28, 0): bytes()); + ABI_CHECK(callContractFunction("h()"), haveReturndata ? encodeArgs(0, 0x40, 0x84) + errorSignature + encodeArgs(0x20, 44, "test1234567890123456789012345678901234567890") + bytes(28, 0): bytes()); + ABI_CHECK(callContractFunction("i()"), haveReturndata ? encodeArgs(0, 0x40, 0x84) + errorSignature + encodeArgs(0x20, 44, "test1234567890123456789012345678901234567890") + bytes(28, 0): bytes()); + ABI_CHECK(callContractFunction("j()"), haveReturndata ? encodeArgs(0, 0x40, 0x84) + errorSignature + encodeArgs(0x20, 44, "test1234567890123456789012345678901234567890") + bytes(28, 0): bytes()); } BOOST_AUTO_TEST_CASE(require_with_message) @@ -10701,6 +10752,7 @@ BOOST_AUTO_TEST_CASE(require_with_message) contract D { bool flag = false; string storageError = "abc"; + string constant constantError = "abc"; function f(uint x) public { require(x > 7, "failed"); } @@ -10718,6 +10770,13 @@ BOOST_AUTO_TEST_CASE(require_with_message) function h() public { require(false, storageError); } + function i() public { + require(false, constantError); + } + function j() public { + string memory errMsg = "msg"; + require(false, errMsg); + } } contract C { D d = new D(); @@ -10741,6 +10800,12 @@ BOOST_AUTO_TEST_CASE(require_with_message) function h() public returns (bool, bytes) { return forward(address(d), msg.data); } + function i() public returns (bool, bytes) { + return forward(address(d), msg.data); + } + function j() public returns (bool, bytes) { + return forward(address(d), msg.data); + } } )"; compileAndRun(sourceCode, 0, "C"); @@ -10751,6 +10816,8 @@ BOOST_AUTO_TEST_CASE(require_with_message) ABI_CHECK(callContractFunction("g()"), haveReturndata ? encodeArgs(1, 0x40, 0) : bytes()); ABI_CHECK(callContractFunction("g()"), haveReturndata ? encodeArgs(0, 0x40, 0x64) + errorSignature + encodeArgs(0x20, 18, "only on second run") + bytes(28, 0) : bytes()); ABI_CHECK(callContractFunction("h()"), haveReturndata ? encodeArgs(0, 0x40, 0x64) + errorSignature + encodeArgs(0x20, 3, "abc") + bytes(28, 0): bytes()); + ABI_CHECK(callContractFunction("i()"), haveReturndata ? encodeArgs(0, 0x40, 0x64) + errorSignature + encodeArgs(0x20, 3, "abc") + bytes(28, 0): bytes()); + ABI_CHECK(callContractFunction("j()"), haveReturndata ? encodeArgs(0, 0x40, 0x64) + errorSignature + encodeArgs(0x20, 3, "msg") + bytes(28, 0): bytes()); } BOOST_AUTO_TEST_CASE(bubble_up_error_messages) @@ -10877,9 +10944,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"); @@ -11065,13 +11132,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(); } } @@ -11093,13 +11160,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()"))); } } @@ -11560,7 +11627,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/abi_encoding_constant.sol b/test/libsolidity/syntaxTests/constants/abi_encoding_constant.sol new file mode 100644 index 00000000..dd6968a0 --- /dev/null +++ b/test/libsolidity/syntaxTests/constants/abi_encoding_constant.sol @@ -0,0 +1,7 @@ +contract C { + bytes32 constant a = keccak256(abi.encode(1, 2)); + bytes32 constant b = keccak256(abi.encodePacked(uint(1), a)); + bytes32 constant c = keccak256(abi.encodeWithSelector(0x12345678, b, 2)); + bytes32 constant d = keccak256(abi.encodeWithSignature("f()", 1, 2)); +} +// ---- 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/multiVariableDeclaration/multiSingleVariableDeclaration.sol b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiSingleVariableDeclaration.sol new file mode 100644 index 00000000..182ba072 --- /dev/null +++ b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiSingleVariableDeclaration.sol @@ -0,0 +1,6 @@ +contract C { + function f() internal returns (uint) { + (uint a) = f(); + a; + } +} diff --git a/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationComplex.sol b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationComplex.sol new file mode 100644 index 00000000..a3ce6a74 --- /dev/null +++ b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationComplex.sol @@ -0,0 +1,11 @@ +contract D { + struct S { uint a; uint b; } +} +contract C { + function f() internal returns (uint, uint, uint, D.S[20] storage, uint) { + (,,,D.S[10*2] storage x,) = f(); + x; + } +} +// ---- +// Warning: (110-117): 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/multiVariableDeclaration/multiVariableDeclarationInvalid.sol b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationInvalid.sol new file mode 100644 index 00000000..c8686ae8 --- /dev/null +++ b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationInvalid.sol @@ -0,0 +1,8 @@ +contract C { + function f() internal returns (uint, uint, uint, uint) { + var (uint a, uint b,,) = f(); + a; b; + } +} +// ---- +// ParserError: (81-85): Expected identifier but got 'uint' diff --git a/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationInvalidType.sol b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationInvalidType.sol new file mode 100644 index 00000000..2b765837 --- /dev/null +++ b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationInvalidType.sol @@ -0,0 +1,9 @@ +contract C { + function f() internal returns (string memory, uint, uint, uint) { + (uint a, string memory b,,) = f(); + a; b; + } +} +// ---- +// TypeError: (85-118): Type string memory is not implicitly convertible to expected type uint256. +// TypeError: (85-118): Type uint256 is not implicitly convertible to expected type string memory. diff --git a/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationScoping.sol b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationScoping.sol new file mode 100644 index 00000000..3ba85f69 --- /dev/null +++ b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationScoping.sol @@ -0,0 +1,12 @@ +pragma experimental "v0.5.0"; + +contract C { + function f() internal { + { + (uint a, uint b, uint c) = (1, 2, 3); + } + a; + } +} +// ---- +// DeclarationError: (130-131): Undeclared identifier. diff --git a/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationScoping2.sol b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationScoping2.sol new file mode 100644 index 00000000..e21181de --- /dev/null +++ b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationScoping2.sol @@ -0,0 +1,13 @@ +pragma experimental "v0.5.0"; + +contract C { + function f() internal { + { + (uint a, uint b, uint c) = (a, b, c); + } + } +} +// ---- +// DeclarationError: (110-111): Undeclared identifier. Did you mean "a"? +// DeclarationError: (113-114): Undeclared identifier. Did you mean "b"? +// DeclarationError: (116-117): Undeclared identifier. Did you mean "c"? diff --git a/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationSimple.sol b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationSimple.sol new file mode 100644 index 00000000..8e06322c --- /dev/null +++ b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationSimple.sol @@ -0,0 +1,12 @@ +contract C { + function f() internal returns (uint, uint, uint, uint) { + (uint a, uint b,,) = f(); + a; b; + } + function g() internal returns (bytes memory, string storage) { + (bytes memory a, string storage b) = g(); + a; b; + } +} +// ---- +// Warning: (163-169): 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/multiVariableDeclaration/multiVariableDeclarationThatIsExpression.sol b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationThatIsExpression.sol new file mode 100644 index 00000000..8ae0eaac --- /dev/null +++ b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationThatIsExpression.sol @@ -0,0 +1,9 @@ +contract C { + struct S { function() returns (S storage)[] x; } + S s; + function f() internal pure returns (uint, uint, uint, S storage, uint, uint) { + (,,,s.x[2](),,) = f(); + } +} +// ---- +// TypeError: (160-168): Expression has to be an lvalue. 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}) |