From 8f8fcbbd397370b407dc2552c4bd6ee4ccb0e93b Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Mon, 27 May 2019 11:39:56 +0200 Subject: [PATCH] Support full variance if autoloading is used Keep track of delayed variance obligations and check them after linking a class is otherwise finished. Obligations may either be unresolved method compatibility (because the necessecary classes aren't available yet) or open parent/interface dependencies. The latter occur because we allow the use of not fully linked classes as parents/interfaces now. An important aspect of the implementation is we do not require classes involved in variance checks to be fully linked in order for the class to be fully linked. Because the involved types do have to exist in the class table (as partially linked classes) and we do check these for correct variance, we have the guarantee that either those classes will successfully link lateron or generate an error, but there is no way to actually use them until that point and as such no possibility of violating the variance contract. This is important because it ensures that a class declaration always either errors or will produce an immediately usable class afterwards -- there are no cases where the finalization of the class declaration has to be delayed until a later time, as earlier variants of this patch did. Because variance checks deal with classes in various stages of linking, we need to use a special instanceof implementation that supports this, and also introduce finer-grained flags that tell us which parts have been linked already and which haven't. Class autoloading for variance checks is delayed into a separate stage after the class is otherwise linked and before delayed variance obligations are processed. This separation is needed to handle cases like A extends B extends C, where B is the autoload root, but C is required to check variance. This could end up loading C while the class structure of B is in an inconsistent state. --- UPGRADING | 4 +- Zend/tests/bug30922.phpt | 2 +- .../variance/class_order_autoload1.phpt | 37 ++ .../variance/class_order_autoload2.phpt | 38 ++ .../variance/class_order_autoload3.phpt | 45 +++ .../variance/class_order_autoload4.phpt | 44 +++ .../variance/class_order_autoload5.phpt | 60 +++ ....phpt => class_order_autoload_error1.phpt} | 10 +- .../variance/class_order_autoload_error2.phpt | 27 ++ .../variance/class_order_autoload_error3.phpt | 38 ++ .../variance/class_order_autoload_error4.phpt | 39 ++ .../variance/class_order_autoload_error5.phpt | 52 +++ .../variance/class_order_autoload_error6.phpt | 39 ++ ...lass_order.phpt => class_order_error.phpt} | 0 Zend/zend_API.c | 2 +- Zend/zend_compile.c | 14 + Zend/zend_compile.h | 11 +- Zend/zend_execute_API.c | 4 +- Zend/zend_globals.h | 3 + Zend/zend_inheritance.c | 350 +++++++++++++++--- Zend/zend_interfaces.c | 4 +- Zend/zend_opcode.c | 4 +- Zend/zend_operators.c | 4 +- 23 files changed, 762 insertions(+), 69 deletions(-) create mode 100644 Zend/tests/type_declarations/variance/class_order_autoload1.phpt create mode 100644 Zend/tests/type_declarations/variance/class_order_autoload2.phpt create mode 100644 Zend/tests/type_declarations/variance/class_order_autoload3.phpt create mode 100644 Zend/tests/type_declarations/variance/class_order_autoload4.phpt create mode 100644 Zend/tests/type_declarations/variance/class_order_autoload5.phpt rename Zend/tests/type_declarations/variance/{class_order_autoload.phpt => class_order_autoload_error1.phpt} (59%) create mode 100644 Zend/tests/type_declarations/variance/class_order_autoload_error2.phpt create mode 100644 Zend/tests/type_declarations/variance/class_order_autoload_error3.phpt create mode 100644 Zend/tests/type_declarations/variance/class_order_autoload_error4.phpt create mode 100644 Zend/tests/type_declarations/variance/class_order_autoload_error5.phpt create mode 100644 Zend/tests/type_declarations/variance/class_order_autoload_error6.phpt rename Zend/tests/type_declarations/variance/{class_order.phpt => class_order_error.phpt} (100%) diff --git a/UPGRADING b/UPGRADING index 6c610858342f0..eceb9b6f0ad9d 100644 --- a/UPGRADING +++ b/UPGRADING @@ -163,7 +163,9 @@ PHP 7.4 UPGRADE NOTES public function method(): B {} } - This feature is currently restricted to non-cyclic type references only. + Full variance support is only available if autoloading is used. Inside a + single file only non-cyclic type references are possible, because all + classes need to be available before they are referenced. RFC: https://wiki.php.net/rfc/covariant-returns-and-contravariant-parameters . Added support for coalesce assign (??=) operator. For example: diff --git a/Zend/tests/bug30922.phpt b/Zend/tests/bug30922.phpt index 0d5d8ae8385a5..001845a7cb38d 100644 --- a/Zend/tests/bug30922.phpt +++ b/Zend/tests/bug30922.phpt @@ -10,4 +10,4 @@ var_dump($a instanceOf A); echo "ok\n"; ?> --EXPECTF-- -Fatal error: Interface 'RecurisiveFooFar' not found in %sbug30922.php on line %d +Fatal error: Interface RecurisiveFooFar cannot implement itself in %s on line %d diff --git a/Zend/tests/type_declarations/variance/class_order_autoload1.phpt b/Zend/tests/type_declarations/variance/class_order_autoload1.phpt new file mode 100644 index 0000000000000..d68d2e8afaf79 --- /dev/null +++ b/Zend/tests/type_declarations/variance/class_order_autoload1.phpt @@ -0,0 +1,37 @@ +--TEST-- +Class order allowed with autoloading (1) +--FILE-- + +===DONE=== +--EXPECT-- +object(A)#2 (0) { +} +object(B)#2 (0) { +} +object(C)#2 (0) { +} +object(C)#2 (0) { +} +===DONE=== diff --git a/Zend/tests/type_declarations/variance/class_order_autoload2.phpt b/Zend/tests/type_declarations/variance/class_order_autoload2.phpt new file mode 100644 index 0000000000000..f6229d3995883 --- /dev/null +++ b/Zend/tests/type_declarations/variance/class_order_autoload2.phpt @@ -0,0 +1,38 @@ +--TEST-- +Class order allowed with autoloading (2) +--FILE-- + +===DONE=== +--EXPECT-- +object(A)#2 (0) { +} +object(C)#2 (0) { +} +object(B)#2 (0) { +} +object(B)#2 (0) { +} +===DONE=== diff --git a/Zend/tests/type_declarations/variance/class_order_autoload3.phpt b/Zend/tests/type_declarations/variance/class_order_autoload3.phpt new file mode 100644 index 0000000000000..d09f2a9c45b35 --- /dev/null +++ b/Zend/tests/type_declarations/variance/class_order_autoload3.phpt @@ -0,0 +1,45 @@ +--TEST-- +Class order allowed with autoloading (3) +--FILE-- + +===DONE=== +--EXPECT-- +object(A)#2 (0) { +} +object(X)#2 (0) { +} +object(Y)#2 (0) { +} +object(B)#2 (0) { +} +object(B)#2 (0) { +} +===DONE=== diff --git a/Zend/tests/type_declarations/variance/class_order_autoload4.phpt b/Zend/tests/type_declarations/variance/class_order_autoload4.phpt new file mode 100644 index 0000000000000..37070b444c6de --- /dev/null +++ b/Zend/tests/type_declarations/variance/class_order_autoload4.phpt @@ -0,0 +1,44 @@ +--TEST-- +Class order allowed with autoloading (4) +--FILE-- + +===DONE=== +--EXPECT-- +object(A)#2 (0) { +} +bool(true) +bool(true) +object(B)#2 (0) { +} +object(B)#2 (0) { +} +===DONE=== diff --git a/Zend/tests/type_declarations/variance/class_order_autoload5.phpt b/Zend/tests/type_declarations/variance/class_order_autoload5.phpt new file mode 100644 index 0000000000000..77e9a0a9ec5f0 --- /dev/null +++ b/Zend/tests/type_declarations/variance/class_order_autoload5.phpt @@ -0,0 +1,60 @@ +--TEST-- +Class order allowed with autoloading (5) +--FILE-- + +===DONE=== +--EXPECT-- +object(A)#2 (0) { +} +object(X)#2 (0) { +} +object(Q)#2 (0) { +} +object(R)#2 (0) { +} +object(Y)#2 (0) { +} +object(B)#2 (0) { +} +object(B)#2 (0) { +} +===DONE=== diff --git a/Zend/tests/type_declarations/variance/class_order_autoload.phpt b/Zend/tests/type_declarations/variance/class_order_autoload_error1.phpt similarity index 59% rename from Zend/tests/type_declarations/variance/class_order_autoload.phpt rename to Zend/tests/type_declarations/variance/class_order_autoload_error1.phpt index a4ea534577fa1..9af404160414d 100644 --- a/Zend/tests/type_declarations/variance/class_order_autoload.phpt +++ b/Zend/tests/type_declarations/variance/class_order_autoload_error1.phpt @@ -1,16 +1,16 @@ --TEST-- -Returns are covariant, but we don't allow the code due to class ordering (autoload variation) +Variance error in the presence of autoloading (1) --FILE-- --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 +Fatal error: Declaration of B::method(): B must be compatible with A::method(): C in %s on line %d diff --git a/Zend/tests/type_declarations/variance/class_order_autoload_error2.phpt b/Zend/tests/type_declarations/variance/class_order_autoload_error2.phpt new file mode 100644 index 0000000000000..48d2e0b9561c7 --- /dev/null +++ b/Zend/tests/type_declarations/variance/class_order_autoload_error2.phpt @@ -0,0 +1,27 @@ +--TEST-- +Variance error in the presence of autoloading (2) +--FILE-- + +--EXPECTF-- +Warning: Declaration of B::method(C $x) should be compatible with A::method(B $x) in %s on line %d diff --git a/Zend/tests/type_declarations/variance/class_order_autoload_error3.phpt b/Zend/tests/type_declarations/variance/class_order_autoload_error3.phpt new file mode 100644 index 0000000000000..23b60b4584f9a --- /dev/null +++ b/Zend/tests/type_declarations/variance/class_order_autoload_error3.phpt @@ -0,0 +1,38 @@ +--TEST-- +Variance error in the presence of autoloading (3) +--FILE-- + +--EXPECTF-- +Fatal error: Declaration of R::method(): A must be compatible with Q::method(): B in %s on line %d diff --git a/Zend/tests/type_declarations/variance/class_order_autoload_error4.phpt b/Zend/tests/type_declarations/variance/class_order_autoload_error4.phpt new file mode 100644 index 0000000000000..6acf9313f4d40 --- /dev/null +++ b/Zend/tests/type_declarations/variance/class_order_autoload_error4.phpt @@ -0,0 +1,39 @@ +--TEST-- +Variance error in the presence of autoloading (4) +--FILE-- + +--EXPECTF-- +object(A)#2 (0) { +} +object(X)#2 (0) { +} + +Fatal error: Declaration of Y::method(): A must be compatible with X::method(): B in %s on line %d diff --git a/Zend/tests/type_declarations/variance/class_order_autoload_error5.phpt b/Zend/tests/type_declarations/variance/class_order_autoload_error5.phpt new file mode 100644 index 0000000000000..a6a46f84a2329 --- /dev/null +++ b/Zend/tests/type_declarations/variance/class_order_autoload_error5.phpt @@ -0,0 +1,52 @@ +--TEST-- +Variance error in the presence of autoloading (5) +--FILE-- + +--EXPECTF-- +object(A)#2 (0) { +} +object(X)#2 (0) { +} + +Warning: Declaration of Y::method(Z $a) should be compatible with X::method(Y $a) in %s on line %d +object(Z)#2 (0) { +} +object(Y)#2 (0) { +} +object(B)#2 (0) { +} +object(B)#2 (0) { +} diff --git a/Zend/tests/type_declarations/variance/class_order_autoload_error6.phpt b/Zend/tests/type_declarations/variance/class_order_autoload_error6.phpt new file mode 100644 index 0000000000000..4b54c9554dc64 --- /dev/null +++ b/Zend/tests/type_declarations/variance/class_order_autoload_error6.phpt @@ -0,0 +1,39 @@ +--TEST-- +Variance error in the presence of autoloading (6) +--FILE-- + +--EXPECTF-- +object(A)#2 (0) { +} +object(X)#2 (0) { +} + +Fatal error: Could not check compatibility between Y::method(): Unknown and X::method(): X, because class Unknown is not available in %s on line %d diff --git a/Zend/tests/type_declarations/variance/class_order.phpt b/Zend/tests/type_declarations/variance/class_order_error.phpt similarity index 100% rename from Zend/tests/type_declarations/variance/class_order.phpt rename to Zend/tests/type_declarations/variance/class_order_error.phpt diff --git a/Zend/zend_API.c b/Zend/zend_API.c index f88fc2caf83d9..13c661699fa50 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -2649,7 +2649,7 @@ static zend_class_entry *do_register_internal_class(zend_class_entry *orig_class class_entry->type = ZEND_INTERNAL_CLASS; zend_initialize_class_data(class_entry, 0); - class_entry->ce_flags = ce_flags | ZEND_ACC_CONSTANTS_UPDATED | ZEND_ACC_LINKED; + class_entry->ce_flags = ce_flags | ZEND_ACC_CONSTANTS_UPDATED | ZEND_ACC_LINKED | ZEND_ACC_RESOLVED_PARENT | ZEND_ACC_RESOLVED_INTERFACES; class_entry->info.internal.module = EG(current_module); if (class_entry->info.internal.builtin_functions) { diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index aa7125cfe1f60..85c0bf8cd5a47 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -370,6 +370,9 @@ void init_compiler(void) /* {{{ */ zend_hash_init(&CG(filenames_table), 8, NULL, ZVAL_PTR_DTOR, 0); zend_llist_init(&CG(open_files), sizeof(zend_file_handle), (void (*)(void *)) file_handle_dtor, 0); CG(unclean_shutdown) = 0; + + CG(delayed_variance_obligations) = NULL; + CG(delayed_autoloads) = NULL; } /* }}} */ @@ -379,6 +382,17 @@ void shutdown_compiler(void) /* {{{ */ zend_stack_destroy(&CG(delayed_oplines_stack)); zend_hash_destroy(&CG(filenames_table)); zend_arena_destroy(CG(arena)); + + if (CG(delayed_variance_obligations)) { + zend_hash_destroy(CG(delayed_variance_obligations)); + FREE_HASHTABLE(CG(delayed_variance_obligations)); + CG(delayed_variance_obligations) = NULL; + } + if (CG(delayed_autoloads)) { + zend_hash_destroy(CG(delayed_autoloads)); + FREE_HASHTABLE(CG(delayed_autoloads)); + CG(delayed_autoloads) = NULL; + } } /* }}} */ diff --git a/Zend/zend_compile.h b/Zend/zend_compile.h index 3f2ec6ba07bd2..b47d762a74878 100644 --- a/Zend/zend_compile.h +++ b/Zend/zend_compile.h @@ -269,8 +269,14 @@ typedef struct _zend_oparray_context { /* Children must reuse parent get_iterator() | | | */ #define ZEND_ACC_REUSE_GET_ITERATOR (1 << 18) /* X | | | */ /* | | | */ -/* Class is being linked. Don't free strings. | | | */ -#define ZEND_ACC_LINKING_IN_PROGRESS (1 << 19) /* X | | | */ +/* Parent class is resolved (CE). | | | */ +#define ZEND_ACC_RESOLVED_PARENT (1 << 19) /* X | | | */ +/* | | | */ +/* Interfaces are resolved (CEs). | | | */ +#define ZEND_ACC_RESOLVED_INTERFACES (1 << 20) /* X | | | */ +/* | | | */ +/* Class has unresolved variance obligations. | | | */ +#define ZEND_ACC_UNRESOLVED_VARIANCE (1 << 21) /* X | | | */ /* | | | */ /* Function Flags (unused: 28...30) | | | */ /* ============== | | | */ @@ -852,6 +858,7 @@ void zend_assert_valid_class_name(const zend_string *const_name); #define ZEND_FETCH_CLASS_NO_AUTOLOAD 0x80 #define ZEND_FETCH_CLASS_SILENT 0x0100 #define ZEND_FETCH_CLASS_EXCEPTION 0x0200 +#define ZEND_FETCH_CLASS_ALLOW_UNLINKED 0x0400 #define ZEND_PARAM_REF (1<<0) #define ZEND_PARAM_VARIADIC (1<<1) diff --git a/Zend/zend_execute_API.c b/Zend/zend_execute_API.c index 431031acba8a0..e0cc560d4bed1 100644 --- a/Zend/zend_execute_API.c +++ b/Zend/zend_execute_API.c @@ -34,6 +34,7 @@ #include "zend_vm.h" #include "zend_float.h" #include "zend_weakrefs.h" +#include "zend_inheritance.h" #ifdef HAVE_SYS_TIME_H #include #endif @@ -916,7 +917,8 @@ ZEND_API zend_class_entry *zend_lookup_class_ex(zend_string *name, zend_string * zend_string_release_ex(lc_name, 0); } ce = (zend_class_entry*)Z_PTR_P(zv); - if (UNEXPECTED(!(ce->ce_flags & ZEND_ACC_LINKED))) { + if (UNEXPECTED(!(ce->ce_flags & ZEND_ACC_LINKED)) && + !(flags & ZEND_FETCH_CLASS_ALLOW_UNLINKED)) { return NULL; } return ce; diff --git a/Zend/zend_globals.h b/Zend/zend_globals.h index c95faef22b67e..d88fdabcf7f2c 100644 --- a/Zend/zend_globals.h +++ b/Zend/zend_globals.h @@ -124,6 +124,9 @@ struct _zend_compiler_globals { void *map_ptr_base; size_t map_ptr_size; size_t map_ptr_last; + + HashTable *delayed_variance_obligations; + HashTable *delayed_autoloads; }; diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c index 738e528308d13..ba1612c189c03 100644 --- a/Zend/zend_inheritance.c +++ b/Zend/zend_inheritance.c @@ -26,6 +26,11 @@ #include "zend_smart_str.h" #include "zend_operators.h" +static void add_dependency_obligation(zend_class_entry *ce, zend_class_entry *dependency_ce); +static void add_compatibility_obligation( + zend_class_entry *ce, const zend_function *child_fn, const zend_function *parent_fn, + zend_bool always_error); + static void overridden_ptr_dtor(zval *zv) /* {{{ */ { efree_size(Z_PTR_P(zv), sizeof(zend_function)); @@ -174,7 +179,7 @@ static zend_string *resolve_class_name(const zend_function *fe, zend_string *nam 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)) { + if (ce->ce_flags & ZEND_ACC_RESOLVED_PARENT) { return ce->parent->name; } else { return ce->parent_name; @@ -199,32 +204,75 @@ static zend_bool class_visible(zend_class_entry *ce) { 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); + uint32_t flags = ZEND_FETCH_CLASS_ALLOW_UNLINKED | ZEND_FETCH_CLASS_NO_AUTOLOAD; + ce = zend_lookup_class_ex(name, NULL, flags); if (ce) { return ce; } + + /* We'll autoload this class and process delayed variance obligations later. */ + if (!CG(delayed_autoloads)) { + ALLOC_HASHTABLE(CG(delayed_autoloads)); + zend_hash_init(CG(delayed_autoloads), 0, NULL, NULL, 0); + } + zend_hash_add_empty_element(CG(delayed_autoloads), name); } else { ce = zend_lookup_class_ex(name, NULL, ZEND_FETCH_CLASS_NO_AUTOLOAD); 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; + /* 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. */ +/* Instanceof that's safe to use on unlinked classes. */ 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))) { + zend_class_entry *ce; + + if (ce1 == ce2) { + return 1; + } + + if (ce1->ce_flags & (ZEND_ACC_LINKED|ZEND_ACC_RESOLVED_INTERFACES)) { return instanceof_function(ce1, ce2); } - return ce1 == ce2; + + ce = ce1; + while (ce->parent) { + if (ce->ce_flags & ZEND_ACC_RESOLVED_PARENT) { + ce = ce->parent; + } else { + ce = zend_lookup_class_ex(ce->parent_name, NULL, + ZEND_FETCH_CLASS_ALLOW_UNLINKED | ZEND_FETCH_CLASS_NO_AUTOLOAD); + if (!ce) { + break; + } + } + if (ce == ce2) { + return 1; + } + } + + if (ce1->num_interfaces) { + uint32_t i; + ZEND_ASSERT(!(ce->ce_flags & ZEND_ACC_RESOLVED_INTERFACES)); + for (i = 0; i < ce1->num_interfaces; i++) { + ce = zend_lookup_class_ex( + ce1->interface_names[i].name, ce1->interface_names[i].lc_name, + ZEND_FETCH_CLASS_ALLOW_UNLINKED | ZEND_FETCH_CLASS_NO_AUTOLOAD); + if (ce && unlinked_instanceof(ce, ce2)) { + return 1; + } + } + } + + return 0; } /* Unresolved means that class declarations that are currently not available are needed to @@ -261,13 +309,14 @@ static inheritance_status zend_perform_covariant_type_check( return INHERITANCE_SUCCESS; } + /* Make sure to always load both classes, to avoid only registering one of them as + * a delayed autoload. */ fe_ce = lookup_class(fe, fe_class_name); + proto_ce = lookup_class(proto, proto_class_name); if (!fe_ce) { *unresolved_class = fe_class_name; return INHERITANCE_UNRESOLVED; } - - proto_ce = lookup_class(proto, proto_class_name); if (!proto_ce) { *unresolved_class = proto_class_name; return INHERITANCE_UNRESOLVED; @@ -440,6 +489,17 @@ static inheritance_status zend_do_perform_implementation_check( } /* }}} */ +static inheritance_status perform_delayable_implementation_check( + zend_string **unresolved_class, zend_class_entry *ce, + const zend_function *fe, const zend_function *proto, zend_bool always_error) { + inheritance_status status = zend_do_perform_implementation_check( + unresolved_class, fe, proto); + if (status == INHERITANCE_UNRESOLVED) { + add_compatibility_obligation(ce, fe, proto, always_error); + } + return status; +} + static ZEND_COLD void zend_append_type_hint(smart_str *str, const zend_function *fptr, zend_arg_info *arg_info, int return_hint) /* {{{ */ { @@ -601,12 +661,13 @@ static ZEND_COLD zend_string *zend_get_function_declaration(const zend_function } /* }}} */ -static zend_always_inline uint32_t func_lineno(zend_function *fn) { +static zend_always_inline uint32_t func_lineno(const zend_function *fn) { return fn->common.type == ZEND_USER_FUNCTION ? fn->op_array.line_start : 0; } static void ZEND_COLD emit_incompatible_method_error( - int error_level, const char *error_verb, zend_function *child, zend_function *parent, + int error_level, const char *error_verb, + const zend_function *child, const 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); @@ -623,6 +684,28 @@ static void ZEND_COLD emit_incompatible_method_error( zend_string_efree(parent_prototype); } +static void ZEND_COLD emit_incompatible_method_error_or_warning( + const zend_function *child, const zend_function *parent, + inheritance_status status, zend_string *unresolved_class, zend_bool always_error) { + int error_level; + const char *error_verb; + if (always_error || + (child->common.prototype && + (child->common.prototype->common.fn_flags & ZEND_ACC_ABSTRACT)) || + ((parent->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) && + (!(child->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) || + zend_perform_covariant_type_check(&unresolved_class, child, child->common.arg_info - 1, parent, parent->common.arg_info - 1) != INHERITANCE_SUCCESS)) + ) { + error_level = E_COMPILE_ERROR; + error_verb = "must"; + } else { + error_level = E_WARNING; + error_verb = "should"; + } + emit_incompatible_method_error( + error_level, error_verb, child, parent, status, unresolved_class); +} + static void do_inheritance_check_on_method(zend_function *child, zend_function *parent, zend_class_entry *ce, zval *child_zv) /* {{{ */ { uint32_t child_flags; @@ -707,26 +790,11 @@ 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"); } - status = zend_do_perform_implementation_check(&unresolved_class, child, parent); - if (UNEXPECTED(status != INHERITANCE_SUCCESS)) { - int error_level; - const char *error_verb; - if (child->common.prototype && ( - child->common.prototype->common.fn_flags & ZEND_ACC_ABSTRACT - )) { - error_level = E_COMPILE_ERROR; - error_verb = "must"; - } else if ((parent->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) && - (!(child->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) || - zend_perform_covariant_type_check(&unresolved_class, child, child->common.arg_info - 1, parent, parent->common.arg_info - 1) != INHERITANCE_SUCCESS)) { - error_level = E_COMPILE_ERROR; - error_verb = "must"; - } else { - error_level = E_WARNING; - error_verb = "should"; - } - emit_incompatible_method_error( - error_level, error_verb, child, parent, status, unresolved_class); + status = perform_delayable_implementation_check( + &unresolved_class, ce, child, parent, /*always_error*/0); + if (status == INHERITANCE_ERROR) { + emit_incompatible_method_error_or_warning( + child, parent, status, unresolved_class, /*always_error*/0); } } } while (0); @@ -904,6 +972,7 @@ static void zend_do_inherit_interfaces(zend_class_entry *ce, const zend_class_en ce->interfaces[ce->num_interfaces++] = entry; } } + ce->ce_flags |= ZEND_ACC_RESOLVED_INTERFACES; /* and now call the implementing handlers */ while (ce_num < ce->num_interfaces) { @@ -1005,6 +1074,7 @@ ZEND_API void zend_do_inheritance(zend_class_entry *ce, zend_class_entry *parent zend_string_release_ex(ce->parent_name, 0); } ce->parent = parent_ce; + ce->ce_flags |= ZEND_ACC_RESOLVED_PARENT; /* Inherit interfaces */ if (parent_ce->num_interfaces) { @@ -1314,10 +1384,11 @@ static void zend_do_implement_interfaces(zend_class_entry *ce) /* {{{ */ } for (i = 0; i < ce->num_interfaces; i++) { - iface = zend_fetch_class_by_name(ce->interface_names[i].name, - ce->interface_names[i].lc_name, ZEND_FETCH_CLASS_INTERFACE); - if (UNEXPECTED(iface == NULL)) { - return; + iface = zend_fetch_class_by_name( + ce->interface_names[i].name, ce->interface_names[i].lc_name, + ZEND_FETCH_CLASS_INTERFACE|ZEND_FETCH_CLASS_ALLOW_UNLINKED); + if (!(iface->ce_flags & ZEND_ACC_LINKED)) { + add_dependency_obligation(ce, iface); } if (UNEXPECTED(!(iface->ce_flags & ZEND_ACC_INTERFACE))) { efree(interfaces); @@ -1354,6 +1425,7 @@ static void zend_do_implement_interfaces(zend_class_entry *ce) /* {{{ */ ce->num_interfaces = num_interfaces; ce->interfaces = interfaces; + ce->ce_flags |= ZEND_ACC_RESOLVED_INTERFACES; i = ce->parent ? ce->parent->num_interfaces : 0; for (; i < ce->num_interfaces; i++) { @@ -1454,18 +1526,18 @@ 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 */ - status = zend_do_perform_implementation_check( - &unresolved_class, fn, existing_fn); - if (status != INHERITANCE_SUCCESS) { + status = perform_delayable_implementation_check( + &unresolved_class, ce, fn, existing_fn, /*always_error*/ 1); + if (status == INHERITANCE_ERROR) { emit_incompatible_method_error( E_COMPILE_ERROR, "must", fn, existing_fn, status, unresolved_class); } } if (fn->common.fn_flags & ZEND_ACC_ABSTRACT) { /* Make sure the abstract declaration is compatible with previous declaration */ - status = zend_do_perform_implementation_check( - &unresolved_class, existing_fn, fn); - if (status != INHERITANCE_SUCCESS) { + status = perform_delayable_implementation_check( + &unresolved_class, ce, existing_fn, fn, /*always_error*/ 1); + if (status == INHERITANCE_ERROR) { emit_incompatible_method_error( E_COMPILE_ERROR, "must", existing_fn, fn, status, unresolved_class); } @@ -1481,15 +1553,17 @@ 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 */ - status = zend_do_perform_implementation_check(&unresolved_class, fn, existing_fn); - if (status != INHERITANCE_SUCCESS) { + status = perform_delayable_implementation_check( + &unresolved_class, ce, fn, existing_fn, /*always_error*/ 1); + if (status == INHERITANCE_ERROR) { emit_incompatible_method_error( E_COMPILE_ERROR, "must", 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 */ - status = zend_do_perform_implementation_check(&unresolved_class, existing_fn, fn); - if (status != INHERITANCE_SUCCESS) { + status = perform_delayable_implementation_check( + &unresolved_class, ce, existing_fn, fn, /*always_error*/ 1); + if (status == INHERITANCE_ERROR) { emit_incompatible_method_error( E_COMPILE_ERROR, "must", existing_fn, fn, status, unresolved_class); } @@ -2130,11 +2204,172 @@ void zend_verify_abstract_class(zend_class_entry *ce) /* {{{ */ } /* }}} */ +typedef struct { + enum { OBLIGATION_DEPENDENCY, OBLIGATION_COMPATIBILITY } type; + union { + zend_class_entry *dependency_ce; + struct { + const zend_function *parent_fn; + const zend_function *child_fn; + zend_bool always_error; + }; + }; +} variance_obligation; + +static void variance_obligation_dtor(zval *zv) { + efree(Z_PTR_P(zv)); +} + +static void variance_obligation_ht_dtor(zval *zv) { + zend_hash_destroy(Z_PTR_P(zv)); + FREE_HASHTABLE(Z_PTR_P(zv)); +} + +static HashTable *get_or_init_obligations_for_class(zend_class_entry *ce) { + HashTable *ht; + zend_ulong key; + if (!CG(delayed_variance_obligations)) { + ALLOC_HASHTABLE(CG(delayed_variance_obligations)); + zend_hash_init(CG(delayed_variance_obligations), 0, NULL, variance_obligation_ht_dtor, 0); + } + + key = (zend_ulong) (uintptr_t) ce; + ht = zend_hash_index_find_ptr(CG(delayed_variance_obligations), key); + if (ht) { + return ht; + } + + ALLOC_HASHTABLE(ht); + zend_hash_init(ht, 0, NULL, variance_obligation_dtor, 0); + zend_hash_index_add_new_ptr(CG(delayed_variance_obligations), key, ht); + ce->ce_flags |= ZEND_ACC_UNRESOLVED_VARIANCE; + return ht; +} + +static void add_dependency_obligation(zend_class_entry *ce, zend_class_entry *dependency_ce) { + HashTable *obligations = get_or_init_obligations_for_class(ce); + variance_obligation *obligation = emalloc(sizeof(variance_obligation)); + obligation->type = OBLIGATION_DEPENDENCY; + obligation->dependency_ce = dependency_ce; + zend_hash_next_index_insert_ptr(obligations, obligation); +} + +static void add_compatibility_obligation( + zend_class_entry *ce, const zend_function *child_fn, const zend_function *parent_fn, + zend_bool always_error) { + HashTable *obligations = get_or_init_obligations_for_class(ce); + variance_obligation *obligation = emalloc(sizeof(variance_obligation)); + obligation->type = OBLIGATION_COMPATIBILITY; + obligation->child_fn = child_fn; + obligation->parent_fn = parent_fn; + obligation->always_error = always_error; + zend_hash_next_index_insert_ptr(obligations, obligation); +} + +static void resolve_delayed_variance_obligations(zend_class_entry *ce); + +static int check_variance_obligation(zval *zv) { + variance_obligation *obligation = Z_PTR_P(zv); + if (obligation->type == OBLIGATION_DEPENDENCY) { + zend_class_entry *dependency_ce = obligation->dependency_ce; + if (dependency_ce->ce_flags & ZEND_ACC_UNRESOLVED_VARIANCE) { + resolve_delayed_variance_obligations(dependency_ce); + } + if (!(dependency_ce->ce_flags & ZEND_ACC_LINKED)) { + return ZEND_HASH_APPLY_KEEP; + } + } else { + zend_string *unresolved_class; + inheritance_status status = zend_do_perform_implementation_check( + &unresolved_class, obligation->child_fn, obligation->parent_fn); + if (status == INHERITANCE_UNRESOLVED) { + return ZEND_HASH_APPLY_KEEP; + } + if (status == INHERITANCE_ERROR) { + emit_incompatible_method_error_or_warning( + obligation->child_fn, obligation->parent_fn, status, unresolved_class, + obligation->always_error); + } + /* Either the compatibility check was successful or only threw a warning. */ + } + return ZEND_HASH_APPLY_REMOVE; +} + +static void load_delayed_classes() { + HashTable *delayed_autoloads = CG(delayed_autoloads); + zend_string *name; + + if (!delayed_autoloads) { + return; + } + + /* Take ownership of this HT, to avoid concurrent modification during autoloading. */ + CG(delayed_autoloads) = NULL; + + ZEND_HASH_FOREACH_STR_KEY(delayed_autoloads, name) { + zend_lookup_class(name); + } ZEND_HASH_FOREACH_END(); + + zend_hash_destroy(delayed_autoloads); + FREE_HASHTABLE(delayed_autoloads); +} + +static void resolve_delayed_variance_obligations(zend_class_entry *ce) { + HashTable *all_obligations = CG(delayed_variance_obligations), *obligations; + zend_ulong num_key = (zend_ulong) (uintptr_t) ce; + + ZEND_ASSERT(all_obligations != NULL); + obligations = zend_hash_index_find_ptr(all_obligations, num_key); + ZEND_ASSERT(obligations != NULL); + + zend_hash_apply(obligations, check_variance_obligation); + if (zend_hash_num_elements(obligations) == 0) { + ce->ce_flags &= ~ZEND_ACC_UNRESOLVED_VARIANCE; + ce->ce_flags |= ZEND_ACC_LINKED; + zend_hash_index_del(all_obligations, num_key); + } +} + +static void report_variance_errors(zend_class_entry *ce) { + HashTable *all_obligations = CG(delayed_variance_obligations), *obligations; + variance_obligation *obligation; + zend_ulong num_key = (zend_ulong) (uintptr_t) ce; + + ZEND_ASSERT(all_obligations != NULL); + obligations = zend_hash_index_find_ptr(all_obligations, num_key); + ZEND_ASSERT(obligations != NULL); + + ZEND_HASH_FOREACH_PTR(obligations, obligation) { + inheritance_status status; + zend_string *unresolved_class; + + /* There should not be any unresolved parents at this point. */ + ZEND_ASSERT(obligation->type == OBLIGATION_COMPATIBILITY); + + /* Just used to fetch the unresolved_class in this case. */ + status = zend_do_perform_implementation_check( + &unresolved_class, obligation->child_fn, obligation->parent_fn); + ZEND_ASSERT(status == INHERITANCE_UNRESOLVED); + emit_incompatible_method_error_or_warning( + obligation->child_fn, obligation->parent_fn, + status, unresolved_class, obligation->always_error); + } ZEND_HASH_FOREACH_END(); + + /* Only warnings were thrown above -- that means that there are incompatibilities, but only + * ones that we permit. Mark all classes with open obligations as fully linked. */ + ce->ce_flags &= ~ZEND_ACC_UNRESOLVED_VARIANCE; + ce->ce_flags |= ZEND_ACC_LINKED; + zend_hash_index_del(all_obligations, num_key); +} + ZEND_API void zend_do_link_class(zend_class_entry *ce) /* {{{ */ { - ce->ce_flags |= ZEND_ACC_LINKING_IN_PROGRESS; if (ce->parent_name) { - zend_class_entry *parent = zend_fetch_class_by_name(ce->parent_name, NULL, 0); + zend_class_entry *parent = zend_fetch_class_by_name( + ce->parent_name, NULL, ZEND_FETCH_CLASS_ALLOW_UNLINKED); + if (!(parent->ce_flags & ZEND_ACC_LINKED)) { + add_dependency_obligation(ce, parent); + } zend_do_inheritance(ce, parent); } if (ce->ce_flags & ZEND_ACC_IMPLEMENT_TRAITS) { @@ -2148,8 +2383,19 @@ ZEND_API void zend_do_link_class(zend_class_entry *ce) /* {{{ */ } zend_build_properties_info_table(ce); - ce->ce_flags &= ~ZEND_ACC_LINKING_IN_PROGRESS; - ce->ce_flags |= ZEND_ACC_LINKED; + + if (!(ce->ce_flags & ZEND_ACC_UNRESOLVED_VARIANCE)) { + ce->ce_flags |= ZEND_ACC_LINKED; + return; + } + + load_delayed_classes(); + if (ce->ce_flags & ZEND_ACC_UNRESOLVED_VARIANCE) { + resolve_delayed_variance_obligations(ce); + if (!(ce->ce_flags & ZEND_ACC_LINKED)) { + report_variance_errors(ce); + } + } } /* Check whether early binding is prevented due to unresolved types in inheritance checks. */ diff --git a/Zend/zend_interfaces.c b/Zend/zend_interfaces.c index 4d58a9898a25b..ff6784be31bea 100644 --- a/Zend/zend_interfaces.c +++ b/Zend/zend_interfaces.c @@ -292,7 +292,7 @@ static int zend_implement_traversable(zend_class_entry *interface, zend_class_en return SUCCESS; } if (class_type->num_interfaces) { - ZEND_ASSERT(class_type->ce_flags & (ZEND_ACC_LINKED|ZEND_ACC_LINKING_IN_PROGRESS)); + ZEND_ASSERT(class_type->ce_flags & ZEND_ACC_RESOLVED_INTERFACES); for (i = 0; i < class_type->num_interfaces; i++) { if (class_type->interfaces[i] == zend_ce_aggregate || class_type->interfaces[i] == zend_ce_iterator) { return SUCCESS; @@ -322,7 +322,7 @@ static int zend_implement_aggregate(zend_class_entry *interface, zend_class_entr } else if (class_type->get_iterator != zend_user_it_get_new_iterator) { /* c-level get_iterator cannot be changed (exception being only Traversable is implemented) */ if (class_type->num_interfaces) { - ZEND_ASSERT(class_type->ce_flags & (ZEND_ACC_LINKED|ZEND_ACC_LINKING_IN_PROGRESS)); + ZEND_ASSERT(class_type->ce_flags & ZEND_ACC_RESOLVED_INTERFACES); for (i = 0; i < class_type->num_interfaces; i++) { if (class_type->interfaces[i] == zend_ce_iterator) { zend_error_noreturn(E_ERROR, "Class %s cannot implement both %s and %s at the same time", diff --git a/Zend/zend_opcode.c b/Zend/zend_opcode.c index ae70dee0ce0d5..6c8ff85b10c22 100644 --- a/Zend/zend_opcode.c +++ b/Zend/zend_opcode.c @@ -238,7 +238,7 @@ ZEND_API void destroy_zend_class(zval *zv) } switch (ce->type) { case ZEND_USER_CLASS: - if (ce->parent_name && !(ce->ce_flags & (ZEND_ACC_LINKED|ZEND_ACC_LINKING_IN_PROGRESS))) { + if (ce->parent_name && !(ce->ce_flags & ZEND_ACC_RESOLVED_PARENT)) { zend_string_release_ex(ce->parent_name, 0); } if (ce->default_properties_table) { @@ -298,7 +298,7 @@ ZEND_API void destroy_zend_class(zval *zv) } zend_hash_destroy(&ce->constants_table); if (ce->num_interfaces > 0) { - if (!(ce->ce_flags & (ZEND_ACC_LINKED|ZEND_ACC_LINKING_IN_PROGRESS))) { + if (!(ce->ce_flags & ZEND_ACC_RESOLVED_INTERFACES)) { uint32_t i; for (i = 0; i < ce->num_interfaces; i++) { diff --git a/Zend/zend_operators.c b/Zend/zend_operators.c index a2da9d1fe3f47..6033fafebe15c 100644 --- a/Zend/zend_operators.c +++ b/Zend/zend_operators.c @@ -2303,7 +2303,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_ACC_LINKING_IN_PROGRESS)); + ZEND_ASSERT(instance_ce->ce_flags & ZEND_ACC_RESOLVED_INTERFACES); for (i = 0; i < instance_ce->num_interfaces; i++) { if (instanceof_interface_only(instance_ce->interfaces[i], ce)) { return 1; @@ -2331,7 +2331,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_ACC_LINKING_IN_PROGRESS)); + ZEND_ASSERT(instance_ce->ce_flags & ZEND_ACC_RESOLVED_INTERFACES); for (i = 0; i < instance_ce->num_interfaces; i++) { if (instanceof_interface(instance_ce->interfaces[i], ce)) { return 1;