diff options
Diffstat (limited to 'libsolidity/analysis')
-rw-r--r-- | libsolidity/analysis/ReferencesResolver.cpp | 8 | ||||
-rw-r--r-- | libsolidity/analysis/StaticAnalyzer.cpp | 10 | ||||
-rw-r--r-- | libsolidity/analysis/StaticAnalyzer.h | 3 | ||||
-rw-r--r-- | libsolidity/analysis/SyntaxChecker.cpp | 21 | ||||
-rw-r--r-- | libsolidity/analysis/SyntaxChecker.h | 6 | ||||
-rw-r--r-- | libsolidity/analysis/TypeChecker.cpp | 44 | ||||
-rw-r--r-- | libsolidity/analysis/TypeChecker.h | 6 | ||||
-rw-r--r-- | libsolidity/analysis/ViewPureChecker.cpp | 127 | ||||
-rw-r--r-- | libsolidity/analysis/ViewPureChecker.h | 25 |
9 files changed, 155 insertions, 95 deletions
diff --git a/libsolidity/analysis/ReferencesResolver.cpp b/libsolidity/analysis/ReferencesResolver.cpp index f33de7b7..8750b47b 100644 --- a/libsolidity/analysis/ReferencesResolver.cpp +++ b/libsolidity/analysis/ReferencesResolver.cpp @@ -332,7 +332,7 @@ void ReferencesResolver::endVisit(VariableDeclaration const& _variable) case Location::Memory: return "\"memory\""; case Location::Storage: return "\"storage\""; case Location::CallData: return "\"calldata\""; - case Location::Default: return "none"; + case Location::Unspecified: return "none"; } return {}; }; @@ -368,12 +368,12 @@ void ReferencesResolver::endVisit(VariableDeclaration const& _variable) // Find correct data location. if (_variable.isEventParameter()) { - solAssert(varLoc == Location::Default, ""); + solAssert(varLoc == Location::Unspecified, ""); typeLoc = DataLocation::Memory; } else if (_variable.isStateVariable()) { - solAssert(varLoc == Location::Default, ""); + solAssert(varLoc == Location::Unspecified, ""); typeLoc = _variable.isConstant() ? DataLocation::Memory : DataLocation::Storage; } else if ( @@ -394,7 +394,7 @@ void ReferencesResolver::endVisit(VariableDeclaration const& _variable) case Location::CallData: typeLoc = DataLocation::CallData; break; - case Location::Default: + case Location::Unspecified: solAssert(!_variable.hasReferenceOrMappingType(), "Data location not properly set."); } diff --git a/libsolidity/analysis/StaticAnalyzer.cpp b/libsolidity/analysis/StaticAnalyzer.cpp index 60a58665..487a5cca 100644 --- a/libsolidity/analysis/StaticAnalyzer.cpp +++ b/libsolidity/analysis/StaticAnalyzer.cpp @@ -56,7 +56,6 @@ bool StaticAnalyzer::visit(FunctionDefinition const& _function) else solAssert(!m_currentFunction, ""); solAssert(m_localVarUseCount.empty(), ""); - m_nonPayablePublic = _function.isPublic() && !_function.isPayable(); m_constructor = _function.isConstructor(); return true; } @@ -64,7 +63,6 @@ bool StaticAnalyzer::visit(FunctionDefinition const& _function) void StaticAnalyzer::endVisit(FunctionDefinition const&) { m_currentFunction = nullptr; - m_nonPayablePublic = false; m_constructor = false; for (auto const& var: m_localVarUseCount) if (var.second == 0) @@ -154,14 +152,6 @@ bool StaticAnalyzer::visit(MemberAccess const& _memberAccess) ); } - if (m_nonPayablePublic && !m_library) - if (MagicType const* type = dynamic_cast<MagicType const*>(_memberAccess.expression().annotation().type.get())) - if (type->kind() == MagicType::Kind::Message && _memberAccess.memberName() == "value") - m_errorReporter.warning( - _memberAccess.location(), - "\"msg.value\" used in non-payable function. Do you want to add the \"payable\" modifier to this function?" - ); - if (_memberAccess.memberName() == "callcode") if (auto const* type = dynamic_cast<FunctionType const*>(_memberAccess.annotation().type.get())) if (type->kind() == FunctionType::Kind::BareCallCode) diff --git a/libsolidity/analysis/StaticAnalyzer.h b/libsolidity/analysis/StaticAnalyzer.h index 2a62e391..7f5c743a 100644 --- a/libsolidity/analysis/StaticAnalyzer.h +++ b/libsolidity/analysis/StaticAnalyzer.h @@ -75,9 +75,6 @@ private: /// Flag that indicates whether the current contract definition is a library. bool m_library = false; - /// Flag that indicates whether a public function does not contain the "payable" modifier. - bool m_nonPayablePublic = false; - /// Number of uses of each (named) local variable in a function, counter is initialized with zero. /// Pairs of AST ids and pointers are used as keys to ensure a deterministic order /// when traversing. diff --git a/libsolidity/analysis/SyntaxChecker.cpp b/libsolidity/analysis/SyntaxChecker.cpp index ac4fa72b..0bc20f2e 100644 --- a/libsolidity/analysis/SyntaxChecker.cpp +++ b/libsolidity/analysis/SyntaxChecker.cpp @@ -138,9 +138,25 @@ void SyntaxChecker::endVisit(ModifierDefinition const& _modifier) m_placeholderFound = false; } -bool SyntaxChecker::visit(WhileStatement const&) +void SyntaxChecker::checkSingleStatementVariableDeclaration(ASTNode const& _statement) +{ + auto varDecl = dynamic_cast<VariableDeclarationStatement const*>(&_statement); + if (varDecl) + m_errorReporter.syntaxError(_statement.location(), "Variable declarations can only be used inside blocks."); +} + +bool SyntaxChecker::visit(IfStatement const& _ifStatement) +{ + checkSingleStatementVariableDeclaration(_ifStatement.trueStatement()); + if (Statement const* _statement = _ifStatement.falseStatement()) + checkSingleStatementVariableDeclaration(*_statement); + return true; +} + +bool SyntaxChecker::visit(WhileStatement const& _whileStatement) { m_inLoopDepth++; + checkSingleStatementVariableDeclaration(_whileStatement.body()); return true; } @@ -149,9 +165,10 @@ void SyntaxChecker::endVisit(WhileStatement const&) m_inLoopDepth--; } -bool SyntaxChecker::visit(ForStatement const&) +bool SyntaxChecker::visit(ForStatement const& _forStatement) { m_inLoopDepth++; + checkSingleStatementVariableDeclaration(_forStatement.body()); return true; } diff --git a/libsolidity/analysis/SyntaxChecker.h b/libsolidity/analysis/SyntaxChecker.h index 897df676..f5716bf9 100644 --- a/libsolidity/analysis/SyntaxChecker.h +++ b/libsolidity/analysis/SyntaxChecker.h @@ -52,6 +52,12 @@ private: virtual bool visit(ModifierDefinition const& _modifier) override; virtual void endVisit(ModifierDefinition const& _modifier) override; + /// Reports an error if _statement is a VariableDeclarationStatement. + /// Used by if/while/for to check for single statement variable declarations + /// without a block. + void checkSingleStatementVariableDeclaration(ASTNode const& _statement); + + virtual bool visit(IfStatement const& _ifStatement) override; virtual bool visit(WhileStatement const& _whileStatement) override; virtual void endVisit(WhileStatement const& _whileStatement) override; virtual bool visit(ForStatement const& _forStatement) override; diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 3b1bfe5c..143ac109 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -525,7 +525,7 @@ void TypeChecker::checkDoubleStorageAssignment(Assignment const& _assignment) ); } -TypePointer TypeChecker::typeCheckABIDecodeAndRetrieveReturnType(FunctionCall const& _functionCall, bool _abiEncoderV2) +TypePointers TypeChecker::typeCheckABIDecodeAndRetrieveReturnType(FunctionCall const& _functionCall, bool _abiEncoderV2) { vector<ASTPointer<Expression const>> arguments = _functionCall.arguments(); if (arguments.size() != 2) @@ -544,10 +544,8 @@ TypePointer TypeChecker::typeCheckABIDecodeAndRetrieveReturnType(FunctionCall co " to bytes memory requested." ); - TypePointer returnType = make_shared<TupleType>(); - if (arguments.size() < 2) - return returnType; + return {}; // The following is a rather syntactic restriction, but we check it here anyway: // The second argument has to be a tuple expression containing type names. @@ -558,10 +556,10 @@ TypePointer TypeChecker::typeCheckABIDecodeAndRetrieveReturnType(FunctionCall co arguments[1]->location(), "The second argument to \"abi.decode\" has to be a tuple of types." ); - return returnType; + return {}; } - vector<TypePointer> components; + TypePointers components; for (auto const& typeArgument: tupleExpression->components()) { solAssert(typeArgument, ""); @@ -591,7 +589,7 @@ TypePointer TypeChecker::typeCheckABIDecodeAndRetrieveReturnType(FunctionCall co components.push_back(make_shared<TupleType>()); } } - return make_shared<TupleType>(components); + return components; } void TypeChecker::endVisit(InheritanceSpecifier const& _inheritance) @@ -686,9 +684,7 @@ bool TypeChecker::visit(StructDefinition const& _struct) bool TypeChecker::visit(FunctionDefinition const& _function) { - bool isLibraryFunction = - dynamic_cast<ContractDefinition const*>(_function.scope()) && - dynamic_cast<ContractDefinition const*>(_function.scope())->isLibrary(); + bool isLibraryFunction = _function.inContractKind() == ContractDefinition::ContractKind::Library; if (_function.isPayable()) { if (isLibraryFunction) @@ -1217,7 +1213,7 @@ bool TypeChecker::visit(VariableDeclarationStatement const& _statement) if (ref->dataStoredIn(DataLocation::Storage)) { string errorText{"Uninitialized storage pointer."}; - if (varDecl.referenceLocation() == VariableDeclaration::Location::Default) + if (varDecl.referenceLocation() == VariableDeclaration::Location::Unspecified) errorText += " Did you mean '<type> memory " + varDecl.name() + "'?"; solAssert(m_scope, ""); m_errorReporter.declarationError(varDecl.location(), errorText); @@ -1782,15 +1778,6 @@ bool TypeChecker::visit(FunctionCall const& _functionCall) if (functionType->kind() == FunctionType::Kind::BareStaticCall && !m_evmVersion.hasStaticCall()) m_errorReporter.typeError(_functionCall.location(), "\"staticcall\" is not supported by the VM version."); - auto returnTypes = - allowDynamicTypes ? - functionType->returnParameterTypes() : - functionType->returnParameterTypesWithoutDynamicTypes(); - if (returnTypes.size() == 1) - _functionCall.annotation().type = returnTypes.front(); - else - _functionCall.annotation().type = make_shared<TupleType>(returnTypes); - if (auto functionName = dynamic_cast<Identifier const*>(&_functionCall.expression())) { if (functionName->name() == "sha3" && functionType->kind() == FunctionType::Kind::KECCAK256) @@ -1826,8 +1813,14 @@ bool TypeChecker::visit(FunctionCall const& _functionCall) bool const abiEncoderV2 = m_scope->sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::ABIEncoderV2); + // Will be assigned to .type at the end (turning multi-elements into a tuple). + TypePointers returnTypes = + allowDynamicTypes ? + functionType->returnParameterTypes() : + functionType->returnParameterTypesWithoutDynamicTypes(); + if (functionType->kind() == FunctionType::Kind::ABIDecode) - _functionCall.annotation().type = typeCheckABIDecodeAndRetrieveReturnType(_functionCall, abiEncoderV2); + returnTypes = typeCheckABIDecodeAndRetrieveReturnType(_functionCall, abiEncoderV2); else if (functionType->takesArbitraryParameters() && arguments.size() < parameterTypes.size()) { solAssert(_functionCall.annotation().kind == FunctionCallKind::FunctionCall, ""); @@ -1985,6 +1978,11 @@ bool TypeChecker::visit(FunctionCall const& _functionCall) } } + if (returnTypes.size() == 1) + _functionCall.annotation().type = returnTypes.front(); + else + _functionCall.annotation().type = make_shared<TupleType>(returnTypes); + return false; } @@ -2105,7 +2103,7 @@ bool TypeChecker::visit(MemberAccess const& _memberAccess) "after argument-dependent lookup in " + exprType->toString() + (memberName == "value" ? " - did you forget the \"payable\" modifier?" : "."); if (exprType->category() == Type::Category::Contract) - for (auto const& addressMember: IntegerType(160, IntegerType::Modifier::Address).nativeMembers(nullptr)) + for (auto const& addressMember: AddressType().nativeMembers(nullptr)) if (addressMember.name == memberName) { Identifier const* var = dynamic_cast<Identifier const*>(&_memberAccess.expression()); @@ -2356,7 +2354,7 @@ void TypeChecker::endVisit(Literal const& _literal) if (_literal.looksLikeAddress()) { // Assign type here if it even looks like an address. This prevents double errors for invalid addresses - _literal.annotation().type = make_shared<IntegerType>(160, IntegerType::Modifier::Address); + _literal.annotation().type = make_shared<AddressType>(); string msg; if (_literal.value().length() != 42) // "0x" + 40 hex digits diff --git a/libsolidity/analysis/TypeChecker.h b/libsolidity/analysis/TypeChecker.h index 4be0d1e4..8d25a88e 100644 --- a/libsolidity/analysis/TypeChecker.h +++ b/libsolidity/analysis/TypeChecker.h @@ -92,9 +92,9 @@ private: void checkExpressionAssignment(Type const& _type, Expression const& _expression); /// Performs type checks for ``abi.decode(bytes memory, (...))`` and returns the - /// return type (which is basically the second argument) if successful. It returns - /// the empty tuple type or error. - TypePointer typeCheckABIDecodeAndRetrieveReturnType(FunctionCall const& _functionCall, bool _abiEncoderV2); + /// vector of return types (which is basically the second argument) if successful. It returns + /// the empty vector on error. + TypePointers typeCheckABIDecodeAndRetrieveReturnType(FunctionCall const& _functionCall, bool _abiEncoderV2); virtual void endVisit(InheritanceSpecifier const& _inheritance) override; virtual void endVisit(UsingForDirective const& _usingFor) override; diff --git a/libsolidity/analysis/ViewPureChecker.cpp b/libsolidity/analysis/ViewPureChecker.cpp index e92ad2fa..113a3177 100644 --- a/libsolidity/analysis/ViewPureChecker.cpp +++ b/libsolidity/analysis/ViewPureChecker.cpp @@ -142,7 +142,7 @@ bool ViewPureChecker::visit(FunctionDefinition const& _funDef) { solAssert(!m_currentFunction, ""); m_currentFunction = &_funDef; - m_currentBestMutability = StateMutability::Pure; + m_bestMutabilityAndLocation = {StateMutability::Pure, _funDef.location()}; return true; } @@ -150,7 +150,7 @@ void ViewPureChecker::endVisit(FunctionDefinition const& _funDef) { solAssert(m_currentFunction == &_funDef, ""); if ( - m_currentBestMutability < _funDef.stateMutability() && + m_bestMutabilityAndLocation.mutability < _funDef.stateMutability() && _funDef.stateMutability() != StateMutability::Payable && _funDef.isImplemented() && !_funDef.isConstructor() && @@ -159,22 +159,22 @@ void ViewPureChecker::endVisit(FunctionDefinition const& _funDef) ) m_errorReporter.warning( _funDef.location(), - "Function state mutability can be restricted to " + stateMutabilityToString(m_currentBestMutability) + "Function state mutability can be restricted to " + stateMutabilityToString(m_bestMutabilityAndLocation.mutability) ); m_currentFunction = nullptr; } -bool ViewPureChecker::visit(ModifierDefinition const&) +bool ViewPureChecker::visit(ModifierDefinition const& _modifier) { solAssert(m_currentFunction == nullptr, ""); - m_currentBestMutability = StateMutability::Pure; + m_bestMutabilityAndLocation = {StateMutability::Pure, _modifier.location()}; return true; } void ViewPureChecker::endVisit(ModifierDefinition const& _modifierDef) { solAssert(m_currentFunction == nullptr, ""); - m_inferredMutability[&_modifierDef] = m_currentBestMutability; + m_inferredMutability[&_modifierDef] = std::move(m_bestMutabilityAndLocation); } void ViewPureChecker::endVisit(Identifier const& _identifier) @@ -219,36 +219,72 @@ void ViewPureChecker::endVisit(InlineAssembly const& _inlineAssembly) }(_inlineAssembly.operations()); } -void ViewPureChecker::reportMutability(StateMutability _mutability, SourceLocation const& _location) +void ViewPureChecker::reportMutability( + StateMutability _mutability, + SourceLocation const& _location, + boost::optional<SourceLocation> const& _nestedLocation +) { - if (m_currentFunction && m_currentFunction->stateMutability() < _mutability) + if (_mutability > m_bestMutabilityAndLocation.mutability) + m_bestMutabilityAndLocation = MutabilityAndLocation{_mutability, _location}; + if (!m_currentFunction || _mutability <= m_currentFunction->stateMutability()) + return; + + // Check for payable here, because any occurrence of `msg.value` + // will set mutability to payable. + if (_mutability == StateMutability::View || ( + _mutability == StateMutability::Payable && + m_currentFunction->stateMutability() == StateMutability::Pure + )) { - if (_mutability == StateMutability::View) - m_errorReporter.typeError( - _location, - "Function declared as pure, but this expression (potentially) reads from the " - "environment or state and thus requires \"view\"." - ); - else if (_mutability == StateMutability::NonPayable) - m_errorReporter.typeError( - _location, - "Function declared as " + - stateMutabilityToString(m_currentFunction->stateMutability()) + - ", but this expression (potentially) modifies the state and thus " - "requires non-payable (the default) or payable." - ); - else - solAssert(false, ""); - - solAssert( - m_currentFunction->stateMutability() == StateMutability::View || - m_currentFunction->stateMutability() == StateMutability::Pure, - "" + m_errorReporter.typeError( + _location, + "Function declared as pure, but this expression (potentially) reads from the " + "environment or state and thus requires \"view\"." ); m_errors = true; } - if (_mutability > m_currentBestMutability) - m_currentBestMutability = _mutability; + else if (_mutability == StateMutability::NonPayable) + { + m_errorReporter.typeError( + _location, + "Function declared as " + + stateMutabilityToString(m_currentFunction->stateMutability()) + + ", but this expression (potentially) modifies the state and thus " + "requires non-payable (the default) or payable." + ); + m_errors = true; + } + else if (_mutability == StateMutability::Payable) + { + // We do not warn for library functions because they cannot be payable anyway. + // Also internal functions should be allowed to use `msg.value`. + if (m_currentFunction->isPublic() && m_currentFunction->inContractKind() != ContractDefinition::ContractKind::Library) + { + if (_nestedLocation) + m_errorReporter.typeError( + _location, + SecondarySourceLocation().append("\"msg.value\" appears here inside the modifier.", *_nestedLocation), + "This modifier uses \"msg.value\" and thus the function has to be payable or internal." + ); + else + m_errorReporter.typeError( + _location, + "\"msg.value\" can only be used in payable public functions. Make the function " + "\"payable\" or use an internal function to avoid this error." + ); + m_errors = true; + } + } + else + solAssert(false, ""); + + solAssert( + m_currentFunction->stateMutability() == StateMutability::View || + m_currentFunction->stateMutability() == StateMutability::Pure || + m_currentFunction->stateMutability() == StateMutability::NonPayable, + "" + ); } void ViewPureChecker::endVisit(FunctionCall const& _functionCall) @@ -287,18 +323,34 @@ void ViewPureChecker::endVisit(MemberAccess const& _memberAccess) ASTString const& member = _memberAccess.memberName(); switch (_memberAccess.expression().annotation().type->category()) { - case Type::Category::Integer: + case Type::Category::Address: if (member == "balance") mutability = StateMutability::View; break; case Type::Category::Magic: { - // we can ignore the kind of magic and only look at the name of the member - set<string> static const pureMembers{ - "encode", "encodePacked", "encodeWithSelector", "encodeWithSignature", "decode", "data", "sig", "blockhash" + using MagicMember = pair<MagicType::Kind, string>; + set<MagicMember> static const pureMembers{ + {MagicType::Kind::ABI, "decode"}, + {MagicType::Kind::ABI, "encode"}, + {MagicType::Kind::ABI, "encodePacked"}, + {MagicType::Kind::ABI, "encodeWithSelector"}, + {MagicType::Kind::ABI, "encodeWithSignature"}, + {MagicType::Kind::Block, "blockhash"}, + {MagicType::Kind::Message, "data"}, + {MagicType::Kind::Message, "sig"} }; - if (!pureMembers.count(member)) + set<MagicMember> static const payableMembers{ + {MagicType::Kind::Message, "value"} + }; + + auto const& type = dynamic_cast<MagicType const&>(*_memberAccess.expression().annotation().type); + MagicMember magicMember(type.kind(), member); + + if (!pureMembers.count(magicMember)) mutability = StateMutability::View; + if (payableMembers.count(magicMember)) + mutability = StateMutability::Payable; break; } case Type::Category::Struct: @@ -338,7 +390,8 @@ void ViewPureChecker::endVisit(ModifierInvocation const& _modifier) if (ModifierDefinition const* mod = dynamic_cast<decltype(mod)>(_modifier.name()->annotation().referencedDeclaration)) { solAssert(m_inferredMutability.count(mod), ""); - reportMutability(m_inferredMutability.at(mod), _modifier.location()); + auto const& mutAndLocation = m_inferredMutability.at(mod); + reportMutability(mutAndLocation.mutability, _modifier.location(), mutAndLocation.location); } else solAssert(dynamic_cast<ContractDefinition const*>(_modifier.name()->annotation().referencedDeclaration), ""); diff --git a/libsolidity/analysis/ViewPureChecker.h b/libsolidity/analysis/ViewPureChecker.h index 3db52e7e..faa5b698 100644 --- a/libsolidity/analysis/ViewPureChecker.h +++ b/libsolidity/analysis/ViewPureChecker.h @@ -31,16 +31,6 @@ namespace dev namespace solidity { -class ASTNode; -class FunctionDefinition; -class ModifierDefinition; -class Identifier; -class MemberAccess; -class IndexAccess; -class ModifierInvocation; -class FunctionCall; -class InlineAssembly; - class ViewPureChecker: private ASTConstVisitor { public: @@ -50,6 +40,11 @@ public: bool check(); private: + struct MutabilityAndLocation + { + StateMutability mutability; + SourceLocation location; + }; virtual bool visit(FunctionDefinition const& _funDef) override; virtual void endVisit(FunctionDefinition const& _funDef) override; @@ -65,15 +60,19 @@ private: /// Called when an element of mutability @a _mutability is encountered. /// Creates appropriate warnings and errors and sets @a m_currentBestMutability. - void reportMutability(StateMutability _mutability, SourceLocation const& _location); + void reportMutability( + StateMutability _mutability, + SourceLocation const& _location, + boost::optional<SourceLocation> const& _nestedLocation = {} + ); std::vector<std::shared_ptr<ASTNode>> const& m_ast; ErrorReporter& m_errorReporter; bool m_errors = false; - StateMutability m_currentBestMutability = StateMutability::Payable; + MutabilityAndLocation m_bestMutabilityAndLocation = MutabilityAndLocation{StateMutability::Payable, SourceLocation()}; FunctionDefinition const* m_currentFunction = nullptr; - std::map<ModifierDefinition const*, StateMutability> m_inferredMutability; + std::map<ModifierDefinition const*, MutabilityAndLocation> m_inferredMutability; }; } |