diff options
23 files changed, 931 insertions, 241 deletions
diff --git a/Changelog.md b/Changelog.md index 5b504a5b..ebd6f121 100644 --- a/Changelog.md +++ b/Changelog.md @@ -11,6 +11,7 @@ Bugfixes: * Commandline interface: Always escape filenames (replace ``/``, ``:`` and ``.`` with ``_``). * Commandline interface: Do not try creating paths ``.`` and ``..``. * Type system: Fix a crash caused by continuing on fatal errors in the code. + * Type system: Detect cyclic dependencies between constants. * Type system: Disallow arrays with negative length. * Type system: Fix a crash related to invalid binary operators. * Type system: Disallow ``var`` declaration with empty tuple type. diff --git a/docs/assembly.rst b/docs/assembly.rst index 23ccfcbe..415bb1a1 100644 --- a/docs/assembly.rst +++ b/docs/assembly.rst @@ -333,11 +333,10 @@ push their entry label (with virtual function resolution applied). The calling s in solidity are: - the caller pushes return label, arg1, arg2, ..., argn - - the call returns with ret1, ret2, ..., retn + - the call returns with ret1, ret2, ..., retm This feature is still a bit cumbersome to use, because the stack offset essentially changes during the call, and thus references to local variables will be wrong. -It is planned that the stack height changes can be specified in inline assembly. .. code:: @@ -361,7 +360,9 @@ Labels Another problem in EVM assembly is that ``jump`` and ``jumpi`` use absolute addresses which can change easily. Solidity inline assembly provides labels to make the use of -jumps easier. The following code computes an element in the Fibonacci series. +jumps easier. Note that labels are a low-level feature and it is possible to write +efficient assembly without labels, just using assembly functions, loops and switch instructions +(see below). The following code computes an element in the Fibonacci series. .. code:: @@ -391,12 +392,14 @@ will have a wrong impression about the stack height at label ``two``: .. code:: { + let x := 8 jump(two) one: - // Here the stack height is 1 (because we pushed 7), - // but the assembler thinks it is 0 because it reads + // Here the stack height is 2 (because we pushed x and 7), + // but the assembler thinks it is 1 because it reads // from top to bottom. - // Accessing stack variables here will lead to errors. + // Accessing the stack variable x here will lead to errors. + x := 9 jump(three) two: 7 // push something onto the stack @@ -404,6 +407,31 @@ will have a wrong impression about the stack height at label ``two``: three: } +This problem can be fixed by manually adjusting the stack height for the +assembler - you can provide a stack height delta that is added +to the stack height just prior to the label. +Note that you will not have to care about these things if you just use +loops and assembly-level functions. + +As an example how this can be done in extreme cases, please see the following. + +.. code:: + + { + let x := 8 + jump(two) + 0 // This code is unreachable but will adjust the stack height correctly + one: + x := 9 // Now x can be accessed properly. + jump(three) + pop // Similar negative correction. + two: + 7 // push something onto the stack + jump(one) + three: + pop // We have to pop the manually pushed value here again. + } + .. note:: ``invalidJumpLabel`` is a pre-defined label. Jumping to this location will always @@ -464,6 +492,9 @@ is performed by replacing the variable's value on the stack by the new value. Switch ------ +.. note:: + Switch is not yet implemented. + You can use a switch statement as a very basic version of "if/else". It takes the value of an expression and compares it to several constants. The branch corresponding to the matching constant is taken. Contrary to the @@ -491,6 +522,9 @@ case does require them. Loops ----- +.. note:: + Loops are not yet implemented. + Assembly supports a simple for-style loop. For-style loops have a header containing an initializing part, a condition and a post-iteration part. The condition has to be a functional-style expression, while @@ -512,6 +546,9 @@ The following example computes the sum of an area in memory. Functions --------- +.. note:: + Functions are not yet implemented. + Assembly allows the definition of low-level functions. These take their arguments (and a return PC) from the stack and also put the results onto the stack. Calling a function looks the same way as executing a functional-style @@ -610,25 +647,19 @@ which follow very simple and regular scoping rules and cleanup of local variable Scoping: An identifier that is declared (label, variable, function, assembly) is only visible in the block where it was declared (including nested blocks inside the current block). It is not legal to access local variables across -function borders, even if they would be in scope. Shadowing is allowed, but -two identifiers with the same name cannot be declared in the same block. +function borders, even if they would be in scope. Shadowing is not allowed. Local variables cannot be accessed before they were declared, but labels, functions and assemblies can. Assemblies are special blocks that are used for e.g. returning runtime code or creating contracts. No identifier from an outer assembly is visible in a sub-assembly. If control flow passes over the end of a block, pop instructions are inserted -that match the number of local variables declared in that block, unless the -``}`` is directly preceded by an opcode that does not have a continuing control -flow path. Whenever a local variable is referenced, the code generator needs +that match the number of local variables declared in that block. +Whenever a local variable is referenced, the code generator needs to know its current relative position in the stack and thus it needs to -keep track of the current so-called stack height. -At the end of a block, this implicit stack height is always reduced by the number -of local variables whether ther is a continuing control flow or not. - -This means that the stack height before and after the block should be the same. -If this is not the case, a warning is issued, -unless the last instruction in the block did not have a continuing control flow path. +keep track of the current so-called stack height. Since all local variables +are removed at the end of a block, the stack height before and after the block +should be the same. If this is not the case, a warning is issued. Why do we use higher-level constructs like ``switch``, ``for`` and functions: @@ -640,10 +671,9 @@ verification and optimization. Furthermore, if manual jumps are allowed, computing the stack height is rather complicated. The position of all local variables on the stack needs to be known, otherwise neither references to local variables nor removing local variables automatically -from the stack at the end of a block will work properly. Because of that, -every label that is preceded by an instruction that ends or diverts control flow -should be annotated with the current stack layout. This annotation is performed -automatically during the desugaring phase. +from the stack at the end of a block will work properly. The desugaring +mechanism correctly inserts operations at unreachable blocks that adjust the +stack height properly in case of jumps that do not have a continuing control flow. Example: @@ -696,17 +726,21 @@ After the desugaring phase it looks as follows:: $case1: { // the function call - we put return label and arguments on the stack - $ret1 calldataload(4) jump($fun_f) - $ret1 [r]: // a label with a [...]-annotation resets the stack height - // to "current block + number of local variables". It also - // introduces a variable, r: - // r is at top of stack, $0 is below (from enclosing block) - $ret2 0x20 jump($fun_allocate) - $ret2 [ret]: // stack here: $0, r, ret (top) + $ret1 calldataload(4) jump(f) + // This is unreachable code. Opcodes are added that mirror the + // effect of the function on the stack height: Arguments are + // removed and return values are introduced. + pop pop + let r := 0 + $ret1: // the actual return point + $ret2 0x20 jump($allocate) + pop pop let ret := 0 + $ret2: mstore(ret, r) return(ret, 0x20) // although it is useless, the jump is automatically inserted, - // since the desugaring process does not analyze control-flow + // since the desugaring process is a purely syntactic operation that + // does not analyze control-flow jump($endswitch) } $caseDefault: @@ -717,20 +751,29 @@ After the desugaring phase it looks as follows:: $endswitch: } jump($afterFunction) - $fun_allocate: + allocate: { - $start[$retpos, size]: - // output variables live in the same scope as the arguments. + // we jump over the unreachable code that introduces the function arguments + jump($start) + let $retpos := 0 let size := 0 + $start: + // output variables live in the same scope as the arguments and is + // actually allocated. let pos := 0 { pos := mload(0x40) mstore(0x40, add(pos, size)) } + // This code replaces the arguments by the return values and jumps back. swap1 pop swap1 jump + // Again unreachable code that corrects stack height. + 0 0 } - $fun_f: + f: { - start [$retpos, x]: + jump($start) + let $retpos := 0 let x := 0 + $start: let y := 0 { let i := 0 @@ -743,8 +786,9 @@ After the desugaring phase it looks as follows:: { i := add(i, 1) } jump($for_begin) $for_end: - } // Here, a pop instruction is inserted for i + } // Here, a pop instruction will be inserted for i swap1 pop swap1 jump + 0 0 } $afterFunction: stop @@ -805,7 +849,7 @@ Grammar:: IdentifierOrList = Identifier | '(' IdentifierList ')' IdentifierList = Identifier ( ',' Identifier)* AssemblyAssignment = '=:' Identifier - LabelDefinition = Identifier ( '[' ( IdentifierList | NumberLiteral ) ']' )? ':' + LabelDefinition = Identifier ':' AssemblySwitch = 'switch' FunctionalAssemblyExpression AssemblyCase* ( 'default' ':' AssemblyBlock )? AssemblyCase = 'case' FunctionalAssemblyExpression ':' AssemblyBlock @@ -838,11 +882,14 @@ Pseudocode:: AssemblyFunctionDefinition('function' name '(' arg1, ..., argn ')' '->' ( '(' ret1, ..., retm ')' body) -> <name>: { - $<name>_start [$retPC, $argn, ..., arg1]: + jump($<name>_start) + let $retPC := 0 let argn := 0 ... let arg1 := 0 + $<name>_start: let ret1 := 0 ... let retm := 0 { desugar(body) } - swap and pop items so that only ret1, ... retn, $retPC are left on the stack - jump + swap and pop items so that only ret1, ... retm, $retPC are left on the stack + jump + 0 (1 + n times) to compensate removal of arg1, ..., argn and $retPC } AssemblyFor('for' { init } condition post body) -> { @@ -862,6 +909,7 @@ Pseudocode:: pop all local variables that are defined at the current point but not at $forI_end jump($forI_end) + 0 (as many as variables were removed above) } 'continue' -> { @@ -869,6 +917,7 @@ Pseudocode:: pop all local variables that are defined at the current point but not at $forI_continue jump($forI_continue) + 0 (as many as variables were removed above) } AssemblySwitch(switch condition cases ( default: defaultBlock )? ) -> { @@ -890,10 +939,13 @@ Pseudocode:: { // find I such that $funcallI_* does not exist $funcallI_return argn ... arg2 arg1 jump(<name>) + pop (n + 1 times) if the current context is `let (id1, ..., idm) := f(...)` -> - $funcallI_return [id1, ..., idm]: + let id1 := 0 ... let idm := 0 + $funcallI_return: else -> - $funcallI_return[m - n - 1]: + 0 (m times) + $funcallI_return: turn the functional expression that leads to the function call into a statement stream } @@ -906,8 +958,16 @@ Pseudocode:: Opcode Stream Generation ------------------------ -During opcode stream generation, we keep track of the current stack height, -so that accessing stack variables by name is possible. +During opcode stream generation, we keep track of the current stack height +in a counter, +so that accessing stack variables by name is possible. The stack height is modified with every opcode +that modifies the stack and with every label that is annotated with a stack +adjustment. Every time a new +local variable is introduced, it is registered together with the current +stack height. If a variable is accessed (either for copying its value or for +assignment), the appropriate DUP or SWAP instruction is selected depending +on the difference bitween the current stack height and the +stack height at the point the variable was introduced. Pseudocode:: @@ -945,13 +1005,8 @@ Pseudocode:: look up id in the syntactic stack of blocks, assert that it is a variable SWAPi where i = 1 + stack_height - stack_height_of_identifier(id) POP - LabelDefinition(name [id1, ..., idn] :) -> - JUMPDEST - // register new variables id1, ..., idn and set the stack height to - // stack_height_at_block_start + number_of_local_variables - LabelDefinition(name [number] :) -> + LabelDefinition(name:) -> JUMPDEST - // adjust stack height by +number (can be negative) NumberLiteral(num) -> PUSH<num interpreted as decimal and right-aligned> HexLiteral(lit) -> diff --git a/docs/control-structures.rst b/docs/control-structures.rst index ebc45965..83d3eac9 100644 --- a/docs/control-structures.rst +++ b/docs/control-structures.rst @@ -399,10 +399,9 @@ Currently, Solidity automatically generates a runtime exception in the following While a user-provided exception is generated in the following situations: #. Calling ``throw``. -#. The condition of ``assert(condition)`` is not met. Internally, Solidity performs a revert operation (instruction ``0xfd``) when a user-provided exception is thrown. In contrast, it performs an invalid operation (instruction ``0xfe``) if a runtime exception is encountered. In both cases, this causes the EVM to revert all changes made to the state. The reason for this is that there is no safe way to continue execution, because an expected effect did not occur. Because we want to retain the atomicity of transactions, the safest thing to do is to revert all changes and make the whole transaction -(or at least call) without effect.
\ No newline at end of file +(or at least call) without effect. diff --git a/docs/index.rst b/docs/index.rst index 904c3a54..fc1a4231 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -39,6 +39,9 @@ Available Solidity Integrations * `Ethereum Studio <https://live.ether.camp/>`_ Specialized web IDE that also provides shell access to a complete Ethereum environment. +* `IntelliJ IDEA plugin <https://plugins.jetbrains.com/plugin/9475-intellij-solidity>`_ + Solidity plugin for IntelliJ IDEA (and all other JetBrains IDEs) + * `Visual Studio Extension <https://visualstudiogallery.msdn.microsoft.com/96221853-33c4-4531-bdd5-d2ea5acc4799/>`_ Solidity plugin for Microsoft Visual Studio that includes the Solidity compiler. diff --git a/docs/miscellaneous.rst b/docs/miscellaneous.rst index 7b0305d5..e3ec0efb 100644 --- a/docs/miscellaneous.rst +++ b/docs/miscellaneous.rst @@ -435,7 +435,7 @@ The following is the order of precedence for operators, listed in order of evalu | *16* | Comma operator | ``,`` | +------------+-------------------------------------+--------------------------------------------+ -.. index:: block, coinbase, difficulty, number, block;number, timestamp, block;timestamp, msg, data, gas, sender, value, now, gas price, origin, assert, revert, keccak256, ripemd160, sha256, ecrecover, addmod, mulmod, cryptography, this, super, selfdestruct, balance, send +.. index:: block, coinbase, difficulty, number, block;number, timestamp, block;timestamp, msg, data, gas, sender, value, now, gas price, origin, revert, keccak256, ripemd160, sha256, ecrecover, addmod, mulmod, cryptography, this, super, selfdestruct, balance, send Global Variables ================ @@ -461,7 +461,6 @@ Global Variables - ``ecrecover(bytes32 hash, uint8 v, bytes32 r, bytes32 s) returns (address)``: recover address associated with the public key from elliptic curve signature, return zero on error - ``addmod(uint x, uint y, uint k) returns (uint)``: compute ``(x + y) % k`` where the addition is performed with arbitrary precision and does not wrap around at ``2**256`` - ``mulmod(uint x, uint y, uint k) returns (uint)``: compute ``(x * y) % k`` where the multiplication is performed with arbitrary precision and does not wrap around at ``2**256`` -- ``assert(bool condition)``: throws if the condition is false - ``this`` (current contract's type): the current contract, explicitly convertible to ``address`` - ``super``: the contract one level higher in the inheritance hierarchy - ``selfdestruct(address recipient)``: destroy the current contract, sending its funds to the given address diff --git a/docs/utils/SolidityLexer.py b/docs/utils/SolidityLexer.py index f5220c8b..1dc99159 100644 --- a/docs/utils/SolidityLexer.py +++ b/docs/utils/SolidityLexer.py @@ -54,7 +54,7 @@ class SolidityLexer(RegexLexer): r'(<<|>>>?|==?|!=?|[-<>+*%&\|\^/])=?', Operator, 'slashstartsregex'), (r'[{(\[;,]', Punctuation, 'slashstartsregex'), (r'[})\].]', Punctuation), - (r'(anonymous|as|assembly|break|constant|continue|do|else|external|hex|if|' + (r'(anonymous|as|assembly|break|constant|continue|do|delete|else|external|for|hex|if|' r'indexed|internal|import|is|mapping|memory|new|payable|public|pragma|' r'private|return|returns|storage|super|this|throw|using|while)\b', Keyword, 'slashstartsregex'), (r'(var|function|event|modifier|struct|enum|contract|library)\b', Keyword.Declaration, 'slashstartsregex'), diff --git a/libsolidity/analysis/GlobalContext.cpp b/libsolidity/analysis/GlobalContext.cpp index 4f100cd0..069d10f5 100644 --- a/libsolidity/analysis/GlobalContext.cpp +++ b/libsolidity/analysis/GlobalContext.cpp @@ -66,8 +66,9 @@ m_magicVariables(vector<shared_ptr<MagicVariableDeclaration const>>{make_shared< make_shared<FunctionType>(strings{"bytes32", "uint8", "bytes32", "bytes32"}, strings{"address"}, FunctionType::Location::ECRecover)), make_shared<MagicVariableDeclaration>("ripemd160", make_shared<FunctionType>(strings(), strings{"bytes20"}, FunctionType::Location::RIPEMD160, true)), - make_shared<MagicVariableDeclaration>("assert", - make_shared<FunctionType>(strings{"bool"}, strings{}, FunctionType::Location::Assert)), +// Disabled until decision about semantics of assert is made. +// make_shared<MagicVariableDeclaration>("assert", +// make_shared<FunctionType>(strings{"bool"}, strings{}, FunctionType::Location::Assert)), make_shared<MagicVariableDeclaration>("revert", make_shared<FunctionType>(strings(), strings(), FunctionType::Location::Revert))}) { diff --git a/libsolidity/analysis/PostTypeChecker.cpp b/libsolidity/analysis/PostTypeChecker.cpp new file mode 100644 index 00000000..cae77c74 --- /dev/null +++ b/libsolidity/analysis/PostTypeChecker.cpp @@ -0,0 +1,108 @@ +/* + 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/PostTypeChecker.h> +#include <libsolidity/ast/AST.h> +#include <libsolidity/analysis/SemVerHandler.h> +#include <libsolidity/interface/Version.h> + +#include <boost/range/adaptor/map.hpp> + +#include <memory> + +using namespace std; +using namespace dev; +using namespace dev::solidity; + + +bool PostTypeChecker::check(ASTNode const& _astRoot) +{ + _astRoot.accept(*this); + return Error::containsOnlyWarnings(m_errors); +} + +void PostTypeChecker::typeError(SourceLocation const& _location, std::string const& _description) +{ + auto err = make_shared<Error>(Error::Type::TypeError); + *err << + errinfo_sourceLocation(_location) << + errinfo_comment(_description); + + m_errors.push_back(err); +} + +bool PostTypeChecker::visit(ContractDefinition const&) +{ + solAssert(!m_currentConstVariable, ""); + solAssert(m_constVariableDependencies.empty(), ""); + return true; +} + +void PostTypeChecker::endVisit(ContractDefinition const&) +{ + solAssert(!m_currentConstVariable, ""); + for (auto declaration: m_constVariables) + if (auto identifier = findCycle(declaration)) + typeError(declaration->location(), "The value of the constant " + declaration->name() + " has a cyclic dependency via " + identifier->name() + "."); +} + +bool PostTypeChecker::visit(VariableDeclaration const& _variable) +{ + solAssert(!m_currentConstVariable, ""); + if (_variable.isConstant()) + { + m_currentConstVariable = &_variable; + m_constVariables.push_back(&_variable); + } + return true; +} + +void PostTypeChecker::endVisit(VariableDeclaration const& _variable) +{ + if (_variable.isConstant()) + { + solAssert(m_currentConstVariable == &_variable, ""); + m_currentConstVariable = nullptr; + } +} + +bool PostTypeChecker::visit(Identifier const& _identifier) +{ + if (m_currentConstVariable) + if (auto var = dynamic_cast<VariableDeclaration const*>(_identifier.annotation().referencedDeclaration)) + if (var->isConstant()) + m_constVariableDependencies[m_currentConstVariable].insert(var); + return true; +} + +VariableDeclaration const* PostTypeChecker::findCycle( + VariableDeclaration const* _startingFrom, + set<VariableDeclaration const*> const& _seen +) +{ + if (_seen.count(_startingFrom)) + return _startingFrom; + else if (m_constVariableDependencies.count(_startingFrom)) + { + set<VariableDeclaration const*> seen(_seen); + seen.insert(_startingFrom); + for (auto v: m_constVariableDependencies[_startingFrom]) + if (findCycle(v, seen)) + return v; + } + return nullptr; +} diff --git a/libsolidity/analysis/PostTypeChecker.h b/libsolidity/analysis/PostTypeChecker.h new file mode 100644 index 00000000..8774f413 --- /dev/null +++ b/libsolidity/analysis/PostTypeChecker.h @@ -0,0 +1,69 @@ +/* + 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/TypeChecker.h> +#include <libsolidity/ast/Types.h> +#include <libsolidity/ast/ASTAnnotations.h> +#include <libsolidity/ast/ASTForward.h> +#include <libsolidity/ast/ASTVisitor.h> + +namespace dev +{ +namespace solidity +{ + +/** + * This module performs analyses on the AST that are done after type checking and assignments of types: + * - whether there are circular references in constant state variables + * @TODO factor out each use-case into an individual class (but do the traversal only once) + */ +class PostTypeChecker: private ASTConstVisitor +{ +public: + /// @param _errors the reference to the list of errors and warnings to add them found during type checking. + PostTypeChecker(ErrorList& _errors): m_errors(_errors) {} + + bool check(ASTNode const& _astRoot); + +private: + /// Adds a new error to the list of errors. + void typeError(SourceLocation const& _location, std::string const& _description); + + virtual bool visit(ContractDefinition const& _contract) override; + virtual void endVisit(ContractDefinition const& _contract) override; + + virtual bool visit(VariableDeclaration const& _declaration) override; + virtual void endVisit(VariableDeclaration const& _declaration) override; + + virtual bool visit(Identifier const& _identifier) override; + + VariableDeclaration const* findCycle( + VariableDeclaration const* _startingFrom, + std::set<VariableDeclaration const*> const& _seen = {} + ); + + ErrorList& m_errors; + + VariableDeclaration const* m_currentConstVariable = nullptr; + std::vector<VariableDeclaration const*> m_constVariables; ///< Required for determinism. + std::map<VariableDeclaration const*, std::set<VariableDeclaration const*>> m_constVariableDependencies; +}; + +} +} diff --git a/libsolidity/analysis/StaticAnalyzer.h b/libsolidity/analysis/StaticAnalyzer.h index b6cf783e..0cb961bd 100644 --- a/libsolidity/analysis/StaticAnalyzer.h +++ b/libsolidity/analysis/StaticAnalyzer.h @@ -36,6 +36,9 @@ namespace solidity /** * The module that performs static analysis on the AST. + * In this context, static analysis is anything that can produce warnings which can help + * programmers write cleaner code. For every warning generated eher, it has to be possible to write + * equivalent code that does generate the warning. */ class StaticAnalyzer: private ASTConstVisitor { diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index ff55ef1f..93b183a2 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -592,8 +592,12 @@ bool TypeChecker::visit(InlineAssembly const& _inlineAssembly) // Inline assembly does not have its own type-checking phase, so we just run the // code-generator and see whether it produces any errors. // External references have already been resolved in a prior stage and stored in the annotation. - assembly::CodeGenerator codeGen(_inlineAssembly.operations(), m_errors); - codeGen.typeCheck([&](assembly::Identifier const& _identifier, eth::Assembly& _assembly, assembly::CodeGenerator::IdentifierContext _context) { + auto identifierAccess = [&]( + assembly::Identifier const& _identifier, + eth::Assembly& _assembly, + assembly::CodeGenerator::IdentifierContext _context + ) + { auto ref = _inlineAssembly.annotation().externalReferences.find(&_identifier); if (ref == _inlineAssembly.annotation().externalReferences.end()) return false; @@ -641,8 +645,11 @@ bool TypeChecker::visit(InlineAssembly const& _inlineAssembly) return false; } return true; - }); - return false; + }; + assembly::CodeGenerator codeGen(_inlineAssembly.operations(), m_errors); + if (!codeGen.typeCheck(identifierAccess)) + return false; + return true; } bool TypeChecker::visit(IfStatement const& _ifStatement) @@ -730,13 +737,16 @@ bool TypeChecker::visit(VariableDeclarationStatement const& _statement) if (auto ref = dynamic_cast<ReferenceType const*>(type(varDecl).get())) { if (ref->dataStoredIn(DataLocation::Storage)) - { warning( varDecl.location(), "Uninitialized storage pointer. Did you mean '<type> memory " + varDecl.name() + "'?" ); - } } + else if (dynamic_cast<MappingType const*>(type(varDecl).get())) + typeError( + varDecl.location(), + "Uninitialized mapping. Mappings cannot be created dynamically, you have to assign them from a state variable." + ); varDecl.accept(*this); return false; } @@ -1580,7 +1590,11 @@ void TypeChecker::endVisit(Literal const& _literal) return; } else - warning(_literal.location(), "This looks like an address but has an invalid checksum."); + warning( + _literal.location(), + "This looks like an address but has an invalid checksum. " + "If this is not used as an address, please prepend '00'." + ); } _literal.annotation().type = Type::forLiteral(_literal); if (!_literal.annotation().type) diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp index aac4c0c6..5192ffa6 100644 --- a/libsolidity/codegen/ExpressionCompiler.cpp +++ b/libsolidity/codegen/ExpressionCompiler.cpp @@ -885,9 +885,8 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) // jump if condition was met m_context << Instruction::ISZERO << Instruction::ISZERO; auto success = m_context.appendConditionalJump(); - // condition was not met, abort - m_context << u256(0) << u256(0); - m_context << Instruction::REVERT; + // condition was not met, flag an error + m_context << Instruction::INVALID; // the success branch m_context << success; break; diff --git a/libsolidity/inlineasm/AsmAnalysis.cpp b/libsolidity/inlineasm/AsmAnalysis.cpp new file mode 100644 index 00000000..a3ddb61d --- /dev/null +++ b/libsolidity/inlineasm/AsmAnalysis.cpp @@ -0,0 +1,180 @@ +/* + 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/>. +*/ +/** + * Analyzer part of inline assembly. + */ + +#include <libsolidity/inlineasm/AsmAnalysis.h> + +#include <libsolidity/inlineasm/AsmData.h> + +#include <libsolidity/interface/Exceptions.h> +#include <libsolidity/interface/Utils.h> + +#include <boost/range/adaptor/reversed.hpp> + +#include <memory> +#include <functional> + +using namespace std; +using namespace dev; +using namespace dev::solidity; +using namespace dev::solidity::assembly; + + +bool Scope::registerLabel(string const& _name) +{ + if (exists(_name)) + return false; + identifiers[_name] = Label(); + return true; +} + +bool Scope::registerVariable(string const& _name) +{ + if (exists(_name)) + return false; + identifiers[_name] = Variable(); + return true; +} + +bool Scope::registerFunction(string const& _name, size_t _arguments, size_t _returns) +{ + if (exists(_name)) + return false; + identifiers[_name] = Function(_arguments, _returns); + return true; +} + +Scope::Identifier* Scope::lookup(string const& _name) +{ + if (identifiers.count(_name)) + return &identifiers[_name]; + else if (superScope && !closedScope) + return superScope->lookup(_name); + else + return nullptr; +} + +bool Scope::exists(string const& _name) +{ + if (identifiers.count(_name)) + return true; + else if (superScope) + return superScope->exists(_name); + else + return false; +} + +AsmAnalyzer::AsmAnalyzer(AsmAnalyzer::Scopes& _scopes, ErrorList& _errors): + m_scopes(_scopes), m_errors(_errors) +{ + // Make the Solidity ErrorTag available to inline assembly + m_scopes[nullptr] = make_shared<Scope>(); + Scope::Label errorLabel; + errorLabel.id = Scope::Label::errorLabelId; + m_scopes[nullptr]->identifiers["invalidJumpLabel"] = errorLabel; + m_currentScope = m_scopes[nullptr].get(); +} + +bool AsmAnalyzer::operator()(assembly::Literal const& _literal) +{ + if (!_literal.isNumber && _literal.value.size() > 32) + { + m_errors.push_back(make_shared<Error>( + Error::Type::TypeError, + "String literal too long (" + boost::lexical_cast<std::string>(_literal.value.size()) + " > 32)" + )); + return false; + } + return true; +} + +bool AsmAnalyzer::operator()(FunctionalInstruction const& _instr) +{ + bool success = true; + for (auto const& arg: _instr.arguments | boost::adaptors::reversed) + if (!boost::apply_visitor(*this, arg)) + success = false; + if (!(*this)(_instr.instruction)) + success = false; + return success; +} + +bool AsmAnalyzer::operator()(Label const& _item) +{ + if (!m_currentScope->registerLabel(_item.name)) + { + //@TODO secondary location + m_errors.push_back(make_shared<Error>( + Error::Type::DeclarationError, + "Label name " + _item.name + " already taken in this scope.", + _item.location + )); + return false; + } + return true; +} + +bool AsmAnalyzer::operator()(FunctionalAssignment const& _assignment) +{ + return boost::apply_visitor(*this, *_assignment.value); +} + +bool AsmAnalyzer::operator()(assembly::VariableDeclaration const& _varDecl) +{ + bool success = boost::apply_visitor(*this, *_varDecl.value); + if (!m_currentScope->registerVariable(_varDecl.name)) + { + //@TODO secondary location + m_errors.push_back(make_shared<Error>( + Error::Type::DeclarationError, + "Variable name " + _varDecl.name + " already taken in this scope.", + _varDecl.location + )); + success = false; + } + return success; +} + +bool AsmAnalyzer::operator()(assembly::FunctionDefinition const&) +{ + // TODO - we cannot throw an exception here because of some tests. + return true; +} + +bool AsmAnalyzer::operator()(assembly::FunctionCall const&) +{ + // TODO - we cannot throw an exception here because of some tests. + return true; +} + +bool AsmAnalyzer::operator()(Block const& _block) +{ + bool success = true; + auto scope = make_shared<Scope>(); + scope->superScope = m_currentScope; + m_scopes[&_block] = scope; + m_currentScope = scope.get(); + + for (auto const& s: _block.statements) + if (!boost::apply_visitor(*this, s)) + success = false; + + m_currentScope = m_currentScope->superScope; + return success; +} diff --git a/libsolidity/inlineasm/AsmAnalysis.h b/libsolidity/inlineasm/AsmAnalysis.h new file mode 100644 index 00000000..9726210d --- /dev/null +++ b/libsolidity/inlineasm/AsmAnalysis.h @@ -0,0 +1,158 @@ +/* + 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/>. +*/ +/** + * Analysis part of inline assembly. + */ + +#pragma once + +#include <libsolidity/interface/Exceptions.h> + +#include <boost/variant.hpp> + +#include <functional> +#include <memory> + +namespace dev +{ +namespace solidity +{ +namespace assembly +{ + +struct Literal; +struct Block; +struct Label; +struct FunctionalInstruction; +struct FunctionalAssignment; +struct VariableDeclaration; +struct Instruction; +struct Identifier; +struct Assignment; +struct FunctionDefinition; +struct FunctionCall; + +template <class...> +struct GenericVisitor{}; + +template <class Visitable, class... Others> +struct GenericVisitor<Visitable, Others...>: public GenericVisitor<Others...> +{ + using GenericVisitor<Others...>::operator (); + explicit GenericVisitor( + std::function<void(Visitable&)> _visitor, + std::function<void(Others&)>... _otherVisitors + ): + GenericVisitor<Others...>(_otherVisitors...), + m_visitor(_visitor) + {} + + void operator()(Visitable& _v) const { m_visitor(_v); } + + std::function<void(Visitable&)> m_visitor; +}; +template <> +struct GenericVisitor<>: public boost::static_visitor<> { + void operator()() const {} +}; + + +struct Scope +{ + struct Variable + { + int stackHeight = 0; + bool active = false; + }; + + struct Label + { + size_t id = unassignedLabelId; + static const size_t errorLabelId = -1; + static const size_t unassignedLabelId = 0; + }; + + struct Function + { + Function(size_t _arguments, size_t _returns): arguments(_arguments), returns(_returns) {} + size_t arguments = 0; + size_t returns = 0; + }; + + using Identifier = boost::variant<Variable, Label, Function>; + using Visitor = GenericVisitor<Variable const, Label const, Function const>; + using NonconstVisitor = GenericVisitor<Variable, Label, Function>; + + bool registerVariable(std::string const& _name); + bool registerLabel(std::string const& _name); + bool registerFunction(std::string const& _name, size_t _arguments, size_t _returns); + + /// Looks up the identifier in this or super scopes (stops and function and assembly boundaries) + /// and returns a valid pointer if found or a nullptr if not found. + /// The pointer will be invalidated if the scope is modified. + Identifier* lookup(std::string const& _name); + /// Looks up the identifier in this and super scopes (stops and function and assembly boundaries) + /// and calls the visitor, returns false if not found. + template <class V> + bool lookup(std::string const& _name, V const& _visitor) + { + if (Identifier* id = lookup(_name)) + { + boost::apply_visitor(_visitor, *id); + return true; + } + else + return false; + } + /// @returns true if the name exists in this scope or in super scopes (also searches + /// across function and assembly boundaries). + bool exists(std::string const& _name); + Scope* superScope = nullptr; + /// If true, identifiers from the super scope are not visible here, but they are still + /// taken into account to prevent shadowing. + bool closedScope = false; + std::map<std::string, Identifier> identifiers; +}; + + +class AsmAnalyzer: public boost::static_visitor<bool> +{ +public: + using Scopes = std::map<assembly::Block const*, std::shared_ptr<Scope>>; + AsmAnalyzer(Scopes& _scopes, ErrorList& _errors); + + bool operator()(assembly::Instruction const&) { return true; } + bool operator()(assembly::Literal const& _literal); + bool operator()(assembly::Identifier const&) { return true; } + bool operator()(assembly::FunctionalInstruction const& _functionalInstruction); + bool operator()(assembly::Label const& _label); + bool operator()(assembly::Assignment const&) { return true; } + bool operator()(assembly::FunctionalAssignment const& _functionalAssignment); + bool operator()(assembly::VariableDeclaration const& _variableDeclaration); + bool operator()(assembly::FunctionDefinition const& _functionDefinition); + bool operator()(assembly::FunctionCall const& _functionCall); + bool operator()(assembly::Block const& _block); + +private: + Scope* m_currentScope = nullptr; + Scopes& m_scopes; + ErrorList& m_errors; +}; + +} +} +} diff --git a/libsolidity/inlineasm/AsmCodeGen.cpp b/libsolidity/inlineasm/AsmCodeGen.cpp index faa7dabd..78a9ee27 100644 --- a/libsolidity/inlineasm/AsmCodeGen.cpp +++ b/libsolidity/inlineasm/AsmCodeGen.cpp @@ -21,14 +21,23 @@ */ #include <libsolidity/inlineasm/AsmCodeGen.h> -#include <memory> -#include <functional> -#include <libdevcore/CommonIO.h> + +#include <libsolidity/inlineasm/AsmParser.h> +#include <libsolidity/inlineasm/AsmData.h> +#include <libsolidity/inlineasm/AsmAnalysis.h> + #include <libevmasm/Assembly.h> #include <libevmasm/SourceLocation.h> #include <libevmasm/Instruction.h> -#include <libsolidity/inlineasm/AsmParser.h> -#include <libsolidity/inlineasm/AsmData.h> + +#include <libdevcore/CommonIO.h> + +#include <boost/range/adaptor/reversed.hpp> +#include <boost/range/adaptor/map.hpp> +#include <boost/range/algorithm/count_if.hpp> + +#include <memory> +#include <functional> using namespace std; using namespace dev; @@ -42,73 +51,26 @@ struct GeneratorState void addError(Error::Type _type, std::string const& _description, SourceLocation const& _location = SourceLocation()) { - auto err = make_shared<Error>(_type); - if (!_location.isEmpty()) - *err << errinfo_sourceLocation(_location); - *err << errinfo_comment(_description); - errors.push_back(err); + errors.push_back(make_shared<Error>(_type, _description, _location)); } - int const* findVariable(string const& _variableName) const + size_t newLabelId() { - auto localVariable = find_if( - variables.rbegin(), - variables.rend(), - [&](pair<string, int> const& _var) { return _var.first == _variableName; } - ); - return localVariable != variables.rend() ? &localVariable->second : nullptr; + return assemblyTagToIdentifier(assembly.newTag()); } - eth::AssemblyItem const* findLabel(string const& _labelName) const + + size_t assemblyTagToIdentifier(eth::AssemblyItem const& _tag) const { - auto label = find_if( - labels.begin(), - labels.end(), - [&](pair<string, eth::AssemblyItem> const& _label) { return _label.first == _labelName; } - ); - return label != labels.end() ? &label->second : nullptr; + u256 id = _tag.data(); + solAssert(id <= std::numeric_limits<size_t>::max(), "Tag id too large."); + return size_t(id); } - map<string, eth::AssemblyItem> labels; - vector<pair<string, int>> variables; ///< name plus stack height + std::map<assembly::Block const*, shared_ptr<Scope>> scopes; ErrorList& errors; eth::Assembly& assembly; }; -/** - * Scans the inline assembly data for labels, creates tags in the assembly and searches for - * duplicate labels. - */ -class LabelOrganizer: public boost::static_visitor<> -{ -public: - LabelOrganizer(GeneratorState& _state): m_state(_state) - { - // Make the Solidity ErrorTag available to inline assembly - m_state.labels.insert(make_pair("invalidJumpLabel", m_state.assembly.errorTag())); - } - - template <class T> - void operator()(T const& /*_item*/) { } - void operator()(Label const& _item) - { - if (m_state.labels.count(_item.name)) - //@TODO secondary location - m_state.addError( - Error::Type::DeclarationError, - "Label " + _item.name + " declared twice.", - _item.location - ); - m_state.labels.insert(make_pair(_item.name, m_state.assembly.newTag())); - } - void operator()(assembly::Block const& _block) - { - std::for_each(_block.statements.begin(), _block.statements.end(), boost::apply_visitor(*this)); - } - -private: - GeneratorState& m_state; -}; - class CodeTransform: public boost::static_visitor<> { public: @@ -117,14 +79,42 @@ public: /// @param _identifierAccess used to resolve identifiers external to the inline assembly explicit CodeTransform( GeneratorState& _state, + assembly::Block const& _block, assembly::CodeGenerator::IdentifierAccess const& _identifierAccess = assembly::CodeGenerator::IdentifierAccess() ): - m_state(_state) + m_state(_state), + m_scope(*m_state.scopes.at(&_block)), + m_initialDeposit(m_state.assembly.deposit()), + m_identifierAccess(_identifierAccess) { - if (_identifierAccess) - m_identifierAccess = _identifierAccess; - else - m_identifierAccess = [](assembly::Identifier const&, eth::Assembly&, CodeGenerator::IdentifierContext) { return false; }; + std::for_each(_block.statements.begin(), _block.statements.end(), boost::apply_visitor(*this)); + + m_state.assembly.setSourceLocation(_block.location); + + // pop variables + for (auto const& identifier: m_scope.identifiers) + if (identifier.second.type() == typeid(Scope::Variable)) + m_state.assembly.append(solidity::Instruction::POP); + + int deposit = m_state.assembly.deposit() - m_initialDeposit; + + // issue warnings for stack height discrepancies + if (deposit < 0) + { + m_state.addError( + Error::Type::Warning, + "Inline assembly block is not balanced. It takes " + toString(-deposit) + " item(s) from the stack.", + _block.location + ); + } + else if (deposit > 0) + { + m_state.addError( + Error::Type::Warning, + "Inline assembly block is not balanced. It leaves " + toString(deposit) + " item(s) on the stack.", + _block.location + ); + } } void operator()(assembly::Instruction const& _instruction) @@ -137,40 +127,38 @@ public: m_state.assembly.setSourceLocation(_literal.location); if (_literal.isNumber) m_state.assembly.append(u256(_literal.value)); - else if (_literal.value.size() > 32) - { - m_state.addError( - Error::Type::TypeError, - "String literal too long (" + boost::lexical_cast<string>(_literal.value.size()) + " > 32)" - ); - m_state.assembly.append(u256(0)); - } else + { + solAssert(_literal.value.size() <= 32, ""); m_state.assembly.append(_literal.value); + } } void operator()(assembly::Identifier const& _identifier) { m_state.assembly.setSourceLocation(_identifier.location); - // First search local variables, then labels, then externals. - if (int const* stackHeight = m_state.findVariable(_identifier.name)) - { - int heightDiff = m_state.assembly.deposit() - *stackHeight; - if (heightDiff <= 0 || heightDiff > 16) + // First search internals, then externals. + if (m_scope.lookup(_identifier.name, Scope::NonconstVisitor( + [=](Scope::Variable& _var) { - m_state.addError( - Error::Type::TypeError, - "Variable inaccessible, too deep inside stack (" + boost::lexical_cast<string>(heightDiff) + ")", - _identifier.location - ); - m_state.assembly.append(u256(0)); + if (int heightDiff = variableHeightDiff(_var, _identifier.location, false)) + m_state.assembly.append(solidity::dupInstruction(heightDiff)); + else + // Store something to balance the stack + m_state.assembly.append(u256(0)); + }, + [=](Scope::Label& _label) + { + assignLabelIdIfUnset(_label); + m_state.assembly.append(eth::AssemblyItem(eth::PushTag, _label.id)); + }, + [=](Scope::Function&) + { + solAssert(false, "Not yet implemented"); } - else - m_state.assembly.append(solidity::dupInstruction(heightDiff)); - return; + ))) + { } - else if (eth::AssemblyItem const* label = m_state.findLabel(_identifier.name)) - m_state.assembly.append(label->pushTag()); - else if (!m_identifierAccess(_identifier, m_state.assembly, CodeGenerator::IdentifierContext::RValue)) + else if (!m_identifierAccess || !m_identifierAccess(_identifier, m_state.assembly, CodeGenerator::IdentifierContext::RValue)) { m_state.addError( Error::Type::DeclarationError, @@ -197,7 +185,10 @@ public: void operator()(Label const& _label) { m_state.assembly.setSourceLocation(_label.location); - m_state.assembly.append(m_state.labels.at(_label.name)); + solAssert(m_scope.identifiers.count(_label.name), ""); + Scope::Label& label = boost::get<Scope::Label>(m_scope.identifiers[_label.name]); + assignLabelIdIfUnset(label); + m_state.assembly.append(eth::AssemblyItem(eth::Tag, label.id)); } void operator()(assembly::Assignment const& _assignment) { @@ -217,42 +208,14 @@ public: int height = m_state.assembly.deposit(); boost::apply_visitor(*this, *_varDecl.value); expectDeposit(1, height, locationOf(*_varDecl.value)); - m_state.variables.push_back(make_pair(_varDecl.name, height)); + solAssert(m_scope.identifiers.count(_varDecl.name), ""); + auto& var = boost::get<Scope::Variable>(m_scope.identifiers[_varDecl.name]); + var.stackHeight = height; + var.active = true; } void operator()(assembly::Block const& _block) { - size_t numVariables = m_state.variables.size(); - int deposit = m_state.assembly.deposit(); - std::for_each(_block.statements.begin(), _block.statements.end(), boost::apply_visitor(*this)); - - // pop variables - while (m_state.variables.size() > numVariables) - { - m_state.assembly.append(solidity::Instruction::POP); - m_state.variables.pop_back(); - } - - m_state.assembly.setSourceLocation(_block.location); - - deposit = m_state.assembly.deposit() - deposit; - - // issue warnings for stack height discrepancies - if (deposit < 0) - { - m_state.addError( - Error::Type::Warning, - "Inline assembly block is not balanced. It takes " + toString(-deposit) + " item(s) from the stack.", - _block.location - ); - } - else if (deposit > 0) - { - m_state.addError( - Error::Type::Warning, - "Inline assembly block is not balanced. It leaves " + toString(deposit) + " item(s) on the stack.", - _block.location - ); - } + CodeTransform(m_state, _block, m_identifierAccess); } void operator()(assembly::FunctionDefinition const&) { @@ -262,27 +225,61 @@ public: private: void generateAssignment(assembly::Identifier const& _variableName, SourceLocation const& _location) { - if (int const* stackHeight = m_state.findVariable(_variableName.name)) - { - int heightDiff = m_state.assembly.deposit() - *stackHeight - 1; - if (heightDiff <= 0 || heightDiff > 16) + if (m_scope.lookup(_variableName.name, Scope::Visitor( + [=](Scope::Variable const& _var) + { + if (int heightDiff = variableHeightDiff(_var, _location, true)) + m_state.assembly.append(solidity::swapInstruction(heightDiff - 1)); + m_state.assembly.append(solidity::Instruction::POP); + }, + [=](Scope::Label const&) + { + m_state.addError( + Error::Type::DeclarationError, + "Label \"" + string(_variableName.name) + "\" used as variable." + ); + }, + [=](Scope::Function const&) + { m_state.addError( - Error::Type::TypeError, - "Variable inaccessible, too deep inside stack (" + boost::lexical_cast<string>(heightDiff) + ")", - _location + Error::Type::DeclarationError, + "Function \"" + string(_variableName.name) + "\" used as variable." ); - else - m_state.assembly.append(solidity::swapInstruction(heightDiff)); - m_state.assembly.append(solidity::Instruction::POP); - return; + } + ))) + { } - else if (!m_identifierAccess(_variableName, m_state.assembly, CodeGenerator::IdentifierContext::LValue)) + else if (!m_identifierAccess || !m_identifierAccess(_variableName, m_state.assembly, CodeGenerator::IdentifierContext::LValue)) m_state.addError( Error::Type::DeclarationError, "Identifier \"" + string(_variableName.name) + "\" not found, not unique or not lvalue." ); } + /// Determines the stack height difference to the given variables. Automatically generates + /// errors if it is not yet in scope or the height difference is too large. Returns 0 on + /// errors and the (positive) stack height difference otherwise. + int variableHeightDiff(Scope::Variable const& _var, SourceLocation const& _location, bool _forSwap) + { + if (!_var.active) + { + m_state.addError( Error::Type::TypeError, "Variable used before it was declared", _location); + return 0; + } + int heightDiff = m_state.assembly.deposit() - _var.stackHeight; + if (heightDiff <= (_forSwap ? 1 : 0) || heightDiff > (_forSwap ? 17 : 16)) + { + m_state.addError( + Error::Type::TypeError, + "Variable inaccessible, too deep inside stack (" + boost::lexical_cast<string>(heightDiff) + ")", + _location + ); + return 0; + } + else + return heightDiff; + } + void expectDeposit(int _deposit, int _oldHeight, SourceLocation const& _location) { if (m_state.assembly.deposit() != _oldHeight + 1) @@ -296,7 +293,19 @@ private: ); } + /// Assigns the label's id to a value taken from eth::Assembly if it has not yet been set. + void assignLabelIdIfUnset(Scope::Label& _label) + { + if (_label.id == Scope::Label::unassignedLabelId) + _label.id = m_state.newLabelId(); + else if (_label.id == Scope::Label::errorLabelId) + _label.id = size_t(m_state.assembly.errorTag().data()); + } + + GeneratorState& m_state; + Scope& m_scope; + int const m_initialDeposit; assembly::CodeGenerator::IdentifierAccess m_identifierAccess; }; @@ -305,8 +314,9 @@ bool assembly::CodeGenerator::typeCheck(assembly::CodeGenerator::IdentifierAcces size_t initialErrorLen = m_errors.size(); eth::Assembly assembly; GeneratorState state(m_errors, assembly); - (LabelOrganizer(state))(m_parsedData); - (CodeTransform(state, _identifierAccess))(m_parsedData); + if (!(AsmAnalyzer(state.scopes, m_errors))(m_parsedData)) + return false; + CodeTransform(state, m_parsedData, _identifierAccess); return m_errors.size() == initialErrorLen; } @@ -314,15 +324,16 @@ eth::Assembly assembly::CodeGenerator::assemble(assembly::CodeGenerator::Identif { eth::Assembly assembly; GeneratorState state(m_errors, assembly); - (LabelOrganizer(state))(m_parsedData); - (CodeTransform(state, _identifierAccess))(m_parsedData); + if (!(AsmAnalyzer(state.scopes, m_errors))(m_parsedData)) + solAssert(false, "Assembly error"); + CodeTransform(state, m_parsedData, _identifierAccess); return assembly; } void assembly::CodeGenerator::assemble(eth::Assembly& _assembly, assembly::CodeGenerator::IdentifierAccess const& _identifierAccess) { GeneratorState state(m_errors, _assembly); - (LabelOrganizer(state))(m_parsedData); - (CodeTransform(state, _identifierAccess))(m_parsedData); + if (!(AsmAnalyzer(state.scopes, m_errors))(m_parsedData)) + solAssert(false, "Assembly error"); + CodeTransform(state, m_parsedData, _identifierAccess); } - diff --git a/libsolidity/inlineasm/AsmStack.cpp b/libsolidity/inlineasm/AsmStack.cpp index 38d688c1..266136a1 100644 --- a/libsolidity/inlineasm/AsmStack.cpp +++ b/libsolidity/inlineasm/AsmStack.cpp @@ -25,6 +25,7 @@ #include <libsolidity/inlineasm/AsmParser.h> #include <libsolidity/inlineasm/AsmCodeGen.h> #include <libsolidity/inlineasm/AsmPrinter.h> +#include <libsolidity/inlineasm/AsmAnalysis.h> #include <libsolidity/parsing/Scanner.h> @@ -45,8 +46,10 @@ bool InlineAssemblyStack::parse(shared_ptr<Scanner> const& _scanner) auto result = parser.parse(_scanner); if (!result) return false; + *m_parserResult = std::move(*result); - return true; + AsmAnalyzer::Scopes scopes; + return (AsmAnalyzer(scopes, m_errors))(*m_parserResult); } string InlineAssemblyStack::toString() diff --git a/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp index 9d8d872f..594efb1e 100644 --- a/libsolidity/interface/CompilerStack.cpp +++ b/libsolidity/interface/CompilerStack.cpp @@ -34,6 +34,7 @@ #include <libsolidity/analysis/TypeChecker.h> #include <libsolidity/analysis/DocStringAnalyser.h> #include <libsolidity/analysis/StaticAnalyzer.h> +#include <libsolidity/analysis/PostTypeChecker.h> #include <libsolidity/analysis/SyntaxChecker.h> #include <libsolidity/codegen/Compiler.h> #include <libsolidity/interface/InterfaceHandler.h> @@ -219,6 +220,14 @@ bool CompilerStack::parse() if (noErrors) { + PostTypeChecker postTypeChecker(m_errors); + for (Source const* source: m_sourceOrder) + if (!postTypeChecker.check(*source->ast)) + noErrors = false; + } + + if (noErrors) + { StaticAnalyzer staticAnalyzer(m_errors); for (Source const* source: m_sourceOrder) if (!staticAnalyzer.analyze(*source->ast)) diff --git a/libsolidity/interface/Exceptions.cpp b/libsolidity/interface/Exceptions.cpp index 41890b91..968a24ad 100644 --- a/libsolidity/interface/Exceptions.cpp +++ b/libsolidity/interface/Exceptions.cpp @@ -27,7 +27,8 @@ using namespace std; using namespace dev; using namespace dev::solidity; -Error::Error(Type _type): m_type(_type) +Error::Error(Type _type, SourceLocation const& _location, string const& _description): + m_type(_type) { switch(m_type) { @@ -56,6 +57,19 @@ Error::Error(Type _type): m_type(_type) solAssert(false, ""); break; } + + if (!_location.isEmpty()) + *this << errinfo_sourceLocation(_location); + if (!_description.empty()) + *this << errinfo_comment(_description); +} + +Error::Error(Error::Type _type, const std::string& _description, const SourceLocation& _location): + Error(_type) +{ + if (!_location.isEmpty()) + *this << errinfo_sourceLocation(_location); + *this << errinfo_comment(_description); } string Exception::lineInfo() const diff --git a/libsolidity/interface/Exceptions.h b/libsolidity/interface/Exceptions.h index 81716c41..0803d8cc 100644 --- a/libsolidity/interface/Exceptions.h +++ b/libsolidity/interface/Exceptions.h @@ -53,7 +53,13 @@ public: Warning }; - explicit Error(Type _type); + explicit Error( + Type _type, + SourceLocation const& _location = SourceLocation(), + std::string const& _description = std::string() + ); + + Error(Type _type, std::string const& _description, SourceLocation const& _location = SourceLocation()); Type type() const { return m_type; } std::string const& typeName() const { return m_typeName; } diff --git a/test/RPCSession.cpp b/test/RPCSession.cpp index be8774bc..f8b364d1 100644 --- a/test/RPCSession.cpp +++ b/test/RPCSession.cpp @@ -207,7 +207,7 @@ void RPCSession::personal_unlockAccount(string const& _address, string const& _p string RPCSession::personal_newAccount(string const& _password) { string addr = rpcCall("personal_newAccount", { quote(_password) }).asString(); - BOOST_MESSAGE("Created account " + addr); + BOOST_TEST_MESSAGE("Created account " + addr); return addr; } diff --git a/test/RPCSession.h b/test/RPCSession.h index 843036e1..b37cc322 100644 --- a/test/RPCSession.h +++ b/test/RPCSession.h @@ -30,6 +30,7 @@ #include <json/value.h> +#include <boost/noncopyable.hpp> #include <boost/test/unit_test.hpp> #include <string> diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index cb0cc168..130b0d3a 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -9119,24 +9119,24 @@ BOOST_AUTO_TEST_CASE(invalid_instruction) BOOST_CHECK(callContractFunction("f()") == encodeArgs()); } -BOOST_AUTO_TEST_CASE(assert) -{ - char const* sourceCode = R"( - contract C { - function f() { - assert(false); - } - function g(bool val) returns (bool) { - assert(val == true); - return true; - } - } - )"; - compileAndRun(sourceCode, 0, "C"); - BOOST_CHECK(callContractFunction("f()") == encodeArgs()); - BOOST_CHECK(callContractFunction("g(bool)", false) == encodeArgs()); - BOOST_CHECK(callContractFunction("g(bool)", true) == encodeArgs(true)); -} +//BOOST_AUTO_TEST_CASE(assert) +//{ +// char const* sourceCode = R"( +// contract C { +// function f() { +// assert(false); +// } +// function g(bool val) returns (bool) { +// assert(val == true); +// return true; +// } +// } +// )"; +// compileAndRun(sourceCode, 0, "C"); +// BOOST_CHECK(callContractFunction("f()") == encodeArgs()); +// BOOST_CHECK(callContractFunction("g(bool)", false) == encodeArgs()); +// BOOST_CHECK(callContractFunction("g(bool)", true) == encodeArgs(true)); +//} BOOST_AUTO_TEST_CASE(revert) { diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 3b137572..3d82fc70 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -20,19 +20,23 @@ * Unit tests for the name and type resolution of the solidity parser. */ -#include <string> +#include <test/libsolidity/ErrorCheck.h> + +#include <test/TestHelper.h> -#include <libdevcore/SHA3.h> #include <libsolidity/parsing/Scanner.h> #include <libsolidity/parsing/Parser.h> #include <libsolidity/analysis/NameAndTypeResolver.h> #include <libsolidity/analysis/StaticAnalyzer.h> +#include <libsolidity/analysis/PostTypeChecker.h> #include <libsolidity/analysis/SyntaxChecker.h> #include <libsolidity/interface/Exceptions.h> #include <libsolidity/analysis/GlobalContext.h> #include <libsolidity/analysis/TypeChecker.h> -#include "../TestHelper.h" -#include "ErrorCheck.h" + +#include <libdevcore/SHA3.h> + +#include <string> using namespace std; @@ -93,10 +97,11 @@ parseAnalyseAndReturnError(string const& _source, bool _reportWarnings = false, BOOST_CHECK(success || !errors.empty()); } if (success) - { - StaticAnalyzer staticAnalyzer(errors); - staticAnalyzer.analyze(*sourceUnit); - } + if (!PostTypeChecker(errors).check(*sourceUnit)) + success = false; + if (success) + if (!StaticAnalyzer(errors).analyze(*sourceUnit)) + success = false; if (errors.size() > 1 && !_allowMultipleErrors) BOOST_FAIL("Multiple errors found"); for (auto const& currentError: errors) @@ -2501,6 +2506,30 @@ BOOST_AUTO_TEST_CASE(storage_assign_to_different_local_variable) CHECK_ERROR(sourceCode, TypeError, ""); } +BOOST_AUTO_TEST_CASE(uninitialized_mapping_variable) +{ + char const* sourceCode = R"( + contract C { + function f() { + mapping(uint => uint) x; + } + } + )"; + CHECK_ERROR(sourceCode, TypeError, "Uninitialized mapping. Mappings cannot be created dynamically, you have to assign them from a state variable"); +} + +BOOST_AUTO_TEST_CASE(uninitialized_mapping_array_variable) +{ + char const* sourceCode = R"( + contract C { + function f() { + mapping(uint => uint)[] x; + } + } + )"; + CHECK_WARNING(sourceCode, "Uninitialized storage pointer"); +} + BOOST_AUTO_TEST_CASE(no_delete_on_storage_pointers) { char const* sourceCode = R"( @@ -5156,6 +5185,34 @@ BOOST_AUTO_TEST_CASE(address_methods) CHECK_SUCCESS(text); } +BOOST_AUTO_TEST_CASE(cyclic_dependency_for_constants) +{ + char const* text = R"( + contract C { + uint constant a = a; + } + )"; + CHECK_ERROR(text, TypeError, "cyclic dependency via a"); + text = R"( + contract C { + uint constant a = b * c; + uint constant b = 7; + uint constant c = b + uint(sha3(d)); + uint constant d = 2 + a; + } + )"; + CHECK_ERROR_ALLOW_MULTI(text, TypeError, "a has a cyclic dependency via c"); + text = R"( + contract C { + uint constant a = b * c; + uint constant b = 7; + uint constant c = 4 + uint(sha3(d)); + uint constant d = 2 + b; + } + )"; + CHECK_SUCCESS(text); +} + BOOST_AUTO_TEST_SUITE_END() } |