From b8e2baf5f4f630da6151371843dcfa8b7d357524 Mon Sep 17 00:00:00 2001 From: Mathias Baumann Date: Thu, 19 Dec 2019 12:13:48 +0000 Subject: [PATCH] Use yul::AstWalker to resolve assembly symbols --- libsolidity/analysis/ReferencesResolver.cpp | 167 +++++++++--------- libsolidity/analysis/ReferencesResolver.h | 13 +- .../ASTJSON/assembly/nested_functions.json | 166 +++++++++++++++++ .../ASTJSON/assembly/nested_functions.sol | 12 ++ .../assembly/nested_functions_legacy.json | 148 ++++++++++++++++ test/libsolidity/ASTJSON/assembly/switch.json | 18 +- .../ASTJSON/assembly/switch_legacy.json | 15 +- .../invalid/nested_function_local_access.sol | 12 ++ 8 files changed, 468 insertions(+), 83 deletions(-) create mode 100644 test/libsolidity/ASTJSON/assembly/nested_functions.json create mode 100644 test/libsolidity/ASTJSON/assembly/nested_functions.sol create mode 100644 test/libsolidity/ASTJSON/assembly/nested_functions_legacy.json create mode 100644 test/libsolidity/syntaxTests/inlineAssembly/invalid/nested_function_local_access.sol diff --git a/libsolidity/analysis/ReferencesResolver.cpp b/libsolidity/analysis/ReferencesResolver.cpp index 889ec4d2d..6765f6710 100644 --- a/libsolidity/analysis/ReferencesResolver.cpp +++ b/libsolidity/analysis/ReferencesResolver.cpp @@ -270,87 +270,10 @@ bool ReferencesResolver::visit(InlineAssembly const& _inlineAssembly) { m_resolver.warnVariablesNamedLikeInstructions(); - // Errors created in this stage are completely ignored because we do not yet know - // the type and size of external identifiers, which would result in false errors. - // The only purpose of this step is to fill the inline assembly annotation with - // external references. - ErrorList errors; - ErrorReporter errorsIgnored(errors); - yul::ExternalIdentifierAccess::Resolver resolver = - [&](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 - if (!declarations.empty()) - // the special identifier exists itself, we should not allow that. - return size_t(-1); - string realName = _identifier.name.str().substr(0, _identifier.name.str().size() - ( - isSlot ? - string("_slot").size() : - string("_offset").size() - )); - if (realName.empty()) - { - declarationError(_identifier.location, "In variable names _slot and _offset can only be used as a suffix."); - return size_t(-1); - } - declarations = m_resolver.nameFromCurrentScope(realName); - } - if (declarations.size() > 1) - { - declarationError(_identifier.location, "Multiple matching identifiers. Resolving overloaded identifiers is not supported."); - return size_t(-1); - } - else if (declarations.size() == 0) - return size_t(-1); - if (auto var = dynamic_cast(declarations.front())) - if (var->isLocalVariable() && _crossesFunctionBoundary) - { - declarationError(_identifier.location, "Cannot access local Solidity variables from inside an inline assembly function."); - return size_t(-1); - } - _inlineAssembly.annotation().externalReferences[&_identifier].isSlot = isSlot; - _inlineAssembly.annotation().externalReferences[&_identifier].isOffset = isOffset; - _inlineAssembly.annotation().externalReferences[&_identifier].declaration = declarations.front(); - return size_t(1); - }; + m_yulAnnotation = &_inlineAssembly.annotation(); + (*this)(_inlineAssembly.operations()); + m_yulAnnotation = nullptr; - // Will be re-generated later with correct information - // We use the latest EVM version because we will re-run it anyway. - yul::AsmAnalysisInfo analysisInfo; - yul::AsmAnalyzer( - analysisInfo, - errorsIgnored, - yul::EVMDialect::strictAssemblyForEVM(m_evmVersion), - resolver - ).analyze(_inlineAssembly.operations()); return false; } @@ -468,6 +391,90 @@ void ReferencesResolver::endVisit(VariableDeclaration const& _variable) _variable.annotation().type = type; } +void ReferencesResolver::operator()(yul::FunctionDefinition const& _function) +{ + bool wasInsideFunction = m_yulInsideFunction; + m_yulInsideFunction = true; + this->operator()(_function.body); + m_yulInsideFunction = wasInsideFunction; +} + +void ReferencesResolver::operator()(yul::Identifier const& _identifier) +{ + bool isSlot = boost::algorithm::ends_with(_identifier.name.str(), "_slot"); + bool isOffset = boost::algorithm::ends_with(_identifier.name.str(), "_offset"); + + auto declarations = m_resolver.nameFromCurrentScope(_identifier.name.str()); + if (isSlot || isOffset) + { + // special mode to access storage variables + if (!declarations.empty()) + // the special identifier exists itself, we should not allow that. + return; + string realName = _identifier.name.str().substr(0, _identifier.name.str().size() - ( + isSlot ? + string("_slot").size() : + string("_offset").size() + )); + if (realName.empty()) + { + declarationError(_identifier.location, "In variable names _slot and _offset can only be used as a suffix."); + return; + } + declarations = m_resolver.nameFromCurrentScope(realName); + } + if (declarations.size() > 1) + { + declarationError(_identifier.location, "Multiple matching identifiers. Resolving overloaded identifiers is not supported."); + return; + } + else if (declarations.size() == 0) + return; + if (auto var = dynamic_cast(declarations.front())) + if (var->isLocalVariable() && m_yulInsideFunction) + { + declarationError(_identifier.location, "Cannot access local Solidity variables from inside an inline assembly function."); + return; + } + + m_yulAnnotation->externalReferences[&_identifier].isSlot = isSlot; + m_yulAnnotation->externalReferences[&_identifier].isOffset = isOffset; + m_yulAnnotation->externalReferences[&_identifier].declaration = declarations.front(); +} + +void ReferencesResolver::operator()(yul::VariableDeclaration const& _varDecl) +{ + for (auto const& identifier: _varDecl.variables) + { + bool isSlot = boost::algorithm::ends_with(identifier.name.str(), "_slot"); + bool isOffset = boost::algorithm::ends_with(identifier.name.str(), "_offset"); + + 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." + ); + } + } + + if (_varDecl.value) + visit(*_varDecl.value); +} + void ReferencesResolver::typeError(SourceLocation const& _location, string const& _description) { m_errorOccurred = true; diff --git a/libsolidity/analysis/ReferencesResolver.h b/libsolidity/analysis/ReferencesResolver.h index a368f7d39..2bbce69d0 100644 --- a/libsolidity/analysis/ReferencesResolver.h +++ b/libsolidity/analysis/ReferencesResolver.h @@ -25,6 +25,7 @@ #include #include #include +#include #include #include @@ -45,7 +46,7 @@ class NameAndTypeResolver; * Resolves references to declarations (of variables and types) and also establishes the link * between a return statement and the return parameter list. */ -class ReferencesResolver: private ASTConstVisitor +class ReferencesResolver: private ASTConstVisitor, private yul::ASTWalker { public: ReferencesResolver( @@ -64,6 +65,9 @@ public: bool resolve(ASTNode const& _root); private: + using yul::ASTWalker::visit; + using yul::ASTWalker::operator(); + bool visit(Block const& _block) override; void endVisit(Block const& _block) override; bool visit(ForStatement const& _for) override; @@ -83,6 +87,10 @@ private: bool visit(Return const& _return) override; void endVisit(VariableDeclaration const& _variable) override; + void operator()(yul::FunctionDefinition const& _function) override; + void operator()(yul::Identifier const& _identifier) override; + void operator()(yul::VariableDeclaration const& _varDecl) override; + /// Adds a new error to the list of errors. void typeError(langutil::SourceLocation const& _location, std::string const& _description); @@ -105,6 +113,9 @@ private: std::vector m_returnParameters; bool const m_resolveInsideCode; bool m_errorOccurred = false; + + InlineAssemblyAnnotation* m_yulAnnotation = nullptr; + bool m_yulInsideFunction = false; }; } diff --git a/test/libsolidity/ASTJSON/assembly/nested_functions.json b/test/libsolidity/ASTJSON/assembly/nested_functions.json new file mode 100644 index 000000000..b990a963f --- /dev/null +++ b/test/libsolidity/ASTJSON/assembly/nested_functions.json @@ -0,0 +1,166 @@ +{ + "absolutePath": "a", + "exportedSymbols": + { + "C": + [ + 8 + ] + }, + "id": 9, + "nodeType": "SourceUnit", + "nodes": + [ + { + "abstract": false, + "baseContracts": [], + "contractDependencies": [], + "contractKind": "contract", + "documentation": null, + "fullyImplemented": true, + "id": 8, + "linearizedBaseContracts": + [ + 8 + ], + "name": "C", + "nodeType": "ContractDefinition", + "nodes": + [ + { + "body": + { + "id": 6, + "nodeType": "Block", + "src": "57:97:1", + "statements": + [ + { + "AST": + { + "nodeType": "YulBlock", + "src": "72:78:1", + "statements": + [ + { + "body": + { + "nodeType": "YulBlock", + "src": "94:50:1", + "statements": + [ + { + "body": + { + "nodeType": "YulBlock", + "src": "118:3:1", + "statements": [] + }, + "name": "f2", + "nodeType": "YulFunctionDefinition", + "src": "104:17:1" + }, + { + "nodeType": "YulAssignment", + "src": "130:6:1", + "value": + { + "kind": "number", + "nodeType": "YulLiteral", + "src": "135:1:1", + "type": "", + "value": "2" + }, + "variableNames": + [ + { + "name": "x", + "nodeType": "YulIdentifier", + "src": "130:1:1" + } + ] + } + ] + }, + "name": "f1", + "nodeType": "YulFunctionDefinition", + "src": "80:64:1" + } + ] + }, + "evmVersion": %EVMVERSION%, + "externalReferences": [], + "id": 5, + "nodeType": "InlineAssembly", + "src": "63:87:1" + } + ] + }, + "documentation": null, + "functionSelector": "26121ff0", + "id": 7, + "implemented": true, + "kind": "function", + "modifiers": [], + "name": "f", + "nodeType": "FunctionDefinition", + "overrides": null, + "parameters": + { + "id": 1, + "nodeType": "ParameterList", + "parameters": [], + "src": "25:2:1" + }, + "returnParameters": + { + "id": 4, + "nodeType": "ParameterList", + "parameters": + [ + { + "constant": false, + "id": 3, + "name": "x", + "nodeType": "VariableDeclaration", + "overrides": null, + "scope": 7, + "src": "49:6:1", + "stateVariable": false, + "storageLocation": "default", + "typeDescriptions": + { + "typeIdentifier": "t_uint256", + "typeString": "uint256" + }, + "typeName": + { + "id": 2, + "name": "uint", + "nodeType": "ElementaryTypeName", + "src": "49:4:1", + "typeDescriptions": + { + "typeIdentifier": "t_uint256", + "typeString": "uint256" + } + }, + "value": null, + "visibility": "internal" + } + ], + "src": "48:8:1" + }, + "scope": 8, + "src": "15:139:1", + "stateMutability": "pure", + "virtual": false, + "visibility": "public" + } + ], + "scope": 9, + "src": "0:156:1" + } + ], + "src": "0:157:1" +} diff --git a/test/libsolidity/ASTJSON/assembly/nested_functions.sol b/test/libsolidity/ASTJSON/assembly/nested_functions.sol new file mode 100644 index 000000000..412dd05a4 --- /dev/null +++ b/test/libsolidity/ASTJSON/assembly/nested_functions.sol @@ -0,0 +1,12 @@ +contract C { + function f() public pure returns (uint x) { + assembly { + function f1() { + function f2() { } + x := 2 + } + } + } +} + +// ---- diff --git a/test/libsolidity/ASTJSON/assembly/nested_functions_legacy.json b/test/libsolidity/ASTJSON/assembly/nested_functions_legacy.json new file mode 100644 index 000000000..abaf60fb7 --- /dev/null +++ b/test/libsolidity/ASTJSON/assembly/nested_functions_legacy.json @@ -0,0 +1,148 @@ +{ + "attributes": + { + "absolutePath": "a", + "exportedSymbols": + { + "C": + [ + 8 + ] + } + }, + "children": + [ + { + "attributes": + { + "abstract": false, + "baseContracts": + [ + null + ], + "contractDependencies": + [ + null + ], + "contractKind": "contract", + "documentation": null, + "fullyImplemented": true, + "linearizedBaseContracts": + [ + 8 + ], + "name": "C", + "scope": 9 + }, + "children": + [ + { + "attributes": + { + "documentation": null, + "functionSelector": "26121ff0", + "implemented": true, + "isConstructor": false, + "kind": "function", + "modifiers": + [ + null + ], + "name": "f", + "overrides": null, + "scope": 8, + "stateMutability": "pure", + "virtual": false, + "visibility": "public" + }, + "children": + [ + { + "attributes": + { + "parameters": + [ + null + ] + }, + "children": [], + "id": 1, + "name": "ParameterList", + "src": "25:2:1" + }, + { + "children": + [ + { + "attributes": + { + "constant": false, + "name": "x", + "overrides": null, + "scope": 7, + "stateVariable": false, + "storageLocation": "default", + "type": "uint256", + "value": null, + "visibility": "internal" + }, + "children": + [ + { + "attributes": + { + "name": "uint", + "type": "uint256" + }, + "id": 2, + "name": "ElementaryTypeName", + "src": "49:4:1" + } + ], + "id": 3, + "name": "VariableDeclaration", + "src": "49:6:1" + } + ], + "id": 4, + "name": "ParameterList", + "src": "48:8:1" + }, + { + "children": + [ + { + "attributes": + { + "evmVersion": %EVMVERSION%, + "externalReferences": + [ + null + ], + "operations": "{\n function f1()\n {\n function f2()\n { }\n x := 2\n }\n}" + }, + "children": [], + "id": 5, + "name": "InlineAssembly", + "src": "63:87:1" + } + ], + "id": 6, + "name": "Block", + "src": "57:97:1" + } + ], + "id": 7, + "name": "FunctionDefinition", + "src": "15:139:1" + } + ], + "id": 8, + "name": "ContractDefinition", + "src": "0:156:1" + } + ], + "id": 9, + "name": "SourceUnit", + "src": "0:157:1" +} diff --git a/test/libsolidity/ASTJSON/assembly/switch.json b/test/libsolidity/ASTJSON/assembly/switch.json index 5e16f8cfb..854c5b6cd 100644 --- a/test/libsolidity/ASTJSON/assembly/switch.json +++ b/test/libsolidity/ASTJSON/assembly/switch.json @@ -158,7 +158,23 @@ ] }, "evmVersion": %EVMVERSION%, - "externalReferences": [], + "externalReferences": + [ + { + "declaration": 5, + "isOffset": false, + "isSlot": false, + "src": "141:1:1", + "valueSize": 18446744073709551615 + }, + { + "declaration": 5, + "isOffset": false, + "isSlot": false, + "src": "172:1:1", + "valueSize": 18446744073709551615 + } + ], "id": 3, "nodeType": "InlineAssembly", "src": "52:138:1" diff --git a/test/libsolidity/ASTJSON/assembly/switch_legacy.json b/test/libsolidity/ASTJSON/assembly/switch_legacy.json index 9d055e52e..5b9322f4c 100644 --- a/test/libsolidity/ASTJSON/assembly/switch_legacy.json +++ b/test/libsolidity/ASTJSON/assembly/switch_legacy.json @@ -92,7 +92,20 @@ "evmVersion": %EVMVERSION%, "externalReferences": [ - null + { + "declaration": 5, + "isOffset": false, + "isSlot": false, + "src": "141:1:1", + "valueSize": 18446744073709551615 + }, + { + "declaration": 5, + "isOffset": false, + "isSlot": false, + "src": "172:1:1", + "valueSize": 18446744073709551615 + } ], "operations": "{\n let f := 0\n switch calldatasize()\n case 0 { f := 1 }\n default { f := 2 }\n}" }, diff --git a/test/libsolidity/syntaxTests/inlineAssembly/invalid/nested_function_local_access.sol b/test/libsolidity/syntaxTests/inlineAssembly/invalid/nested_function_local_access.sol new file mode 100644 index 000000000..3bb6223a8 --- /dev/null +++ b/test/libsolidity/syntaxTests/inlineAssembly/invalid/nested_function_local_access.sol @@ -0,0 +1,12 @@ +contract C { + function f() public pure returns (uint x) { + assembly { + function f1() { + function f2() { } + x := 2 + } + } + } +} +// ---- +// DeclarationError: (130-131): Cannot access local Solidity variables from inside an inline assembly function.