From a0d6c118604dd20896601b3c85344c8882da1d4a Mon Sep 17 00:00:00 2001 From: nishant-sachdeva Date: Thu, 16 Dec 2021 17:05:39 +0530 Subject: [PATCH] Equality operator allowed for external function types --- Changelog.md | 1 + libsolidity/ast/Types.cpp | 12 ++- libsolidity/codegen/ExpressionCompiler.cpp | 26 +++++- libsolidity/codegen/YulUtilFunctions.cpp | 29 +++++++ libsolidity/codegen/YulUtilFunctions.h | 3 + .../codegen/ir/IRGeneratorForStatements.cpp | 26 ++++-- ...or_external_function_cleans_dirty_bits.sol | 17 ++++ ...rison_operators_for_external_functions.sol | 81 +++++++++++++++++++ ...xternal_functions_with_extra_gas_slots.sol | 12 +++ ...nternal_and_external_function_pointers.sol | 23 ++++++ ...al_functions_with_different_parameters.sol | 26 ++++++ 11 files changed, 244 insertions(+), 12 deletions(-) create mode 100644 test/libsolidity/semanticTests/functionTypes/comparison_operator_for_external_function_cleans_dirty_bits.sol create mode 100644 test/libsolidity/semanticTests/functionTypes/comparison_operators_for_external_functions.sol create mode 100644 test/libsolidity/syntaxTests/functionTypes/comparison_operator_for_external_functions_with_extra_gas_slots.sol create mode 100644 test/libsolidity/syntaxTests/functionTypes/comparison_operators_between_internal_and_external_function_pointers.sol create mode 100644 test/libsolidity/syntaxTests/functionTypes/comparison_operators_external_functions_with_different_parameters.sol diff --git a/Changelog.md b/Changelog.md index bb20bd813..d8e9d8f40 100644 --- a/Changelog.md +++ b/Changelog.md @@ -2,6 +2,7 @@ Language Features: * General: Support ``ContractName.functionName`` for ``abi.encodeCall``, in addition to external function pointers. + * General: Add equality-comparison operators for external function types. Compiler Features: diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index da2811a21..fb31a9303 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -3018,8 +3018,18 @@ TypeResult FunctionType::binaryOperatorResult(Token _operator, Type const* _othe if (_other->category() != category() || !(_operator == Token::Equal || _operator == Token::NotEqual)) return nullptr; FunctionType const& other = dynamic_cast(*_other); - if (kind() == Kind::Internal && other.kind() == Kind::Internal && sizeOnStack() == 1 && other.sizeOnStack() == 1) + if (kind() == Kind::Internal && sizeOnStack() == 1 && other.kind() == Kind::Internal && other.sizeOnStack() == 1) return commonType(this, _other); + else if ( + kind() == Kind::External && + sizeOnStack() == 2 && + !bound() && + other.kind() == Kind::External && + other.sizeOnStack() == 2 && + !other.bound() + ) + return commonType(this, _other); + return nullptr; } diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp index 34a9a1ee8..b14a032c4 100644 --- a/libsolidity/codegen/ExpressionCompiler.cpp +++ b/libsolidity/codegen/ExpressionCompiler.cpp @@ -2278,12 +2278,29 @@ void ExpressionCompiler::appendAndOrOperatorCode(BinaryOperation const& _binaryO void ExpressionCompiler::appendCompareOperatorCode(Token _operator, Type const& _type) { - solAssert(_type.sizeOnStack() == 1, "Comparison of multi-slot types."); if (_operator == Token::Equal || _operator == Token::NotEqual) { - if (FunctionType const* funType = dynamic_cast(&_type)) + FunctionType const* functionType = dynamic_cast(&_type); + if (functionType && functionType->kind() == FunctionType::Kind::External) { - if (funType->kind() == FunctionType::Kind::Internal) + solUnimplementedAssert(functionType->sizeOnStack() == 2, ""); + m_context << Instruction::SWAP3; + + m_context << ((u256(1) << 160) - 1) << Instruction::AND; + m_context << Instruction::SWAP1; + m_context << ((u256(1) << 160) - 1) << Instruction::AND; + m_context << Instruction::EQ; + m_context << Instruction::SWAP2; + m_context << ((u256(1) << 32) - 1) << Instruction::AND; + m_context << Instruction::SWAP1; + m_context << ((u256(1) << 32) - 1) << Instruction::AND; + m_context << Instruction::EQ; + m_context << Instruction::AND; + } + else + { + solAssert(_type.sizeOnStack() == 1, "Comparison of multi-slot types."); + if (functionType && functionType->kind() == FunctionType::Kind::Internal) { // We have to remove the upper bits (construction time value) because they might // be "unknown" in one of the operands and not in the other. @@ -2291,13 +2308,14 @@ void ExpressionCompiler::appendCompareOperatorCode(Token _operator, Type const& m_context << Instruction::SWAP1; m_context << ((u256(1) << 32) - 1) << Instruction::AND; } + m_context << Instruction::EQ; } - m_context << Instruction::EQ; if (_operator == Token::NotEqual) m_context << Instruction::ISZERO; } else { + solAssert(_type.sizeOnStack() == 1, "Comparison of multi-slot types."); bool isSigned = false; if (auto type = dynamic_cast(&_type)) isSigned = type->isSigned(); diff --git a/libsolidity/codegen/YulUtilFunctions.cpp b/libsolidity/codegen/YulUtilFunctions.cpp index 106ad260f..cbe8b8204 100644 --- a/libsolidity/codegen/YulUtilFunctions.cpp +++ b/libsolidity/codegen/YulUtilFunctions.cpp @@ -29,6 +29,7 @@ #include #include #include +#include using namespace std; using namespace solidity; @@ -4548,3 +4549,31 @@ string YulUtilFunctions::externalCodeFunction() .render(); }); } + +std::string YulUtilFunctions::externalFunctionPointersEqualFunction() +{ + std::string const functionName = "externalFunctionPointersEqualFunction"; + return m_functionCollector.createFunction(functionName, [&]() { + return util::Whiskers(R"( + function ( + leftAddress, + leftSelector, + rightAddress, + rightSelector + ) -> result { + result := and( + eq( + (leftAddress), (rightAddress) + ), + eq( + (leftSelector), (rightSelector) + ) + ) + } + )") + ("functionName", functionName) + ("addressCleanUpFunction", cleanupFunction(*TypeProvider::address())) + ("selectorCleanUpFunction", cleanupFunction(*TypeProvider::uint(32))) + .render(); + }); +} diff --git a/libsolidity/codegen/YulUtilFunctions.h b/libsolidity/codegen/YulUtilFunctions.h index a03ee75c2..52d672b1f 100644 --- a/libsolidity/codegen/YulUtilFunctions.h +++ b/libsolidity/codegen/YulUtilFunctions.h @@ -522,6 +522,9 @@ public: /// Signature: (address) -> mpos std::string externalCodeFunction(); + /// @return the name of a function that that checks if two external functions pointers are equal or not + std::string externalFunctionPointersEqualFunction(); + private: /// @returns the name of a function that copies a struct from calldata or memory to storage /// signature: (slot, value) -> diff --git a/libsolidity/codegen/ir/IRGeneratorForStatements.cpp b/libsolidity/codegen/ir/IRGeneratorForStatements.cpp index d35685037..24d695d39 100644 --- a/libsolidity/codegen/ir/IRGeneratorForStatements.cpp +++ b/libsolidity/codegen/ir/IRGeneratorForStatements.cpp @@ -799,13 +799,8 @@ bool IRGeneratorForStatements::visit(BinaryOperation const& _binOp) if (TokenTraits::isCompareOp(op)) { - if (auto type = dynamic_cast(commonType)) - { - solAssert(op == Token::Equal || op == Token::NotEqual, "Invalid function pointer comparison!"); - solAssert(type->kind() != FunctionType::Kind::External, "External function comparison not allowed!"); - } - solAssert(commonType->isValueType(), ""); + bool isSigned = false; if (auto type = dynamic_cast(commonType)) isSigned = type->isSigned(); @@ -813,8 +808,25 @@ bool IRGeneratorForStatements::visit(BinaryOperation const& _binOp) string args = expressionAsType(_binOp.leftExpression(), *commonType, true); args += ", " + expressionAsType(_binOp.rightExpression(), *commonType, true); + auto functionType = dynamic_cast(commonType); + solAssert(functionType ? (op == Token::Equal || op == Token::NotEqual) : true, "Invalid function pointer comparison!"); + string expr; - if (op == Token::Equal) + + if (functionType && functionType->kind() == FunctionType::Kind::External) + { + solUnimplementedAssert(functionType->sizeOnStack() == 2, ""); + expr = m_utils.externalFunctionPointersEqualFunction() + + "(" + + IRVariable{_binOp.leftExpression()}.part("address").name() + "," + + IRVariable{_binOp.leftExpression()}.part("functionSelector").name() + "," + + IRVariable{_binOp.rightExpression()}.part("address").name() + "," + + IRVariable{_binOp.rightExpression()}.part("functionSelector").name() + + ")"; + if (op == Token::NotEqual) + expr = "iszero(" + expr + ")"; + } + else if (op == Token::Equal) expr = "eq(" + move(args) + ")"; else if (op == Token::NotEqual) expr = "iszero(eq(" + move(args) + "))"; diff --git a/test/libsolidity/semanticTests/functionTypes/comparison_operator_for_external_function_cleans_dirty_bits.sol b/test/libsolidity/semanticTests/functionTypes/comparison_operator_for_external_function_cleans_dirty_bits.sol new file mode 100644 index 000000000..3347d9b77 --- /dev/null +++ b/test/libsolidity/semanticTests/functionTypes/comparison_operator_for_external_function_cleans_dirty_bits.sol @@ -0,0 +1,17 @@ +contract C { + function g() external {} + function comparison_operators_for_external_function_pointers_with_dirty_bits() external returns (bool) { + function() external g_ptr_dirty = this.g; + assembly { + g_ptr_dirty.address := or(g_ptr_dirty.address, shl(160, sub(0,1))) + g_ptr_dirty.selector := or(g_ptr_dirty.selector, shl(32, sub(0,1))) + } + function() external g_ptr = this.g; + return g_ptr == g_ptr_dirty; + } +} +// ==== +// compileViaYul: also +// EVMVersion: >=constantinople +// ---- +// comparison_operators_for_external_function_pointers_with_dirty_bits() -> true diff --git a/test/libsolidity/semanticTests/functionTypes/comparison_operators_for_external_functions.sol b/test/libsolidity/semanticTests/functionTypes/comparison_operators_for_external_functions.sol new file mode 100644 index 000000000..da75ea071 --- /dev/null +++ b/test/libsolidity/semanticTests/functionTypes/comparison_operators_for_external_functions.sol @@ -0,0 +1,81 @@ +contract C { + function f() external {} + function g() external {} + function h() pure external {} + function i() view external {} + + function comparison_operators_for_external_functions() public returns (bool) { + assert( + this.f != this.g && + this.f != this.h && + this.f != this.i && + + this.g != this.h && + this.g != this.i && + + this.h != this.i && + + this.f == this.f && + this.g == this.g && + this.h == this.h && + this.i == this.i + ); + return true; + } + + function comparison_operators_for_local_external_function_pointers() public returns (bool) { + function () external f_local = this.f; + function () external g_local = this.g; + function () external pure h_local = this.h; + function () external view i_local = this.i; + + assert( + f_local == this.f && + g_local == this.g && + h_local == this.h && + i_local == this.i && + + f_local != this.g && + f_local != this.h && + f_local != this.i && + + g_local != this.f && + g_local != this.h && + g_local != this.i && + + h_local != this.f && + h_local != this.g && + h_local != this.i && + + i_local != this.f && + i_local != this.g && + i_local != this.h + ); + + assert( + f_local == f_local && + f_local != g_local && + f_local != h_local && + f_local != i_local + ); + + assert( + g_local == g_local && + g_local != h_local && + g_local != i_local + ); + + assert( + h_local == h_local && + i_local == i_local && + h_local != i_local + ); + + return true; + } +} +// ==== +// compileViaYul: also +// ---- +// comparison_operators_for_external_functions() -> true +// comparison_operators_for_local_external_function_pointers() -> true diff --git a/test/libsolidity/syntaxTests/functionTypes/comparison_operator_for_external_functions_with_extra_gas_slots.sol b/test/libsolidity/syntaxTests/functionTypes/comparison_operator_for_external_functions_with_extra_gas_slots.sol new file mode 100644 index 000000000..7b0713caf --- /dev/null +++ b/test/libsolidity/syntaxTests/functionTypes/comparison_operator_for_external_functions_with_extra_gas_slots.sol @@ -0,0 +1,12 @@ +contract C { + function external_test_function() external {} + function comparison_operator_for_external_function_with_extra_slots() external returns (bool) { + return ( + (this.external_test_function{gas: 4} == this.external_test_function) && + (this.external_test_function{gas: 4} == this.external_test_function{gas: 4}) + ); + } +} +// ---- +// TypeError 2271: (193-259): Operator == not compatible with types function () external and function () external +// TypeError 2271: (277-351): Operator == not compatible with types function () external and function () external diff --git a/test/libsolidity/syntaxTests/functionTypes/comparison_operators_between_internal_and_external_function_pointers.sol b/test/libsolidity/syntaxTests/functionTypes/comparison_operators_between_internal_and_external_function_pointers.sol new file mode 100644 index 000000000..19f44658f --- /dev/null +++ b/test/libsolidity/syntaxTests/functionTypes/comparison_operators_between_internal_and_external_function_pointers.sol @@ -0,0 +1,23 @@ +contract C { + function external_test_function() external {} + function internal_test_function() internal {} + + function comparison_operator_between_internal_and_external_function_pointers() external returns (bool) { + function () external external_function_pointer_local = this.external_test_function; + function () internal internal_function_pointer_local = internal_test_function; + + assert( + this.external_test_function == external_function_pointer_local && + internal_function_pointer_local == internal_test_function + ); + assert( + internal_function_pointer_local != external_function_pointer_local && + internal_test_function != this.external_test_function + ); + + return true; + } +} +// ---- +// TypeError 2271: (606-672): Operator != not compatible with types function () and function () external +// TypeError 2271: (688-741): Operator != not compatible with types function () and function () external diff --git a/test/libsolidity/syntaxTests/functionTypes/comparison_operators_external_functions_with_different_parameters.sol b/test/libsolidity/syntaxTests/functionTypes/comparison_operators_external_functions_with_different_parameters.sol new file mode 100644 index 000000000..39381f454 --- /dev/null +++ b/test/libsolidity/syntaxTests/functionTypes/comparison_operators_external_functions_with_different_parameters.sol @@ -0,0 +1,26 @@ +contract C { + function external_test_function1(uint num) external {} + function external_test_function2(bool val) external {} + + function comparison_operator_between_internal_and_external_function_pointers() external returns (bool) { + function () external external_function_pointer_local1 = this.external_test_function1; + function () external external_function_pointer_local2 = this.external_test_function2; + + assert( + this.external_test_function1 == external_function_pointer_local1 && + this.external_test_function2 == external_function_pointer_local2 + ); + assert( + external_function_pointer_local2 != external_function_pointer_local1 && + this.external_test_function2 != this.external_test_function1 + ); + + return true; + } +} +// ---- +// TypeError 9574: (249-333): Type function (uint256) external is not implicitly convertible to expected type function () external. +// TypeError 9574: (343-427): Type function (bool) external is not implicitly convertible to expected type function () external. +// TypeError 2271: (458-522): Operator == not compatible with types function (uint256) external and function () external +// TypeError 2271: (538-602): Operator == not compatible with types function (bool) external and function () external +// TypeError 2271: (726-786): Operator != not compatible with types function (bool) external and function (uint256) external