From 50374cb3cf6cea4fd3c472beb290324ca2aeebfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1t=C3=A9=20Kocsis?= Date: Sat, 8 May 2021 16:30:12 +0200 Subject: [PATCH 1/9] Add reproducer for possible issue with object return type inheritance --- ext/reflection/php_reflection.stub.php | 2 +- ext/reflection/php_reflection_arginfo.h | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/ext/reflection/php_reflection.stub.php b/ext/reflection/php_reflection.stub.php index 69384f88bb320..ed79762e89b46 100644 --- a/ext/reflection/php_reflection.stub.php +++ b/ext/reflection/php_reflection.stub.php @@ -459,7 +459,7 @@ public function __toString(): string {} public function getName() {} /** @return mixed */ - public function getValue() {} + public function getValue(): mixed {} /** @return bool */ public function isPublic() {} diff --git a/ext/reflection/php_reflection_arginfo.h b/ext/reflection/php_reflection_arginfo.h index b1ea070d2ac3a..551085e4c624c 100644 --- a/ext/reflection/php_reflection_arginfo.h +++ b/ext/reflection/php_reflection_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 47ac64b027cdeb0e9996147277f79fa9d6b876bd */ + * Stub hash: cef133f3196139e48f8ce1ada0994e538503856d */ ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Reflection_getModifierNames, 0, 0, 1) ZEND_ARG_TYPE_INFO(0, modifiers, IS_LONG, 0) @@ -342,7 +342,8 @@ ZEND_END_ARG_INFO() #define arginfo_class_ReflectionClassConstant_getName arginfo_class_ReflectionFunctionAbstract_inNamespace -#define arginfo_class_ReflectionClassConstant_getValue arginfo_class_ReflectionFunctionAbstract_inNamespace +ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class_ReflectionClassConstant_getValue, 0, 0, IS_MIXED, 0) +ZEND_END_ARG_INFO() #define arginfo_class_ReflectionClassConstant_isPublic arginfo_class_ReflectionFunctionAbstract_inNamespace From 7720286238fc65971cb269969b69d78c9696648c Mon Sep 17 00:00:00 2001 From: Joe Watkins Date: Mon, 10 May 2021 07:37:21 +0200 Subject: [PATCH 2/9] Must check that the class table is ready for lookups --- Zend/zend_inheritance.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c index 04ec5dc3c61ab..70993ce066af2 100644 --- a/Zend/zend_inheritance.c +++ b/Zend/zend_inheritance.c @@ -203,7 +203,12 @@ static bool class_visible(zend_class_entry *ce) { static zend_class_entry *lookup_class( zend_class_entry *scope, zend_string *name, bool register_unresolved) { uint32_t flags = ZEND_FETCH_CLASS_ALLOW_UNLINKED | ZEND_FETCH_CLASS_NO_AUTOLOAD; - zend_class_entry *ce = zend_lookup_class_ex(name, NULL, flags); + zend_class_entry *ce = NULL; + + if (EG(class_table)) { + ce = zend_lookup_class_ex(name, NULL, flags); + } + if (!CG(in_compilation)) { if (ce) { return ce; From 403eebfbc99ab257e37a6425d97202453b13a49e Mon Sep 17 00:00:00 2001 From: Joe Watkins Date: Mon, 10 May 2021 09:49:33 +0200 Subject: [PATCH 3/9] fix nicer --- Zend/zend_inheritance.c | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c index 70993ce066af2..5327bc853c5aa 100644 --- a/Zend/zend_inheritance.c +++ b/Zend/zend_inheritance.c @@ -200,27 +200,39 @@ static bool class_visible(zend_class_entry *ce) { } } +static zend_always_inline void register_unresolved_class(zend_string *name) { + /* 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); +} + static zend_class_entry *lookup_class( zend_class_entry *scope, zend_string *name, bool register_unresolved) { - uint32_t flags = ZEND_FETCH_CLASS_ALLOW_UNLINKED | ZEND_FETCH_CLASS_NO_AUTOLOAD; - zend_class_entry *ce = NULL; + if (UNEXPECTED(!EG(active))) { + if (zend_string_equals_ci(scope->name, name)) { + return scope; + } - if (EG(class_table)) { - ce = zend_lookup_class_ex(name, NULL, flags); + if (register_unresolved) { + register_unresolved_class(name); + } + + return NULL; } + zend_class_entry *ce = zend_lookup_class_ex( + name, NULL, ZEND_FETCH_CLASS_ALLOW_UNLINKED | ZEND_FETCH_CLASS_NO_AUTOLOAD); + if (!CG(in_compilation)) { if (ce) { return ce; } if (register_unresolved) { - /* 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); + register_unresolved_class(name); } } else { if (ce && class_visible(ce)) { From 08aa30ca6b2df9c846751cfe440e682d69122c69 Mon Sep 17 00:00:00 2001 From: Joe Watkins Date: Mon, 10 May 2021 11:01:04 +0200 Subject: [PATCH 4/9] add test --- Zend/tests/internal_class_variance.phpt | 12 ++++++++++++ ext/zend_test/test.c | 10 ++++++++++ ext/zend_test/test.stub.php | 3 +++ ext/zend_test/test_arginfo.h | 12 +++++++++++- 4 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 Zend/tests/internal_class_variance.phpt diff --git a/Zend/tests/internal_class_variance.phpt b/Zend/tests/internal_class_variance.phpt new file mode 100644 index 0000000000000..cc3e5fb559b99 --- /dev/null +++ b/Zend/tests/internal_class_variance.phpt @@ -0,0 +1,12 @@ +--TEST-- +Internal class variance +--SKIPIF-- + +--FILE-- +returnsThrowable()); +?> +--EXPECT-- +string(16) "returnsThrowable" diff --git a/ext/zend_test/test.c b/ext/zend_test/test.c index b6f6737f086ed..8f69b30006f8f 100644 --- a/ext/zend_test/test.c +++ b/ext/zend_test/test.c @@ -322,6 +322,16 @@ static ZEND_METHOD(_ZendTestClass, returnsStatic) { object_init_ex(return_value, zend_get_called_scope(execute_data)); } +static ZEND_METHOD(_ZendTestClass, returnsThrowable) { + ZEND_PARSE_PARAMETERS_NONE(); + zend_throw_error(NULL, "Dummy"); +} + +static ZEND_METHOD(_ZendTestChildClass, returnsThrowable) { + ZEND_PARSE_PARAMETERS_NONE(); + zend_throw_error(NULL, "Dummy"); +} + static ZEND_METHOD(_ZendTestTrait, testMethod) { ZEND_PARSE_PARAMETERS_NONE(); RETURN_TRUE; diff --git a/ext/zend_test/test.stub.php b/ext/zend_test/test.stub.php index 624ba1195f3cd..3571556d238ca 100644 --- a/ext/zend_test/test.stub.php +++ b/ext/zend_test/test.stub.php @@ -24,10 +24,13 @@ public static function is_object(): int {} public function __toString(): string {} public function returnsStatic(): static {} + + public function returnsThrowable(): Throwable {} } class _ZendTestChildClass extends _ZendTestClass { + public function returnsThrowable(): Exception {} } trait _ZendTestTrait { diff --git a/ext/zend_test/test_arginfo.h b/ext/zend_test/test_arginfo.h index dc040a568043f..d4d77e125dd8d 100644 --- a/ext/zend_test/test_arginfo.h +++ b/ext/zend_test/test_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: cf8958513064fb7257203b3304c8dc67c8e008b9 */ + * Stub hash: 70374ed7b55604eb98e85148d7ff19e79258ce92 */ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_test_array_return, 0, 0, IS_ARRAY, 0) ZEND_END_ARG_INFO() @@ -63,6 +63,12 @@ ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class__ZendTestClass_returnsStatic, 0, 0, IS_STATIC, 0) ZEND_END_ARG_INFO() +ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO_EX(arginfo_class__ZendTestClass_returnsThrowable, 0, 0, Throwable, 0) +ZEND_END_ARG_INFO() + +ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO_EX(arginfo_class__ZendTestChildClass_returnsThrowable, 0, 0, Exception, 0) +ZEND_END_ARG_INFO() + #define arginfo_class__ZendTestTrait_testMethod arginfo_ZendTestNS2_ZendSubNS_namespaced_func #define arginfo_class_ZendTestNS_Foo_method arginfo_zend_test_void_return @@ -89,6 +95,8 @@ static ZEND_FUNCTION(namespaced_func); static ZEND_METHOD(_ZendTestClass, is_object); static ZEND_METHOD(_ZendTestClass, __toString); static ZEND_METHOD(_ZendTestClass, returnsStatic); +static ZEND_METHOD(_ZendTestClass, returnsThrowable); +static ZEND_METHOD(_ZendTestChildClass, returnsThrowable); static ZEND_METHOD(_ZendTestTrait, testMethod); static ZEND_METHOD(ZendTestNS_Foo, method); static ZEND_METHOD(ZendTestNS2_Foo, method); @@ -123,11 +131,13 @@ static const zend_function_entry class__ZendTestClass_methods[] = { ZEND_ME(_ZendTestClass, is_object, arginfo_class__ZendTestClass_is_object, ZEND_ACC_PUBLIC|ZEND_ACC_STATIC) ZEND_ME(_ZendTestClass, __toString, arginfo_class__ZendTestClass___toString, ZEND_ACC_PUBLIC|ZEND_ACC_DEPRECATED) ZEND_ME(_ZendTestClass, returnsStatic, arginfo_class__ZendTestClass_returnsStatic, ZEND_ACC_PUBLIC) + ZEND_ME(_ZendTestClass, returnsThrowable, arginfo_class__ZendTestClass_returnsThrowable, ZEND_ACC_PUBLIC) ZEND_FE_END }; static const zend_function_entry class__ZendTestChildClass_methods[] = { + ZEND_ME(_ZendTestChildClass, returnsThrowable, arginfo_class__ZendTestChildClass_returnsThrowable, ZEND_ACC_PUBLIC) ZEND_FE_END }; From 80606032ef5e4308752046be80ce7e81c5b3219c Mon Sep 17 00:00:00 2001 From: Joe Watkins Date: Mon, 10 May 2021 11:14:33 +0200 Subject: [PATCH 5/9] make test sensible --- Zend/tests/internal_class_variance.phpt | 8 ++++-- ext/zend_test/test.c | 35 ++++++++++++++----------- 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/Zend/tests/internal_class_variance.phpt b/Zend/tests/internal_class_variance.phpt index cc3e5fb559b99..94e2e2c15fe36 100644 --- a/Zend/tests/internal_class_variance.phpt +++ b/Zend/tests/internal_class_variance.phpt @@ -6,7 +6,11 @@ Internal class variance returnsThrowable()); +try { + $test->returnsThrowable(); +} catch (\Error) { + echo "OK"; +} ?> --EXPECT-- -string(16) "returnsThrowable" +OK diff --git a/ext/zend_test/test.c b/ext/zend_test/test.c index 8f69b30006f8f..54e70a9ab25b3 100644 --- a/ext/zend_test/test.c +++ b/ext/zend_test/test.c @@ -257,22 +257,25 @@ static zend_object *zend_test_class_new(zend_class_entry *class_type) /* {{{ */ /* }}} */ static zend_function *zend_test_class_method_get(zend_object **object, zend_string *name, const zval *key) /* {{{ */ { - zend_internal_function *fptr; - - if (EXPECTED(EG(trampoline).common.function_name == NULL)) { - fptr = (zend_internal_function *) &EG(trampoline); - } else { - fptr = emalloc(sizeof(zend_internal_function)); - } - memset(fptr, 0, sizeof(zend_internal_function)); - fptr->type = ZEND_INTERNAL_FUNCTION; - fptr->num_args = 1; - fptr->scope = (*object)->ce; - fptr->fn_flags = ZEND_ACC_CALL_VIA_HANDLER; - fptr->function_name = zend_string_copy(name); - fptr->handler = ZEND_FN(zend_test_func); - - return (zend_function*)fptr; + if (zend_string_equals_literal_ci(name, "test")) { + zend_internal_function *fptr; + + if (EXPECTED(EG(trampoline).common.function_name == NULL)) { + fptr = (zend_internal_function *) &EG(trampoline); + } else { + fptr = emalloc(sizeof(zend_internal_function)); + } + memset(fptr, 0, sizeof(zend_internal_function)); + fptr->type = ZEND_INTERNAL_FUNCTION; + fptr->num_args = 1; + fptr->scope = (*object)->ce; + fptr->fn_flags = ZEND_ACC_CALL_VIA_HANDLER; + fptr->function_name = zend_string_copy(name); + fptr->handler = ZEND_FN(zend_test_func); + + return (zend_function*)fptr; + } + return zend_std_get_method(object, name, key); } /* }}} */ From 973679ac6ba9e41c858f687e613e973e59a2ea19 Mon Sep 17 00:00:00 2001 From: Joe Watkins Date: Mon, 10 May 2021 11:51:47 +0200 Subject: [PATCH 6/9] better ? --- Zend/zend_inheritance.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c index 5327bc853c5aa..68d6d9bd7bb2c 100644 --- a/Zend/zend_inheritance.c +++ b/Zend/zend_inheritance.c @@ -211,19 +211,26 @@ static zend_always_inline void register_unresolved_class(zend_string *name) { static zend_class_entry *lookup_class( zend_class_entry *scope, zend_string *name, bool register_unresolved) { + zend_class_entry *ce; + if (UNEXPECTED(!EG(active))) { - if (zend_string_equals_ci(scope->name, name)) { - return scope; - } + zend_string *lc_name = + zend_string_tolower(name); - if (register_unresolved) { - register_unresolved_class(name); - } + ce = zend_hash_find_ptr(CG(class_table), lc_name); + + zend_string_release(lc_name); - return NULL; + if (!ce) { + zend_error_noreturn( + E_COMPILE_ERROR, "%s must be loaded before %s", + ZSTR_VAL(name), ZSTR_VAL(scope->name)); + } + + return ce; } - zend_class_entry *ce = zend_lookup_class_ex( + ce = zend_lookup_class_ex( name, NULL, ZEND_FETCH_CLASS_ALLOW_UNLINKED | ZEND_FETCH_CLASS_NO_AUTOLOAD); if (!CG(in_compilation)) { From 5ce60d1987d4bab0c4acb750ad8169497b713fb7 Mon Sep 17 00:00:00 2001 From: Joe Watkins Date: Mon, 10 May 2021 12:02:52 +0200 Subject: [PATCH 7/9] cleanup --- Zend/tests/internal_class_variance.phpt | 4 ++-- Zend/zend_inheritance.c | 7 +++---- ext/reflection/php_reflection.stub.php | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/Zend/tests/internal_class_variance.phpt b/Zend/tests/internal_class_variance.phpt index 94e2e2c15fe36..8c446c2a72c1f 100644 --- a/Zend/tests/internal_class_variance.phpt +++ b/Zend/tests/internal_class_variance.phpt @@ -1,7 +1,7 @@ --TEST-- Internal class variance ---SKIPIF-- - +--EXTENSIONS-- +zend_test --FILE-- name)); } diff --git a/ext/reflection/php_reflection.stub.php b/ext/reflection/php_reflection.stub.php index ed79762e89b46..bd200c56058c5 100644 --- a/ext/reflection/php_reflection.stub.php +++ b/ext/reflection/php_reflection.stub.php @@ -459,7 +459,7 @@ public function __toString(): string {} public function getName() {} /** @return mixed */ - public function getValue(): mixed {} + public function getValue(): {} /** @return bool */ public function isPublic() {} From f5a759e88a4a7d4f5ac858e3c7a8450a4d931782 Mon Sep 17 00:00:00 2001 From: Joe Watkins Date: Mon, 10 May 2021 12:12:03 +0200 Subject: [PATCH 8/9] stray : --- ext/reflection/php_reflection.stub.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/reflection/php_reflection.stub.php b/ext/reflection/php_reflection.stub.php index bd200c56058c5..69384f88bb320 100644 --- a/ext/reflection/php_reflection.stub.php +++ b/ext/reflection/php_reflection.stub.php @@ -459,7 +459,7 @@ public function __toString(): string {} public function getName() {} /** @return mixed */ - public function getValue(): {} + public function getValue() {} /** @return bool */ public function isPublic() {} From b1ff8da1cbc0df57f97315ccf99723f24c887195 Mon Sep 17 00:00:00 2001 From: Joe Watkins Date: Mon, 10 May 2021 12:13:18 +0200 Subject: [PATCH 9/9] stub --- ext/reflection/php_reflection_arginfo.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ext/reflection/php_reflection_arginfo.h b/ext/reflection/php_reflection_arginfo.h index 551085e4c624c..b1ea070d2ac3a 100644 --- a/ext/reflection/php_reflection_arginfo.h +++ b/ext/reflection/php_reflection_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: cef133f3196139e48f8ce1ada0994e538503856d */ + * Stub hash: 47ac64b027cdeb0e9996147277f79fa9d6b876bd */ ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Reflection_getModifierNames, 0, 0, 1) ZEND_ARG_TYPE_INFO(0, modifiers, IS_LONG, 0) @@ -342,8 +342,7 @@ ZEND_END_ARG_INFO() #define arginfo_class_ReflectionClassConstant_getName arginfo_class_ReflectionFunctionAbstract_inNamespace -ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class_ReflectionClassConstant_getValue, 0, 0, IS_MIXED, 0) -ZEND_END_ARG_INFO() +#define arginfo_class_ReflectionClassConstant_getValue arginfo_class_ReflectionFunctionAbstract_inNamespace #define arginfo_class_ReflectionClassConstant_isPublic arginfo_class_ReflectionFunctionAbstract_inNamespace