From 9f880d27c0c80fe5b648adc36c0f2acebce7b694 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 9 Jan 2020 15:04:33 +0100 Subject: [PATCH 1/5] Check abstract method signatures coming from traits --- Zend/tests/traits/abstract_method_1.phpt | 18 ++++++++ Zend/tests/traits/abstract_method_2.phpt | 22 ++++++++++ Zend/tests/traits/bug60217b.phpt | 2 +- Zend/tests/traits/bug60217c.phpt | 2 +- Zend/zend_inheritance.c | 53 ++++++------------------ 5 files changed, 54 insertions(+), 43 deletions(-) create mode 100644 Zend/tests/traits/abstract_method_1.phpt create mode 100644 Zend/tests/traits/abstract_method_2.phpt diff --git a/Zend/tests/traits/abstract_method_1.phpt b/Zend/tests/traits/abstract_method_1.phpt new file mode 100644 index 0000000000000..d3eb7d80d188c --- /dev/null +++ b/Zend/tests/traits/abstract_method_1.phpt @@ -0,0 +1,18 @@ +--TEST-- +Abstract method from trait enforced in class +--FILE-- + +--EXPECTF-- +Fatal error: Declaration of C::neededByTheTrait(array $a, object $b) must be compatible with T::neededByTheTrait(int $a, string $b) in %s on line %d diff --git a/Zend/tests/traits/abstract_method_2.phpt b/Zend/tests/traits/abstract_method_2.phpt new file mode 100644 index 0000000000000..dd522a14f9eb9 --- /dev/null +++ b/Zend/tests/traits/abstract_method_2.phpt @@ -0,0 +1,22 @@ +--TEST-- +Mutually incompatible methods from traits are fine as long as the final method is compatible +--FILE-- + +===DONE=== +--EXPECT-- +===DONE=== diff --git a/Zend/tests/traits/bug60217b.phpt b/Zend/tests/traits/bug60217b.phpt index b349cf2c540b2..05a3adc700cff 100644 --- a/Zend/tests/traits/bug60217b.phpt +++ b/Zend/tests/traits/bug60217b.phpt @@ -22,4 +22,4 @@ class CBroken { $o = new CBroken; $o->foo(1); --EXPECTF-- -Fatal error: Declaration of TBroken1::foo($a) must be compatible with TBroken2::foo($a, $b = 0) in %s +Fatal error: Declaration of CBroken::foo($a) must be compatible with TBroken2::foo($a, $b = 0) in %s on line %d diff --git a/Zend/tests/traits/bug60217c.phpt b/Zend/tests/traits/bug60217c.phpt index 401444e978f83..334487aeaa944 100644 --- a/Zend/tests/traits/bug60217c.phpt +++ b/Zend/tests/traits/bug60217c.phpt @@ -22,4 +22,4 @@ class CBroken { $o = new CBroken; $o->foo(1); --EXPECTF-- -Fatal error: Declaration of TBroken2::foo($a) must be compatible with TBroken1::foo($a, $b = 0) in %s on line %d +Fatal error: Declaration of CBroken::foo($a) must be compatible with TBroken1::foo($a, $b = 0) in %s on line %d diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c index f15c325d67679..fd4028b04a665 100644 --- a/Zend/zend_inheritance.c +++ b/Zend/zend_inheritance.c @@ -34,12 +34,6 @@ static void add_property_compatibility_obligation( zend_class_entry *ce, const zend_property_info *child_prop, const zend_property_info *parent_prop); -static void overridden_ptr_dtor(zval *zv) /* {{{ */ -{ - efree_size(Z_PTR_P(zv), sizeof(zend_function)); -} -/* }}} */ - static void zend_type_copy_ctor(zend_type *type, zend_bool persistent) { if (ZEND_TYPE_HAS_LIST(*type)) { zend_type_list *old_list = ZEND_TYPE_LIST(*type); @@ -1580,7 +1574,7 @@ static void zend_add_magic_methods(zend_class_entry* ce, zend_string* mname, zen } /* }}} */ -static void zend_add_trait_method(zend_class_entry *ce, zend_string *name, zend_string *key, zend_function *fn, HashTable **overridden) /* {{{ */ +static void zend_add_trait_method(zend_class_entry *ce, zend_string *name, zend_string *key, zend_function *fn) /* {{{ */ { zend_function *existing_fn = NULL; zend_function *new_fn; @@ -1594,31 +1588,14 @@ static void zend_add_trait_method(zend_class_entry *ce, zend_string *name, zend_ return; } + /* Abstract method signatures from the trait must be satisfied. */ + if (fn->common.fn_flags & ZEND_ACC_ABSTRACT) { + perform_delayable_implementation_check(ce, existing_fn, fn); + return; + } + if (existing_fn->common.scope == ce) { /* members from the current class override trait methods */ - /* use temporary *overridden HashTable to detect hidden conflict */ - if (*overridden) { - if ((existing_fn = zend_hash_find_ptr(*overridden, key)) != NULL) { - if (existing_fn->common.fn_flags & ZEND_ACC_ABSTRACT) { - /* Make sure the trait method is compatible with previosly declared abstract method */ - perform_delayable_implementation_check(ce, fn, existing_fn); - } - if (fn->common.fn_flags & ZEND_ACC_ABSTRACT) { - /* Make sure the abstract declaration is compatible with previous declaration */ - perform_delayable_implementation_check(ce, existing_fn, fn); - return; - } - } - } else { - ALLOC_HASHTABLE(*overridden); - zend_hash_init_ex(*overridden, 8, NULL, overridden_ptr_dtor, 0, 0); - } - zend_hash_update_mem(*overridden, key, fn, sizeof(zend_function)); - return; - } else if ((fn->common.fn_flags & ZEND_ACC_ABSTRACT) - && !(existing_fn->common.fn_flags & ZEND_ACC_ABSTRACT)) { - /* Make sure the abstract declaration is compatible with previous declaration */ - perform_delayable_implementation_check(ce, existing_fn, fn); return; } else if (UNEXPECTED((existing_fn->common.scope->ce_flags & ZEND_ACC_TRAIT) && !(existing_fn->common.fn_flags & ZEND_ACC_ABSTRACT))) { @@ -1675,7 +1652,7 @@ static void zend_fixup_trait_method(zend_function *fn, zend_class_entry *ce) /* } /* }}} */ -static void zend_traits_copy_functions(zend_string *fnname, zend_function *fn, zend_class_entry *ce, HashTable **overridden, HashTable *exclude_table, zend_class_entry **aliases) /* {{{ */ +static void zend_traits_copy_functions(zend_string *fnname, zend_function *fn, zend_class_entry *ce, HashTable *exclude_table, zend_class_entry **aliases) /* {{{ */ { zend_trait_alias *alias, **alias_ptr; zend_string *lcname; @@ -1701,7 +1678,7 @@ static void zend_traits_copy_functions(zend_string *fnname, zend_function *fn, z } lcname = zend_string_tolower(alias->alias); - zend_add_trait_method(ce, alias->alias, lcname, &fn_copy, overridden); + zend_add_trait_method(ce, alias->alias, lcname, &fn_copy); zend_string_release_ex(lcname, 0); } alias_ptr++; @@ -1733,7 +1710,7 @@ static void zend_traits_copy_functions(zend_string *fnname, zend_function *fn, z } } - zend_add_trait_method(ce, fn->common.function_name, fnname, &fn_copy, overridden); + zend_add_trait_method(ce, fn->common.function_name, fnname, &fn_copy); } } /* }}} */ @@ -1907,7 +1884,6 @@ static void zend_traits_init_trait_structures(zend_class_entry *ce, zend_class_e static void zend_do_traits_method_binding(zend_class_entry *ce, zend_class_entry **traits, HashTable **exclude_tables, zend_class_entry **aliases) /* {{{ */ { uint32_t i; - HashTable *overridden = NULL; zend_string *key; zend_function *fn; @@ -1916,7 +1892,7 @@ static void zend_do_traits_method_binding(zend_class_entry *ce, zend_class_entry if (traits[i]) { /* copies functions, applies defined aliasing, and excludes unused trait methods */ ZEND_HASH_FOREACH_STR_KEY_PTR(&traits[i]->function_table, key, fn) { - zend_traits_copy_functions(key, fn, ce, &overridden, exclude_tables[i], aliases); + zend_traits_copy_functions(key, fn, ce, exclude_tables[i], aliases); } ZEND_HASH_FOREACH_END(); if (exclude_tables[i]) { @@ -1930,7 +1906,7 @@ static void zend_do_traits_method_binding(zend_class_entry *ce, zend_class_entry for (i = 0; i < ce->num_traits; i++) { if (traits[i]) { ZEND_HASH_FOREACH_STR_KEY_PTR(&traits[i]->function_table, key, fn) { - zend_traits_copy_functions(key, fn, ce, &overridden, NULL, aliases); + zend_traits_copy_functions(key, fn, ce, NULL, aliases); } ZEND_HASH_FOREACH_END(); } } @@ -1939,11 +1915,6 @@ static void zend_do_traits_method_binding(zend_class_entry *ce, zend_class_entry ZEND_HASH_FOREACH_PTR(&ce->function_table, fn) { zend_fixup_trait_method(fn, ce); } ZEND_HASH_FOREACH_END(); - - if (overridden) { - zend_hash_destroy(overridden); - FREE_HASHTABLE(overridden); - } } /* }}} */ From ca149dda24ac7372ab96c4161741d5bd39027bcb Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 6 Feb 2020 15:36:13 +0100 Subject: [PATCH 2/5] Properly check visibility/staticness and allow abstract private --- Zend/tests/traits/abstract_method_1.phpt | 4 ++-- Zend/tests/traits/abstract_method_2.phpt | 2 +- Zend/tests/traits/abstract_method_3.phpt | 18 ++++++++++++++++++ Zend/tests/traits/abstract_method_4.phpt | 18 ++++++++++++++++++ Zend/tests/traits/abstract_method_5.phpt | 18 ++++++++++++++++++ Zend/zend_compile.c | 2 +- Zend/zend_inheritance.c | 11 +++++++---- 7 files changed, 65 insertions(+), 8 deletions(-) create mode 100644 Zend/tests/traits/abstract_method_3.phpt create mode 100644 Zend/tests/traits/abstract_method_4.phpt create mode 100644 Zend/tests/traits/abstract_method_5.phpt diff --git a/Zend/tests/traits/abstract_method_1.phpt b/Zend/tests/traits/abstract_method_1.phpt index d3eb7d80d188c..7ec5316b8a586 100644 --- a/Zend/tests/traits/abstract_method_1.phpt +++ b/Zend/tests/traits/abstract_method_1.phpt @@ -4,13 +4,13 @@ Abstract method from trait enforced in class diff --git a/Zend/tests/traits/abstract_method_2.phpt b/Zend/tests/traits/abstract_method_2.phpt index dd522a14f9eb9..3633f9f4e1bba 100644 --- a/Zend/tests/traits/abstract_method_2.phpt +++ b/Zend/tests/traits/abstract_method_2.phpt @@ -13,7 +13,7 @@ trait T2 { class C { use T1, T2; - function test(): int {} + public function test(): int {} } ?> diff --git a/Zend/tests/traits/abstract_method_3.phpt b/Zend/tests/traits/abstract_method_3.phpt new file mode 100644 index 0000000000000..935cc5cd0d353 --- /dev/null +++ b/Zend/tests/traits/abstract_method_3.phpt @@ -0,0 +1,18 @@ +--TEST-- +Private abstract method from trait enforced in class +--FILE-- + +--EXPECTF-- +Fatal error: Declaration of C::neededByTheTrait(array $a, object $b) must be compatible with T::neededByTheTrait(int $a, string $b) in %s on line %d diff --git a/Zend/tests/traits/abstract_method_4.phpt b/Zend/tests/traits/abstract_method_4.phpt new file mode 100644 index 0000000000000..3ff85243dbb2d --- /dev/null +++ b/Zend/tests/traits/abstract_method_4.phpt @@ -0,0 +1,18 @@ +--TEST-- +Visibility enforcement on abstract trait methods +--FILE-- + +--EXPECTF-- +Fatal error: Access level to C::method() must be public (as in class T) in %s on line %d diff --git a/Zend/tests/traits/abstract_method_5.phpt b/Zend/tests/traits/abstract_method_5.phpt new file mode 100644 index 0000000000000..fa6482aa42860 --- /dev/null +++ b/Zend/tests/traits/abstract_method_5.phpt @@ -0,0 +1,18 @@ +--TEST-- +Staticness enforcement on abstract trait methods +--FILE-- + +--EXPECTF-- +Fatal error: Cannot make static method T::method() non static in class C in %s on line %d diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index fc5b92f09cf19..d49837740f157 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -6101,7 +6101,7 @@ void zend_begin_method_decl(zend_op_array *op_array, zend_string *name, zend_boo } if (op_array->fn_flags & ZEND_ACC_ABSTRACT) { - if (op_array->fn_flags & ZEND_ACC_PRIVATE) { + if ((op_array->fn_flags & ZEND_ACC_PRIVATE) && !(ce->ce_flags & ZEND_ACC_TRAIT)) { zend_error_noreturn(E_COMPILE_ERROR, "%s function %s::%s() cannot be declared private", in_interface ? "Interface" : "Abstract", ZSTR_VAL(ce->name), ZSTR_VAL(name)); } diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c index fd4028b04a665..9e071ccd8903c 100644 --- a/Zend/zend_inheritance.c +++ b/Zend/zend_inheritance.c @@ -534,8 +534,10 @@ static inheritance_status zend_do_perform_implementation_check( && ((proto->common.scope->ce_flags & ZEND_ACC_INTERFACE) == 0 && (proto->common.fn_flags & ZEND_ACC_ABSTRACT) == 0))); - /* If the prototype method is private do not enforce a signature */ - ZEND_ASSERT(!(proto->common.fn_flags & ZEND_ACC_PRIVATE)); + /* If the prototype method is private and not abstract, we do not enforce a signature. + * private abstract methods can only occur in traits. */ + ZEND_ASSERT(!(proto->common.fn_flags & ZEND_ACC_PRIVATE) + || (proto->common.fn_flags & ZEND_ACC_ABSTRACT)); /* The number of required arguments cannot increase. */ if (proto->common.required_num_args < fe->common.required_num_args) { @@ -850,7 +852,7 @@ static zend_always_inline inheritance_status do_inheritance_check_on_method_ex(z child->common.fn_flags |= ZEND_ACC_CHANGED; } - if (parent_flags & ZEND_ACC_PRIVATE) { + if ((parent_flags & ZEND_ACC_PRIVATE) && !(parent_flags & ZEND_ACC_ABSTRACT)) { return INHERITANCE_SUCCESS; } @@ -1590,7 +1592,8 @@ static void zend_add_trait_method(zend_class_entry *ce, zend_string *name, zend_ /* Abstract method signatures from the trait must be satisfied. */ if (fn->common.fn_flags & ZEND_ACC_ABSTRACT) { - perform_delayable_implementation_check(ce, existing_fn, fn); + do_inheritance_check_on_method(existing_fn, fn, ce, NULL); + fn->common.prototype = NULL; return; } From bec80b36b51ac62d61fe7444746cdfe09090bdf7 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 6 Feb 2020 19:00:56 +0100 Subject: [PATCH 3/5] Don't try to modify the method prototype --- Zend/zend_inheritance.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c index 9e071ccd8903c..9efe89f6e2774 100644 --- a/Zend/zend_inheritance.c +++ b/Zend/zend_inheritance.c @@ -868,7 +868,7 @@ static zend_always_inline inheritance_status do_inheritance_check_on_method_ex(z parent = proto; } - if (!check_only && child->common.prototype != proto) { + if (!check_only && child->common.prototype != proto && child_zv) { do { if (child->common.scope != ce && child->type == ZEND_USER_FUNCTION @@ -876,7 +876,7 @@ static zend_always_inline inheritance_status do_inheritance_check_on_method_ex(z if (ce->ce_flags & ZEND_ACC_INTERFACE) { /* Few parent interfaces contain the same method */ break; - } else if (child_zv) { + } 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)); @@ -1593,7 +1593,6 @@ static void zend_add_trait_method(zend_class_entry *ce, zend_string *name, zend_ /* Abstract method signatures from the trait must be satisfied. */ if (fn->common.fn_flags & ZEND_ACC_ABSTRACT) { do_inheritance_check_on_method(existing_fn, fn, ce, NULL); - fn->common.prototype = NULL; return; } @@ -1616,7 +1615,6 @@ static void zend_add_trait_method(zend_class_entry *ce, zend_string *name, zend_ /* inherited members are overridden by members inserted by traits */ /* check whether the trait method fulfills the inheritance requirements */ do_inheritance_check_on_method(fn, existing_fn, ce, NULL); - fn->common.prototype = NULL; } } From ed3b3604b1a1ad84d1a57479abfadbd7a6f7224e Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Fri, 7 Feb 2020 10:57:10 +0100 Subject: [PATCH 4/5] Make sure that abstract private is implemented in same class Normally it's enough to mark the class as "abstract" if an abstract method was not implemented. However, if there are abstract private methods, then even they really must be implemented, or it must be forwarded to a method with higher visibility. --- Zend/tests/traits/abstract_method_6.phpt | 20 ++++++++++++ Zend/tests/traits/abstract_method_7.phpt | 23 +++++++++++++ Zend/zend_inheritance.c | 41 ++++++++++++++---------- 3 files changed, 67 insertions(+), 17 deletions(-) create mode 100644 Zend/tests/traits/abstract_method_6.phpt create mode 100644 Zend/tests/traits/abstract_method_7.phpt diff --git a/Zend/tests/traits/abstract_method_6.phpt b/Zend/tests/traits/abstract_method_6.phpt new file mode 100644 index 0000000000000..a8a3fc4138689 --- /dev/null +++ b/Zend/tests/traits/abstract_method_6.phpt @@ -0,0 +1,20 @@ +--TEST-- +Abstract private trait method not implemented +--FILE-- + +--EXPECTF-- +Fatal error: Class C must implement 1 abstract private method (C::method) in %s on line %d diff --git a/Zend/tests/traits/abstract_method_7.phpt b/Zend/tests/traits/abstract_method_7.phpt new file mode 100644 index 0000000000000..5e276eb17df50 --- /dev/null +++ b/Zend/tests/traits/abstract_method_7.phpt @@ -0,0 +1,23 @@ +--TEST-- +Abstract private trait method forwarded as abstract protected method +--FILE-- + +===DONE=== +--EXPECT-- +===DONE=== diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c index 9efe89f6e2774..4c510f436de0d 100644 --- a/Zend/zend_inheritance.c +++ b/Zend/zend_inheritance.c @@ -2122,20 +2122,18 @@ typedef struct _zend_abstract_info { static void zend_verify_abstract_class_function(zend_function *fn, zend_abstract_info *ai) /* {{{ */ { - if (fn->common.fn_flags & ZEND_ACC_ABSTRACT) { - if (ai->cnt < MAX_ABSTRACT_INFO_CNT) { - ai->afn[ai->cnt] = fn; - } - if (fn->common.fn_flags & ZEND_ACC_CTOR) { - if (!ai->ctor) { - ai->cnt++; - ai->ctor = 1; - } else { - ai->afn[ai->cnt] = NULL; - } - } else { + if (ai->cnt < MAX_ABSTRACT_INFO_CNT) { + ai->afn[ai->cnt] = fn; + } + if (fn->common.fn_flags & ZEND_ACC_CTOR) { + if (!ai->ctor) { ai->cnt++; + ai->ctor = 1; + } else { + ai->afn[ai->cnt] = NULL; } + } else { + ai->cnt++; } } /* }}} */ @@ -2144,16 +2142,23 @@ void zend_verify_abstract_class(zend_class_entry *ce) /* {{{ */ { zend_function *func; zend_abstract_info ai; - - ZEND_ASSERT((ce->ce_flags & (ZEND_ACC_IMPLICIT_ABSTRACT_CLASS|ZEND_ACC_INTERFACE|ZEND_ACC_TRAIT|ZEND_ACC_EXPLICIT_ABSTRACT_CLASS)) == ZEND_ACC_IMPLICIT_ABSTRACT_CLASS); + zend_bool is_explicit_abstract = (ce->ce_flags & ZEND_ACC_EXPLICIT_ABSTRACT_CLASS) != 0; memset(&ai, 0, sizeof(ai)); ZEND_HASH_FOREACH_PTR(&ce->function_table, func) { - zend_verify_abstract_class_function(func, &ai); + if (func->common.fn_flags & ZEND_ACC_ABSTRACT) { + /* If the class is explicitly abstract, we only check private abstract methods, + * because only they must be declared in the same class. */ + if (!is_explicit_abstract || (func->common.fn_flags & ZEND_ACC_PRIVATE)) { + zend_verify_abstract_class_function(func, &ai); + } + } } ZEND_HASH_FOREACH_END(); if (ai.cnt) { - zend_error_noreturn(E_ERROR, "Class %s contains %d abstract method%s and must therefore be declared abstract or implement the remaining methods (" MAX_ABSTRACT_INFO_FMT MAX_ABSTRACT_INFO_FMT MAX_ABSTRACT_INFO_FMT ")", + zend_error_noreturn(E_ERROR, !is_explicit_abstract + ? "Class %s contains %d abstract method%s and must therefore be declared abstract or implement the remaining methods (" MAX_ABSTRACT_INFO_FMT MAX_ABSTRACT_INFO_FMT MAX_ABSTRACT_INFO_FMT ")" + : "Class %s must implement %d abstract private method%s (" MAX_ABSTRACT_INFO_FMT MAX_ABSTRACT_INFO_FMT MAX_ABSTRACT_INFO_FMT ")", ZSTR_VAL(ce->name), ai.cnt, ai.cnt > 1 ? "s" : "", DISPLAY_ABSTRACT_FN(0), @@ -2432,7 +2437,9 @@ ZEND_API int zend_do_link_class(zend_class_entry *ce, zend_string *lc_parent_nam if (interfaces) { zend_do_implement_interfaces(ce, interfaces); } - if ((ce->ce_flags & (ZEND_ACC_IMPLICIT_ABSTRACT_CLASS|ZEND_ACC_INTERFACE|ZEND_ACC_TRAIT|ZEND_ACC_EXPLICIT_ABSTRACT_CLASS)) == ZEND_ACC_IMPLICIT_ABSTRACT_CLASS) { + if (!(ce->ce_flags & (ZEND_ACC_INTERFACE|ZEND_ACC_TRAIT)) + && (ce->ce_flags & (ZEND_ACC_IMPLICIT_ABSTRACT_CLASS|ZEND_ACC_EXPLICIT_ABSTRACT_CLASS)) + ) { zend_verify_abstract_class(ce); } From e81598bf38fb13e64adeabdca0e3db7bb0bb3f53 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 3 Mar 2020 15:32:26 +0100 Subject: [PATCH 5/5] Don't enforce visibility for BC reasons --- Zend/tests/traits/abstract_method_4.phpt | 6 ++++-- Zend/zend_inheritance.c | 25 +++++++++++++++--------- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/Zend/tests/traits/abstract_method_4.phpt b/Zend/tests/traits/abstract_method_4.phpt index 3ff85243dbb2d..10b7219a170ff 100644 --- a/Zend/tests/traits/abstract_method_4.phpt +++ b/Zend/tests/traits/abstract_method_4.phpt @@ -10,9 +10,11 @@ trait T { class C { use T; + /* For backwards-compatiblity reasons, visibility is not enforced here. */ private function method(int $a, string $b) {} } ?> ---EXPECTF-- -Fatal error: Access level to C::method() must be public (as in class T) in %s on line %d +===DONE=== +--EXPECT-- +===DONE=== diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c index 4c510f436de0d..54234b9ebf913 100644 --- a/Zend/zend_inheritance.c +++ b/Zend/zend_inheritance.c @@ -805,7 +805,7 @@ static void perform_delayable_implementation_check( } } -static zend_always_inline inheritance_status do_inheritance_check_on_method_ex(zend_function *child, zend_function *parent, zend_class_entry *ce, zval *child_zv, zend_bool check_only, zend_bool checked) /* {{{ */ +static zend_always_inline inheritance_status do_inheritance_check_on_method_ex(zend_function *child, zend_function *parent, zend_class_entry *ce, zval *child_zv, zend_bool check_visibility, zend_bool check_only, zend_bool checked) /* {{{ */ { uint32_t child_flags; uint32_t parent_flags = parent->common.fn_flags; @@ -888,7 +888,8 @@ static zend_always_inline inheritance_status do_inheritance_check_on_method_ex(z } /* Prevent derived classes from restricting access that was available in parent classes (except deriving from non-abstract ctors) */ - if (!checked && (child_flags & ZEND_ACC_PPP_MASK) > (parent_flags & ZEND_ACC_PPP_MASK)) { + if (!checked && check_visibility + && (child_flags & ZEND_ACC_PPP_MASK) > (parent_flags & ZEND_ACC_PPP_MASK)) { if (check_only) { return INHERITANCE_ERROR; } @@ -907,9 +908,9 @@ static zend_always_inline inheritance_status do_inheritance_check_on_method_ex(z } /* }}} */ -static zend_never_inline void do_inheritance_check_on_method(zend_function *child, zend_function *parent, zend_class_entry *ce, zval *child_zv) /* {{{ */ +static zend_never_inline void do_inheritance_check_on_method(zend_function *child, zend_function *parent, zend_class_entry *ce, zval *child_zv, zend_bool check_visibility) /* {{{ */ { - do_inheritance_check_on_method_ex(child, parent, ce, child_zv, 0, 0); + do_inheritance_check_on_method_ex(child, parent, ce, child_zv, check_visibility, 0, 0); } /* }}} */ @@ -926,9 +927,10 @@ static zend_always_inline void do_inherit_method(zend_string *key, zend_function } if (checked) { - do_inheritance_check_on_method_ex(func, parent, ce, child, 0, checked); + do_inheritance_check_on_method_ex( + func, parent, ce, child, /* check_visibility */ 1, 0, checked); } else { - do_inheritance_check_on_method(func, parent, ce, child); + do_inheritance_check_on_method(func, parent, ce, child, /* check_visibility */ 1); } } else { @@ -1592,7 +1594,11 @@ static void zend_add_trait_method(zend_class_entry *ce, zend_string *name, zend_ /* Abstract method signatures from the trait must be satisfied. */ if (fn->common.fn_flags & ZEND_ACC_ABSTRACT) { - do_inheritance_check_on_method(existing_fn, fn, ce, NULL); + /* "abstract private" methods in traits were not available prior to PHP 8. + * As such, "abstract protected" was sometimes used to indicate trait requirements, + * even though the "implementing" method was private. Do not check visibility + * requirements to maintain backwards-compatibility with such usage. */ + do_inheritance_check_on_method(existing_fn, fn, ce, NULL, /* check_visibility */ 0); return; } @@ -1614,7 +1620,7 @@ static void zend_add_trait_method(zend_class_entry *ce, zend_string *name, zend_ } else { /* inherited members are overridden by members inserted by traits */ /* check whether the trait method fulfills the inheritance requirements */ - do_inheritance_check_on_method(fn, existing_fn, ce, NULL); + do_inheritance_check_on_method(fn, existing_fn, ce, NULL, /* check_visibility */ 1); } } @@ -2475,7 +2481,8 @@ static inheritance_status zend_can_early_bind(zend_class_entry *ce, zend_class_e if (zv) { zend_function *child_func = Z_FUNC_P(zv); inheritance_status status = - do_inheritance_check_on_method_ex(child_func, parent_func, ce, NULL, 1, 0); + do_inheritance_check_on_method_ex( + child_func, parent_func, ce, NULL, /* check_visibility */ 1, 1, 0); if (UNEXPECTED(status != INHERITANCE_SUCCESS)) { return status;