From 0479f7f16c3c85a52b0ae895006f655da5ac76f3 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 6 Mar 2017 14:11:55 +0100 Subject: [PATCH 1/4] Test for trying to construct an inconstructible contract before its definition. --- .../SolidityNameAndTypeResolution.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 3b1375721..437f2adc7 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -5060,6 +5060,24 @@ BOOST_AUTO_TEST_CASE(inconstructible_internal_constructor) CHECK_ERROR(text, TypeError, "Contract with internal constructor cannot be created directly."); } +BOOST_AUTO_TEST_CASE(inconstructible_internal_constructor_inverted) +{ + // Previously, the type information for A was not yet available at the point of + // "new A". + char const* text = R"( + contract B { + A a; + function B() { + a = new A(this); + } + } + contract A { + function A(address a) internal {} + } + )"; + CHECK_ERROR(text, TypeError, "Contract with internal constructor cannot be created directly."); +} + BOOST_AUTO_TEST_CASE(constructible_internal_constructor) { char const* text = R"( From ca4e2933dd9b149653070815dd6255e3b8735d6d Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 6 Mar 2017 14:00:46 +0100 Subject: [PATCH 2/4] Strict checking for AST annotation types. --- libsolidity/ast/AST.cpp | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/libsolidity/ast/AST.cpp b/libsolidity/ast/AST.cpp index 616de54e1..8d137bac4 100644 --- a/libsolidity/ast/AST.cpp +++ b/libsolidity/ast/AST.cpp @@ -83,7 +83,7 @@ SourceUnitAnnotation& SourceUnit::annotation() const { if (!m_annotation) m_annotation = new SourceUnitAnnotation(); - return static_cast(*m_annotation); + return dynamic_cast(*m_annotation); } string Declaration::sourceUnitName() const @@ -99,7 +99,7 @@ ImportAnnotation& ImportDirective::annotation() const { if (!m_annotation) m_annotation = new ImportAnnotation(); - return static_cast(*m_annotation); + return dynamic_cast(*m_annotation); } TypePointer ImportDirective::type() const @@ -255,14 +255,14 @@ ContractDefinitionAnnotation& ContractDefinition::annotation() const { if (!m_annotation) m_annotation = new ContractDefinitionAnnotation(); - return static_cast(*m_annotation); + return dynamic_cast(*m_annotation); } TypeNameAnnotation& TypeName::annotation() const { if (!m_annotation) m_annotation = new TypeNameAnnotation(); - return static_cast(*m_annotation); + return dynamic_cast(*m_annotation); } TypePointer StructDefinition::type() const @@ -274,7 +274,7 @@ TypeDeclarationAnnotation& StructDefinition::annotation() const { if (!m_annotation) m_annotation = new TypeDeclarationAnnotation(); - return static_cast(*m_annotation); + return dynamic_cast(*m_annotation); } TypePointer EnumValue::type() const @@ -293,7 +293,7 @@ TypeDeclarationAnnotation& EnumDefinition::annotation() const { if (!m_annotation) m_annotation = new TypeDeclarationAnnotation(); - return static_cast(*m_annotation); + return dynamic_cast(*m_annotation); } shared_ptr FunctionDefinition::functionType(bool _internal) const @@ -349,7 +349,7 @@ FunctionDefinitionAnnotation& FunctionDefinition::annotation() const { if (!m_annotation) m_annotation = new FunctionDefinitionAnnotation(); - return static_cast(*m_annotation); + return dynamic_cast(*m_annotation); } TypePointer ModifierDefinition::type() const @@ -361,7 +361,7 @@ ModifierDefinitionAnnotation& ModifierDefinition::annotation() const { if (!m_annotation) m_annotation = new ModifierDefinitionAnnotation(); - return static_cast(*m_annotation); + return dynamic_cast(*m_annotation); } TypePointer EventDefinition::type() const @@ -381,14 +381,14 @@ EventDefinitionAnnotation& EventDefinition::annotation() const { if (!m_annotation) m_annotation = new EventDefinitionAnnotation(); - return static_cast(*m_annotation); + return dynamic_cast(*m_annotation); } UserDefinedTypeNameAnnotation& UserDefinedTypeName::annotation() const { if (!m_annotation) m_annotation = new UserDefinedTypeNameAnnotation(); - return static_cast(*m_annotation); + return dynamic_cast(*m_annotation); } bool VariableDeclaration::isLValue() const @@ -460,70 +460,70 @@ VariableDeclarationAnnotation& VariableDeclaration::annotation() const { if (!m_annotation) m_annotation = new VariableDeclarationAnnotation(); - return static_cast(*m_annotation); + return dynamic_cast(*m_annotation); } StatementAnnotation& Statement::annotation() const { if (!m_annotation) m_annotation = new StatementAnnotation(); - return static_cast(*m_annotation); + return dynamic_cast(*m_annotation); } InlineAssemblyAnnotation& InlineAssembly::annotation() const { if (!m_annotation) m_annotation = new InlineAssemblyAnnotation(); - return static_cast(*m_annotation); + return dynamic_cast(*m_annotation); } ReturnAnnotation& Return::annotation() const { if (!m_annotation) m_annotation = new ReturnAnnotation(); - return static_cast(*m_annotation); + return dynamic_cast(*m_annotation); } VariableDeclarationStatementAnnotation& VariableDeclarationStatement::annotation() const { if (!m_annotation) m_annotation = new VariableDeclarationStatementAnnotation(); - return static_cast(*m_annotation); + return dynamic_cast(*m_annotation); } ExpressionAnnotation& Expression::annotation() const { if (!m_annotation) m_annotation = new ExpressionAnnotation(); - return static_cast(*m_annotation); + return dynamic_cast(*m_annotation); } MemberAccessAnnotation& MemberAccess::annotation() const { if (!m_annotation) m_annotation = new MemberAccessAnnotation(); - return static_cast(*m_annotation); + return dynamic_cast(*m_annotation); } BinaryOperationAnnotation& BinaryOperation::annotation() const { if (!m_annotation) m_annotation = new BinaryOperationAnnotation(); - return static_cast(*m_annotation); + return dynamic_cast(*m_annotation); } FunctionCallAnnotation& FunctionCall::annotation() const { if (!m_annotation) m_annotation = new FunctionCallAnnotation(); - return static_cast(*m_annotation); + return dynamic_cast(*m_annotation); } IdentifierAnnotation& Identifier::annotation() const { if (!m_annotation) m_annotation = new IdentifierAnnotation(); - return static_cast(*m_annotation); + return dynamic_cast(*m_annotation); } bool Literal::looksLikeAddress() const From f300bdb020ca55b05f1725dc2157fb636c1e4801 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 6 Mar 2017 14:12:20 +0100 Subject: [PATCH 3/4] Move public constructor property into AST itself. --- libsolidity/analysis/TypeChecker.cpp | 4 +--- libsolidity/ast/AST.cpp | 6 ++++++ libsolidity/ast/AST.h | 2 ++ libsolidity/ast/ASTAnnotations.h | 2 -- libsolidity/interface/CompilerStack.cpp | 2 +- 5 files changed, 10 insertions(+), 6 deletions(-) diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index ff55ef1f9..a9a2c82ff 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -77,8 +77,6 @@ bool TypeChecker::visit(ContractDefinition const& _contract) FunctionDefinition const* function = _contract.constructor(); if (function) { - if (!function->isPublic()) - _contract.annotation().hasPublicConstructor = false; if (!function->returnParameters().empty()) typeError(function->returnParameterList()->location(), "Non-empty \"returns\" directive for constructor."); if (function->isDeclaredConst()) @@ -1290,7 +1288,7 @@ void TypeChecker::endVisit(NewExpression const& _newExpression) fatalTypeError(_newExpression.location(), "Identifier is not a contract."); if (!contract->annotation().isFullyImplemented) typeError(_newExpression.location(), "Trying to create an instance of an abstract contract."); - if (!contract->annotation().hasPublicConstructor) + if (!contract->constructorIsPublic()) typeError(_newExpression.location(), "Contract with internal constructor cannot be created directly."); solAssert(!!m_scope, ""); diff --git a/libsolidity/ast/AST.cpp b/libsolidity/ast/AST.cpp index 8d137bac4..03112d2d9 100644 --- a/libsolidity/ast/AST.cpp +++ b/libsolidity/ast/AST.cpp @@ -132,6 +132,12 @@ FunctionDefinition const* ContractDefinition::constructor() const return nullptr; } +bool ContractDefinition::constructorIsPublic() const +{ + FunctionDefinition const* f = constructor(); + return !f || f->isPublic(); +} + FunctionDefinition const* ContractDefinition::fallbackFunction() const { for (ContractDefinition const* contract: annotation().linearizedBaseContracts) diff --git a/libsolidity/ast/AST.h b/libsolidity/ast/AST.h index 743fdaa1d..8031760d6 100644 --- a/libsolidity/ast/AST.h +++ b/libsolidity/ast/AST.h @@ -356,6 +356,8 @@ public: /// Returns the constructor or nullptr if no constructor was specified. FunctionDefinition const* constructor() const; + /// @returns true iff the constructor of this contract is public (or non-existing). + bool constructorIsPublic() const; /// Returns the fallback function or nullptr if no fallback function was specified. FunctionDefinition const* fallbackFunction() const; diff --git a/libsolidity/ast/ASTAnnotations.h b/libsolidity/ast/ASTAnnotations.h index 61e97a55b..9c4c3ae8b 100644 --- a/libsolidity/ast/ASTAnnotations.h +++ b/libsolidity/ast/ASTAnnotations.h @@ -80,8 +80,6 @@ struct ContractDefinitionAnnotation: TypeDeclarationAnnotation, DocumentedAnnota { /// Whether all functions are implemented. bool isFullyImplemented = true; - /// Whether a public constructor (even the default one) is available. - bool hasPublicConstructor = true; /// List of all (direct and indirect) base contracts in order from derived to /// base, including the contract itself. std::vector linearizedBaseContracts; diff --git a/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp index 9d8d872f4..edfca094c 100644 --- a/libsolidity/interface/CompilerStack.cpp +++ b/libsolidity/interface/CompilerStack.cpp @@ -628,7 +628,7 @@ void CompilerStack::compileContract( if ( _compiledContracts.count(&_contract) || !_contract.annotation().isFullyImplemented || - !_contract.annotation().hasPublicConstructor + !_contract.constructorIsPublic() ) return; for (auto const* dependency: _contract.annotation().contractDependencies) From a3cb69b14b13a64deda8a71387e480a0eec45698 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 6 Mar 2017 14:16:06 +0100 Subject: [PATCH 4/4] Changelog entry. --- Changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/Changelog.md b/Changelog.md index 5eb1e401e..df0fde215 100644 --- a/Changelog.md +++ b/Changelog.md @@ -15,6 +15,7 @@ Bugfixes: * Type system: Fix a crash related to invalid binary operators. * Type system: Disallow ``var`` declaration with empty tuple type. * Type system: Correctly convert function argument types to pointers for member functions. + * Type system: Move privateness of constructor into AST itself. * Inline assembly: Charge one stack slot for non-value types during analysis. * Assembly output: Print source location before the operation it refers to instead of after.