From 2c4c8264e4428ff45cfd5f8f9b5458f1044ba8aa Mon Sep 17 00:00:00 2001 From: Marenz Date: Wed, 12 Jan 2022 18:58:17 +0100 Subject: [PATCH] Fix wrong error with immutables when base contract c'tor uses return --- Changelog.md | 1 + libsolidity/analysis/ImmutableValidator.cpp | 25 ++++++++++++------- libsolidity/analysis/ImmutableValidator.h | 5 ++-- .../base_ctor_return_no_immutables.sol | 10 ++++++++ 4 files changed, 30 insertions(+), 11 deletions(-) create mode 100644 test/libsolidity/syntaxTests/immutable/base_ctor_return_no_immutables.sol diff --git a/Changelog.md b/Changelog.md index b83e083d0..bd492e4bf 100644 --- a/Changelog.md +++ b/Changelog.md @@ -11,6 +11,7 @@ Compiler Features: Bugfixes: * Control Flow Graph: Perform proper virtual lookup for modifiers for uninitialized variable and unreachable code analysis. + * Immutables: Fix wrong error when the constructor of a base contract uses ``return`` and the parent contract contains immutable variables. Solc-Js: diff --git a/libsolidity/analysis/ImmutableValidator.cpp b/libsolidity/analysis/ImmutableValidator.cpp index e28236fb6..485aa3fa2 100644 --- a/libsolidity/analysis/ImmutableValidator.cpp +++ b/libsolidity/analysis/ImmutableValidator.cpp @@ -29,7 +29,7 @@ void ImmutableValidator::analyze() { m_inCreationContext = true; - auto linearizedContracts = m_currentContract.annotation().linearizedBaseContracts | ranges::views::reverse; + auto linearizedContracts = m_mostDerivedContract.annotation().linearizedBaseContracts | ranges::views::reverse; for (ContractDefinition const* contract: linearizedContracts) for (VariableDeclaration const* stateVar: contract->stateVariables()) @@ -62,7 +62,7 @@ void ImmutableValidator::analyze() visitCallableIfNew(*modDef); } - checkAllVariablesInitialized(m_currentContract.location()); + checkAllVariablesInitialized(m_mostDerivedContract.location()); } bool ImmutableValidator::visit(Assignment const& _assignment) @@ -137,7 +137,7 @@ void ImmutableValidator::endVisit(IdentifierPath const& _identifierPath) if (auto const callableDef = dynamic_cast(_identifierPath.annotation().referencedDeclaration)) visitCallableIfNew( *_identifierPath.annotation().requiredLookup == VirtualLookup::Virtual ? - callableDef->resolveVirtual(m_currentContract) : + callableDef->resolveVirtual(m_mostDerivedContract) : *callableDef ); @@ -147,7 +147,7 @@ void ImmutableValidator::endVisit(IdentifierPath const& _identifierPath) void ImmutableValidator::endVisit(Identifier const& _identifier) { if (auto const callableDef = dynamic_cast(_identifier.annotation().referencedDeclaration)) - visitCallableIfNew(*_identifier.annotation().requiredLookup == VirtualLookup::Virtual ? callableDef->resolveVirtual(m_currentContract) : *callableDef); + visitCallableIfNew(*_identifier.annotation().requiredLookup == VirtualLookup::Virtual ? callableDef->resolveVirtual(m_mostDerivedContract) : *callableDef); if (auto const varDecl = dynamic_cast(_identifier.annotation().referencedDeclaration)) analyseVariableReference(*varDecl, _identifier); } @@ -160,15 +160,18 @@ void ImmutableValidator::endVisit(Return const& _return) bool ImmutableValidator::analyseCallable(CallableDeclaration const& _callableDeclaration) { - FunctionDefinition const* prevConstructor = m_currentConstructor; - m_currentConstructor = nullptr; + ScopedSaveAndRestore constructorGuard{m_currentConstructor, {}}; + ScopedSaveAndRestore constructorContractGuard{m_currentConstructorContract, {}}; if (FunctionDefinition const* funcDef = dynamic_cast(&_callableDeclaration)) { ASTNode::listAccept(funcDef->modifiers(), *this); if (funcDef->isConstructor()) + { + m_currentConstructorContract = funcDef->annotation().contract; m_currentConstructor = funcDef; + } if (funcDef->isImplemented()) funcDef->body().accept(*this); @@ -177,8 +180,6 @@ bool ImmutableValidator::analyseCallable(CallableDeclaration const& _callableDec if (modDef->isImplemented()) modDef->body().accept(*this); - m_currentConstructor = prevConstructor; - return false; } @@ -253,7 +254,8 @@ void ImmutableValidator::analyseVariableReference(VariableDeclaration const& _va void ImmutableValidator::checkAllVariablesInitialized(solidity::langutil::SourceLocation const& _location) { - for (ContractDefinition const* contract: m_currentContract.annotation().linearizedBaseContracts) + for (ContractDefinition const* contract: m_mostDerivedContract.annotation().linearizedBaseContracts | ranges::views::reverse) + { for (VariableDeclaration const* varDecl: contract->stateVariables()) if (varDecl->immutable()) if (!util::contains(m_initializedStateVariables, varDecl)) @@ -263,6 +265,11 @@ void ImmutableValidator::checkAllVariablesInitialized(solidity::langutil::Source solidity::langutil::SecondarySourceLocation().append("Not initialized: ", varDecl->location()), "Construction control flow ends without initializing all immutable state variables." ); + + // Don't check further than the current c'tors contract + if (contract == m_currentConstructorContract) + break; + } } void ImmutableValidator::visitCallableIfNew(Declaration const& _declaration) diff --git a/libsolidity/analysis/ImmutableValidator.h b/libsolidity/analysis/ImmutableValidator.h index e6d837efe..452e91d7c 100644 --- a/libsolidity/analysis/ImmutableValidator.h +++ b/libsolidity/analysis/ImmutableValidator.h @@ -42,7 +42,7 @@ class ImmutableValidator: private ASTConstVisitor public: ImmutableValidator(langutil::ErrorReporter& _errorReporter, ContractDefinition const& _contractDefinition): - m_currentContract(_contractDefinition), + m_mostDerivedContract(_contractDefinition), m_errorReporter(_errorReporter) { } @@ -66,7 +66,7 @@ private: void visitCallableIfNew(Declaration const& _declaration); - ContractDefinition const& m_currentContract; + ContractDefinition const& m_mostDerivedContract; CallableDeclarationSet m_visitedCallables; @@ -74,6 +74,7 @@ private: langutil::ErrorReporter& m_errorReporter; FunctionDefinition const* m_currentConstructor = nullptr; + ContractDefinition const* m_currentConstructorContract = nullptr; bool m_inLoop = false; bool m_inBranch = false; bool m_inCreationContext = true; diff --git a/test/libsolidity/syntaxTests/immutable/base_ctor_return_no_immutables.sol b/test/libsolidity/syntaxTests/immutable/base_ctor_return_no_immutables.sol new file mode 100644 index 000000000..e6e668bc0 --- /dev/null +++ b/test/libsolidity/syntaxTests/immutable/base_ctor_return_no_immutables.sol @@ -0,0 +1,10 @@ +contract Parent { + constructor() { + return; + } +} + +contract Child is Parent { + uint public immutable baked = 123; +} +