From 6f06154eb5a0e9380bcc7775fafbb946f026397e Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Wed, 15 Apr 2020 11:37:39 +0200 Subject: [PATCH] Move direct struct recursion check to detect recursion in global structs. --- .../analysis/DeclarationTypeChecker.cpp | 37 ++++++++++++++++++- libsolidity/analysis/DeclarationTypeChecker.h | 3 ++ libsolidity/analysis/TypeChecker.cpp | 33 ----------------- libsolidity/analysis/TypeChecker.h | 1 - .../recursive_struct_forward_reference.sol | 2 +- .../syntaxTests/structs/struct_var_member.sol | 7 ++++ .../types/global_struct_recursive.sol | 8 ++++ 7 files changed, 54 insertions(+), 37 deletions(-) create mode 100644 test/libsolidity/syntaxTests/structs/struct_var_member.sol create mode 100644 test/libsolidity/syntaxTests/types/global_struct_recursive.sol diff --git a/libsolidity/analysis/DeclarationTypeChecker.cpp b/libsolidity/analysis/DeclarationTypeChecker.cpp index 71fd52f14..193ad83bd 100644 --- a/libsolidity/analysis/DeclarationTypeChecker.cpp +++ b/libsolidity/analysis/DeclarationTypeChecker.cpp @@ -23,6 +23,8 @@ #include +#include + #include using namespace std; @@ -79,10 +81,12 @@ bool DeclarationTypeChecker::visit(StructDefinition const& _struct) m_currentStructsSeen.insert(&_struct); - for (auto const& _member: _struct.members()) + for (auto const& member: _struct.members()) { m_recursiveStructSeen = false; - _member->accept(*this); + member->accept(*this); + solAssert(member->annotation().type, ""); + solAssert(member->annotation().type->canBeStored(), "Type cannot be used in struct."); if (m_recursiveStructSeen) hasRecursiveChild = true; } @@ -94,6 +98,29 @@ bool DeclarationTypeChecker::visit(StructDefinition const& _struct) if (m_currentStructsSeen.empty()) m_recursiveStructSeen = false; + // Check direct recursion, fatal error if detected. + auto visitor = [&](StructDefinition const& _struct, auto& _cycleDetector, size_t _depth) + { + if (_depth >= 256) + fatalDeclarationError(_struct.location(), "Struct definition exhausting cyclic dependency validator."); + + for (ASTPointer const& member: _struct.members()) + { + Type const* memberType = member->annotation().type; + while (auto arrayType = dynamic_cast(memberType)) + { + if (arrayType->isDynamicallySized()) + break; + memberType = arrayType->baseType(); + } + if (auto structType = dynamic_cast(memberType)) + if (_cycleDetector.run(structType->structDefinition())) + return; + } + }; + if (util::CycleDetector(visitor).run(_struct) != nullptr) + fatalTypeError(_struct.location(), "Recursive struct definition."); + return false; } @@ -342,6 +369,12 @@ void DeclarationTypeChecker::fatalTypeError(SourceLocation const& _location, str m_errorReporter.fatalTypeError(_location, _description); } +void DeclarationTypeChecker::fatalDeclarationError(SourceLocation const& _location, string const& _description) +{ + m_errorOccurred = true; + m_errorReporter.fatalDeclarationError(_location, _description); +} + bool DeclarationTypeChecker::check(ASTNode const& _node) { _node.accept(*this); diff --git a/libsolidity/analysis/DeclarationTypeChecker.h b/libsolidity/analysis/DeclarationTypeChecker.h index a3f2c0c8f..b704c4fd2 100644 --- a/libsolidity/analysis/DeclarationTypeChecker.h +++ b/libsolidity/analysis/DeclarationTypeChecker.h @@ -65,6 +65,9 @@ private: /// Adds a new error to the list of errors and throws to abort reference resolving. void fatalTypeError(langutil::SourceLocation const& _location, 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); + langutil::ErrorReporter& m_errorReporter; bool m_errorOccurred = false; langutil::EVMVersion m_evmVersion; diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 920b453d6..b17f5b773 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -289,39 +289,6 @@ void TypeChecker::endVisit(UsingForDirective const& _usingFor) m_errorReporter.fatalTypeError(_usingFor.libraryName().location(), "Library name expected."); } -bool TypeChecker::visit(StructDefinition const& _struct) -{ - for (ASTPointer const& member: _struct.members()) - solAssert(type(*member)->canBeStored(), "Type cannot be used in struct."); - - // Check recursion, fatal error if detected. - auto visitor = [&](StructDefinition const& _struct, CycleDetector& _cycleDetector, size_t _depth) - { - if (_depth >= 256) - m_errorReporter.fatalDeclarationError(_struct.location(), "Struct definition exhausting cyclic dependency validator."); - - for (ASTPointer const& member: _struct.members()) - { - Type const* memberType = type(*member); - while (auto arrayType = dynamic_cast(memberType)) - { - if (arrayType->isDynamicallySized()) - break; - memberType = arrayType->baseType(); - } - if (auto structType = dynamic_cast(memberType)) - if (_cycleDetector.run(structType->structDefinition())) - return; - } - }; - if (CycleDetector(visitor).run(_struct) != nullptr) - m_errorReporter.fatalTypeError(_struct.location(), "Recursive struct definition."); - - ASTNode::listAccept(_struct.members(), *this); - - return false; -} - bool TypeChecker::visit(FunctionDefinition const& _function) { bool isLibraryFunction = _function.inContractKind() == ContractKind::Library; diff --git a/libsolidity/analysis/TypeChecker.h b/libsolidity/analysis/TypeChecker.h index 5aaba15f7..585f7d95f 100644 --- a/libsolidity/analysis/TypeChecker.h +++ b/libsolidity/analysis/TypeChecker.h @@ -112,7 +112,6 @@ private: void endVisit(InheritanceSpecifier const& _inheritance) override; void endVisit(UsingForDirective const& _usingFor) override; - bool visit(StructDefinition const& _struct) override; bool visit(FunctionDefinition const& _function) override; bool visit(VariableDeclaration const& _variable) override; /// We need to do this manually because we want to pass the bases of the current contract in diff --git a/test/libsolidity/syntaxTests/structs/recursion/recursive_struct_forward_reference.sol b/test/libsolidity/syntaxTests/structs/recursion/recursive_struct_forward_reference.sol index 6e1ad1cf3..573f22744 100644 --- a/test/libsolidity/syntaxTests/structs/recursion/recursive_struct_forward_reference.sol +++ b/test/libsolidity/syntaxTests/structs/recursion/recursive_struct_forward_reference.sol @@ -4,7 +4,7 @@ contract C { function f(Data.S memory a) public {} } contract Data { - struct S { S x; } + struct S { S[] x; } } // ---- // TypeError: (63-78): Recursive type not allowed for public or external contract functions. diff --git a/test/libsolidity/syntaxTests/structs/struct_var_member.sol b/test/libsolidity/syntaxTests/structs/struct_var_member.sol new file mode 100644 index 000000000..04a274bcb --- /dev/null +++ b/test/libsolidity/syntaxTests/structs/struct_var_member.sol @@ -0,0 +1,7 @@ +contract C { + struct S { + var x; + } +} +// ---- +// ParserError: (27-30): Expected explicit type name. diff --git a/test/libsolidity/syntaxTests/types/global_struct_recursive.sol b/test/libsolidity/syntaxTests/types/global_struct_recursive.sol new file mode 100644 index 000000000..dc4becaae --- /dev/null +++ b/test/libsolidity/syntaxTests/types/global_struct_recursive.sol @@ -0,0 +1,8 @@ +struct s1 { s2 x; } +struct s2 { s1 y; } + +contract C { + // whatever +} +// ---- +// TypeError: (0-19): Recursive struct definition.