From dc5a6dd0428a62651d84393975e5657d183ceaca Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Sat, 20 Apr 2024 12:54:29 +0200 Subject: [PATCH] Fix prototype for trait methods Fixes GH-14009 --- Zend/tests/gh14009_001.phpt | 41 +++++++++++++++++ Zend/tests/gh14009_002.phpt | 22 +++++++++ Zend/tests/gh14009_003.phpt | 19 ++++++++ Zend/tests/gh14009_004.phpt | 37 +++++++++++++++ .../interface_constructor_prototype_001.phpt | 19 ++++++++ .../interface_constructor_prototype_002.phpt | 26 +++++++++++ Zend/zend_inheritance.c | 46 ++++++++++++------- 7 files changed, 194 insertions(+), 16 deletions(-) create mode 100644 Zend/tests/gh14009_001.phpt create mode 100644 Zend/tests/gh14009_002.phpt create mode 100644 Zend/tests/gh14009_003.phpt create mode 100644 Zend/tests/gh14009_004.phpt create mode 100644 Zend/tests/interface_constructor_prototype_001.phpt create mode 100644 Zend/tests/interface_constructor_prototype_002.phpt diff --git a/Zend/tests/gh14009_001.phpt b/Zend/tests/gh14009_001.phpt new file mode 100644 index 0000000000000..81325e814c271 --- /dev/null +++ b/Zend/tests/gh14009_001.phpt @@ -0,0 +1,41 @@ +--TEST-- +GH-14009: Traits inherit prototype +--FILE-- +common(); + } +} + +class B extends P { + protected function common() { + echo __METHOD__, "\n"; + } +} + +trait T { + protected function common() { + echo __METHOD__, "\n"; + } +} + +class C extends P { + use T; +} + +$a = new A(); +$a->test(new B()); +$a->test(new C()); + +?> +--EXPECT-- +B::common +T::common diff --git a/Zend/tests/gh14009_002.phpt b/Zend/tests/gh14009_002.phpt new file mode 100644 index 0000000000000..86047e020205e --- /dev/null +++ b/Zend/tests/gh14009_002.phpt @@ -0,0 +1,22 @@ +--TEST-- +GH-14009: Traits inherit prototype +--FILE-- + +--EXPECTF-- +Fatal error: Declaration of P::common() must be compatible with T::common(int $param) in %s on line %d diff --git a/Zend/tests/gh14009_003.phpt b/Zend/tests/gh14009_003.phpt new file mode 100644 index 0000000000000..71ee5baa360a6 --- /dev/null +++ b/Zend/tests/gh14009_003.phpt @@ -0,0 +1,19 @@ +--TEST-- +GH-14009: Traits inherit prototype +--FILE-- + +--EXPECT-- diff --git a/Zend/tests/gh14009_004.phpt b/Zend/tests/gh14009_004.phpt new file mode 100644 index 0000000000000..01bad46fedcb7 --- /dev/null +++ b/Zend/tests/gh14009_004.phpt @@ -0,0 +1,37 @@ +--TEST-- +GH-14009: Traits inherit prototype +--FILE-- +test(); + } +} + +D::callTest(new C()); + +?> +--EXPECT-- +B::test diff --git a/Zend/tests/interface_constructor_prototype_001.phpt b/Zend/tests/interface_constructor_prototype_001.phpt new file mode 100644 index 0000000000000..67341367ac58f --- /dev/null +++ b/Zend/tests/interface_constructor_prototype_001.phpt @@ -0,0 +1,19 @@ +--TEST-- +Interfaces don't set prototypes to their parent method +--FILE-- + +--EXPECTF-- +Fatal error: Declaration of Test::__construct(int $param) must be compatible with B::__construct(int|float $param) in %s on line %d diff --git a/Zend/tests/interface_constructor_prototype_002.phpt b/Zend/tests/interface_constructor_prototype_002.phpt new file mode 100644 index 0000000000000..76398d2d215b9 --- /dev/null +++ b/Zend/tests/interface_constructor_prototype_002.phpt @@ -0,0 +1,26 @@ +--TEST-- +Interfaces don't set prototypes to their parent method +--XFAIL-- +X::__constructor()'s prototype is set to B::__construct(). Y::__construct() then +uses prototype to verify LSP, but misses A::__construct() which has a stricter +signature. +--FILE-- + +--EXPECTF-- +Fatal error: Declaration of Y::__construct(int $param) must be compatible with A::__construct(int|float $param) in %s on line %d diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c index cec0ffdb1853b..6f9b10f88d686 100644 --- a/Zend/zend_inheritance.c +++ b/Zend/zend_inheritance.c @@ -1085,8 +1085,19 @@ static zend_always_inline inheritance_status do_inheritance_check_on_method_ex( uint32_t parent_flags = parent->common.fn_flags; zend_function *proto; +#define SEPARATE_METHOD() do { \ + if (child_scope != ce && child->type == ZEND_USER_FUNCTION && child_zv) { \ + /* op_array wasn't duplicated yet */ \ + zend_function *new_function = zend_arena_alloc(&CG(arena), sizeof(zend_op_array)); \ + memcpy(new_function, child, sizeof(zend_op_array)); \ + Z_PTR_P(child_zv) = child = new_function; \ + child_zv = NULL; \ + } \ + } while(0) + if (UNEXPECTED((parent_flags & ZEND_ACC_PRIVATE) && !(parent_flags & ZEND_ACC_ABSTRACT) && !(parent_flags & ZEND_ACC_CTOR))) { if (!check_only) { + SEPARATE_METHOD(); child->common.fn_flags |= ZEND_ACC_CHANGED; } /* The parent method is private and not an abstract so we don't need to check any inheritance rules */ @@ -1130,7 +1141,12 @@ static zend_always_inline inheritance_status do_inheritance_check_on_method_ex( ZEND_FN_SCOPE_NAME(parent), ZSTR_VAL(child->common.function_name), ZEND_FN_SCOPE_NAME(child)); } - if (!check_only && (parent_flags & (ZEND_ACC_PRIVATE|ZEND_ACC_CHANGED))) { + if (!check_only + && (parent_flags & (ZEND_ACC_PRIVATE|ZEND_ACC_CHANGED)) + /* In the case of abstract traits, don't mark the method as changed. We don't actually have a + * private parent method. */ + && !(parent->common.scope->ce_flags & ZEND_ACC_TRAIT)) { + SEPARATE_METHOD(); child->common.fn_flags |= ZEND_ACC_CHANGED; } @@ -1146,21 +1162,16 @@ static zend_always_inline inheritance_status do_inheritance_check_on_method_ex( parent = proto; } - if (!check_only && child->common.prototype != proto && child_zv) { - do { - if (child->common.scope != ce && child->type == ZEND_USER_FUNCTION) { - if (ce->ce_flags & ZEND_ACC_INTERFACE) { - /* Few parent interfaces contain the same method */ - break; - } else { - /* op_array wasn't duplicated yet */ - zend_function *new_function = zend_arena_alloc(&CG(arena), sizeof(zend_op_array)); - memcpy(new_function, child, sizeof(zend_op_array)); - Z_PTR_P(child_zv) = child = new_function; - } - } - child->common.prototype = proto; - } while (0); + if (!check_only + && child->common.prototype != proto + /* We are not setting the prototype of overridden interface methods because of abstract + * constructors. See Zend/tests/interface_constructor_prototype_001.phpt. */ + && !(ce->ce_flags & ZEND_ACC_INTERFACE) + /* Parent is a trait when its method is abstract. We only need to verify its signature then, + * don't actually use it as a prototype. See Zend/tests/gh14009_004.phpt. */ + && !(parent->common.scope->ce_flags & ZEND_ACC_TRAIT)) { + SEPARATE_METHOD(); + child->common.prototype = proto; } /* Prevent derived classes from restricting access that was available in parent classes (except deriving from non-abstract ctors) */ @@ -1180,6 +1191,9 @@ static zend_always_inline inheritance_status do_inheritance_check_on_method_ex( } perform_delayable_implementation_check(ce, child, child_scope, parent, parent_scope); } + +#undef SEPARATE_METHOD + return INHERITANCE_SUCCESS; } /* }}} */