From 0fbea8a1a0724aa92ff6e45c6045827916cfaf6c Mon Sep 17 00:00:00 2001 From: Mathias Baumann Date: Mon, 18 Mar 2019 13:22:42 +0100 Subject: [PATCH 1/4] Change return type for interfaceType() to ResultType --- libsolidity/analysis/ReferencesResolver.cpp | 2 +- libsolidity/analysis/TypeChecker.cpp | 6 ++-- libsolidity/ast/Types.cpp | 20 ++++++------- libsolidity/ast/Types.h | 32 ++++++++++----------- libsolidity/interface/ABI.cpp | 8 +++--- 5 files changed, 34 insertions(+), 34 deletions(-) diff --git a/libsolidity/analysis/ReferencesResolver.cpp b/libsolidity/analysis/ReferencesResolver.cpp index d624a0135..492adb5fe 100644 --- a/libsolidity/analysis/ReferencesResolver.cpp +++ b/libsolidity/analysis/ReferencesResolver.cpp @@ -213,7 +213,7 @@ void ReferencesResolver::endVisit(FunctionTypeName const& _typeName) for (auto const& t: _typeName.parameterTypes() + _typeName.returnParameterTypes()) { solAssert(t->annotation().type, "Type not set for parameter."); - if (!t->annotation().type->interfaceType(false)) + if (!t->annotation().type->interfaceType(false).get()) { fatalTypeError(t->location(), "Internal type cannot be used for external function type."); return; diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index d1e4516b0..9399e7f88 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -355,7 +355,7 @@ bool TypeChecker::visit(FunctionDefinition const& _function) { if (!type(var)->canLiveOutsideStorage() && _function.isPublic()) m_errorReporter.typeError(var.location(), "Type is required to live outside storage."); - if (_function.isPublic() && !(type(var)->interfaceType(isLibraryFunction))) + if (_function.isPublic() && !(type(var)->interfaceType(isLibraryFunction).get())) m_errorReporter.fatalTypeError(var.location(), "Internal or recursive type is not allowed for public or external functions."); } if ( @@ -576,7 +576,7 @@ bool TypeChecker::visit(EventDefinition const& _eventDef) numIndexed++; if (!type(*var)->canLiveOutsideStorage()) m_errorReporter.typeError(var->location(), "Type is required to live outside storage."); - if (!type(*var)->interfaceType(false)) + if (!type(*var)->interfaceType(false).get()) m_errorReporter.typeError(var->location(), "Internal or recursive type is not allowed as event parameter type."); if ( !_eventDef.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::ABIEncoderV2) && @@ -599,7 +599,7 @@ void TypeChecker::endVisit(FunctionTypeName const& _funType) { FunctionType const& fun = dynamic_cast(*_funType.annotation().type); if (fun.kind() == FunctionType::Kind::External) - solAssert(fun.interfaceType(false), "External function type uses internal types."); + solAssert(fun.interfaceType(false).get(), "External function type uses internal types."); } bool TypeChecker::visit(InlineAssembly const& _inlineAssembly) diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index 3ba1f928d..754f006ba 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -1869,7 +1869,7 @@ TypePointer ArrayType::decodingType() const return shared_from_this(); } -TypePointer ArrayType::interfaceType(bool _inLibrary) const +TypeResult ArrayType::interfaceType(bool _inLibrary) const { if (_inLibrary && m_interfaceType_library.is_initialized()) return *m_interfaceType_library; @@ -2135,7 +2135,7 @@ MemberList::MemberMap StructType::nativeMembers(ContractDefinition const*) const return members; } -TypePointer StructType::interfaceType(bool _inLibrary) const +TypeResult StructType::interfaceType(bool _inLibrary) const { if (_inLibrary && m_interfaceType_library.is_initialized()) return *m_interfaceType_library; @@ -2168,7 +2168,7 @@ TypePointer StructType::interfaceType(bool _inLibrary) const allOkay = false; break; } - if (!var->annotation().type->interfaceType(false)) + if (!var->annotation().type->interfaceType(false).get()) { allOkay = false; break; @@ -2204,8 +2204,8 @@ string StructType::signatureInExternalFunction(bool _structsByName) const { solAssert(_t, "Parameter should have external type."); auto t = _t->interfaceType(_structsByName); - solAssert(t, ""); - return t->signatureInExternalFunction(_structsByName); + solAssert(t.get(), ""); + return t.get()->signatureInExternalFunction(_structsByName); }); return "(" + boost::algorithm::join(memberTypeStrings, ",") + ")"; } @@ -2574,7 +2574,7 @@ FunctionType::FunctionType(FunctionTypeName const& _typeName): solAssert(t->annotation().type, "Type not set for parameter."); if (m_kind == Kind::External) solAssert( - t->annotation().type->interfaceType(false), + t->annotation().type->interfaceType(false).get(), "Internal type used as parameter for external function." ); m_parameterTypes.push_back(t->annotation().type); @@ -2584,7 +2584,7 @@ FunctionType::FunctionType(FunctionTypeName const& _typeName): solAssert(t->annotation().type, "Type not set for return parameter."); if (m_kind == Kind::External) solAssert( - t->annotation().type->interfaceType(false), + t->annotation().type->interfaceType(false).get(), "Internal type used as return parameter for external function." ); m_returnParameterTypes.push_back(t->annotation().type); @@ -2885,14 +2885,14 @@ FunctionTypePointer FunctionType::interfaceFunctionType() const for (auto type: m_parameterTypes) { - if (auto ext = type->interfaceType(isLibraryFunction)) + if (auto ext = type->interfaceType(isLibraryFunction).get()) paramTypes.push_back(ext); else return FunctionTypePointer(); } for (auto type: m_returnParameterTypes) { - if (auto ext = type->interfaceType(isLibraryFunction)) + if (auto ext = type->interfaceType(isLibraryFunction).get()) retParamTypes.push_back(ext); else return FunctionTypePointer(); @@ -2978,7 +2978,7 @@ TypePointer FunctionType::encodingType() const return TypePointer(); } -TypePointer FunctionType::interfaceType(bool /*_inLibrary*/) const +TypeResult FunctionType::interfaceType(bool /*_inLibrary*/) const { if (m_kind == Kind::External) return shared_from_this(); diff --git a/libsolidity/ast/Types.h b/libsolidity/ast/Types.h index a6c299187..b79baa602 100644 --- a/libsolidity/ast/Types.h +++ b/libsolidity/ast/Types.h @@ -304,7 +304,7 @@ public: /// If there is no such type, returns an empty shared pointer. /// @param _inLibrary if set, returns types as used in a library, e.g. struct and contract types /// are returned without modification. - virtual TypePointer interfaceType(bool /*_inLibrary*/) const { return TypePointer(); } + virtual TypeResult interfaceType(bool /*_inLibrary*/) const { return TypePointer(); } private: /// @returns a member list containing all members added to this type by `using for` directives. @@ -355,7 +355,7 @@ public: u256 literalValue(Literal const* _literal) const override; TypePointer encodingType() const override { return shared_from_this(); } - TypePointer interfaceType(bool) const override { return shared_from_this(); } + TypeResult interfaceType(bool) const override { return shared_from_this(); } StateMutability stateMutability(void) const { return m_stateMutability; } @@ -395,7 +395,7 @@ public: std::string toString(bool _short) const override; TypePointer encodingType() const override { return shared_from_this(); } - TypePointer interfaceType(bool) const override { return shared_from_this(); } + TypeResult interfaceType(bool) const override { return shared_from_this(); } unsigned numBits() const { return m_bits; } bool isSigned() const { return m_modifier == Modifier::Signed; } @@ -437,7 +437,7 @@ public: std::string toString(bool _short) const override; TypePointer encodingType() const override { return shared_from_this(); } - TypePointer interfaceType(bool) const override { return shared_from_this(); } + TypeResult interfaceType(bool) const override { return shared_from_this(); } /// Number of bits used for this type in total. unsigned numBits() const { return m_totalBits; } @@ -583,7 +583,7 @@ public: std::string toString(bool) const override { return "bytes" + dev::toString(m_bytes); } MemberList::MemberMap nativeMembers(ContractDefinition const*) const override; TypePointer encodingType() const override { return shared_from_this(); } - TypePointer interfaceType(bool) const override { return shared_from_this(); } + TypeResult interfaceType(bool) const override { return shared_from_this(); } unsigned numBytes() const { return m_bytes; } @@ -609,7 +609,7 @@ public: std::string toString(bool) const override { return "bool"; } u256 literalValue(Literal const* _literal) const override; TypePointer encodingType() const override { return shared_from_this(); } - TypePointer interfaceType(bool) const override { return shared_from_this(); } + TypeResult interfaceType(bool) const override { return shared_from_this(); } }; /** @@ -716,7 +716,7 @@ public: MemberList::MemberMap nativeMembers(ContractDefinition const* _currentScope) const override; TypePointer encodingType() const override; TypePointer decodingType() const override; - TypePointer interfaceType(bool _inLibrary) const override; + TypeResult interfaceType(bool _inLibrary) const override; /// @returns true if this is valid to be stored in calldata bool validForCalldata() const; @@ -749,8 +749,8 @@ private: TypePointer m_baseType; bool m_hasDynamicLength = true; u256 m_length; - mutable boost::optional m_interfaceType; - mutable boost::optional m_interfaceType_library; + mutable boost::optional m_interfaceType; + mutable boost::optional m_interfaceType_library; }; /** @@ -788,7 +788,7 @@ public: return TypePointer{}; return std::make_shared(isPayable() ? StateMutability::Payable : StateMutability::NonPayable); } - TypePointer interfaceType(bool _inLibrary) const override + TypeResult interfaceType(bool _inLibrary) const override { if (isSuper()) return TypePointer{}; @@ -842,7 +842,7 @@ public: { return location() == DataLocation::Storage ? std::make_shared(256) : shared_from_this(); } - TypePointer interfaceType(bool _inLibrary) const override; + TypeResult interfaceType(bool _inLibrary) const override; TypePointer copyForLocation(DataLocation _location, bool _isPointer) const override; @@ -872,8 +872,8 @@ private: StructDefinition const& m_struct; /// Cache for the recursive() function. mutable boost::optional m_recursive; - mutable boost::optional m_interfaceType; - mutable boost::optional m_interfaceType_library; + mutable boost::optional m_interfaceType; + mutable boost::optional m_interfaceType_library; }; /** @@ -902,7 +902,7 @@ public: { return std::make_shared(8 * int(storageBytes())); } - TypePointer interfaceType(bool _inLibrary) const override + TypeResult interfaceType(bool _inLibrary) const override { return _inLibrary ? shared_from_this() : encodingType(); } @@ -1098,7 +1098,7 @@ public: bool hasSimpleZeroValueInMemory() const override { return false; } MemberList::MemberMap nativeMembers(ContractDefinition const* _currentScope) const override; TypePointer encodingType() const override; - TypePointer interfaceType(bool _inLibrary) const override; + TypeResult interfaceType(bool _inLibrary) const override; /// @returns TypePointer of a new FunctionType object. All input/return parameters are an /// appropriate external types (i.e. the interfaceType()s) of input/return parameters of @@ -1223,7 +1223,7 @@ public: { return std::make_shared(256); } - TypePointer interfaceType(bool _inLibrary) const override + TypeResult interfaceType(bool _inLibrary) const override { return _inLibrary ? shared_from_this() : TypePointer(); } diff --git a/libsolidity/interface/ABI.cpp b/libsolidity/interface/ABI.cpp index f8a9c7ec8..e97bd5b65 100644 --- a/libsolidity/interface/ABI.cpp +++ b/libsolidity/interface/ABI.cpp @@ -107,9 +107,9 @@ Json::Value ABI::generate(ContractDefinition const& _contractDef) for (auto const& p: it->parameters()) { auto type = p->annotation().type->interfaceType(false); - solAssert(type, ""); + solAssert(type.get(), ""); Json::Value input; - auto param = formatType(p->name(), *type, false); + auto param = formatType(p->name(), *type.get(), false); param["indexed"] = p->isIndexed(); params.append(param); } @@ -173,8 +173,8 @@ Json::Value ABI::formatType(string const& _name, Type const& _type, bool _forLib { solAssert(member.type, ""); auto t = member.type->interfaceType(_forLibrary); - solAssert(t, ""); - ret["components"].append(formatType(member.name, *t, _forLibrary)); + solAssert(t.get(), ""); + ret["components"].append(formatType(member.name, *t.get(), _forLibrary)); } } else From 8e899a0d32de5032ec9986e14cf1161e657bd061 Mon Sep 17 00:00:00 2001 From: Mathias Baumann Date: Thu, 7 Mar 2019 17:12:10 +0100 Subject: [PATCH 2/4] Disallow internal function types as parameters for public/external library function --- Changelog.md | 1 + libsolidity/analysis/TypeChecker.cpp | 12 +- libsolidity/ast/Types.cpp | 161 +++++++++++------- libsolidity/ast/Types.h | 13 +- ...external_parameter_in_library_external.sol | 6 + ...external_parameter_in_library_external.sol | 6 + ...nternal_function_as_external_parameter.sol | 2 +- ...external_parameter_in_library_external.sol | 2 +- ...function_returned_from_public_function.sol | 2 +- ...external_parameter_in_library_external.sol | 9 + ...cts_of_non_external_types_in_interface.sol | 2 +- ...s_of_non_external_types_in_interface_2.sol | 2 +- ...non_external_types_in_interface_nested.sol | 2 +- ..._struct_as_contract_function_parameter.sol | 10 ++ ...e_struct_as_library_function_parameter.sol | 9 + ...t_as_memory_library_function_parameter.sol | 14 ++ .../recursive_struct_forward_reference.sol | 2 +- ...function_as_library_function_parameter.sol | 11 ++ .../recursion/return_recursive_structs.sol | 2 +- .../recursion/return_recursive_structs2.sol | 2 +- .../recursion/return_recursive_structs3.sol | 2 +- ..._data_location_function_param_external.sol | 2 +- 22 files changed, 188 insertions(+), 86 deletions(-) create mode 100644 test/libsolidity/syntaxTests/functionTypes/internal_function_array_memory_as_external_parameter_in_library_external.sol create mode 100644 test/libsolidity/syntaxTests/functionTypes/internal_function_array_storage_as_external_parameter_in_library_external.sol create mode 100644 test/libsolidity/syntaxTests/functionTypes/internal_function_struct_as_external_parameter_in_library_external.sol create mode 100644 test/libsolidity/syntaxTests/structs/recursion/recursive_struct_as_contract_function_parameter.sol create mode 100644 test/libsolidity/syntaxTests/structs/recursion/recursive_struct_as_library_function_parameter.sol create mode 100644 test/libsolidity/syntaxTests/structs/recursion/recursive_struct_as_memory_library_function_parameter.sol create mode 100644 test/libsolidity/syntaxTests/structs/recursion/recursive_struct_with_internal_function_as_library_function_parameter.sol diff --git a/Changelog.md b/Changelog.md index 49bc87bee..3292041fe 100644 --- a/Changelog.md +++ b/Changelog.md @@ -13,6 +13,7 @@ Compiler Features: Bugfixes: * Code Generator: Defensively pad memory for ``type(Contract).name`` to multiples of 32. + * Type System: Detect and disallow internal function pointers as parameters for public/external library functions, even when they are nested/wrapped in structs, arrays or other types. * Yul Optimizer: Properly determine whether a variable can be eliminated during stack compression pass. diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 9399e7f88..d8a92097c 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -355,8 +355,16 @@ bool TypeChecker::visit(FunctionDefinition const& _function) { if (!type(var)->canLiveOutsideStorage() && _function.isPublic()) m_errorReporter.typeError(var.location(), "Type is required to live outside storage."); - if (_function.isPublic() && !(type(var)->interfaceType(isLibraryFunction).get())) - m_errorReporter.fatalTypeError(var.location(), "Internal or recursive type is not allowed for public or external functions."); + if (_function.isPublic()) + { + auto iType = type(var)->interfaceType(isLibraryFunction); + + if (!iType.get()) + { + solAssert(!iType.message().empty(), "Expected detailed error message!"); + m_errorReporter.fatalTypeError(var.location(), iType.message()); + } + } } if ( _function.isPublic() && diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index 754f006ba..fb321ea24 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -1877,23 +1877,22 @@ TypeResult ArrayType::interfaceType(bool _inLibrary) const if (!_inLibrary && m_interfaceType.is_initialized()) return *m_interfaceType; - TypePointer result; + TypeResult result{TypePointer{}}; + TypeResult baseExt = m_baseType->interfaceType(_inLibrary); - if (_inLibrary && location() == DataLocation::Storage) + if (!baseExt.get()) + { + solAssert(!baseExt.message().empty(), "Expected detailed error message!"); + result = baseExt; + } + else if (_inLibrary && location() == DataLocation::Storage) result = shared_from_this(); else if (m_arrayKind != ArrayKind::Ordinary) result = this->copyForLocation(DataLocation::Memory, true); + else if (isDynamicallySized()) + result = TypePointer{make_shared(DataLocation::Memory, baseExt)}; else - { - TypePointer baseExt = m_baseType->interfaceType(_inLibrary); - - if (!baseExt) - result = TypePointer(); - else if (isDynamicallySized()) - result = make_shared(DataLocation::Memory, baseExt); - else - result = make_shared(DataLocation::Memory, baseExt, m_length); - } + result = TypePointer{make_shared(DataLocation::Memory, baseExt, m_length)}; if (_inLibrary) m_interfaceType_library = result; @@ -2084,7 +2083,7 @@ unsigned StructType::calldataOffsetOfMember(std::string const& _member) const bool StructType::isDynamicallyEncoded() const { - solAssert(!recursive(), ""); + solAssert(interfaceType(false).get(), ""); for (auto t: memoryMemberTypes()) { solAssert(t, "Parameter should have external type."); @@ -2143,47 +2142,86 @@ TypeResult StructType::interfaceType(bool _inLibrary) const if (!_inLibrary && m_interfaceType.is_initialized()) return *m_interfaceType; - TypePointer result = TypePointer{}; + bool recursion = false; + TypeResult result{TypePointer{}}; - if (_inLibrary && location() == DataLocation::Storage) - result = shared_from_this(); - else if (recursive()) - result = TypePointer{}; - else + auto visitor = [&]( + StructDefinition const& _struct, + CycleDetector& _cycleDetector, + size_t /*_depth*/ + ) { // Check that all members have interface types. - // We pass "false" to canBeUsedExternally (_inLibrary), because this struct will be - // passed by value and thus the encoding does not differ, but it will disallow - // mappings. - // Also return false if at least one struct member does not have a type. - // This might happen, for example, if the type of the member does not exist, - // which is reported as an error. - bool allOkay = true; - for (auto const& var: m_struct.members()) + // Return an error if at least one struct member does not have a type. + // This might happen, for example, if the type of the member does not exist. + for (ASTPointer const& variable: _struct.members()) { // If the struct member does not have a type return false. // A TypeError is expected in this case. - if (!var->annotation().type) + if (!variable->annotation().type) { - allOkay = false; - break; + result = TypeResult::err("Invalid type!"); + return; } - if (!var->annotation().type->interfaceType(false).get()) + + Type const* memberType = variable->annotation().type.get(); + + while (dynamic_cast(memberType)) + memberType = dynamic_cast(memberType)->baseType().get(); + + if (StructType const* innerStruct = dynamic_cast(memberType)) + if ( + innerStruct->m_recursive == true || + _cycleDetector.run(innerStruct->structDefinition()) + ) + { + recursion = true; + if (_inLibrary && location() == DataLocation::Storage) + continue; + else + { + result = TypeResult::err("Recursive structs can only be passed as storage pointers to libraries, not as memory objects to contract functions."); + return; + } + } + + auto iType = memberType->interfaceType(_inLibrary); + if (!iType.get()) { - allOkay = false; - break; + solAssert(!iType.message().empty(), "Expected detailed error message!"); + result = iType; + return; } } - if (allOkay) - result = copyForLocation(DataLocation::Memory, true); - } + }; + + recursion = recursion || (CycleDetector(visitor).run(structDefinition()) != nullptr); + + std::string const recursiveErrMsg = "Recursive type not allowed for public or external contract functions."; if (_inLibrary) - m_interfaceType_library = result; - else - m_interfaceType = result; + { + if (!result.message().empty()) + m_interfaceType_library = result; + else if (location() == DataLocation::Storage) + m_interfaceType_library = shared_from_this(); + else + m_interfaceType_library = copyForLocation(DataLocation::Memory, true); - return result; + if (recursion) + m_interfaceType = TypeResult::err(recursiveErrMsg); + + return *m_interfaceType_library; + } + + if (recursion) + m_interfaceType = TypeResult::err(recursiveErrMsg); + else if (!result.message().empty()) + m_interfaceType = result; + else + m_interfaceType = copyForLocation(DataLocation::Memory, true); + + return *m_interfaceType; } TypePointer StructType::copyForLocation(DataLocation _location, bool _isPointer) const @@ -2273,27 +2311,6 @@ set StructType::membersMissingInMemory() const return missing; } -bool StructType::recursive() const -{ - if (!m_recursive.is_initialized()) - { - auto visitor = [&](StructDefinition const& _struct, CycleDetector& _cycleDetector, size_t /*_depth*/) - { - for (ASTPointer const& variable: _struct.members()) - { - Type const* memberType = variable->annotation().type.get(); - while (dynamic_cast(memberType)) - memberType = dynamic_cast(memberType)->baseType().get(); - if (StructType const* innerStruct = dynamic_cast(memberType)) - if (_cycleDetector.run(innerStruct->structDefinition())) - return; - } - }; - m_recursive = (CycleDetector(visitor).run(structDefinition()) != nullptr); - } - return *m_recursive; -} - TypeResult EnumType::unaryOperatorResult(Token _operator) const { return _operator == Token::Delete ? make_shared() : TypePointer(); @@ -2983,7 +3000,7 @@ TypeResult FunctionType::interfaceType(bool /*_inLibrary*/) const if (m_kind == Kind::External) return shared_from_this(); else - return TypePointer(); + return TypeResult::err("Internal type is not allowed for public or external functions."); } bool FunctionType::canTakeArguments( @@ -3277,6 +3294,26 @@ string MappingType::canonicalName() const return "mapping(" + keyType()->canonicalName() + " => " + valueType()->canonicalName() + ")"; } +TypeResult MappingType::interfaceType(bool _inLibrary) const +{ + solAssert(keyType()->interfaceType(_inLibrary).get(), "Must be an elementary type!"); + + if (_inLibrary) + { + auto iType = valueType()->interfaceType(_inLibrary); + + if (!iType.get()) + { + solAssert(!iType.message().empty(), "Expected detailed error message!"); + return iType; + } + } + else + return TypeResult::err("Only libraries are allowed to use the mapping type in public or external functions."); + + return shared_from_this(); +} + string TypeType::richIdentifier() const { return "t_type" + identifierList(actualType()); diff --git a/libsolidity/ast/Types.h b/libsolidity/ast/Types.h index b79baa602..c2445fc4c 100644 --- a/libsolidity/ast/Types.h +++ b/libsolidity/ast/Types.h @@ -863,15 +863,9 @@ public: TypePointers memoryMemberTypes() const; /// @returns the set of all members that are removed in the memory version (typically mappings). std::set membersMissingInMemory() const; - - /// @returns true if the same struct is used recursively in one of its members. Only - /// analyses the "memory" representation, i.e. mappings are ignored in all structs. - bool recursive() const; - private: StructDefinition const& m_struct; - /// Cache for the recursive() function. - mutable boost::optional m_recursive; + // Caches for interfaceType(bool) mutable boost::optional m_interfaceType; mutable boost::optional m_interfaceType_library; }; @@ -1223,10 +1217,7 @@ public: { return std::make_shared(256); } - TypeResult interfaceType(bool _inLibrary) const override - { - return _inLibrary ? shared_from_this() : TypePointer(); - } + TypeResult interfaceType(bool _inLibrary) const override; bool dataStoredIn(DataLocation _location) const override { return _location == DataLocation::Storage; } /// Cannot be stored in memory, but just in case. bool hasSimpleZeroValueInMemory() const override { solAssert(false, ""); } diff --git a/test/libsolidity/syntaxTests/functionTypes/internal_function_array_memory_as_external_parameter_in_library_external.sol b/test/libsolidity/syntaxTests/functionTypes/internal_function_array_memory_as_external_parameter_in_library_external.sol new file mode 100644 index 000000000..5ca4721b7 --- /dev/null +++ b/test/libsolidity/syntaxTests/functionTypes/internal_function_array_memory_as_external_parameter_in_library_external.sol @@ -0,0 +1,6 @@ +library L { + // Used to cause internal error + function f(function(uint) internal returns (uint)[] memory x) public { } +} +// ---- +// TypeError: (63-112): Internal type is not allowed for public or external functions. diff --git a/test/libsolidity/syntaxTests/functionTypes/internal_function_array_storage_as_external_parameter_in_library_external.sol b/test/libsolidity/syntaxTests/functionTypes/internal_function_array_storage_as_external_parameter_in_library_external.sol new file mode 100644 index 000000000..9ae10f44d --- /dev/null +++ b/test/libsolidity/syntaxTests/functionTypes/internal_function_array_storage_as_external_parameter_in_library_external.sol @@ -0,0 +1,6 @@ +library L { + // Used to cause internal error + function g(function(uint) internal returns (uint)[] storage x) public { } +} +// ---- +// TypeError: (63-113): Internal type is not allowed for public or external functions. diff --git a/test/libsolidity/syntaxTests/functionTypes/internal_function_as_external_parameter.sol b/test/libsolidity/syntaxTests/functionTypes/internal_function_as_external_parameter.sol index fa92d5597..c3aaa30cc 100644 --- a/test/libsolidity/syntaxTests/functionTypes/internal_function_as_external_parameter.sol +++ b/test/libsolidity/syntaxTests/functionTypes/internal_function_as_external_parameter.sol @@ -5,4 +5,4 @@ contract C { } } // ---- -// TypeError: (124-164): Internal or recursive type is not allowed for public or external functions. +// TypeError: (124-164): Internal type is not allowed for public or external functions. diff --git a/test/libsolidity/syntaxTests/functionTypes/internal_function_as_external_parameter_in_library_external.sol b/test/libsolidity/syntaxTests/functionTypes/internal_function_as_external_parameter_in_library_external.sol index b37fb285c..d464dc35e 100644 --- a/test/libsolidity/syntaxTests/functionTypes/internal_function_as_external_parameter_in_library_external.sol +++ b/test/libsolidity/syntaxTests/functionTypes/internal_function_as_external_parameter_in_library_external.sol @@ -3,4 +3,4 @@ library L { } } // ---- -// TypeError: (27-67): Internal or recursive type is not allowed for public or external functions. +// TypeError: (27-67): Internal type is not allowed for public or external functions. diff --git a/test/libsolidity/syntaxTests/functionTypes/internal_function_returned_from_public_function.sol b/test/libsolidity/syntaxTests/functionTypes/internal_function_returned_from_public_function.sol index 41fcd0a44..5b36cc8b8 100644 --- a/test/libsolidity/syntaxTests/functionTypes/internal_function_returned_from_public_function.sol +++ b/test/libsolidity/syntaxTests/functionTypes/internal_function_returned_from_public_function.sol @@ -4,4 +4,4 @@ contract C { } } // ---- -// TypeError: (129-169): Internal or recursive type is not allowed for public or external functions. +// TypeError: (129-169): Internal type is not allowed for public or external functions. diff --git a/test/libsolidity/syntaxTests/functionTypes/internal_function_struct_as_external_parameter_in_library_external.sol b/test/libsolidity/syntaxTests/functionTypes/internal_function_struct_as_external_parameter_in_library_external.sol new file mode 100644 index 000000000..02d5cca91 --- /dev/null +++ b/test/libsolidity/syntaxTests/functionTypes/internal_function_struct_as_external_parameter_in_library_external.sol @@ -0,0 +1,9 @@ +library L { + struct S + { + function(uint) internal returns (uint)[] x; + } + function f(S storage s) public { } +} +// ---- +// TypeError: (104-115): Internal type is not allowed for public or external functions. diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/041_functions_with_stucts_of_non_external_types_in_interface.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/041_functions_with_stucts_of_non_external_types_in_interface.sol index 73b608aed..57c60d893 100644 --- a/test/libsolidity/syntaxTests/nameAndTypeResolution/041_functions_with_stucts_of_non_external_types_in_interface.sol +++ b/test/libsolidity/syntaxTests/nameAndTypeResolution/041_functions_with_stucts_of_non_external_types_in_interface.sol @@ -6,4 +6,4 @@ contract C { } // ---- // Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments. -// TypeError: (103-111): Internal or recursive type is not allowed for public or external functions. +// TypeError: (103-111): Internal type is not allowed for public or external functions. diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/042_functions_with_stucts_of_non_external_types_in_interface_2.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/042_functions_with_stucts_of_non_external_types_in_interface_2.sol index 607a4a685..1d1da7f1f 100644 --- a/test/libsolidity/syntaxTests/nameAndTypeResolution/042_functions_with_stucts_of_non_external_types_in_interface_2.sol +++ b/test/libsolidity/syntaxTests/nameAndTypeResolution/042_functions_with_stucts_of_non_external_types_in_interface_2.sol @@ -6,4 +6,4 @@ contract C { } // ---- // Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments. -// TypeError: (105-113): Internal or recursive type is not allowed for public or external functions. +// TypeError: (105-113): Only libraries are allowed to use the mapping type in public or external functions. diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/043_functions_with_stucts_of_non_external_types_in_interface_nested.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/043_functions_with_stucts_of_non_external_types_in_interface_nested.sol index da73d8ddf..0ddcf438a 100644 --- a/test/libsolidity/syntaxTests/nameAndTypeResolution/043_functions_with_stucts_of_non_external_types_in_interface_nested.sol +++ b/test/libsolidity/syntaxTests/nameAndTypeResolution/043_functions_with_stucts_of_non_external_types_in_interface_nested.sol @@ -7,4 +7,4 @@ contract C { } // ---- // Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments. -// TypeError: (132-140): Internal or recursive type is not allowed for public or external functions. +// TypeError: (132-140): Only libraries are allowed to use the mapping type in public or external functions. diff --git a/test/libsolidity/syntaxTests/structs/recursion/recursive_struct_as_contract_function_parameter.sol b/test/libsolidity/syntaxTests/structs/recursion/recursive_struct_as_contract_function_parameter.sol new file mode 100644 index 000000000..4bce69bca --- /dev/null +++ b/test/libsolidity/syntaxTests/structs/recursion/recursive_struct_as_contract_function_parameter.sol @@ -0,0 +1,10 @@ +contract Test { + struct MyStructName { + address addr; + MyStructName[] x; + } + + function f(MyStructName memory s) public {} +} +// ---- +// TypeError: (112-133): Recursive type not allowed for public or external contract functions. diff --git a/test/libsolidity/syntaxTests/structs/recursion/recursive_struct_as_library_function_parameter.sol b/test/libsolidity/syntaxTests/structs/recursion/recursive_struct_as_library_function_parameter.sol new file mode 100644 index 000000000..37685dc8d --- /dev/null +++ b/test/libsolidity/syntaxTests/structs/recursion/recursive_struct_as_library_function_parameter.sol @@ -0,0 +1,9 @@ +library Test { + struct MyStructName { + address addr; + MyStructName[] x; + } + + function f(MyStructName storage s) public {} +} +// ---- diff --git a/test/libsolidity/syntaxTests/structs/recursion/recursive_struct_as_memory_library_function_parameter.sol b/test/libsolidity/syntaxTests/structs/recursion/recursive_struct_as_memory_library_function_parameter.sol new file mode 100644 index 000000000..0c5d5dac9 --- /dev/null +++ b/test/libsolidity/syntaxTests/structs/recursion/recursive_struct_as_memory_library_function_parameter.sol @@ -0,0 +1,14 @@ +pragma experimental ABIEncoderV2; + +library Test { + struct MyStructName { + address addr; + MyStructName[] x; + } + + function f(MyStructName memory _x) public { + } +} +// ---- +// Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments. +// TypeError: (146-168): Recursive structs can only be passed as storage pointers to libraries, not as memory objects to contract functions. 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 d2a411ec8..e2d1a4d11 100644 --- a/test/libsolidity/syntaxTests/structs/recursion/recursive_struct_forward_reference.sol +++ b/test/libsolidity/syntaxTests/structs/recursion/recursive_struct_forward_reference.sol @@ -8,4 +8,4 @@ contract Data { } // ---- // Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments. -// TypeError: (63-78): Internal or recursive type is not allowed for public or external functions. +// TypeError: (63-78): Recursive type not allowed for public or external contract functions. diff --git a/test/libsolidity/syntaxTests/structs/recursion/recursive_struct_with_internal_function_as_library_function_parameter.sol b/test/libsolidity/syntaxTests/structs/recursion/recursive_struct_with_internal_function_as_library_function_parameter.sol new file mode 100644 index 000000000..f9418bea5 --- /dev/null +++ b/test/libsolidity/syntaxTests/structs/recursion/recursive_struct_with_internal_function_as_library_function_parameter.sol @@ -0,0 +1,11 @@ +library Test { + struct MyStructName { + address addr; + MyStructName[] x; + function() internal y; + } + + function f(MyStructName storage s) public {} +} +// ---- +// TypeError: (142-164): Internal type is not allowed for public or external functions. diff --git a/test/libsolidity/syntaxTests/structs/recursion/return_recursive_structs.sol b/test/libsolidity/syntaxTests/structs/recursion/return_recursive_structs.sol index c8f9185c1..22885e956 100644 --- a/test/libsolidity/syntaxTests/structs/recursion/return_recursive_structs.sol +++ b/test/libsolidity/syntaxTests/structs/recursion/return_recursive_structs.sol @@ -4,4 +4,4 @@ contract C { } } // ---- -// TypeError: (91-99): Internal or recursive type is not allowed for public or external functions. +// TypeError: (91-99): Recursive type not allowed for public or external contract functions. diff --git a/test/libsolidity/syntaxTests/structs/recursion/return_recursive_structs2.sol b/test/libsolidity/syntaxTests/structs/recursion/return_recursive_structs2.sol index a8b7ac759..2ead307d6 100644 --- a/test/libsolidity/syntaxTests/structs/recursion/return_recursive_structs2.sol +++ b/test/libsolidity/syntaxTests/structs/recursion/return_recursive_structs2.sol @@ -4,4 +4,4 @@ contract C { } } // ---- -// TypeError: (94-102): Internal or recursive type is not allowed for public or external functions. +// TypeError: (94-102): Recursive type not allowed for public or external contract functions. diff --git a/test/libsolidity/syntaxTests/structs/recursion/return_recursive_structs3.sol b/test/libsolidity/syntaxTests/structs/recursion/return_recursive_structs3.sol index 0a5b1bc8d..c47df25bf 100644 --- a/test/libsolidity/syntaxTests/structs/recursion/return_recursive_structs3.sol +++ b/test/libsolidity/syntaxTests/structs/recursion/return_recursive_structs3.sol @@ -5,4 +5,4 @@ contract C { } } // ---- -// TypeError: (119-129): Internal or recursive type is not allowed for public or external functions. +// TypeError: (119-129): Recursive type not allowed for public or external contract functions. diff --git a/test/libsolidity/syntaxTests/types/mapping/mapping_array_data_location_function_param_external.sol b/test/libsolidity/syntaxTests/types/mapping/mapping_array_data_location_function_param_external.sol index 0c29ebd8b..ffe757474 100644 --- a/test/libsolidity/syntaxTests/types/mapping/mapping_array_data_location_function_param_external.sol +++ b/test/libsolidity/syntaxTests/types/mapping/mapping_array_data_location_function_param_external.sol @@ -3,4 +3,4 @@ contract c { } // ---- // TypeError: (29-61): Type is required to live outside storage. -// TypeError: (29-61): Internal or recursive type is not allowed for public or external functions. +// TypeError: (29-61): Only libraries are allowed to use the mapping type in public or external functions. From 7d809df91a3f7d1c3e30008f81666b316e215d7e Mon Sep 17 00:00:00 2001 From: Mathias Baumann Date: Tue, 19 Mar 2019 11:47:58 +0100 Subject: [PATCH 3/4] Add back StructType::recursive() --- libsolidity/ast/Types.cpp | 11 ++++++----- libsolidity/ast/Types.h | 11 +++++++++++ 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index fb321ea24..1b3e0c723 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -2142,9 +2142,10 @@ TypeResult StructType::interfaceType(bool _inLibrary) const if (!_inLibrary && m_interfaceType.is_initialized()) return *m_interfaceType; - bool recursion = false; TypeResult result{TypePointer{}}; + m_recursive = false; + auto visitor = [&]( StructDefinition const& _struct, CycleDetector& _cycleDetector, @@ -2175,7 +2176,7 @@ TypeResult StructType::interfaceType(bool _inLibrary) const _cycleDetector.run(innerStruct->structDefinition()) ) { - recursion = true; + m_recursive = true; if (_inLibrary && location() == DataLocation::Storage) continue; else @@ -2195,7 +2196,7 @@ TypeResult StructType::interfaceType(bool _inLibrary) const } }; - recursion = recursion || (CycleDetector(visitor).run(structDefinition()) != nullptr); + m_recursive = m_recursive.get() || (CycleDetector(visitor).run(structDefinition()) != nullptr); std::string const recursiveErrMsg = "Recursive type not allowed for public or external contract functions."; @@ -2208,13 +2209,13 @@ TypeResult StructType::interfaceType(bool _inLibrary) const else m_interfaceType_library = copyForLocation(DataLocation::Memory, true); - if (recursion) + if (m_recursive.get()) m_interfaceType = TypeResult::err(recursiveErrMsg); return *m_interfaceType_library; } - if (recursion) + if (m_recursive.get()) m_interfaceType = TypeResult::err(recursiveErrMsg); else if (!result.message().empty()) m_interfaceType = result; diff --git a/libsolidity/ast/Types.h b/libsolidity/ast/Types.h index c2445fc4c..86ae52ff4 100644 --- a/libsolidity/ast/Types.h +++ b/libsolidity/ast/Types.h @@ -844,6 +844,16 @@ public: } TypeResult interfaceType(bool _inLibrary) const override; + bool recursive() const + { + if (m_recursive.is_initialized()) + return m_recursive.get(); + + interfaceType(false); + + return m_recursive.get(); + } + TypePointer copyForLocation(DataLocation _location, bool _isPointer) const override; std::string canonicalName() const override; @@ -868,6 +878,7 @@ private: // Caches for interfaceType(bool) mutable boost::optional m_interfaceType; mutable boost::optional m_interfaceType_library; + mutable boost::optional m_recursive; }; /** From 4c2b1c1f291fa68c7ee7b5dbbea1b0dae1f1e5c1 Mon Sep 17 00:00:00 2001 From: Mathias Baumann Date: Wed, 20 Mar 2019 11:39:34 +0100 Subject: [PATCH 4/4] ArrayType::interfaceType(): Rename local variable to make more sense --- libsolidity/ast/Types.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index 1b3e0c723..1d592489d 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -1878,21 +1878,21 @@ TypeResult ArrayType::interfaceType(bool _inLibrary) const return *m_interfaceType; TypeResult result{TypePointer{}}; - TypeResult baseExt = m_baseType->interfaceType(_inLibrary); + TypeResult baseInterfaceType = m_baseType->interfaceType(_inLibrary); - if (!baseExt.get()) + if (!baseInterfaceType.get()) { - solAssert(!baseExt.message().empty(), "Expected detailed error message!"); - result = baseExt; + solAssert(!baseInterfaceType.message().empty(), "Expected detailed error message!"); + result = baseInterfaceType; } else if (_inLibrary && location() == DataLocation::Storage) result = shared_from_this(); else if (m_arrayKind != ArrayKind::Ordinary) result = this->copyForLocation(DataLocation::Memory, true); else if (isDynamicallySized()) - result = TypePointer{make_shared(DataLocation::Memory, baseExt)}; + result = TypePointer{make_shared(DataLocation::Memory, baseInterfaceType)}; else - result = TypePointer{make_shared(DataLocation::Memory, baseExt, m_length)}; + result = TypePointer{make_shared(DataLocation::Memory, baseInterfaceType, m_length)}; if (_inLibrary) m_interfaceType_library = result;