Merge pull request #101 from LianaHus/sol_Disallow_access_if_not_initialized

Disallow access to non-initialized references in storage

8 files changed, 124 insertions, 43 deletions

diff --git a/libsolidity/CompilerStack.cpp b/libsolidity/CompilerStack.cpp
index e6b87264..6ee19d58 100644
--- a/libsolidity/CompilerStack.cpp
+++ b/libsolidity/CompilerStack.cpp
@@ -122,6 +122,7 @@ bool CompilerStack::parse()
 			}
 
 	InterfaceHandler interfaceHandler;
+	bool typesFine = true;
 	for (Source const* source: m_sourceOrder)
 		for (ASTPointer<ASTNode> const& node: source->ast->nodes())
 			if (ContractDefinition* contract = dynamic_cast<ContractDefinition*>(node.get()))
@@ -129,14 +130,14 @@ bool CompilerStack::parse()
 				m_globalContext->setCurrentContract(*contract);
 				resolver.updateDeclaration(*m_globalContext->currentThis());
 				TypeChecker typeChecker;
-				bool typesFine = typeChecker.checkTypeRequirements(*contract);
-				if (!typesFine)
-					m_errors += typeChecker.errors();
+				if (!typeChecker.checkTypeRequirements(*contract))
+					typesFine = false;
+				m_errors += typeChecker.errors();
 				contract->setDevDocumentation(interfaceHandler.devDocumentation(*contract));
 				contract->setUserDocumentation(interfaceHandler.userDocumentation(*contract));
 				m_contracts[contract->name()].contract = contract;
 			}
-	m_parseSuccessful = m_errors.empty();
+	m_parseSuccessful = typesFine;
 	return m_parseSuccessful;
 }
 
diff --git a/libsolidity/Exceptions.h b/libsolidity/Exceptions.h
index 645b368f..8ab33999 100644
--- a/libsolidity/Exceptions.h
+++ b/libsolidity/Exceptions.h
@@ -31,13 +31,13 @@ namespace dev
 {
 namespace solidity
 {
-
 struct Error: virtual Exception {};
 
 struct ParserError: virtual Error {};
 struct TypeError: virtual Error {};
 struct DeclarationError: virtual Error {};
 struct DocstringParsingError: virtual Error {};
+struct Warning: virtual Error {};
 
 struct CompilerError: virtual Exception {};
 struct InternalCompilerError: virtual Exception {};
@@ -53,7 +53,6 @@ public:
 		infos.push_back(std::make_pair(_errMsg, _sourceLocation));
 		return *this;
 	}
-
 	std::vector<errorSourceLocationInfo> infos;
 };
 
diff --git a/libsolidity/Parser.cpp b/libsolidity/Parser.cpp
index 3fbe4d68..f3b654ea 100644
--- a/libsolidity/Parser.cpp
+++ b/libsolidity/Parser.cpp
@@ -274,9 +274,17 @@ ASTPointer<FunctionDefinition> Parser::parseFunctionDefinition(ASTString const*
 	else
 		m_scanner->next(); // just consume the ';'
 	bool const c_isConstructor = (_contractName && *name == *_contractName);
-	return nodeFactory.createNode<FunctionDefinition>(name, visibility, c_isConstructor, docstring,
-													  parameters, isDeclaredConst, modifiers,
-													  returnParameters, block);
+	return nodeFactory.createNode<FunctionDefinition>(
+		name,
+		visibility,
+		c_isConstructor,
+		docstring,
+		parameters,
+		isDeclaredConst,
+		modifiers,
+		returnParameters,
+		block
+	);
 }
 
 ASTPointer<StructDefinition> Parser::parseStructDefinition()
