Analyze inline assembly variable declarations for invalid or shadowing names.

Changelog.md

@@ -13,6 +13,7 @@ Breaking changes:
  * Syntax: ``push(element)`` for dynamic storage arrays do not return the new length anymore.
  * Syntax: Abstract contracts need to be marked explicitly as abstract by using the ``abstract`` keyword.
  * Inline Assembly: Only strict inline assembly is allowed.
+ * Inline Assembly: Variable declarations cannot shadow declarations outside the assembly block.
  * Type checker: Resulting type of exponentiation is equal to the type of the base. Also allow signed types for the base.
  * Source mappings: Add "modifier depth" as a fifth field in the source mappings.
 

libsolidity/analysis/ReferencesResolver.cpp

@@ -279,10 +279,34 @@ bool ReferencesResolver::visit(InlineAssembly const& _inlineAssembly)
 	ErrorList errors;
 	ErrorReporter errorsIgnored(errors);
 	yul::ExternalIdentifierAccess::Resolver resolver =
-	[&](yul::Identifier const& _identifier, yul::IdentifierContext, bool _crossesFunctionBoundary) {
+	[&](yul::Identifier const& _identifier, yul::IdentifierContext _context, bool _crossesFunctionBoundary) {
-		auto declarations = m_resolver.nameFromCurrentScope(_identifier.name.str());
 		bool isSlot = boost::algorithm::ends_with(_identifier.name.str(), "_slot");
 		bool isOffset = boost::algorithm::ends_with(_identifier.name.str(), "_offset");
+		if (_context == yul::IdentifierContext::VariableDeclaration)
+		{
+			string namePrefix = _identifier.name.str().substr(0, _identifier.name.str().find('.'));
+			if (isSlot || isOffset)
+				declarationError(_identifier.location, "In variable declarations _slot and _offset can not be used as a suffix.");
+			else if (
+				auto declarations = m_resolver.nameFromCurrentScope(namePrefix);
+				!declarations.empty()
+			)
+			{
+				SecondarySourceLocation ssl;
+				for (auto const* decl: declarations)
+					ssl.append("The shadowed declaration is here:", decl->location());
+				if (!ssl.infos.empty())
+					declarationError(
+						_identifier.location,
+						ssl,
+						namePrefix.size() < _identifier.name.str().size() ?
+						"The prefix of this declaration conflicts with a declaration outside the inline assembly block." :
+						"This declaration shadows a declaration outside the inline assembly block."
+					);
+			}
+			return size_t(-1);
+		}
+		auto declarations = m_resolver.nameFromCurrentScope(_identifier.name.str());
 		if (isSlot || isOffset)
 		{
 			// special mode to access storage variables
@@ -464,6 +488,12 @@ void ReferencesResolver::declarationError(SourceLocation const& _location, strin
 	m_errorReporter.declarationError(_location, _description);
 }
 
+void ReferencesResolver::declarationError(SourceLocation const& _location, SecondarySourceLocation const& _ssl, string const& _description)
+{
+	m_errorOccurred = true;
+	m_errorReporter.declarationError(_location, _ssl, _description);
+}
+
 void ReferencesResolver::fatalDeclarationError(SourceLocation const& _location, string const& _description)
 {
 	m_errorOccurred = true;

libsolidity/analysis/ReferencesResolver.h

@@ -91,6 +91,9 @@ private:
 	/// Adds a new error to the list of errors.
 	void declarationError(langutil::SourceLocation const& _location, std::string const& _description);
 
+	/// Adds a new error to the list of errors.
+	void declarationError(langutil::SourceLocation const& _location, langutil::SecondarySourceLocation const& _ssl, std::string const& _description);
+
 	/// Adds a new error to the list of errors and throws to abort reference resolving.
 	void fatalDeclarationError(langutil::SourceLocation const& _location, std::string const& _description);
 

libyul/AsmAnalysis.cpp

@@ -244,6 +244,14 @@ bool AsmAnalyzer::operator()(VariableDeclaration const& _varDecl)
 {
 	bool success = true;
 	int const numVariables = _varDecl.variables.size();
+	if (m_resolver)
+		for (auto const& variable: _varDecl.variables)
+			// Call the resolver for variable declarations to allow it to raise errors on shadowing.
+			m_resolver(
+				yul::Identifier{variable.location, variable.name},
+				yul::IdentifierContext::VariableDeclaration,
+				m_currentScope->insideFunction()
+			);
 	if (_varDecl.value)
 	{
 		int const stackHeight = m_stackHeight;

libyul/backends/evm/AbstractAssembly.h

@@ -111,7 +111,7 @@ public:
 	virtual SubID appendData(dev::bytes const& _data) = 0;
 };
 
-enum class IdentifierContext { LValue, RValue };
+enum class IdentifierContext { LValue, RValue, VariableDeclaration };
 
 /// Object that is used to resolve references and generate code for access to identifiers external
 /// to inline assembly (not used in standalone assembly mode).

test/libsolidity/syntaxTests/inlineAssembly/invalid/variable_declaration_suffix_offset.sol

@@ -0,0 +1,15 @@
+contract C {
+    function f() public pure {
+        assembly {
+            let x_offset := 1
+            let x_slot := 1
+            let _offset := 1
+            let _slot := 1
+        }
+    }
+}
+// ----
+// DeclarationError: (79-87): In variable declarations _slot and _offset can not be used as a suffix.
+// DeclarationError: (109-115): In variable declarations _slot and _offset can not be used as a suffix.
+// DeclarationError: (137-144): In variable declarations _slot and _offset can not be used as a suffix.
+// DeclarationError: (166-171): In variable declarations _slot and _offset can not be used as a suffix.

test/libsolidity/syntaxTests/inlineAssembly/shadowing/argument.sol

@@ -0,0 +1,9 @@
+contract C {
+    function f(uint a) public pure {
+        assembly {
+            let a := 1
+        }
+    }
+}
+// ----
+// DeclarationError: (85-86): This declaration shadows a declaration outside the inline assembly block.

test/libsolidity/syntaxTests/inlineAssembly/shadowing/constant.sol

@@ -0,0 +1,10 @@
+contract C {
+    uint constant a;
+    function f() public pure {
+        assembly {
+            let a := 1
+        }
+    }
+}
+// ----
+// DeclarationError: (100-101): This declaration shadows a declaration outside the inline assembly block.

test/libsolidity/syntaxTests/inlineAssembly/shadowing/contract.sol

@@ -0,0 +1,9 @@
+contract C {
+    function f() public pure {
+        assembly {
+            let C := 1
+        }
+    }
+}
+// ----
+// DeclarationError: (79-80): This declaration shadows a declaration outside the inline assembly block.

test/libsolidity/syntaxTests/inlineAssembly/shadowing/function.sol

@@ -0,0 +1,9 @@
+contract C {
+    function f() public pure {
+        assembly {
+            let f := 1
+        }
+    }
+}
+// ----
+// DeclarationError: (79-80): This declaration shadows a declaration outside the inline assembly block.

test/libsolidity/syntaxTests/inlineAssembly/shadowing/local_variable.sol

@@ -0,0 +1,10 @@
+contract C {
+    function f() public pure {
+        uint a;
+        assembly {
+            let a := 1
+        }
+    }
+}
+// ----
+// DeclarationError: (95-96): This declaration shadows a declaration outside the inline assembly block.

test/libsolidity/syntaxTests/inlineAssembly/shadowing/name_clash_in_import.sol

@@ -0,0 +1,18 @@
+==== Source: a ====
+contract A
+{
+	uint constant a = 42;
+}
+==== Source: b ====
+import {A as b} from "a";
+contract B {
+    function f() public pure {
+        assembly {
+            let b := 3
+            let b.a := 4
+        }
+    }
+}
+// ----
+// DeclarationError: (b:105-106): This declaration shadows a declaration outside the inline assembly block.
+// DeclarationError: (b:128-131): The prefix of this declaration conflicts with a declaration outside the inline assembly block.

test/libsolidity/syntaxTests/inlineAssembly/shadowing/no_name_clash_in_import.sol

@@ -0,0 +1,15 @@
+==== Source: a ====
+contract A
+{
+	uint constant a = 42;
+}
+==== Source: b ====
+import {A as b} from "a";
+contract B {
+    function f() public pure {
+        assembly {
+            let A := 1
+            let A.b := 2
+        }
+    }
+}

test/libsolidity/syntaxTests/inlineAssembly/shadowing/qualified_names.sol

@@ -0,0 +1,14 @@
+contract D {
+    uint constant a;
+}
+contract C {
+    function f() public pure {
+        assembly {
+            let D.a := 1
+            let D.b := 1 // shadowing the prefix only is also an error
+        }
+    }
+}
+// ----
+// DeclarationError: (115-118): The prefix of this declaration conflicts with a declaration outside the inline assembly block.
+// DeclarationError: (140-143): The prefix of this declaration conflicts with a declaration outside the inline assembly block.