diff --git a/Changelog.md b/Changelog.md index 40ff714ab..a638fa9e8 100644 --- a/Changelog.md +++ b/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. diff --git a/libsolidity/analysis/ReferencesResolver.cpp b/libsolidity/analysis/ReferencesResolver.cpp index ffeb32ee5..f7bae57b8 100644 --- a/libsolidity/analysis/ReferencesResolver.cpp +++ b/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) { - auto declarations = m_resolver.nameFromCurrentScope(_identifier.name.str()); + [&](yul::Identifier const& _identifier, yul::IdentifierContext _context, bool _crossesFunctionBoundary) { 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; diff --git a/libsolidity/analysis/ReferencesResolver.h b/libsolidity/analysis/ReferencesResolver.h index 5631fb801..56394416a 100644 --- a/libsolidity/analysis/ReferencesResolver.h +++ b/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); diff --git a/libyul/AsmAnalysis.cpp b/libyul/AsmAnalysis.cpp index 57706578c..dce6bc013 100644 --- a/libyul/AsmAnalysis.cpp +++ b/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; diff --git a/libyul/backends/evm/AbstractAssembly.h b/libyul/backends/evm/AbstractAssembly.h index b87710522..0d9a558df 100644 --- a/libyul/backends/evm/AbstractAssembly.h +++ b/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). diff --git a/test/libsolidity/syntaxTests/inlineAssembly/invalid/variable_declaration_suffix_offset.sol b/test/libsolidity/syntaxTests/inlineAssembly/invalid/variable_declaration_suffix_offset.sol new file mode 100644 index 000000000..e27fb041b --- /dev/null +++ b/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. diff --git a/test/libsolidity/syntaxTests/inlineAssembly/shadowing/argument.sol b/test/libsolidity/syntaxTests/inlineAssembly/shadowing/argument.sol new file mode 100644 index 000000000..0f84c9587 --- /dev/null +++ b/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. diff --git a/test/libsolidity/syntaxTests/inlineAssembly/shadowing/constant.sol b/test/libsolidity/syntaxTests/inlineAssembly/shadowing/constant.sol new file mode 100644 index 000000000..1249e9da5 --- /dev/null +++ b/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. diff --git a/test/libsolidity/syntaxTests/inlineAssembly/shadowing/contract.sol b/test/libsolidity/syntaxTests/inlineAssembly/shadowing/contract.sol new file mode 100644 index 000000000..abf0ca879 --- /dev/null +++ b/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. diff --git a/test/libsolidity/syntaxTests/inlineAssembly/shadowing/function.sol b/test/libsolidity/syntaxTests/inlineAssembly/shadowing/function.sol new file mode 100644 index 000000000..2d28623ff --- /dev/null +++ b/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. diff --git a/test/libsolidity/syntaxTests/inlineAssembly/shadowing/local_variable.sol b/test/libsolidity/syntaxTests/inlineAssembly/shadowing/local_variable.sol new file mode 100644 index 000000000..1cb39eac4 --- /dev/null +++ b/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. diff --git a/test/libsolidity/syntaxTests/inlineAssembly/shadowing/name_clash_in_import.sol b/test/libsolidity/syntaxTests/inlineAssembly/shadowing/name_clash_in_import.sol new file mode 100644 index 000000000..d6e73871e --- /dev/null +++ b/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. diff --git a/test/libsolidity/syntaxTests/inlineAssembly/shadowing/no_name_clash_in_import.sol b/test/libsolidity/syntaxTests/inlineAssembly/shadowing/no_name_clash_in_import.sol new file mode 100644 index 000000000..86426aad9 --- /dev/null +++ b/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 + } + } +} diff --git a/test/libsolidity/syntaxTests/inlineAssembly/shadowing/qualified_names.sol b/test/libsolidity/syntaxTests/inlineAssembly/shadowing/qualified_names.sol new file mode 100644 index 000000000..3ae885243 --- /dev/null +++ b/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.