@@ -753,7 +761,8 @@ ASTPointer<Statement> Parser::parseSimpleStatement()
 }
 
 ASTPointer<VariableDeclarationStatement> Parser::parseVariableDeclarationStatement(
-	ASTPointer<TypeName> const& _lookAheadArrayType)
+	ASTPointer<TypeName> const& _lookAheadArrayType
+)
 {
 	VarDeclParserOptions options;
 	options.allowVar = true;
@@ -765,14 +774,16 @@ ASTPointer<VariableDeclarationStatement> Parser::parseVariableDeclarationStateme
 }
 
 ASTPointer<ExpressionStatement> Parser::parseExpressionStatement(
-	ASTPointer<Expression> const& _lookAheadIndexAccessStructure)
+	ASTPointer<Expression> const& _lookAheadIndexAccessStructure
+)
 {
 	ASTPointer<Expression> expression = parseExpression(_lookAheadIndexAccessStructure);
 	return ASTNodeFactory(*this, expression).createNode<ExpressionStatement>(expression);
 }
 
 ASTPointer<Expression> Parser::parseExpression(
-		ASTPointer<Expression> const& _lookAheadIndexAccessStructure)
+	ASTPointer<Expression> const& _lookAheadIndexAccessStructure
+)
 {
 	ASTPointer<Expression> expression = parseBinaryExpression(4, _lookAheadIndexAccessStructure);
 	if (!Token::isAssignmentOp(m_scanner->currentToken()))
@@ -784,8 +795,10 @@ ASTPointer<Expression> Parser::parseExpression(
 	return nodeFactory.createNode<Assignment>(expression, assignmentOperator, rightHandSide);
 }
 
-ASTPointer<Expression> Parser::parseBinaryExpression(int _minPrecedence,
-	ASTPointer<Expression> const& _lookAheadIndexAccessStructure)
+ASTPointer<Expression> Parser::parseBinaryExpression(
+	int _minPrecedence,
+	ASTPointer<Expression> const& _lookAheadIndexAccessStructure
+)
 {
 	ASTPointer<Expression> expression = parseUnaryExpression(_lookAheadIndexAccessStructure);
 	ASTNodeFactory nodeFactory(*this, expression);
@@ -803,7 +816,8 @@ ASTPointer<Expression> Parser::parseBinaryExpression(int _minPrecedence,
 }
 
 ASTPointer<Expression> Parser::parseUnaryExpression(
-	ASTPointer<Expression> const& _lookAheadIndexAccessStructure)
+	ASTPointer<Expression> const& _lookAheadIndexAccessStructure
+)
 {
 	ASTNodeFactory nodeFactory = _lookAheadIndexAccessStructure ?
 		ASTNodeFactory(*this, _lookAheadIndexAccessStructure) : ASTNodeFactory(*this);
@@ -830,7 +844,8 @@ ASTPointer<Expression> Parser::parseUnaryExpression(
 }
 
 ASTPointer<Expression> Parser::parseLeftHandSideExpression(
-	ASTPointer<Expression> const& _lookAheadIndexAccessStructure)
+	ASTPointer<Expression> const& _lookAheadIndexAccessStructure
+)
 {
 	ASTNodeFactory nodeFactory = _lookAheadIndexAccessStructure ?
 		ASTNodeFactory(*this, _lookAheadIndexAccessStructure) : ASTNodeFactory(*this);
@@ -1014,7 +1029,9 @@ Parser::LookAheadInfo Parser::peekStatementType() const
 }
 
 ASTPointer<TypeName> Parser::typeNameIndexAccessStructure(
-	ASTPointer<PrimaryExpression> const& _primary, vector<pair<ASTPointer<Expression>, SourceLocation>> const& _indices)
+	ASTPointer<PrimaryExpression> const& _primary,
+	vector<pair<ASTPointer<Expression>, SourceLocation>> const& _indices
+)
 {
 	ASTNodeFactory nodeFactory(*this, _primary);
 	ASTPointer<TypeName> type;
@@ -1033,7 +1050,9 @@ ASTPointer<TypeName> Parser::typeNameIndexAccessStructure(
 }
 
 ASTPointer<Expression> Parser::expressionFromIndexAccessStructure(
-	ASTPointer<PrimaryExpression> const& _primary, vector<pair<ASTPointer<Expression>, SourceLocation>> const& _indices)
+	ASTPointer<PrimaryExpression> const& _primary,
+	vector<pair<ASTPointer<Expression>, SourceLocation>> const& _indices
+)
 {
 	ASTNodeFactory nodeFactory(*this, _primary);
 	ASTPointer<Expression> expression(_primary);
diff --git a/libsolidity/Parser.h b/libsolidity/Parser.h
index 79eb73f0..043d022b 100644
--- a/libsolidity/Parser.h
+++ b/libsolidity/Parser.h
@@ -90,17 +90,23 @@ private:
 	/// A "simple statement" can be a variable declaration statement or an expression statement.
 	ASTPointer<Statement> parseSimpleStatement();
 	ASTPointer<VariableDeclarationStatement> parseVariableDeclarationStatement(
-		ASTPointer<TypeName> const& _lookAheadArrayType = ASTPointer<TypeName>());
+		ASTPointer<TypeName> const& _lookAheadArrayType = ASTPointer<TypeName>()
+	);
 	ASTPointer<ExpressionStatement> parseExpressionStatement(
-		ASTPointer<Expression> const& _lookAheadIndexAccessStructure = ASTPointer<Expression>());
+		ASTPointer<Expression> const& _lookAheadIndexAccessStructure = ASTPointer<Expression>()
+	);
 	ASTPointer<Expression> parseExpression(
-		ASTPointer<Expression> const& _lookAheadIndexAccessStructure = ASTPointer<Expression>());
+		ASTPointer<Expression> const& _lookAheadIndexAccessStructure = ASTPointer<Expression>()
+	);
 	ASTPointer<Expression> parseBinaryExpression(int _minPrecedence = 4,
-		ASTPointer<Expression> const& _lookAheadIndexAccessStructure = ASTPointer<Expression>());
+		ASTPointer<Expression> const& _lookAheadIndexAccessStructure = ASTPointer<Expression>()
+	);
 	ASTPointer<Expression> parseUnaryExpression(
-		ASTPointer<Expression> const& _lookAheadIndexAccessStructure = ASTPointer<Expression>());
+		ASTPointer<Expression> const& _lookAheadIndexAccessStructure = ASTPointer<Expression>()
+	);
 	ASTPointer<Expression> parseLeftHandSideExpression(
-		ASTPointer<Expression> const& _lookAheadIndexAccessStructure = ASTPointer<Expression>());
+		ASTPointer<Expression> const& _lookAheadIndexAccessStructure = ASTPointer<Expression>()
+	);
 	ASTPointer<Expression> parsePrimaryExpression();
 	std::vector<ASTPointer<Expression>> parseFunctionCallListArguments();
 	std::pair<std::vector<ASTPointer<Expression>>, std::vector<ASTPointer<ASTString>>> parseFunctionCallArguments();
