From a4c94a1b5b9c9d256c271926754301997600725f Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 4 Aug 2021 18:11:00 +0200 Subject: [PATCH] Fixed inline assembly external identifier access. --- libsolidity/analysis/TypeChecker.cpp | 9 ++++++- libsolidity/codegen/CompilerContext.cpp | 1 + libsolidity/codegen/ContractCompiler.cpp | 1 + libyul/AsmAnalysis.cpp | 27 ++++++++++++++++++- libyul/backends/evm/AbstractAssembly.h | 2 +- .../external_identifier_access_shadowing.sol | 12 +++++++++ .../external_identifier_access_shadowing.sol | 12 +++++++++ 7 files changed, 61 insertions(+), 3 deletions(-) create mode 100644 test/libsolidity/semanticTests/inlineAssembly/external_identifier_access_shadowing.sol create mode 100644 test/libsolidity/syntaxTests/inlineAssembly/external_identifier_access_shadowing.sol diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index f15fe730e..7f36746d3 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -742,8 +742,15 @@ bool TypeChecker::visit(InlineAssembly const& _inlineAssembly) yul::Identifier const& _identifier, yul::IdentifierContext _context, bool - ) + ) -> bool { + if (_context == yul::IdentifierContext::NonExternal) + { + // Hack until we can disallow any shadowing: If we found an internal reference, + // clear the external references, so that codegen does not use it. + _inlineAssembly.annotation().externalReferences.erase(& _identifier); + return false; + } auto ref = _inlineAssembly.annotation().externalReferences.find(&_identifier); if (ref == _inlineAssembly.annotation().externalReferences.end()) return false; diff --git a/libsolidity/codegen/CompilerContext.cpp b/libsolidity/codegen/CompilerContext.cpp index 3a939dcd7..5503c3ecd 100644 --- a/libsolidity/codegen/CompilerContext.cpp +++ b/libsolidity/codegen/CompilerContext.cpp @@ -411,6 +411,7 @@ void CompilerContext::appendInlineAssembly( yul::AbstractAssembly& _assembly ) { + solAssert(_context == yul::IdentifierContext::RValue || _context == yul::IdentifierContext::LValue, ""); auto it = std::find(_localVariables.begin(), _localVariables.end(), _identifier.name.str()); solAssert(it != _localVariables.end(), ""); auto stackDepth = static_cast(distance(it, _localVariables.end())); diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index 378179d5e..6c5a0abe0 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -708,6 +708,7 @@ bool ContractCompiler::visit(InlineAssembly const& _inlineAssembly) yul::AbstractAssembly& _assembly ) { + solAssert(_context == yul::IdentifierContext::RValue || _context == yul::IdentifierContext::LValue, ""); auto ref = _inlineAssembly.annotation().externalReferences.find(&_identifier); solAssert(ref != _inlineAssembly.annotation().externalReferences.end(), ""); Declaration const* decl = ref->second.declaration; diff --git a/libyul/AsmAnalysis.cpp b/libyul/AsmAnalysis.cpp index 43b78d6b2..1c0867568 100644 --- a/libyul/AsmAnalysis.cpp +++ b/libyul/AsmAnalysis.cpp @@ -146,6 +146,13 @@ vector AsmAnalyzer::operator()(Identifier const& _identifier) } })) { + if (m_resolver) + // We found a local reference, make sure there is no external reference. + m_resolver( + _identifier, + yul::IdentifierContext::NonExternal, + m_currentScope->insideFunction() + ); } else { @@ -308,7 +315,7 @@ vector AsmAnalyzer::operator()(FunctionCall const& _funCall) validateInstructions(_funCall); } - else if (!m_currentScope->lookup(_funCall.functionName.name, GenericVisitor{ + else if (m_currentScope->lookup(_funCall.functionName.name, GenericVisitor{ [&](Scope::Variable const&) { m_errorReporter.typeError( @@ -323,6 +330,16 @@ vector AsmAnalyzer::operator()(FunctionCall const& _funCall) returnTypes = &_fun.returns; } })) + { + if (m_resolver) + // We found a local reference, make sure there is no external reference. + m_resolver( + _funCall.functionName, + yul::IdentifierContext::NonExternal, + m_currentScope->insideFunction() + ); + } + else { if (!validateInstructions(_funCall)) m_errorReporter.declarationError( @@ -539,6 +556,14 @@ void AsmAnalyzer::checkAssignment(Identifier const& _variable, YulString _valueT bool found = false; if (Scope::Identifier const* var = m_currentScope->lookup(_variable.name)) { + if (m_resolver) + // We found a local reference, make sure there is no external reference. + m_resolver( + _variable, + yul::IdentifierContext::NonExternal, + m_currentScope->insideFunction() + ); + if (!holds_alternative(*var)) m_errorReporter.typeError(2657_error, _variable.debugData->location, "Assignment requires variable."); else if (!m_activeVariables.count(&std::get(*var))) diff --git a/libyul/backends/evm/AbstractAssembly.h b/libyul/backends/evm/AbstractAssembly.h index 9e1a28dab..60a745bee 100644 --- a/libyul/backends/evm/AbstractAssembly.h +++ b/libyul/backends/evm/AbstractAssembly.h @@ -117,7 +117,7 @@ public: virtual void markAsInvalid() = 0; }; -enum class IdentifierContext { LValue, RValue, VariableDeclaration }; +enum class IdentifierContext { LValue, RValue, VariableDeclaration, NonExternal }; /// 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/semanticTests/inlineAssembly/external_identifier_access_shadowing.sol b/test/libsolidity/semanticTests/inlineAssembly/external_identifier_access_shadowing.sol new file mode 100644 index 000000000..f21dddfb4 --- /dev/null +++ b/test/libsolidity/semanticTests/inlineAssembly/external_identifier_access_shadowing.sol @@ -0,0 +1,12 @@ +contract C { + function f() public returns (uint x) { + assembly { + function g() -> f { f := 2 } + x := g() + } + } +} +// ==== +// compileViaYul: also +// ---- +// f() -> 2 diff --git a/test/libsolidity/syntaxTests/inlineAssembly/external_identifier_access_shadowing.sol b/test/libsolidity/syntaxTests/inlineAssembly/external_identifier_access_shadowing.sol new file mode 100644 index 000000000..fb073bd38 --- /dev/null +++ b/test/libsolidity/syntaxTests/inlineAssembly/external_identifier_access_shadowing.sol @@ -0,0 +1,12 @@ +contract C { + function f() public returns (uint x) { + assembly { + function g() -> x { + x := 42 + } + x := g() + } + } +} +// ---- +// DeclarationError 6578: (123-124): Cannot access local Solidity variables from inside an inline assembly function.