Validate immutable variables

libsolidity/CMakeLists.txt

@@ -14,6 +14,8 @@ set(sources
 	analysis/DeclarationContainer.h
 	analysis/DocStringAnalyser.cpp
 	analysis/DocStringAnalyser.h
+	analysis/ImmutableValidator.cpp
+	analysis/ImmutableValidator.h
 	analysis/GlobalContext.cpp
 	analysis/GlobalContext.h
 	analysis/NameAndTypeResolver.cpp

libsolidity/analysis/ImmutableValidator.cpp

@@ -0,0 +1,214 @@
+/*
+	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/ImmutableValidator.h>
+
+#include <libsolutil/CommonData.h>
+
+#include <boost/range/adaptor/reversed.hpp>
+
+using namespace solidity::frontend;
+
+void ImmutableValidator::analyze()
+{
+	m_inConstructionContext = true;
+
+	auto linearizedContracts = m_currentContract.annotation().linearizedBaseContracts | boost::adaptors::reversed;
+
+	for (ContractDefinition const* contract: linearizedContracts)
+		for (VariableDeclaration const* stateVar: contract->stateVariables())
+			if (stateVar->value())
+			{
+				stateVar->value()->accept(*this);
+				solAssert(m_initializedStateVariables.emplace(stateVar).second, "");
+			}
+
+	for (ContractDefinition const* contract: linearizedContracts)
+		if (contract->constructor())
+			visitCallableIfNew(*contract->constructor());
+
+	for (ContractDefinition const* contract: linearizedContracts)
+		for (std::shared_ptr<InheritanceSpecifier> const inheritSpec: contract->baseContracts())
+			if (auto args = inheritSpec->arguments())
+				ASTNode::listAccept(*args, *this);
+
+	m_inConstructionContext = false;
+
+	for (ContractDefinition const* contract: linearizedContracts)
+	{
+		for (auto funcDef: contract->definedFunctions())
+			visitCallableIfNew(*funcDef);
+
+		for (auto modDef: contract->functionModifiers())
+			visitCallableIfNew(*modDef);
+	}
+
+	checkAllVariablesInitialized(m_currentContract.location());
+}
+
+bool ImmutableValidator::visit(FunctionDefinition const& _functionDefinition)
+{
+	return analyseCallable(_functionDefinition);
+}
+
+bool ImmutableValidator::visit(ModifierDefinition const& _modifierDefinition)
+{
+	return analyseCallable(_modifierDefinition);
+}
+
+bool ImmutableValidator::visit(MemberAccess const& _memberAccess)
+{
+	_memberAccess.expression().accept(*this);
+
+	if (auto varDecl = dynamic_cast<VariableDeclaration const*>(_memberAccess.annotation().referencedDeclaration))
+		analyseVariableReference(*varDecl, _memberAccess);
+	else if (auto funcType = dynamic_cast<FunctionType const*>(_memberAccess.annotation().type))
+		if (funcType->kind() == FunctionType::Kind::Internal && funcType->hasDeclaration())
+			visitCallableIfNew(funcType->declaration());
+
+	return false;
+}
+
+bool ImmutableValidator::visit(IfStatement const& _ifStatement)
+{
+	bool prevInBranch = m_inBranch;
+
+	_ifStatement.condition().accept(*this);
+
+	m_inBranch = true;
+	_ifStatement.trueStatement().accept(*this);
+
+	if (auto falseStatement = _ifStatement.falseStatement())
+		falseStatement->accept(*this);
+
+	m_inBranch = prevInBranch;
+
+	return false;
+}
+
+bool ImmutableValidator::visit(WhileStatement const& _whileStatement)
+{
+	bool prevInLoop = m_inLoop;
+	m_inLoop = true;
+
+	_whileStatement.condition().accept(*this);
+	_whileStatement.body().accept(*this);
+
+	m_inLoop = prevInLoop;
+
+	return false;
+}
+
+void ImmutableValidator::endVisit(Identifier const& _identifier)
+{
+	if (auto const callableDef = dynamic_cast<CallableDeclaration const*>(_identifier.annotation().referencedDeclaration))
+		visitCallableIfNew(callableDef->resolveVirtual(m_currentContract));
+	if (auto const varDecl = dynamic_cast<VariableDeclaration const*>(_identifier.annotation().referencedDeclaration))
+		analyseVariableReference(*varDecl, _identifier);
+}
+
+void ImmutableValidator::endVisit(Return const& _return)
+{
+	if (m_currentConstructor != nullptr)
+		checkAllVariablesInitialized(_return.location());
+}
+
+bool ImmutableValidator::analyseCallable(CallableDeclaration const& _callableDeclaration)
+{
+	FunctionDefinition const* prevConstructor = m_currentConstructor;
+	m_currentConstructor = nullptr;
+
+	if (FunctionDefinition const* funcDef = dynamic_cast<decltype(funcDef)>(&_callableDeclaration))
+	{
+		ASTNode::listAccept(funcDef->modifiers(), *this);
+
+		if (funcDef->isConstructor())
+			m_currentConstructor = funcDef;
+
+		if (funcDef->isImplemented())
+			funcDef->body().accept(*this);
+	}
+	else if (ModifierDefinition const* modDef = dynamic_cast<decltype(modDef)>(&_callableDeclaration))
+		modDef->body().accept(*this);
+
+	m_currentConstructor = prevConstructor;
+
+	return false;
+}
+
+void ImmutableValidator::analyseVariableReference(VariableDeclaration const& _variableReference, Expression const& _expression)
+{
+	if (!_variableReference.isStateVariable() || !_variableReference.immutable())
+		return;
+
+	if (_expression.annotation().lValueRequested && _expression.annotation().lValueOfOrdinaryAssignment)
+	{
+		if (!m_currentConstructor)
+			m_errorReporter.typeError(
+				_expression.location(),
+				"Immutable variables can only be initialized inline or assigned directly in the constructor."
+			);
+		else if (m_currentConstructor->annotation().contract->id() != _variableReference.annotation().contract->id())
+			m_errorReporter.typeError(
+				_expression.location(),
+				"Immutable variables must be initialized in the constructor of the contract they are defined in."
+			);
+		else if (m_inLoop)
+			m_errorReporter.typeError(
+				_expression.location(),
+				"Immutable variables can only be initialized once, not in a while statement."
+			);
+		else if (m_inBranch)
+			m_errorReporter.typeError(
+				_expression.location(),
+				"Immutable variables must be initialized unconditionally, not in an if statement."
+			);
+
+		if (!m_initializedStateVariables.emplace(&_variableReference).second)
+			m_errorReporter.typeError(
+				_expression.location(),
+				"Immutable state variable already initialized."
+			);
+	}
+	else if (m_inConstructionContext)
+		m_errorReporter.typeError(
+			_expression.location(),
+			"Immutable variables cannot be read during contract creation time, which means they cannot be read in the constructor or any function or modifier called from it."
+		);
+}
+
+void ImmutableValidator::checkAllVariablesInitialized(solidity::langutil::SourceLocation const& _location)
+{
+	for (ContractDefinition const* contract: m_currentContract.annotation().linearizedBaseContracts)
+		for (VariableDeclaration const* varDecl: contract->stateVariables())
+			if (varDecl->immutable())
+				if (!util::contains(m_initializedStateVariables, varDecl))
+					m_errorReporter.typeError(
+						_location,
+						solidity::langutil::SecondarySourceLocation().append("Not initialized: ", varDecl->location()),
+						"Construction control flow ends without initializing all immutable state variables."
+					);
+}
+
+void ImmutableValidator::visitCallableIfNew(Declaration const& _declaration)
+{
+	CallableDeclaration const* _callable = dynamic_cast<CallableDeclaration const*>(&_declaration);
+	solAssert(_callable != nullptr, "");
+
+	if (m_visitedCallables.emplace(_callable).second)
+		_declaration.accept(*this);
+}

libsolidity/analysis/ImmutableValidator.h

@@ -0,0 +1,78 @@
+/*
+	This file is part of solidity.
+
+	solidity is free software: you can redistribute it and/or modify
+	it under the terms of the GNU General Public License as published by
+	the Free Software Foundation, either version 3 of the License, or
+	(at your option) any later version.
+
+	solidity is distributed in the hope that it will be useful,
+	but WITHOUT ANY WARRANTY; without even the implied warranty of
+	MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+	GNU General Public License for more details.
+
+	You should have received a copy of the GNU General Public License
+	along with solidity.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#pragma once
+
+#include <libsolidity/ast/ASTVisitor.h>
+#include <liblangutil/ErrorReporter.h>
+
+#include <memory>
+
+namespace solidity::frontend
+{
+
+/**
+ * Validates access and initialization of immutable variables:
+ * must be directly initialized in their respective c'tor
+ * can not be read by any function/modifier called by the c'tor (or the c'tor itself)
+ * must be initialized outside loops (only one initialization)
+ * must be initialized outside ifs (must be initialized unconditionally)
+ * must be initialized exactly once (no multiple statements)
+ * must be initialized exactly once (no early return to skip initialization)
+*/
+class ImmutableValidator: private ASTConstVisitor
+{
+	using CallableDeclarationSet = std::set<CallableDeclaration const*, ASTNode::CompareByID>;
+
+public:
+	ImmutableValidator(langutil::ErrorReporter& _errorReporter, ContractDefinition const& _contractDefinition):
+		m_currentContract(_contractDefinition),
+		m_errorReporter(_errorReporter)
+	{ }
+
+	void analyze();
+
+private:
+	bool visit(FunctionDefinition const& _functionDefinition);
+	bool visit(ModifierDefinition const& _modifierDefinition);
+	bool visit(MemberAccess const& _memberAccess);
+	bool visit(IfStatement const& _ifStatement);
+	bool visit(WhileStatement const& _whileStatement);
+	void endVisit(Identifier const& _identifier);
+	void endVisit(Return const& _return);
+
+	bool analyseCallable(CallableDeclaration const& _callableDeclaration);
+	void analyseVariableReference(VariableDeclaration const& _variableReference, Expression const& _expression);
+
+	void checkAllVariablesInitialized(langutil::SourceLocation const& _location);
+
+	void visitCallableIfNew(Declaration const& _declaration);
+
+	ContractDefinition const& m_currentContract;
+
+	CallableDeclarationSet m_visitedCallables;
+
+	std::set<VariableDeclaration const*, ASTNode::CompareByID> m_initializedStateVariables;
+	langutil::ErrorReporter& m_errorReporter;
+
+	FunctionDefinition const* m_currentConstructor = nullptr;
+	bool m_inLoop = false;
+	bool m_inBranch = false;
+	bool m_inConstructionContext = false;
+};
+
+}