@@ -122,11 +128,13 @@ private:
 	/// Returns a typename parsed in look-ahead fashion from something like "a[8][2**70]".
 	ASTPointer<TypeName> typeNameIndexAccessStructure(
 		ASTPointer<PrimaryExpression> const& _primary,
-		std::vector<std::pair<ASTPointer<Expression>, SourceLocation>> const& _indices);
+		std::vector<std::pair<ASTPointer<Expression>, SourceLocation>> const& _indices
+	);
 	/// Returns an expression parsed in look-ahead fashion from something like "a[8][2**70]".
 	ASTPointer<Expression> expressionFromIndexAccessStructure(
 		ASTPointer<PrimaryExpression> const& _primary,
-		std::vector<std::pair<ASTPointer<Expression>, SourceLocation>> const& _indices);
+		std::vector<std::pair<ASTPointer<Expression>, SourceLocation>> const& _indices
+	);
 	/// If current token value is not _value, throw exception otherwise advance token.
 	void expectToken(Token::Value _value);
 	Token::Value expectAssignmentOperator();
diff --git a/libsolidity/TypeChecker.cpp b/libsolidity/TypeChecker.cpp
index f453e2fa..48a8a536 100644
--- a/libsolidity/TypeChecker.cpp
+++ b/libsolidity/TypeChecker.cpp
@@ -43,8 +43,14 @@ bool TypeChecker::checkTypeRequirements(const ContractDefinition& _contract)
 		if (m_errors.empty())
 			throw; // Something is weird here, rather throw again.
 	}
-
-	return m_errors.empty();
+	bool success = true;
+	for (auto const& it: m_errors)
+		if (!dynamic_cast<Warning const*>(it.get()))
+		{
+			success = false;
+			break;
+		}
+	return success;
 }
 
 TypePointer const& TypeChecker::type(Expression const& _expression) const
