Merge pull request #6722 from ethereum/smt_fix_variable_usage

[SMTChecker] Fix VariableUsage for IndexAccess

libsolidity/formal/SMTChecker.cpp

@@ -1020,6 +1020,7 @@ void SMTChecker::endVisit(IndexAccess const& _indexAccess)
 	}
 	else
 	{
+		createExpr(_indexAccess);
 		m_errorReporter.warning(
 			_indexAccess.location(),
 			"Assertion checker does not yet implement this expression."
@@ -1096,10 +1097,19 @@ void SMTChecker::arrayIndexAssignment(Expression const& _expr, smt::Expression c
 		));
 	}
 	else if (dynamic_cast<IndexAccess const*>(&indexAccess.baseExpression()))
+	{
+		auto identifier = dynamic_cast<Identifier const*>(leftmostBase(indexAccess));
+		if (identifier)
+		{
+			auto varDecl = identifierToVariable(*identifier);
+			newValue(*varDecl);
+		}
+
 		m_errorReporter.warning(
 			indexAccess.location(),
 			"Assertion checker does not yet implement assignments to multi-dimensional mappings or arrays."
 		);
+	}
 	else
 		m_errorReporter.warning(
 			_expr.location(),
@@ -1982,6 +1992,14 @@ FunctionDefinition const* SMTChecker::inlinedFunctionCallToDefinition(FunctionCa
 	return nullptr;
 }
 
+Expression const* SMTChecker::leftmostBase(IndexAccess const& _indexAccess)
+{
+	Expression const* base = &_indexAccess.baseExpression();
+	while (auto access = dynamic_cast<IndexAccess const*>(base))
+		base = &access->baseExpression();
+	return base;
+}
+
 set<VariableDeclaration const*> SMTChecker::touchedVariables(ASTNode const& _node)
 {
 	solAssert(!m_callStack.empty(), "");

libsolidity/formal/SMTChecker.h

@@ -55,9 +55,11 @@ public:
 	/// the constructor.
 	std::vector<std::string> unhandledQueries() { return m_interface->unhandledQueries(); }
 
-	/// @return the FunctionDefinition of a called function if possible and should inline,
+	/// @returns the FunctionDefinition of a called function if possible and should inline,
 	/// otherwise nullptr.
 	static FunctionDefinition const* inlinedFunctionCallToDefinition(FunctionCall const& _funCall);
+	/// @returns the leftmost identifier in a multi-d IndexAccess.
+	static Expression const* leftmostBase(IndexAccess const& _indexAccess);
 
 private:
 	// TODO: Check that we do not have concurrent reads and writes to a variable,

libsolidity/formal/VariableUsage.cpp

@@ -25,17 +25,32 @@ using namespace std;
 using namespace dev;
 using namespace dev::solidity;
 
+set<VariableDeclaration const*> VariableUsage::touchedVariables(ASTNode const& _node, vector<CallableDeclaration const*> const& _outerCallstack)
+{
+	m_touchedVariables.clear();
+	m_callStack.clear();
+	m_callStack += _outerCallstack;
+	m_lastCall = m_callStack.back();
+	_node.accept(*this);
+	return m_touchedVariables;
+}
+
 void VariableUsage::endVisit(Identifier const& _identifier)
 {
-	Declaration const* declaration = _identifier.annotation().referencedDeclaration;
-	solAssert(declaration, "");
-	if (VariableDeclaration const* varDecl = dynamic_cast<VariableDeclaration const*>(declaration))
-		if (_identifier.annotation().lValueRequested)
-		{
-			solAssert(m_lastCall, "");
-			if (!varDecl->isLocalVariable() || varDecl->functionOrModifierDefinition() == m_lastCall)
-				m_touchedVariables.insert(varDecl);
-		}
+	if (_identifier.annotation().lValueRequested)
+		checkIdentifier(_identifier);
+}
+
+void VariableUsage::endVisit(IndexAccess const& _indexAccess)
+{
+	if (_indexAccess.annotation().lValueRequested)
+	{
+		/// identifier.annotation().lValueRequested == false, that's why we
+		/// need to check that before.
+		auto identifier = dynamic_cast<Identifier const*>(SMTChecker::leftmostBase(_indexAccess));
+		if (identifier)
+			checkIdentifier(*identifier);
+	}
 }
 
 void VariableUsage::endVisit(FunctionCall const& _funCall)
@@ -75,12 +90,14 @@ void VariableUsage::endVisit(PlaceholderStatement const&)
 		funDef->body().accept(*this);
 }
 
-set<VariableDeclaration const*> VariableUsage::touchedVariables(ASTNode const& _node, vector<CallableDeclaration const*> const& _outerCallstack)
+void VariableUsage::checkIdentifier(Identifier const& _identifier)
 {
-	m_touchedVariables.clear();
-	m_callStack.clear();
-	m_callStack += _outerCallstack;
-	m_lastCall = m_callStack.back();
-	_node.accept(*this);
-	return m_touchedVariables;
+	Declaration const* declaration = _identifier.annotation().referencedDeclaration;
+	solAssert(declaration, "");
+	if (VariableDeclaration const* varDecl = dynamic_cast<VariableDeclaration const*>(declaration))
+	{
+		solAssert(m_lastCall, "");
+		if (!varDecl->isLocalVariable() || varDecl->functionOrModifierDefinition() == m_lastCall)
+			m_touchedVariables.insert(varDecl);
+	}
 }

libsolidity/formal/VariableUsage.h

@@ -38,12 +38,16 @@ public:
 
 private:
 	void endVisit(Identifier const& _node) override;
+	void endVisit(IndexAccess const& _node) override;
 	void endVisit(FunctionCall const& _node) override;
 	bool visit(FunctionDefinition const& _node) override;
 	void endVisit(FunctionDefinition const& _node) override;
 	void endVisit(ModifierInvocation const& _node) override;
 	void endVisit(PlaceholderStatement const& _node) override;
 
+	/// Checks whether an identifier should be added to touchedVariables.
+	void checkIdentifier(Identifier const& _identifier);
+
 	std::set<VariableDeclaration const*> m_touchedVariables;
 	std::vector<CallableDeclaration const*> m_callStack;
 	CallableDeclaration const* m_lastCall = nullptr;