diff options
| -rw-r--r-- | Changelog.md | 1 | ||||
| -rw-r--r-- | libsolidity/analysis/NameAndTypeResolver.cpp | 141 | ||||
| -rw-r--r-- | libsolidity/analysis/NameAndTypeResolver.h | 3 | ||||
| -rw-r--r-- | libsolidity/analysis/ReferencesResolver.cpp | 9 | ||||
| -rw-r--r-- | libsolidity/analysis/ReferencesResolver.h | 2 | ||||
| -rw-r--r-- | libsolidity/ast/Types.cpp | 4 | ||||
| -rw-r--r-- | test/libsolidity/SolidityNameAndTypeResolution.cpp | 16 |
7 files changed, 103 insertions, 73 deletions
diff --git a/Changelog.md b/Changelog.md index bd514cfe..72947dab 100644 --- a/Changelog.md +++ b/Changelog.md @@ -9,6 +9,7 @@ Features: Bugfixes: * Commandline interface: Always escape filenames (replace ``/``, ``:`` and ``.`` with ``_``). * Commandline interface: Do not try creating paths ``.`` and ``..``. + * Type system: Fix a crash caused by continuing on fatal errors in the code. * Type system: Disallow arrays with negative length. ### 0.4.9 (2017-01-31) diff --git a/libsolidity/analysis/NameAndTypeResolver.cpp b/libsolidity/analysis/NameAndTypeResolver.cpp index 01384260..336dc894 100644 --- a/libsolidity/analysis/NameAndTypeResolver.cpp +++ b/libsolidity/analysis/NameAndTypeResolver.cpp @@ -21,6 +21,7 @@ */ #include <libsolidity/analysis/NameAndTypeResolver.h> + #include <libsolidity/ast/AST.h> #include <libsolidity/analysis/TypeChecker.h> #include <libsolidity/interface/Exceptions.h> @@ -130,62 +131,9 @@ bool NameAndTypeResolver::performImports(SourceUnit& _sourceUnit, map<string, So bool NameAndTypeResolver::resolveNamesAndTypes(ASTNode& _node, bool _resolveInsideCode) { - bool success = true; try { - if (ContractDefinition* contract = dynamic_cast<ContractDefinition*>(&_node)) - { - m_currentScope = m_scopes[contract->scope()].get(); - solAssert(!!m_currentScope, ""); - - for (ASTPointer<InheritanceSpecifier> const& baseContract: contract->baseContracts()) - if (!resolveNamesAndTypes(*baseContract, true)) - success = false; - - m_currentScope = m_scopes[contract].get(); - - if (success) - { - linearizeBaseContracts(*contract); - vector<ContractDefinition const*> properBases( - ++contract->annotation().linearizedBaseContracts.begin(), - contract->annotation().linearizedBaseContracts.end() - ); - - for (ContractDefinition const* base: properBases) - importInheritedScope(*base); - } - - // these can contain code, only resolve parameters for now - for (ASTPointer<ASTNode> const& node: contract->subNodes()) - { - m_currentScope = m_scopes[contract].get(); - if (!resolveNamesAndTypes(*node, false)) - success = false; - } - - if (!success) - return false; - - if (!_resolveInsideCode) - return success; - - m_currentScope = m_scopes[contract].get(); - - // now resolve references inside the code - for (ASTPointer<ASTNode> const& node: contract->subNodes()) - { - m_currentScope = m_scopes[contract].get(); - if (!resolveNamesAndTypes(*node, true)) - success = false; - } - } - else - { - if (m_scopes.count(&_node)) - m_currentScope = m_scopes[&_node].get(); - return ReferencesResolver(m_errors, *this, _resolveInsideCode).resolve(_node); - } + return resolveNamesAndTypesInternal(_node, _resolveInsideCode); } catch (FatalError const&) { @@ -193,7 +141,6 @@ bool NameAndTypeResolver::resolveNamesAndTypes(ASTNode& _node, bool _resolveInsi throw; // Something is weird here, rather throw again. return false; } - return success; } bool NameAndTypeResolver::updateDeclaration(Declaration const& _declaration) @@ -249,21 +196,25 @@ vector<Declaration const*> NameAndTypeResolver::cleanedDeclarations( solAssert(_declarations.size() > 1, ""); vector<Declaration const*> uniqueFunctions; - for (auto it = _declarations.begin(); it != _declarations.end(); ++it) + for (Declaration const* declaration: _declarations) { - solAssert(*it, ""); + solAssert(declaration, ""); // the declaration is functionDefinition, eventDefinition or a VariableDeclaration while declarations > 1 - solAssert(dynamic_cast<FunctionDefinition const*>(*it) || dynamic_cast<EventDefinition const*>(*it) || dynamic_cast<VariableDeclaration const*>(*it), - "Found overloading involving something not a function or a variable"); + solAssert( + dynamic_cast<FunctionDefinition const*>(declaration) || + dynamic_cast<EventDefinition const*>(declaration) || + dynamic_cast<VariableDeclaration const*>(declaration), + "Found overloading involving something not a function or a variable." + ); - shared_ptr<FunctionType const> functionType { (*it)->functionType(false) }; + FunctionTypePointer functionType { declaration->functionType(false) }; if (!functionType) - functionType = (*it)->functionType(true); - solAssert(functionType, "failed to determine the function type of the overloaded"); + functionType = declaration->functionType(true); + solAssert(functionType, "Failed to determine the function type of the overloaded."); for (auto parameter: functionType->parameterTypes() + functionType->returnParameterTypes()) if (!parameter) - reportFatalDeclarationError(_identifier.location(), "Function type can not be used in this context"); + reportFatalDeclarationError(_identifier.location(), "Function type can not be used in this context."); if (uniqueFunctions.end() == find_if( uniqueFunctions.begin(), @@ -276,11 +227,73 @@ vector<Declaration const*> NameAndTypeResolver::cleanedDeclarations( return newFunctionType && functionType->hasEqualArgumentTypes(*newFunctionType); } )) - uniqueFunctions.push_back(*it); + uniqueFunctions.push_back(declaration); } return uniqueFunctions; } +bool NameAndTypeResolver::resolveNamesAndTypesInternal(ASTNode& _node, bool _resolveInsideCode) +{ + if (ContractDefinition* contract = dynamic_cast<ContractDefinition*>(&_node)) + { + bool success = true; + m_currentScope = m_scopes[contract->scope()].get(); + solAssert(!!m_currentScope, ""); + + for (ASTPointer<InheritanceSpecifier> const& baseContract: contract->baseContracts()) + if (!resolveNamesAndTypes(*baseContract, true)) + success = false; + + m_currentScope = m_scopes[contract].get(); + + if (success) + { + linearizeBaseContracts(*contract); + vector<ContractDefinition const*> properBases( + ++contract->annotation().linearizedBaseContracts.begin(), + contract->annotation().linearizedBaseContracts.end() + ); + + for (ContractDefinition const* base: properBases) + importInheritedScope(*base); + } + + // these can contain code, only resolve parameters for now + for (ASTPointer<ASTNode> const& node: contract->subNodes()) + { + m_currentScope = m_scopes[contract].get(); + if (!resolveNamesAndTypes(*node, false)) + { + success = false; + break; + } + } + + if (!success) + return false; + + if (!_resolveInsideCode) + return success; + + m_currentScope = m_scopes[contract].get(); + + // now resolve references inside the code + for (ASTPointer<ASTNode> const& node: contract->subNodes()) + { + m_currentScope = m_scopes[contract].get(); + if (!resolveNamesAndTypes(*node, true)) + success = false; + } + return success; + } + else + { + if (m_scopes.count(&_node)) + m_currentScope = m_scopes[&_node].get(); + return ReferencesResolver(m_errors, *this, _resolveInsideCode).resolve(_node); + } +} + void NameAndTypeResolver::importInheritedScope(ContractDefinition const& _base) { auto iterator = m_scopes.find(&_base); diff --git a/libsolidity/analysis/NameAndTypeResolver.h b/libsolidity/analysis/NameAndTypeResolver.h index 828b566f..038a887b 100644 --- a/libsolidity/analysis/NameAndTypeResolver.h +++ b/libsolidity/analysis/NameAndTypeResolver.h @@ -89,6 +89,9 @@ public: ); private: + /// Internal version of @a resolveNamesAndTypes (called from there) throws exceptions on fatal errors. + bool resolveNamesAndTypesInternal(ASTNode& _node, bool _resolveInsideCode = true); + /// Imports all members declared directly in the given contract (i.e. does not import inherited members) /// into the current scope if they are not present already. void importInheritedScope(ContractDefinition const& _base); diff --git a/libsolidity/analysis/ReferencesResolver.cpp b/libsolidity/analysis/ReferencesResolver.cpp index c06181d8..37bcb2d9 100644 --- a/libsolidity/analysis/ReferencesResolver.cpp +++ b/libsolidity/analysis/ReferencesResolver.cpp @@ -35,14 +35,7 @@ using namespace dev::solidity; bool ReferencesResolver::resolve(ASTNode const& _root) { - try - { - _root.accept(*this); - } - catch (FatalError const&) - { - solAssert(m_errorOccurred, ""); - } + _root.accept(*this); return !m_errorOccurred; } diff --git a/libsolidity/analysis/ReferencesResolver.h b/libsolidity/analysis/ReferencesResolver.h index 23ac6b07..dce343d3 100644 --- a/libsolidity/analysis/ReferencesResolver.h +++ b/libsolidity/analysis/ReferencesResolver.h @@ -52,7 +52,7 @@ public: m_resolveInsideCode(_resolveInsideCode) {} - /// @returns true if no errors during resolving + /// @returns true if no errors during resolving and throws exceptions on fatal errors. bool resolve(ASTNode const& _root); private: diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index 5b7b4a2c..96b3ed17 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -1652,6 +1652,7 @@ MemberList::MemberMap StructType::nativeMembers(ContractDefinition const*) const for (ASTPointer<VariableDeclaration> const& variable: m_struct.members()) { TypePointer type = variable->annotation().type; + solAssert(type, ""); // Skip all mapping members if we are not in storage. if (location() != DataLocation::Storage && !type->canLiveOutsideStorage()) continue; @@ -1964,6 +1965,8 @@ FunctionType::FunctionType(VariableDeclaration const& _varDecl): if (auto structType = dynamic_cast<StructType const*>(returnType.get())) { for (auto const& member: structType->members(nullptr)) + { + solAssert(member.type, ""); if (member.type->category() != Category::Mapping) { if (auto arrayType = dynamic_cast<ArrayType const*>(member.type.get())) @@ -1972,6 +1975,7 @@ FunctionType::FunctionType(VariableDeclaration const& _varDecl): retParams.push_back(member.type); retParamNames.push_back(member.name); } + } } else { diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 1a4f3cdc..507d9057 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -5079,6 +5079,22 @@ BOOST_AUTO_TEST_CASE(invalid_address_length) CHECK_WARNING(text, "checksum"); } +BOOST_AUTO_TEST_CASE(early_exit_on_fatal_errors) +{ + // This tests a crash that occured because we did not stop for fatal errors. + char const* text = R"( + contract C { + struct S { + ftring a; + } + S public s; + function s() s { + } + } + )"; + CHECK_ERROR(text, DeclarationError, "Identifier not found or not unique"); +} + BOOST_AUTO_TEST_SUITE_END() } |