@@ -443,6 +449,18 @@ bool TypeChecker::visit(VariableDeclaration const& _variable)
 	{
 		if (_variable.value())
 			expectType(*_variable.value(), *varType);
+		else
+		{
+			if (auto ref = dynamic_cast<ReferenceType const *>(varType.get()))
+				if (ref->dataStoredIn(DataLocation::Storage) && _variable.isLocalVariable() && !_variable.isCallableParameter())
+				{
+					auto err = make_shared<Warning>();
+					*err <<
+						errinfo_sourceLocation(_variable.location()) <<
+						errinfo_comment("Uninitialized storage pointer. Did you mean '<type> memory " + _variable.name() + "'?");
+					m_errors.push_back(err);
+				}
+		}
 	}
 	else
 	{
diff --git a/libsolidity/TypeChecker.h b/libsolidity/TypeChecker.h
index cc539e22..3f740b7e 100644
--- a/libsolidity/TypeChecker.h
+++ b/libsolidity/TypeChecker.h
@@ -43,10 +43,10 @@ class TypeChecker: private ASTConstVisitor
 {
 public:
 	/// Performs type checking on the given contract and all of its sub-nodes.
-	/// @returns true iff all checks passed.
+	/// @returns true iff all checks passed. Note even if all checks passed, errors() can still contain warnings
 	bool checkTypeRequirements(ContractDefinition const& _contract);
 
-	/// @returns the list of errors found during type checking.
+	/// @returns the list of errors and warnings found during type checking.
 	std::vector<std::shared_ptr<Error const>> const& errors() const { return m_errors; }
 
 	/// @returns the type of an expression and asserts that it is present.
@@ -57,6 +57,7 @@ public:
 
 	/// Adds a new error to the list of errors.
 	void typeError(ASTNode const& _node, std::string const& _description);
+
 	/// Adds a new error to the list of errors and throws to abort type checking.
 	void fatalTypeError(ASTNode const& _node, std::string const& _description);
 
diff --git a/solc/CommandLineInterface.cpp b/solc/CommandLineInterface.cpp
index a1f5fd66..be10faa8 100644
--- a/solc/CommandLineInterface.cpp
+++ b/solc/CommandLineInterface.cpp
@@ -491,12 +491,15 @@ bool CommandLineInterface::processInput()
 		// TODO: Perhaps we should not compile unless requested
 		bool optimize = m_args.count("optimize") > 0;
 		unsigned runs = m_args["optimize-runs"].as<unsigned>();
-		if (!m_compiler->compile(optimize, runs))
-		{
-			for (auto const& error: m_compiler->errors())
-				SourceReferenceFormatter::printExceptionInformation(cerr, *error, "Error", *m_compiler);
+		bool successful = m_compiler->compile(optimize, runs);
+		for (auto const& error: m_compiler->errors())
+			SourceReferenceFormatter::printExceptionInformation(
+				cerr,
+				*error,
+				(dynamic_pointer_cast<Warning const>(error)) ? "Warning" : "Error", *m_compiler
+			);
+		if (!successful)
 			return false;
-		}
 		m_compiler->link(m_libraries);
 	}
 	catch (ParserError const& _exception)
diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp
index cac16682..961c10b4 100644
--- a/test/libsolidity/SolidityNameAndTypeResolution.cpp
+++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp
@@ -45,7 +45,7 @@ namespace
 {
 
 pair<ASTPointer<SourceUnit>, shared_ptr<Exception const>>
-parseAnalyseAndReturnError(string const& _source)
+parseAnalyseAndReturnError(string const& _source, bool _reportWarnings = false)
 {
 	Parser parser;
 	ASTPointer<SourceUnit> sourceUnit;
@@ -74,7 +74,19 @@ parseAnalyseAndReturnError(string const& _source)
 				TypeChecker typeChecker;
 				if (!typeChecker.checkTypeRequirements(*contract))
 				{
-					err = typeChecker.errors().front();
+					for (auto const& firstError: typeChecker.errors())
+					{
+						if (_reportWarnings || !dynamic_pointer_cast<Warning const>(firstError))
+						{
+							err = firstError;
+							break;
+						}
+						else if (_reportWarnings)
+						{
+							err = firstError;
+							break;
+						}
+					}
 					break;
 				}
 			}
@@ -101,9 +113,9 @@ ASTPointer<SourceUnit> parseAndAnalyse(string const& _source)
 	return sourceAndError.first;
 }
 
-shared_ptr<Exception const> parseAndAnalyseReturnError(std::string const& _source)
+shared_ptr<Exception const> parseAndAnalyseReturnError(std::string const& _source, bool _warning = false)
 {
-	auto sourceAndError = parseAnalyseAndReturnError(_source);
+	auto sourceAndError = parseAnalyseAndReturnError(_source, _warning);
 	BOOST_REQUIRE(!!sourceAndError.second);
 	return sourceAndError.second;
 }
@@ -119,8 +131,10 @@ static ContractDefinition const* retrieveContract(ASTPointer<SourceUnit> _source
 	return nullptr;
 }
 
-static FunctionTypePointer const& retrieveFunctionBySignature(ContractDefinition const* _contract,
-															  std::string const& _signature)
+static FunctionTypePointer const& retrieveFunctionBySignature(
+	ContractDefinition const* _contract,
+	std::string const& _signature
+)
 {
 	FixedHash<4> hash(dev::sha3(_signature));
 	return _contract->interfaceFunctions()[hash];
@@ -155,8 +169,8 @@ BOOST_AUTO_TEST_CASE(double_stateVariable_declaration)
 BOOST_AUTO_TEST_CASE(double_function_declaration)
 {
 	char const* text = "contract test {\n"
-					   "  function fun() { var x; }\n"
-					   "  function fun() { var x; }\n"
+					   "  function fun() { uint x; }\n"
+					   "  function fun() { uint x; }\n"
 					   "}\n";
 	SOLIDITY_CHECK_ERROR_TYPE(parseAndAnalyseReturnError(text), DeclarationError);
 }
@@ -2318,6 +2332,24 @@ BOOST_AUTO_TEST_CASE(literal_string_to_storage_pointer)
 	SOLIDITY_CHECK_ERROR_TYPE(parseAndAnalyseReturnError(text), TypeError);
 }
 
+BOOST_AUTO_TEST_CASE(non_initialized_references)
+{
+	char const* text = R"(
+		contract c
+		{
+			struct s{
+				uint a;
+			}
+			function f()
+			{
+				s x;
+				x.a = 2;
+			}
+		}
+	)";
+	SOLIDITY_CHECK_ERROR_TYPE(parseAndAnalyseReturnError(text, true), Warning);
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 
 }