diff options
23 files changed, 544 insertions, 96 deletions
diff --git a/Zend/tests/bug62441.phpt b/Zend/tests/bug62441.phpt index 581c0dad68..2b9f35cb30 100644 --- a/Zend/tests/bug62441.phpt +++ b/Zend/tests/bug62441.phpt @@ -16,4 +16,4 @@ namespace ns { } ?> --EXPECTF-- -Fatal error: Declaration of ns\Foo::method(ns\stdClass $o) must be compatible with Iface::method(stdClass $o) in %s on line %d +Fatal error: Could not check compatibility between ns\Foo::method(ns\stdClass $o) and Iface::method(stdClass $o), because class ns\stdClass is not available in %s on line %d diff --git a/Zend/tests/bug76451.inc b/Zend/tests/bug76451.inc new file mode 100644 index 0000000000..18d3b50789 --- /dev/null +++ b/Zend/tests/bug76451.inc @@ -0,0 +1,4 @@ +<?php + +class Foo {} +class_alias('Foo', 'Bar'); diff --git a/Zend/tests/bug76451.phpt b/Zend/tests/bug76451.phpt new file mode 100644 index 0000000000..ad72056e4b --- /dev/null +++ b/Zend/tests/bug76451.phpt @@ -0,0 +1,16 @@ +--TEST-- +Bug #76451: Aliases during inheritance type checks affected by opcache +--FILE-- +<?php +require __DIR__ . "/bug76451.inc"; + +class A { + public function test(Foo $foo) {} +} +class B extends A { + public function test(Bar $foo) {} +} +?> +===DONE=== +--EXPECT-- +===DONE=== diff --git a/Zend/tests/bug76451_2.inc b/Zend/tests/bug76451_2.inc new file mode 100644 index 0000000000..df7cfd6645 --- /dev/null +++ b/Zend/tests/bug76451_2.inc @@ -0,0 +1,8 @@ +<?php +class A { + public function test(Foo $foo) {} +} +class B extends A { + public function test(Bar $foo) {} +} +?> diff --git a/Zend/tests/bug76451_2.phpt b/Zend/tests/bug76451_2.phpt new file mode 100644 index 0000000000..75cacccf30 --- /dev/null +++ b/Zend/tests/bug76451_2.phpt @@ -0,0 +1,12 @@ +--TEST-- +Bug #76451: Aliases during inheritance type checks affected by opcache (variation) +--FILE-- +<?php +class Foo {} +class_alias('Foo', 'Bar'); + +require __DIR__ . '/bug76451_2.inc'; +?> +===DONE=== +--EXPECT-- +===DONE=== diff --git a/Zend/tests/return_types/008.phpt b/Zend/tests/return_types/008.phpt index a178607feb..d5178ebd9d 100644 --- a/Zend/tests/return_types/008.phpt +++ b/Zend/tests/return_types/008.phpt @@ -14,6 +14,8 @@ class qux implements foo { } $qux = new qux(); -var_dump($qux->bar()); ---EXPECTF-- -Fatal error: Declaration of qux::bar(): qux must be compatible with foo::bar(): foo in %s008.php on line 8 +echo get_class($qux->bar()); + +?> +--EXPECT-- +qux diff --git a/Zend/tests/return_types/generators003.phpt b/Zend/tests/return_types/generators003.phpt index 489ef895a7..3b524eff2b 100644 --- a/Zend/tests/return_types/generators003.phpt +++ b/Zend/tests/return_types/generators003.phpt @@ -15,6 +15,8 @@ class SomeCollection implements Collection { } $some = new SomeCollection(); -var_dump($some->getIterator()); ---EXPECTF-- -Fatal error: Declaration of SomeCollection::getIterator(): Generator must be compatible with Collection::getIterator(): Iterator in %sgenerators003.php on line 7 +echo get_class($some->getIterator()); + +?> +--EXPECT-- +Generator diff --git a/Zend/tests/return_types/inheritance005.phpt b/Zend/tests/return_types/inheritance005.phpt index 6d8dfc8da9..9793051da8 100644 --- a/Zend/tests/return_types/inheritance005.phpt +++ b/Zend/tests/return_types/inheritance005.phpt @@ -13,5 +13,9 @@ class Bar extends Foo { return new Bar; } } ---EXPECTF-- -Fatal error: Declaration of Bar::test(): Bar must be compatible with Foo::test(): Foo in %sinheritance005.php on line 9 + +echo get_class(Bar::test()); + +?> +--EXPECT-- +Bar diff --git a/Zend/tests/return_types/inheritance006.phpt b/Zend/tests/return_types/inheritance006.phpt index 65cb1d2f72..8b7a37a1f4 100644 --- a/Zend/tests/return_types/inheritance006.phpt +++ b/Zend/tests/return_types/inheritance006.phpt @@ -17,5 +17,9 @@ class Bar extends Foo { return new B; } } ---EXPECTF-- -Fatal error: Declaration of Bar::test(): B must be compatible with Foo::test(): A in %sinheritance006.php on line 11 + +echo get_class(Bar::test()); + +?> +--EXPECT-- +B diff --git a/Zend/tests/return_types/inheritance007.phpt b/Zend/tests/return_types/inheritance007.phpt index d04ef71b1c..b08aaf9e99 100644 --- a/Zend/tests/return_types/inheritance007.phpt +++ b/Zend/tests/return_types/inheritance007.phpt @@ -15,5 +15,9 @@ class Bar extends Foo { return new ArrayObject([1, 2]); } } ---EXPECTF-- -Fatal error: Declaration of Bar::test(): ArrayObject must be compatible with Foo::test(): Traversable in %sinheritance007.php on line 9 + +echo get_class(Bar::test()); + +?> +--EXPECT-- +ArrayObject diff --git a/Zend/tests/type_declarations/variance/class_order.phpt b/Zend/tests/type_declarations/variance/class_order.phpt new file mode 100644 index 0000000000..df66e78b78 --- /dev/null +++ b/Zend/tests/type_declarations/variance/class_order.phpt @@ -0,0 +1,19 @@ +--TEST-- +Returns are covariant, but we don't allow the code due to class ordering +--FILE-- +<?php + +class A { + public function method() : B {} +} +class B extends A { + public function method() : C {} +} +class C extends B { +} + +new C; + +?> +--EXPECTF-- +Fatal error: Could not check compatibility between B::method(): C and A::method(): B, because class C is not available in %s on line %d diff --git a/Zend/tests/type_declarations/variance/class_order_autoload.phpt b/Zend/tests/type_declarations/variance/class_order_autoload.phpt new file mode 100644 index 0000000000..a4ea534577 --- /dev/null +++ b/Zend/tests/type_declarations/variance/class_order_autoload.phpt @@ -0,0 +1,25 @@ +--TEST-- +Returns are covariant, but we don't allow the code due to class ordering (autoload variation) +--FILE-- +<?php + +spl_autoload_register(function($class) { + if ($class === 'A') { + class A { + public function method() : B {} + } + } else if ($class == 'B') { + class B extends A { + public function method() : C {} + } + } else { + class C extends B { + } + } +}); + +$c = new C; + +?> +--EXPECTF-- +Fatal error: Could not check compatibility between B::method(): C and A::method(): B, because class C is not available in %s on line %d diff --git a/Zend/tests/type_declarations/variance/enum_forward_compat.phpt b/Zend/tests/type_declarations/variance/enum_forward_compat.phpt new file mode 100644 index 0000000000..3213aaaf76 --- /dev/null +++ b/Zend/tests/type_declarations/variance/enum_forward_compat.phpt @@ -0,0 +1,26 @@ +--TEST-- +Forward compatibility with types that look like classes but aren't +--FILE-- +<?php + +spl_autoload_register(function($class) { + var_dump($class); + if ($class === 'X') { + class X {} + } else { + class Y {} + } +}); + +class A { + public function method(X $param) : object {} +} + +class B extends A { + public function method(object $param) : Y {} +} + +?> +--EXPECT-- +string(1) "X" +string(1) "Y" diff --git a/Zend/tests/type_declarations/variance/object_variance.phpt b/Zend/tests/type_declarations/variance/object_variance.phpt new file mode 100644 index 0000000000..3ab0a311bf --- /dev/null +++ b/Zend/tests/type_declarations/variance/object_variance.phpt @@ -0,0 +1,22 @@ +--TEST-- +Testing object's variance in inheritance +--FILE-- +<?php + +interface I1 { + function method1(I1 $o): object; +} +interface I2 extends I1 { + function method1(object $o): I1; +} +final class C1 implements I2 { + function method1($o = null): self { + return $this; + } +} + +$o = new C1(); +echo get_class($o->method1()); +?> +--EXPECT-- +C1 diff --git a/Zend/tests/type_declarations/variance/parent_in_class.phpt b/Zend/tests/type_declarations/variance/parent_in_class.phpt new file mode 100644 index 0000000000..88b711bddd --- /dev/null +++ b/Zend/tests/type_declarations/variance/parent_in_class.phpt @@ -0,0 +1,45 @@ +--TEST-- +Use of parent inside a class that has / has no parent +--FILE-- +<?php + +// Illegal: A::parent is ill-defined +class A { + public function method(parent $x) {} +} +class B extends A { + public function method(parent $x) {} +} + +// Legal: A2::parent == P2 +class P2 {} +class A2 extends P2 { + public function method(parent $x) {} +} +class B2 extends A2 { + public function method(P2 $x) {} +} + +// Legal: B3::parent == A3 is subclass of A3::parent == P3 in covariant position +class P3 {} +class A3 extends P3 { + public function method($x): parent {} +} +class B3 extends A3 { + public function method($x): parent {} +} + +// Illegal: B4::parent == A4 is subclass of A4::parent == P4 in contravariant position +class P4 {} +class A4 extends P4 { + public function method(parent $x) {} +} +class B4 extends A4 { + public function method(parent $x) {} +} + +?> +--EXPECTF-- +Warning: Declaration of B4::method(A4 $x) should be compatible with A4::method(P4 $x) in %s on line %d + +Warning: Could not check compatibility between B::method(A $x) and A::method(parent $x), because class parent is not available in %s on line %d diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index d3bc68f893..99140e1818 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -1121,14 +1121,16 @@ ZEND_API void zend_do_delayed_early_binding(const zend_op_array *op_array, uint3 if (first_early_binding_opline != (uint32_t)-1) { zend_bool orig_in_compilation = CG(in_compilation); uint32_t opline_num = first_early_binding_opline; - zend_class_entry *ce; CG(in_compilation) = 1; while (opline_num != (uint32_t)-1) { const zend_op *opline = &op_array->opcodes[opline_num]; + zval *lcname = RT_CONSTANT(opline, opline->op1); zval *parent_name = RT_CONSTANT(opline, opline->op2); - if ((ce = zend_lookup_class_ex(Z_STR_P(parent_name), Z_STR_P(parent_name + 1), 0)) != NULL) { - do_bind_class(RT_CONSTANT(&op_array->opcodes[opline_num], op_array->opcodes[opline_num].op1), ce); + zend_class_entry *ce = zend_hash_find_ptr(EG(class_table), Z_STR_P(lcname + 1)); + zend_class_entry *parent_ce = zend_lookup_class_ex(Z_STR_P(parent_name), Z_STR_P(parent_name + 1), 0); + if (ce && parent_ce && zend_can_early_bind(ce, parent_ce)) { + do_bind_class(lcname, parent_ce); } opline_num = op_array->opcodes[opline_num].result.opline_num; } @@ -6278,6 +6280,7 @@ zend_op *zend_compile_class_decl(zend_ast *ast, zend_bool toplevel) /* {{{ */ && !(CG(compiler_options) & ZEND_COMPILE_PRELOAD) /* delay inheritance till preloading */ && ((parent_ce->type != ZEND_INTERNAL_CLASS) || !(CG(compiler_options) & ZEND_COMPILE_IGNORE_INTERNAL_CLASSES)) && ((parent_ce->type != ZEND_USER_CLASS) || !(CG(compiler_options) & ZEND_COMPILE_IGNORE_OTHER_FILES) || (parent_ce->info.user.filename == ce->info.user.filename)) + && zend_can_early_bind(ce, parent_ce) ) { if (EXPECTED(zend_hash_add_ptr(CG(class_table), lcname, ce) != NULL)) { CG(zend_lineno) = decl->end_lineno; diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c index e4292b3484..dc26d115de 100644 --- a/Zend/zend_inheritance.c +++ b/Zend/zend_inheritance.c @@ -22,6 +22,7 @@ #include "zend_compile.h" #include "zend_execute.h" #include "zend_inheritance.h" +#include "zend_interfaces.h" #include "zend_smart_str.h" #include "zend_operators.h" @@ -170,17 +171,73 @@ char *zend_visibility_string(uint32_t fn_flags) /* {{{ */ /* }}} */ static zend_string *resolve_class_name(const zend_function *fe, zend_string *name) { - ZEND_ASSERT(fe->common.scope); - if (zend_string_equals_literal_ci(name, "parent") && fe->common.scope->parent) { - return fe->common.scope->parent->name; + zend_class_entry *ce = fe->common.scope; + ZEND_ASSERT(ce); + if (zend_string_equals_literal_ci(name, "parent") && ce->parent) { + if (ce->ce_flags & (ZEND_ACC_LINKED|ZEND_ACC_LINKING_IN_PROGRESS)) { + return ce->parent->name; + } else { + return ce->parent_name; + } } else if (zend_string_equals_literal_ci(name, "self")) { - return fe->common.scope->name; + return ce->name; } else { return name; } } -static int zend_perform_covariant_type_check( +static zend_bool class_visible(zend_class_entry *ce) { + if (ce->type == ZEND_INTERNAL_CLASS) { + return !(CG(compiler_options) & ZEND_COMPILE_IGNORE_INTERNAL_CLASSES); + } else { + ZEND_ASSERT(ce->type == ZEND_USER_CLASS); + return !(CG(compiler_options) & ZEND_COMPILE_IGNORE_OTHER_FILES) + || ce->info.user.filename == CG(compiled_filename); + } +} + +static zend_class_entry *lookup_class(const zend_function *fe, zend_string *name) { + zend_class_entry *ce; + if (!CG(in_compilation)) { + ce = zend_lookup_class(name); + if (ce) { + return ce; + } + } else { + ce = zend_lookup_class_ex(name, NULL, /* autoload */ 0); + if (ce && class_visible(ce)) { + return ce; + } + } + + /* The current class may not be registered yet, so check for it explicitly. */ + if (zend_string_equals_ci(fe->common.scope->name, name)) { + return fe->common.scope; + } + + return NULL; +} + +/* Instanceof that's safe to use on unlinked classes. For the unlinked case, we only handle + * class identity here. */ +static zend_bool unlinked_instanceof(zend_class_entry *ce1, zend_class_entry *ce2) { + if ((ce1->ce_flags & (ZEND_ACC_LINKED|ZEND_ACC_LINKING_IN_PROGRESS))) { + return instanceof_function(ce1, ce2); + } + return ce1 == ce2; +} + +/* Unresolved means that class declarations that are currently not available are needed to + * determine whether the inheritance is valid or not. At runtime UNRESOLVED should be treated + * as an ERROR. */ +typedef enum { + INHERITANCE_UNRESOLVED = -1, + INHERITANCE_ERROR = 0, + INHERITANCE_SUCCESS = 1, +} inheritance_status; + +static inheritance_status zend_perform_covariant_type_check( + zend_string **unresolved_class, const zend_function *fe, zend_arg_info *fe_arg_info, const zend_function *proto, zend_arg_info *proto_arg_info) /* {{{ */ { @@ -188,79 +245,105 @@ static int zend_perform_covariant_type_check( ZEND_ASSERT(ZEND_TYPE_IS_SET(fe_type) && ZEND_TYPE_IS_SET(proto_type)); if (ZEND_TYPE_ALLOW_NULL(fe_type) && !ZEND_TYPE_ALLOW_NULL(proto_type)) { - return 0; + return INHERITANCE_ERROR; } - if (ZEND_TYPE_IS_CLASS(fe_type) && ZEND_TYPE_IS_CLASS(proto_type)) { - zend_string *fe_class_name = resolve_class_name(fe, ZEND_TYPE_NAME(fe_type)); - zend_string *proto_class_name = resolve_class_name(proto, ZEND_TYPE_NAME(proto_type)); + if (ZEND_TYPE_IS_CLASS(proto_type)) { + zend_string *fe_class_name, *proto_class_name; + zend_class_entry *fe_ce, *proto_ce; + if (!ZEND_TYPE_IS_CLASS(fe_type)) { + return INHERITANCE_ERROR; + } - if (fe_class_name != proto_class_name && strcasecmp(ZSTR_VAL(fe_class_name), ZSTR_VAL(proto_class_name)) != 0) { - if (fe->common.type != ZEND_USER_FUNCTION) { - return 0; - } else { - zend_class_entry *fe_ce, *proto_ce; + fe_class_name = resolve_class_name(fe, ZEND_TYPE_NAME(fe_type)); + proto_class_name = resolve_class_name(proto, ZEND_TYPE_NAME(proto_type)); + if (zend_string_equals_ci(fe_class_name, proto_class_name)) { + return INHERITANCE_SUCCESS; + } - fe_ce = zend_lookup_class(fe_class_name); - proto_ce = zend_lookup_class(proto_class_name); + fe_ce = lookup_class(fe, fe_class_name); + if (!fe_ce) { + *unresolved_class = fe_class_name; + return INHERITANCE_UNRESOLVED; + } - /* Check for class alias */ - if (!fe_ce || !proto_ce || - fe_ce->type == ZEND_INTERNAL_CLASS || - proto_ce->type == ZEND_INTERNAL_CLASS || - fe_ce != proto_ce) { - return 0; - } - } + proto_ce = lookup_class(proto, proto_class_name); + if (!proto_ce) { + *unresolved_class = proto_class_name; + return INHERITANCE_UNRESOLVED; } - } else if (ZEND_TYPE_CODE(fe_type) != ZEND_TYPE_CODE(proto_type)) { - if (ZEND_TYPE_CODE(proto_type) == IS_ITERABLE) { - if (ZEND_TYPE_CODE(fe_type) == IS_ARRAY) { - return 1; - } - if (ZEND_TYPE_IS_CLASS(fe_type) && - zend_string_equals_literal_ci(ZEND_TYPE_NAME(fe_type), "Traversable")) { - return 1; + return unlinked_instanceof(fe_ce, proto_ce) ? INHERITANCE_SUCCESS : INHERITANCE_ERROR; + } else if (ZEND_TYPE_CODE(proto_type) == IS_ITERABLE) { + if (ZEND_TYPE_IS_CLASS(fe_type)) { + zend_string *fe_class_name = resolve_class_name(fe, ZEND_TYPE_NAME(fe_type)); + zend_class_entry *fe_ce = lookup_class(fe, fe_class_name); + if (!fe_ce) { + *unresolved_class = fe_class_name; + return INHERITANCE_UNRESOLVED; + } + return unlinked_instanceof(fe_ce, zend_ce_traversable) + ? INHERITANCE_SUCCESS : INHERITANCE_ERROR; + } + + return ZEND_TYPE_CODE(fe_type) == IS_ITERABLE || ZEND_TYPE_CODE(fe_type) == IS_ARRAY + ? INHERITANCE_SUCCESS : INHERITANCE_ERROR; + } else if (ZEND_TYPE_CODE(proto_type) == IS_OBJECT) { + if (ZEND_TYPE_IS_CLASS(fe_type)) { + /* Currently, any class name would be allowed here. We still perform a class lookup + * for forward-compatibility reasons, as we may have named types in the future that + * are not classes (such as enums or typedefs). */ + zend_string *fe_class_name = resolve_class_name(fe, ZEND_TYPE_NAME(fe_type)); + zend_class_entry *fe_ce = lookup_class(fe, fe_class_name); + if (!fe_ce) { + *unresolved_class = fe_class_name; + return INHERITANCE_UNRESOLVED; } + return INHERITANCE_SUCCESS; } - /* Incompatible built-in types */ - return 0; + return ZEND_TYPE_CODE(fe_type) == IS_OBJECT ? INHERITANCE_SUCCESS : INHERITANCE_ERROR; + } else { + return ZEND_TYPE_CODE(fe_type) == ZEND_TYPE_CODE(proto_type) + ? INHERITANCE_SUCCESS : INHERITANCE_ERROR; } - - return 1; } /* }}} */ -static int zend_do_perform_arg_type_hint_check(const zend_function *fe, zend_arg_info *fe_arg_info, const zend_function *proto, zend_arg_info *proto_arg_info) /* {{{ */ +static inheritance_status zend_do_perform_arg_type_hint_check( + zend_string **unresolved_class, + const zend_function *fe, zend_arg_info *fe_arg_info, + const zend_function *proto, zend_arg_info *proto_arg_info) /* {{{ */ { if (!ZEND_TYPE_IS_SET(fe_arg_info->type)) { /* Child with no type is always compatible */ - return 1; + return INHERITANCE_SUCCESS; } if (!ZEND_TYPE_IS_SET(proto_arg_info->type)) { /* Child defines a type, but parent doesn't, violates LSP */ - return 0; + return INHERITANCE_ERROR; } /* Contravariant type check is performed as a covariant type check with swapped * argument order. */ - return zend_perform_covariant_type_check(proto, proto_arg_info, fe, fe_arg_info); + return zend_perform_covariant_type_check( + unresolved_class, proto, proto_arg_info, fe, fe_arg_info); } /* }}} */ -static zend_bool zend_do_perform_implementation_check(const zend_function *fe, const zend_function *proto) /* {{{ */ +static inheritance_status zend_do_perform_implementation_check( + zend_string **unresolved_class, const zend_function *fe, const zend_function *proto) /* {{{ */ { uint32_t i, num_args; + inheritance_status status, local_status; /* If it's a user function then arg_info == NULL means we don't have any parameters but * we still need to do the arg number checks. We are only willing to ignore this for internal * functions because extensions don't always define arg_info. */ if (!proto->common.arg_info && proto->common.type != ZEND_USER_FUNCTION) { - return 1; + return INHERITANCE_SUCCESS; } /* Checks for constructors only if they are declared in an interface, @@ -269,29 +352,29 @@ static zend_bool zend_do_perform_implementation_check(const zend_function *fe, c if ((fe->common.fn_flags & ZEND_ACC_CTOR) && ((proto->common.scope->ce_flags & ZEND_ACC_INTERFACE) == 0 && (proto->common.fn_flags & ZEND_ACC_ABSTRACT) == 0)) { - return 1; + return INHERITANCE_SUCCESS; } /* If the prototype method is private do not enforce a signature */ if (proto->common.fn_flags & ZEND_ACC_PRIVATE) { - return 1; + return INHERITANCE_SUCCESS; } /* check number of arguments */ if (proto->common.required_num_args < fe->common.required_num_args || proto->common.num_args > fe->common.num_args) { - return 0; + return INHERITANCE_ERROR; } /* by-ref constraints on return values are covariant */ if ((proto->common.fn_flags & ZEND_ACC_RETURN_REFERENCE) && !(fe->common.fn_flags & ZEND_ACC_RETURN_REFERENCE)) { - return 0; + return INHERITANCE_ERROR; } if ((proto->common.fn_flags & ZEND_ACC_VARIADIC) && !(fe->common.fn_flags & ZEND_ACC_VARIADIC)) { - return 0; + return INHERITANCE_ERROR; } /* For variadic functions any additional (optional) arguments that were added must be @@ -309,6 +392,7 @@ static zend_bool zend_do_perform_implementation_check(const zend_function *fe, c } } + status = INHERITANCE_SUCCESS; for (i = 0; i < num_args; i++) { zend_arg_info *fe_arg_info = &fe->common.arg_info[i]; @@ -319,13 +403,18 @@ static zend_bool zend_do_perform_implementation_check(const zend_function *fe, c proto_arg_info = &proto->common.arg_info[proto->common.num_args]; } - if (!zend_do_perform_arg_type_hint_check(fe, fe_arg_info, proto, proto_arg_info)) { - return 0; + local_status = zend_do_perform_arg_type_hint_check( + unresolved_class, fe, fe_arg_info, proto, proto_arg_info); + if (local_status == INHERITANCE_ERROR) { + return INHERITANCE_ERROR; + } + if (local_status == INHERITANCE_UNRESOLVED) { + status = INHERITANCE_UNRESOLVED; } /* by-ref constraints on arguments are invariant */ if (fe_arg_info->pass_by_reference != proto_arg_info->pass_by_reference) { - return 0; + return INHERITANCE_ERROR; } } @@ -334,14 +423,20 @@ static zend_bool zend_do_perform_implementation_check(const zend_function *fe, c if (proto->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) { /* Removing a return type is not valid. */ if (!(fe->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE)) { - return 0; + return INHERITANCE_ERROR; } - if (!zend_perform_covariant_type_check(fe, fe->common.arg_info - 1, proto, proto->common.arg_info - 1)) { - return 0; + local_status = zend_perform_covariant_type_check( + unresolved_class, fe, fe->common.arg_info - 1, proto, proto->common.arg_info - 1); + if (local_status == INHERITANCE_ERROR) { + return INHERITANCE_ERROR; + } + if (local_status == INHERITANCE_UNRESOLVED) { + status = INHERITANCE_UNRESOLVED; } } - return 1; + + return status; } /* }}} */ @@ -510,10 +605,30 @@ static zend_always_inline uint32_t func_lineno(zend_function *fn) { return fn->common.type == ZEND_USER_FUNCTION ? fn->op_array.line_start : 0; } +static void ZEND_COLD emit_incompatible_method_error( + zend_function *child, zend_function *parent, + inheritance_status status, zend_string *unresolved_class) { + zend_string *parent_prototype = zend_get_function_declaration(parent); + zend_string *child_prototype = zend_get_function_declaration(child); + if (status == INHERITANCE_UNRESOLVED) { + zend_error_at(E_COMPILE_ERROR, NULL, func_lineno(child), + "Could not check compatibility between %s and %s, because class %s is not available", + ZSTR_VAL(child_prototype), ZSTR_VAL(parent_prototype), ZSTR_VAL(unresolved_class)); + } else { + zend_error_at(E_COMPILE_ERROR, NULL, func_lineno(child), + "Declaration of %s must be compatible with %s", + ZSTR_VAL(child_prototype), ZSTR_VAL(parent_prototype)); + } + zend_string_efree(child_prototype); + zend_string_efree(parent_prototype); +} + static void do_inheritance_check_on_method(zend_function *child, zend_function *parent, zend_class_entry *ce, zval *child_zv) /* {{{ */ { uint32_t child_flags; uint32_t parent_flags = parent->common.fn_flags; + inheritance_status status; + zend_string *unresolved_class; if (UNEXPECTED(parent_flags & ZEND_ACC_FINAL)) { zend_error_at_noreturn(E_COMPILE_ERROR, NULL, func_lineno(child), @@ -592,14 +707,9 @@ static void do_inheritance_check_on_method(zend_function *child, zend_function * ZEND_FN_SCOPE_NAME(child), ZSTR_VAL(child->common.function_name), zend_visibility_string(parent_flags), ZEND_FN_SCOPE_NAME(parent), (parent_flags&ZEND_ACC_PUBLIC) ? "" : " or weaker"); } - if (UNEXPECTED(!zend_do_perform_implementation_check(child, parent))) { - zend_string *method_prototype = zend_get_function_declaration(parent); - zend_string *child_prototype = zend_get_function_declaration(child); - zend_error_at(E_COMPILE_ERROR, NULL, func_lineno(child), - "Declaration of %s must be compatible with %s", - ZSTR_VAL(child_prototype), ZSTR_VAL(method_prototype)); - zend_string_efree(child_prototype); - zend_string_efree(method_prototype); + status = zend_do_perform_implementation_check(&unresolved_class, child, parent); + if (status != INHERITANCE_SUCCESS) { + emit_incompatible_method_error(child, parent, status, unresolved_class); } } } while (0); @@ -1294,6 +1404,8 @@ static void zend_add_trait_method(zend_class_entry *ce, const char *name, zend_s { zend_function *existing_fn = NULL; zend_function *new_fn; + zend_string *unresolved_class; + inheritance_status status; if ((existing_fn = zend_hash_find_ptr(&ce->function_table, key)) != NULL) { /* if it is the same function with the same visibility and has not been assigned a class scope yet, regardless @@ -1311,18 +1423,20 @@ static void zend_add_trait_method(zend_class_entry *ce, const char *name, zend_s 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 */ - if (UNEXPECTED(!zend_do_perform_implementation_check(fn, existing_fn))) { - zend_error_noreturn(E_COMPILE_ERROR, "Declaration of %s must be compatible with %s", - ZSTR_VAL(zend_get_function_declaration(fn)), - ZSTR_VAL(zend_get_function_declaration(existing_fn))); + status = zend_do_perform_implementation_check( + &unresolved_class, fn, existing_fn); + if (status != INHERITANCE_SUCCESS) { + emit_incompatible_method_error( + fn, existing_fn, status, unresolved_class); } } if (fn->common.fn_flags & ZEND_ACC_ABSTRACT) { /* Make sure the abstract declaration is compatible with previous declaration */ - if (UNEXPECTED(!zend_do_perform_implementation_check(existing_fn, fn))) { - zend_error_noreturn(E_COMPILE_ERROR, "Declaration of %s must be compatible with %s", - ZSTR_VAL(zend_get_function_declaration(existing_fn)), - ZSTR_VAL(zend_get_function_declaration(fn))); + status = zend_do_perform_implementation_check( + &unresolved_class, existing_fn, fn); + if (status != INHERITANCE_SUCCESS) { + emit_incompatible_method_error( + existing_fn, fn, status, unresolved_class); } return; } @@ -1336,17 +1450,15 @@ static void zend_add_trait_method(zend_class_entry *ce, const char *name, zend_s } else if (existing_fn->common.fn_flags & ZEND_ACC_ABSTRACT && (existing_fn->common.scope->ce_flags & ZEND_ACC_INTERFACE) == 0) { /* Make sure the trait method is compatible with previosly declared abstract method */ - if (UNEXPECTED(!zend_do_perform_implementation_check(fn, existing_fn))) { - zend_error_noreturn(E_COMPILE_ERROR, "Declaration of %s must be compatible with %s", - ZSTR_VAL(zend_get_function_declaration(fn)), - ZSTR_VAL(zend_get_function_declaration(existing_fn))); + status = zend_do_perform_implementation_check(&unresolved_class, fn, existing_fn); + if (status != INHERITANCE_SUCCESS) { + emit_incompatible_method_error(fn, existing_fn, status, unresolved_class); } } else if (fn->common.fn_flags & ZEND_ACC_ABSTRACT) { /* Make sure the abstract declaration is compatible with previous declaration */ - if (UNEXPECTED(!zend_do_perform_implementation_check(existing_fn, fn))) { - zend_error_noreturn(E_COMPILE_ERROR, "Declaration of %s must be compatible with %s", - ZSTR_VAL(zend_get_function_declaration(existing_fn)), - ZSTR_VAL(zend_get_function_declaration(fn))); + status = zend_do_perform_implementation_check(&unresolved_class, existing_fn, fn); + if (status != INHERITANCE_SUCCESS) { + emit_incompatible_method_error(existing_fn, fn, status, unresolved_class); } return; } else if (UNEXPECTED(existing_fn->common.scope->ce_flags & ZEND_ACC_TRAIT)) { @@ -1979,4 +2091,24 @@ ZEND_API void zend_do_link_class(zend_class_entry *ce, zend_class_entry *parent) ce->ce_flags &= ~ZEND_ACC_LINKING_IN_PROGRESS; ce->ce_flags |= ZEND_ACC_LINKED; } + +/* Check whether early binding is prevented due to unresolved types in inheritance checks. */ +zend_bool zend_can_early_bind(zend_class_entry *ce, zend_class_entry *parent_ce) { + zend_string *key; + zend_function *parent_func; + ZEND_HASH_FOREACH_STR_KEY_PTR(&parent_ce->function_table, key, parent_func) { + uint32_t parent_flags = parent_func->common.fn_flags; + zend_function *func = zend_hash_find_ptr(&ce->function_table, key); + zend_string *unresolved_class; + if (!func || (parent_flags & ZEND_ACC_PRIVATE) || + ((parent_flags & ZEND_ACC_CTOR) && !(parent_flags & ZEND_ACC_ABSTRACT)) + ) { + continue; + } + if (zend_do_perform_implementation_check(&unresolved_class, func, parent_func) == INHERITANCE_UNRESOLVED) { + return 0; + } + } ZEND_HASH_FOREACH_END(); + return 1; +} /* }}} */ diff --git a/Zend/zend_inheritance.h b/Zend/zend_inheritance.h index eb68a6d278..cba8efe50b 100644 --- a/Zend/zend_inheritance.h +++ b/Zend/zend_inheritance.h @@ -31,6 +31,7 @@ ZEND_API void zend_do_link_class(zend_class_entry *ce, zend_class_entry *parent_ void zend_verify_abstract_class(zend_class_entry *ce); void zend_build_properties_info_table(zend_class_entry *ce); +zend_bool zend_can_early_bind(zend_class_entry *ce, zend_class_entry *parent_ce); END_EXTERN_C() diff --git a/Zend/zend_operators.c b/Zend/zend_operators.c index 361223a7c0..fd784b7534 100644 --- a/Zend/zend_operators.c +++ b/Zend/zend_operators.c @@ -2260,7 +2260,7 @@ static zend_bool ZEND_FASTCALL instanceof_interface_only(const zend_class_entry uint32_t i; if (instance_ce->num_interfaces) { - ZEND_ASSERT(instance_ce->ce_flags & ZEND_ACC_LINKED); + ZEND_ASSERT(instance_ce->ce_flags & (ZEND_ACC_LINKED|ZEND_ACC_LINKING_IN_PROGRESS)); for (i = 0; i < instance_ce->num_interfaces; i++) { if (instanceof_interface_only(instance_ce->interfaces[i], ce)) { return 1; @@ -2288,7 +2288,7 @@ static zend_bool ZEND_FASTCALL instanceof_interface(const zend_class_entry *inst uint32_t i; if (instance_ce->num_interfaces) { - ZEND_ASSERT(instance_ce->ce_flags & ZEND_ACC_LINKED); + ZEND_ASSERT(instance_ce->ce_flags & (ZEND_ACC_LINKED|ZEND_ACC_LINKING_IN_PROGRESS)); for (i = 0; i < instance_ce->num_interfaces; i++) { if (instanceof_interface(instance_ce->interfaces[i], ce)) { return 1; diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index 6704481cbc..99fae3f9f0 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -3452,6 +3452,44 @@ static zend_bool preload_try_resolve_property_types(zend_class_entry *ce) return ok; } +static zend_bool preload_is_type_known(zend_class_entry *ce, zend_type type) { + zend_string *name, *lcname; + zend_bool known; + if (!ZEND_TYPE_IS_NAME(type)) { + return 1; + } + + name = ZEND_TYPE_NAME(type); + if (zend_string_equals_literal_ci(name, "self") || + zend_string_equals_literal_ci(name, "parent") || + zend_string_equals_ci(name, ce->name)) { + return 1; + } + + lcname = zend_string_tolower(name); + known = zend_hash_exists(EG(class_table), lcname); + zend_string_release(lcname); + return known; +} + +static zend_bool preload_all_types_known(zend_class_entry *ce) { + zend_function *fptr; + ZEND_HASH_FOREACH_PTR(&ce->function_table, fptr) { + uint32_t i; + if (fptr->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) { + if (!preload_is_type_known(ce, fptr->common.arg_info[-1].type)) { + return 0; + } + } + for (i = 0; i < fptr->common.num_args; i++) { + if (!preload_is_type_known(ce, fptr->common.arg_info[i].type)) { + return 0; + } + } + } ZEND_HASH_FOREACH_END(); + return 1; +} + static void preload_link(void) { zval *zv; @@ -3545,6 +3583,14 @@ static void preload_link(void) if (!found) continue; } + /* TODO: This is much more restrictive than necessary. We only need to actually + * know the types for covariant checks, but don't need them if we can ensure + * compatibility through a simple string comparison. We could improve this using + * a more general version of zend_can_early_bind(). */ + if (!preload_all_types_known(ce)) { + continue; + } + zend_string *key = zend_string_tolower(ce->name); zv = zend_hash_set_bucket_key(EG(class_table), (Bucket*)zv, key); zend_string_release(key); diff --git a/ext/opcache/tests/preload_011.phpt b/ext/opcache/tests/preload_011.phpt new file mode 100644 index 0000000000..438cd80500 --- /dev/null +++ b/ext/opcache/tests/preload_011.phpt @@ -0,0 +1,30 @@ +--TEST-- +Argument/return types must be available for preloading +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.optimization_level=-1 +opcache.preload={PWD}/preload_variance_ind.inc +--SKIPIF-- +<?php require_once('skipif.inc'); ?> +--FILE-- +<?php +interface K {} +interface L extends K {} +require __DIR__ . '/preload_variance.inc'; + +$a = new A; +$b = new B; +$d = new D; +$f = new F; +$g = new G; + +?> +===DONE=== +--EXPECTF-- +Warning: Can't preload unlinked class H in %s on line %d + +Warning: Can't preload unlinked class B in %s on line %d + +Warning: Can't preload unlinked class A in %s on line %d +===DONE=== diff --git a/ext/opcache/tests/preload_variance.inc b/ext/opcache/tests/preload_variance.inc new file mode 100644 index 0000000000..09f057442c --- /dev/null +++ b/ext/opcache/tests/preload_variance.inc @@ -0,0 +1,41 @@ +<?php + +// Requires X, delay to runtime. +// TODO: It is not actually required, because we don't need X to check inheritance in this case. +class A extends Z { + public function method(X $a) {} +} +class B extends Z { + public function method($a) : X {} +} + +// Works. +class C extends Z { + public function method($a): self {} + public function method2($a): C {} +} +class D extends C { + public function method($a): self {} + public function method2($a): D {} +} + +// Works. +interface I {} +interface J extends I {} +class E { + public function method($a): I {} +} +class F extends E { + public function method($a): J {} +} + +// Requires K & L, delay to runtime. +class G { + public function method($a): K {} +} +class H extends G { + public function method($a): L {} +} + +// Early-binding preventer. +class Z {} diff --git a/ext/opcache/tests/preload_variance_ind.inc b/ext/opcache/tests/preload_variance_ind.inc new file mode 100644 index 0000000000..6e331dd48c --- /dev/null +++ b/ext/opcache/tests/preload_variance_ind.inc @@ -0,0 +1,2 @@ +<?php +opcache_compile_file(__DIR__ . '/preload_variance.inc'); |