libsolidity/analysis/TypeChecker.cpp

@@ -1337,7 +1337,10 @@ void TypeChecker::checkExpressionAssignment(Type const& _type, Expression const&
 
 bool TypeChecker::visit(Assignment const& _assignment)
 {
-	requireLValue(_assignment.leftHandSide());
+	requireLValue(
+		_assignment.leftHandSide(),
+		_assignment.assignmentOperator() == Token::Assign
+	);
 	TypePointer t = type(_assignment.leftHandSide());
 	_assignment.annotation().type = t;
 
@@ -1395,7 +1398,10 @@ bool TypeChecker::visit(TupleExpression const& _tuple)
 		for (auto const& component: components)
 			if (component)
 			{
-				requireLValue(*component);
+				requireLValue(
+					*component,
+					_tuple.annotation().lValueOfOrdinaryAssignment
+				);
 				types.push_back(type(*component));
 			}
 			else
@@ -1480,7 +1486,7 @@ bool TypeChecker::visit(UnaryOperation const& _operation)
 	Token op = _operation.getOperator();
 	bool const modifying = (op == Token::Inc || op == Token::Dec || op == Token::Delete);
 	if (modifying)
-		requireLValue(_operation.subExpression());
+		requireLValue(_operation.subExpression(), false);
 	else
 		_operation.subExpression().accept(*this);
 	TypePointer const& subExprType = type(_operation.subExpression());
@@ -2988,9 +2994,10 @@ bool TypeChecker::expectType(Expression const& _expression, Type const& _expecte
 	return true;
 }
 
-void TypeChecker::requireLValue(Expression const& _expression)
+void TypeChecker::requireLValue(Expression const& _expression, bool _ordinaryAssignment)
 {
 	_expression.annotation().lValueRequested = true;
+	_expression.annotation().lValueOfOrdinaryAssignment = _ordinaryAssignment;
 	_expression.accept(*this);
 
 	if (_expression.annotation().isLValue)

libsolidity/analysis/TypeChecker.h

@@ -158,7 +158,7 @@ private:
 	/// convertible to @a _expectedType.
 	bool expectType(Expression const& _expression, Type const& _expectedType);
 	/// Runs type checks on @a _expression to infer its type and then checks that it is an LValue.
-	void requireLValue(Expression const& _expression);
+	void requireLValue(Expression const& _expression, bool _ordinaryAssignment);
 
 	ContractDefinition const* m_scope = nullptr;
 

libsolidity/ast/AST.h

@@ -62,6 +62,24 @@ class ASTConstVisitor;
 class ASTNode: private boost::noncopyable
 {
 public:
+	struct CompareByID
+	{
+		using is_transparent = void;
+
+		bool operator()(ASTNode const* _lhs, ASTNode const* _rhs) const
+		{
+			return _lhs->id() < _rhs->id();
+		}
+		bool operator()(ASTNode const* _lhs, int64_t _rhs) const
+		{
+			return _lhs->id() < _rhs;
+		}
+		bool operator()(int64_t _lhs, ASTNode const* _rhs) const
+		{
+			return _lhs < _rhs->id();
+		}
+	};
+
 	using SourceLocation = langutil::SourceLocation;
 
 	explicit ASTNode(int64_t _id, SourceLocation const& _location);

libsolidity/ast/ASTAnnotations.h

@@ -208,6 +208,9 @@ struct ExpressionAnnotation: ASTAnnotation
 	bool isLValue = false;
 	/// Whether the expression is used in a context where the LValue is actually required.
 	bool lValueRequested = false;
+	/// Whether the expression is an lvalue that is only assigned.
+	/// Would be false for --, ++, delete, +=, -=, ....
+	bool lValueOfOrdinaryAssignment = false;
 
 	/// Types and - if given - names of arguments if the expr. is a function
 	/// that is called, used for overload resoultion

libsolidity/interface/CompilerStack.cpp

@@ -35,6 +35,7 @@
 #include <libsolidity/analysis/SyntaxChecker.h>
 #include <libsolidity/analysis/TypeChecker.h>
 #include <libsolidity/analysis/ViewPureChecker.h>
+#include <libsolidity/analysis/ImmutableValidator.h>
 
 #include <libsolidity/ast/AST.h>
 #include <libsolidity/ast/TypeProvider.h>
@@ -383,6 +384,15 @@ bool CompilerStack::analyze()
 					noErrors = false;
 		}
 
+		// Check that immutable variables are never read in c'tors and assigned
+		// exactly once
+		if (noErrors)
+			for (Source const* source: m_sourceOrder)
+				if (source->ast)
+					for (ASTPointer<ASTNode> const& node: source->ast->nodes())
+						if (ContractDefinition* contract = dynamic_cast<ContractDefinition*>(node.get()))
+							ImmutableValidator(m_errorReporter, *contract).analyze();
+
 		if (noErrors)
 		{
 			// Control flow graph generator and analyzer. It can check for issues such as