diff --git a/Changelog.md b/Changelog.md index 51686bc740f6..ebaa17f24d89 100644 --- a/Changelog.md +++ b/Changelog.md @@ -2,6 +2,7 @@ Important Bugfixes: * ABI Encoder: When ABI-encoding values from calldata that contain nested arrays, correctly validate the nested array length against ``calldatasize()`` in all cases. + * Override Checker: Allow changing data location for parameters only when overriding external functions. Compiler Features: diff --git a/docs/bugs.json b/docs/bugs.json index 6613f1964d3d..7bee4d10f3d3 100644 --- a/docs/bugs.json +++ b/docs/bugs.json @@ -1,4 +1,15 @@ [ + { + "uid": "SOL-2022-3", + "name": "DataLocationChangeInInternalOverride", + "summary": "It was possible to change the data location of the parameters or return variables from ``calldata`` to ``memory`` and vice-versa while overriding internal and public functions. This caused invalid code to be generated when calling such a function internally through virtual function calls.", + "description": "When calling external functions, it is irrelevant if the data location of the parameters is ``calldata`` or ``memory``, the encoding of the data does not change. Because of that, changing the data location when overriding external functions is allowed. The compiler incorrectly also allowed a change in the data location for overriding public and internal functions. Since public functions can be called internally as well as externally, this causes invalid code to be generated when such an incorrectly overridden function is called internally through the base contract. The caller provides a memory pointer, but the called function interprets it as a calldata pointer or vice-versa.", + "link": "https://blog.soliditylang.org/2022/05/17/data-location-inheritance-bug/", + "introduced": "0.6.9", + "fixed": "0.8.14", + "severity": "very low" + + }, { "uid": "SOL-2022-2", "name": "NestedCallataArrayAbiReencodingSizeValidation", diff --git a/docs/bugs_by_version.json b/docs/bugs_by_version.json index 0d103279ff6e..d6ee8bb8a981 100644 --- a/docs/bugs_by_version.json +++ b/docs/bugs_by_version.json @@ -1345,6 +1345,7 @@ }, "0.6.10": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1356,6 +1357,7 @@ }, "0.6.11": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1367,6 +1369,7 @@ }, "0.6.12": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1477,6 +1480,7 @@ }, "0.6.9": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1489,6 +1493,7 @@ }, "0.7.0": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1500,6 +1505,7 @@ }, "0.7.1": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1512,6 +1518,7 @@ }, "0.7.2": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1523,6 +1530,7 @@ }, "0.7.3": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1533,6 +1541,7 @@ }, "0.7.4": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1542,6 +1551,7 @@ }, "0.7.5": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1551,6 +1561,7 @@ }, "0.7.6": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1560,6 +1571,7 @@ }, "0.8.0": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1569,6 +1581,7 @@ }, "0.8.1": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1578,12 +1591,14 @@ }, "0.8.10": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation" ], "released": "2021-11-09" }, "0.8.11": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "AbiEncodeCallLiteralAsFixedBytesBug" ], @@ -1591,6 +1606,7 @@ }, "0.8.12": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "AbiEncodeCallLiteralAsFixedBytesBug" ], @@ -1598,12 +1614,14 @@ }, "0.8.13": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation" ], "released": "2022-03-16" }, "0.8.2": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1613,6 +1631,7 @@ }, "0.8.3": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory" @@ -1621,6 +1640,7 @@ }, "0.8.4": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables" ], @@ -1628,6 +1648,7 @@ }, "0.8.5": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables" ], @@ -1635,6 +1656,7 @@ }, "0.8.6": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables" ], @@ -1642,6 +1664,7 @@ }, "0.8.7": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables" ], @@ -1649,6 +1672,7 @@ }, "0.8.8": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "UserDefinedValueTypesBug", "SignedImmutables" @@ -1657,6 +1681,7 @@ }, "0.8.9": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation" ], "released": "2021-09-29" diff --git a/libsolidity/analysis/ContractLevelChecker.cpp b/libsolidity/analysis/ContractLevelChecker.cpp index c159b835761d..9ecf76f1303c 100644 --- a/libsolidity/analysis/ContractLevelChecker.cpp +++ b/libsolidity/analysis/ContractLevelChecker.cpp @@ -39,7 +39,7 @@ namespace { template -bool hasEqualParameters(T const& _a, B const& _b) +bool hasEqualExternalCallableParameters(T const& _a, B const& _b) { return FunctionType(_a).asExternallyCallableFunction(false)->hasEqualParameterTypes( *FunctionType(_b).asExternallyCallableFunction(false) @@ -204,7 +204,7 @@ void ContractLevelChecker::findDuplicateDefinitions(map> const SecondarySourceLocation ssl; for (size_t j = i + 1; j < overloads.size(); ++j) - if (hasEqualParameters(*overloads[i], *overloads[j])) + if (hasEqualExternalCallableParameters(*overloads[i], *overloads[j])) { solAssert( ( diff --git a/libsolidity/analysis/OverrideChecker.cpp b/libsolidity/analysis/OverrideChecker.cpp index 2f83748fb4aa..d342ab100359 100644 --- a/libsolidity/analysis/OverrideChecker.cpp +++ b/libsolidity/analysis/OverrideChecker.cpp @@ -313,7 +313,7 @@ Token OverrideProxy::functionKind() const }, m_item); } -FunctionType const* OverrideProxy::functionType() const +FunctionType const* OverrideProxy::externalFunctionType() const { return std::visit(GenericVisitor{ [&](FunctionDefinition const* _item) { return FunctionType(*_item).asExternallyCallableFunction(false); }, @@ -322,6 +322,15 @@ FunctionType const* OverrideProxy::functionType() const }, m_item); } +FunctionType const* OverrideProxy::originalFunctionType() const +{ + return std::visit(GenericVisitor{ + [&](FunctionDefinition const* _item) { return TypeProvider::function(*_item); }, + [&](VariableDeclaration const*) -> FunctionType const* { solAssert(false, "Requested specific function type of variable."); return nullptr; }, + [&](ModifierDefinition const*) -> FunctionType const* { solAssert(false, "Requested specific function type of modifier."); return nullptr; } + }, m_item); +} + ModifierType const* OverrideProxy::modifierType() const { return std::visit(GenericVisitor{ @@ -413,7 +422,7 @@ OverrideProxy::OverrideComparator const& OverrideProxy::overrideComparator() con [&](FunctionDefinition const* _function) { vector paramTypes; - for (Type const* t: functionType()->parameterTypes()) + for (Type const* t: externalFunctionType()->parameterTypes()) paramTypes.emplace_back(t->richIdentifier()); return OverrideComparator{ _function->name(), @@ -424,7 +433,7 @@ OverrideProxy::OverrideComparator const& OverrideProxy::overrideComparator() con [&](VariableDeclaration const* _var) { vector paramTypes; - for (Type const* t: functionType()->parameterTypes()) + for (Type const* t: externalFunctionType()->parameterTypes()) paramTypes.emplace_back(t->richIdentifier()); return OverrideComparator{ _var->name(), @@ -589,14 +598,17 @@ void OverrideChecker::checkOverride(OverrideProxy const& _overriding, OverridePr if (_super.isFunction()) { - FunctionType const* functionType = _overriding.functionType(); - FunctionType const* superType = _super.functionType(); + FunctionType const* functionType = _overriding.externalFunctionType(); + FunctionType const* superType = _super.externalFunctionType(); + bool returnTypesDifferAlready = false; if (_overriding.functionKind() != Token::Fallback) { solAssert(functionType->hasEqualParameterTypes(*superType), "Override doesn't have equal parameters!"); if (!functionType->hasEqualReturnTypes(*superType)) + { + returnTypesDifferAlready = true; overrideError( _overriding, _super, @@ -604,6 +616,36 @@ void OverrideChecker::checkOverride(OverrideProxy const& _overriding, OverridePr "Overriding " + _overriding.astNodeName() + " return types differ.", "Overridden " + _overriding.astNodeName() + " is here:" ); + } + } + + // The override proxy considers calldata and memory the same data location. + // Here we do a more specific check: + // Data locations of parameters and return variables have to match + // unless we have a public function overriding an external one. + if ( + _overriding.isFunction() && + !returnTypesDifferAlready && + _super.visibility() != Visibility::External && + _overriding.functionKind() != Token::Fallback + ) + { + if (!_overriding.originalFunctionType()->hasEqualParameterTypes(*_super.originalFunctionType())) + overrideError( + _overriding, + _super, + 7723_error, + "Data locations of parameters have to be the same when overriding non-external functions, but they differ.", + "Overridden " + _overriding.astNodeName() + " is here:" + ); + if (!_overriding.originalFunctionType()->hasEqualReturnTypes(*_super.originalFunctionType())) + overrideError( + _overriding, + _super, + 1443_error, + "Data locations of return variables have to be the same when overriding non-external functions, but they differ.", + "Overridden " + _overriding.astNodeName() + " is here:" + ); } // Stricter mutability is always okay except when super is Payable diff --git a/libsolidity/analysis/OverrideChecker.h b/libsolidity/analysis/OverrideChecker.h index 9178d179de8a..824248325353 100644 --- a/libsolidity/analysis/OverrideChecker.h +++ b/libsolidity/analysis/OverrideChecker.h @@ -84,7 +84,10 @@ class OverrideProxy /// @returns receive / fallback / function (only the latter for modifiers and variables); langutil::Token functionKind() const; - FunctionType const* functionType() const; + /// @returns the externally callable function type + FunctionType const* externalFunctionType() const; + /// @returns the (unmodified) function type + FunctionType const* originalFunctionType() const; ModifierType const* modifierType() const; Declaration const* declaration() const; @@ -101,6 +104,7 @@ class OverrideProxy /** * Struct to help comparing override items about whether they override each other. + * Compares functions based on their "externally callable" type. * Does not produce a total order. */ struct OverrideComparator diff --git a/libsolidity/ast/AST.cpp b/libsolidity/ast/AST.cpp index be6f2fe9be66..04c47a155fdc 100644 --- a/libsolidity/ast/AST.cpp +++ b/libsolidity/ast/AST.cpp @@ -480,7 +480,9 @@ FunctionDefinition const& FunctionDefinition::resolveVirtual( solAssert(isOrdinary(), ""); solAssert(!libraryFunction(), ""); - FunctionType const* functionType = TypeProvider::function(*this)->asExternallyCallableFunction(false); + // We actually do not want the externally callable function here. + // This is just to add an assertion since the comparison used to be less strict. + FunctionType const* externalFunctionType = TypeProvider::function(*this)->asExternallyCallableFunction(false); bool foundSearchStart = (_searchStart == nullptr); for (ContractDefinition const* c: _mostDerivedContract.annotation().linearizedBaseContracts) @@ -495,9 +497,12 @@ FunctionDefinition const& FunctionDefinition::resolveVirtual( // With super lookup analysis guarantees that there is an implemented function in the chain. // With virtual lookup there are valid cases where returning an unimplemented one is fine. (function->isImplemented() || _searchStart == nullptr) && - FunctionType(*function).asExternallyCallableFunction(false)->hasEqualParameterTypes(*functionType) + FunctionType(*function).asExternallyCallableFunction(false)->hasEqualParameterTypes(*externalFunctionType) ) + { + solAssert(FunctionType(*function).hasEqualParameterTypes(*TypeProvider::function(*this))); return *function; + } } solAssert(false, "Virtual function " + name() + " not found."); diff --git a/test/externalTests/bleeps.sh b/test/externalTests/bleeps.sh index bd3b0255b117..a9d2b58a0644 100755 --- a/test/externalTests/bleeps.sh +++ b/test/externalTests/bleeps.sh @@ -86,6 +86,14 @@ function bleeps_test npm install npm-run-all npm install + # TODO: Bleeps depends on OpenZeppelin 4.3.2, which is affected by + # https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3293. + # Forcing OZ >= 4.6.0 fixes this but it also causes a lot of unrelated compilation errors. + # Remove this when Bleeps gets updated to support newer OpenZeppelin. + perl -i -0pe \ + "s/(function hashProposal\(\n address\[\] )calldata( targets,\n uint256\[\] )calldata( values,\n bytes\[\] )calldata( calldatas,)/\1memory\2memory\3memory\4/g" \ + node_modules/@openzeppelin/contracts/governance/IGovernor.sol + replace_version_pragmas for preset in $SELECTED_PRESETS; do diff --git a/test/externalTests/brink.sh b/test/externalTests/brink.sh index 1513350bbf12..bfbd993f23ee 100755 --- a/test/externalTests/brink.sh +++ b/test/externalTests/brink.sh @@ -62,6 +62,9 @@ function brink_test setup_solc "$DIR" "$BINARY_TYPE" "$BINARY_PATH" download_project "$repo" "$ref_type" "$ref" "$DIR" + # TODO: Remove this when Brink merges https://github.com/brinktrade/brink-core/pull/52 + sed -i "s|\(function isValidSignature(bytes \)calldata\( _data, bytes \)calldata\( _signature)\)|\1memory\2memory\3|g" contracts/Test/MockEIP1271Validator.sol + neutralize_package_lock neutralize_package_json_hooks force_hardhat_compiler_binary "$config_file" "$BINARY_TYPE" "$BINARY_PATH" diff --git a/test/externalTests/ens.sh b/test/externalTests/ens.sh index bf018d0ce699..2b177afb2b18 100755 --- a/test/externalTests/ens.sh +++ b/test/externalTests/ens.sh @@ -37,8 +37,8 @@ function test_fn { yarn test; } function ens_test { local repo="https://github.com/ensdomains/ens-contracts.git" - local ref_type=tag - local ref="v0.0.8" # The project is in flux right now and master might be too unstable for us + local ref_type=branch + local ref="master" local config_file="hardhat.config.js" local compile_only_presets=( @@ -72,6 +72,10 @@ function ens_test replace_version_pragmas neutralize_packaged_contracts + # In some cases Hardhat does not detect revert reasons properly via IR. + # TODO: Remove this when https://github.com/NomicFoundation/hardhat/issues/2115 gets fixed. + sed -i "s|it\(('Does not allow wrapping a name you do not own',\)|it.skip\1|g" test/wrapper/NameWrapper.js + find . -name "*.sol" -exec sed -i -e 's/^\(\s*\)\(assembly\)/\1\/\/\/ @solidity memory-safe-assembly\n\1\2/' '{}' \; for preset in $SELECTED_PRESETS; do diff --git a/test/externalTests/gnosis.sh b/test/externalTests/gnosis.sh index 7d579f76c697..8995231e1bd4 100755 --- a/test/externalTests/gnosis.sh +++ b/test/externalTests/gnosis.sh @@ -79,6 +79,9 @@ function gnosis_safe_test sed -i 's|\(describe\)\(("Upgrade from Safe 1.1.1"\)|\1.skip\2|g' test/migration/UpgradeFromSafe111.spec.ts sed -i 's|\(describe\)\(("Upgrade from Safe 1.2.0"\)|\1.skip\2|g' test/migration/UpgradeFromSafe120.spec.ts + # TODO: Remove this when Gnosis merges https://github.com/gnosis/safe-contracts/pull/394 + sed -i "s|\(function isValidSignature(bytes \)calldata\( _data, bytes \)calldata\( _signature)\)|\1memory\2memory\3|g" contracts/handler/CompatibilityFallbackHandler.sol + neutralize_package_lock neutralize_package_json_hooks force_hardhat_compiler_binary "$config_file" "$BINARY_TYPE" "$BINARY_PATH" diff --git a/test/libsolidity/semanticTests/inheritance/dataLocation/external_public_calldata.sol b/test/libsolidity/semanticTests/inheritance/dataLocation/external_public_calldata.sol new file mode 100644 index 000000000000..4de95b0f1c4f --- /dev/null +++ b/test/libsolidity/semanticTests/inheritance/dataLocation/external_public_calldata.sol @@ -0,0 +1,18 @@ +abstract contract A { + function f(uint256[] calldata a) external virtual returns (uint256[] calldata); +} + +contract B is A { + function f(uint256[] memory a) public override returns (uint256[] memory) { + return a; + } + + function g(uint[] calldata x) public returns (uint256[] memory) { + return f(x); + } +} +// ==== +// compileViaYul: also +// ---- +// f(uint256[]): 0x20, 2, 9, 8 -> 0x20, 2, 9, 8 +// g(uint256[]): 0x20, 2, 9, 8 -> 0x20, 2, 9, 8 diff --git a/test/libsolidity/syntaxTests/inheritance/dataLocation/external_overriding_external.sol b/test/libsolidity/syntaxTests/inheritance/dataLocation/external_overriding_external.sol new file mode 100644 index 000000000000..6105a50a283e --- /dev/null +++ b/test/libsolidity/syntaxTests/inheritance/dataLocation/external_overriding_external.sol @@ -0,0 +1,14 @@ +abstract contract A { + function f(uint256[1] memory a) external virtual returns (uint256); +} +contract B is A { + function f(uint256[1] calldata a) external pure virtual override returns (uint256) { + return a[0]; + } +} +contract C is A, B { + function f(uint256[1] memory a) external pure override(B, A) returns (uint256) { + return a[0]; + } +} +// ---- diff --git a/test/libsolidity/syntaxTests/inheritance/dataLocation/modifier_parameter_data_location_change_illegal_internal.sol b/test/libsolidity/syntaxTests/inheritance/dataLocation/modifier_parameter_data_location_change_illegal_internal.sol new file mode 100644 index 000000000000..a4856d9f0335 --- /dev/null +++ b/test/libsolidity/syntaxTests/inheritance/dataLocation/modifier_parameter_data_location_change_illegal_internal.sol @@ -0,0 +1,13 @@ +abstract contract A { + modifier m(uint256[1] memory a) virtual; + function test(uint256[1] memory a) m(a) external { + } +} + +contract B is A { + modifier m(uint256[1] calldata a) override { + _; + } +} +// ---- +// TypeError 1078: (153-214): Override changes modifier signature. diff --git a/test/libsolidity/syntaxTests/inheritance/dataLocation/parameter_data_location_change_calldata_memory_illegal_public.sol b/test/libsolidity/syntaxTests/inheritance/dataLocation/parameter_data_location_change_calldata_memory_illegal_public.sol new file mode 100644 index 000000000000..e3fb069af239 --- /dev/null +++ b/test/libsolidity/syntaxTests/inheritance/dataLocation/parameter_data_location_change_calldata_memory_illegal_public.sol @@ -0,0 +1,11 @@ +abstract contract A { + function f(uint256[1] calldata a) public virtual returns (uint256); +} + +contract B is A { + function f(uint256[1] memory a) public override returns (uint256) { + return a[0]; + } +} +// ---- +// TypeError 7723: (119-213): Data locations of parameters have to be the same when overriding non-external functions, but they differ. diff --git a/test/libsolidity/syntaxTests/inheritance/dataLocation/parameter_data_location_change_illegal_internal.sol b/test/libsolidity/syntaxTests/inheritance/dataLocation/parameter_data_location_change_illegal_internal.sol new file mode 100644 index 000000000000..55b5b5fe6bc5 --- /dev/null +++ b/test/libsolidity/syntaxTests/inheritance/dataLocation/parameter_data_location_change_illegal_internal.sol @@ -0,0 +1,16 @@ +abstract contract A { + function f(uint256[1] memory a) internal virtual returns (uint256); + function test() external returns (uint) { + uint[1] memory t; + t[0] = 7; + return f(t); + } +} + +contract B is A { + function f(uint256[1] calldata a) internal override returns (uint256) { + return a[0]; + } +} +// ---- +// TypeError 7723: (236-334): Data locations of parameters have to be the same when overriding non-external functions, but they differ. diff --git a/test/libsolidity/syntaxTests/inheritance/dataLocation/parameter_data_location_change_illegal_public.sol b/test/libsolidity/syntaxTests/inheritance/dataLocation/parameter_data_location_change_illegal_public.sol new file mode 100644 index 000000000000..52123f68d475 --- /dev/null +++ b/test/libsolidity/syntaxTests/inheritance/dataLocation/parameter_data_location_change_illegal_public.sol @@ -0,0 +1,16 @@ +abstract contract A { + function f(uint256[1] memory a) public virtual returns (uint256); + function test() external returns (uint) { + uint[1] memory t; + t[0] = 7; + return f(t); + } +} + +contract B is A { + function f(uint256[1] calldata a) public override returns (uint256) { + return a[0]; + } +} +// ---- +// TypeError 7723: (234-330): Data locations of parameters have to be the same when overriding non-external functions, but they differ. diff --git a/test/libsolidity/syntaxTests/inheritance/dataLocation/return_type_data_location.sol b/test/libsolidity/syntaxTests/inheritance/dataLocation/return_type_data_location.sol new file mode 100644 index 000000000000..1c7f4e926c5f --- /dev/null +++ b/test/libsolidity/syntaxTests/inheritance/dataLocation/return_type_data_location.sol @@ -0,0 +1,7 @@ +abstract contract A { + function foo() external virtual view returns(uint[] calldata); +} +contract X is A { + function foo() public view override returns(uint[] memory) { } +} +// ---- diff --git a/test/libsolidity/syntaxTests/inheritance/dataLocation/return_type_data_location_change_illegal.sol b/test/libsolidity/syntaxTests/inheritance/dataLocation/return_type_data_location_change_illegal.sol new file mode 100644 index 000000000000..e06cd43ae573 --- /dev/null +++ b/test/libsolidity/syntaxTests/inheritance/dataLocation/return_type_data_location_change_illegal.sol @@ -0,0 +1,8 @@ +abstract contract A { + function foo() public virtual view returns(uint[] calldata); +} +contract X is A { + function foo() public view override returns(uint[] memory) { } +} +// ---- +// TypeError 1443: (105-168): Data locations of return variables have to be the same when overriding non-external functions, but they differ. diff --git a/tools/solidityUpgrade/Upgrade060.cpp b/tools/solidityUpgrade/Upgrade060.cpp index 52010db39b81..8caff02c9f82 100644 --- a/tools/solidityUpgrade/Upgrade060.cpp +++ b/tools/solidityUpgrade/Upgrade060.cpp @@ -72,7 +72,7 @@ void OverridingFunction::endVisit(ContractDefinition const& _contract) { auto& super = (*begin); auto functionType = FunctionType(*function).asExternallyCallableFunction(false); - auto superType = super.functionType()->asExternallyCallableFunction(false); + auto superType = super.externalFunctionType(); if (functionType && functionType->hasEqualParameterTypes(*superType)) {