From 07bb42bd386c1ba0bfc6e9b1ad0f5477bcfab384 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Fri, 21 Feb 2025 21:01:46 +0100 Subject: [PATCH 1/5] Fix GH-17866: zend_mm_heap corrupted error after upgrading from 8.4.3 to 8.4.4 This regressed in GH-17592. The function is with its attributes HashTable* is copied in zend_get_closure_invoke_method() but its refcount is not increased. This caused a crash in the Symfony demo page. --- .../deprecated/functions/gh17866.phpt | 21 +++++++++++++++++++ Zend/zend_closures.c | 5 +++++ 2 files changed, 26 insertions(+) create mode 100644 Zend/tests/attributes/deprecated/functions/gh17866.phpt diff --git a/Zend/tests/attributes/deprecated/functions/gh17866.phpt b/Zend/tests/attributes/deprecated/functions/gh17866.phpt new file mode 100644 index 000000000000..b1a686cacd70 --- /dev/null +++ b/Zend/tests/attributes/deprecated/functions/gh17866.phpt @@ -0,0 +1,21 @@ +--TEST-- +GH-17866 (zend_mm_heap corrupted error after upgrading from 8.4.3 to 8.4.4) +--FILE-- +__invoke(...); +$test(); + +?> +--EXPECTF-- +Deprecated: Method Foo::__invoke() is deprecated, xyzzy in %s on line %d +In __invoke diff --git a/Zend/zend_closures.c b/Zend/zend_closures.c index 629aa7d51a84..c3a06ad7360f 100644 --- a/Zend/zend_closures.c +++ b/Zend/zend_closures.c @@ -467,6 +467,11 @@ ZEND_API zend_function *zend_get_closure_invoke_method(zend_object *object) /* { ZEND_ACC_RETURN_REFERENCE | ZEND_ACC_VARIADIC | ZEND_ACC_HAS_RETURN_TYPE; invoke->common = closure->func.common; + + if (invoke->common.attributes) { + GC_TRY_ADDREF(invoke->common.attributes); + } + /* We return ZEND_INTERNAL_FUNCTION, but arg_info representation is the * same as for ZEND_USER_FUNCTION (uses zend_string* instead of char*). * This is not a problem, because ZEND_ACC_HAS_TYPE_HINTS is never set, From 3fedff1785c89fabecd96a0c0ba5ca321e3754c3 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 22 Feb 2025 00:30:57 +0100 Subject: [PATCH 2/5] Update test with reflection part --- .../attributes/deprecated/functions/gh17866.phpt | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Zend/tests/attributes/deprecated/functions/gh17866.phpt b/Zend/tests/attributes/deprecated/functions/gh17866.phpt index b1a686cacd70..d1e9171cc832 100644 --- a/Zend/tests/attributes/deprecated/functions/gh17866.phpt +++ b/Zend/tests/attributes/deprecated/functions/gh17866.phpt @@ -13,9 +13,21 @@ class Foo { $foo = new Foo; $closure = Closure::fromCallable($foo); $test = $closure->__invoke(...); + +$rc = new ReflectionMethod($test, '__invoke'); +var_dump($rc->getAttributes()); + $test(); ?> --EXPECTF-- +array(1) { + [0]=> + object(ReflectionAttribute)#%d (1) { + ["name"]=> + string(10) "Deprecated" + } +} + Deprecated: Method Foo::__invoke() is deprecated, xyzzy in %s on line %d In __invoke From 0bb2f81f715286ea20c9c17626597fcb09ea1f7b Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 24 Feb 2025 20:32:55 +0100 Subject: [PATCH 3/5] Don't refcount --- Zend/zend_closures.c | 5 ----- Zend/zend_object_handlers.c | 7 +------ Zend/zend_object_handlers.h | 4 ---- 3 files changed, 1 insertion(+), 15 deletions(-) diff --git a/Zend/zend_closures.c b/Zend/zend_closures.c index c3a06ad7360f..629aa7d51a84 100644 --- a/Zend/zend_closures.c +++ b/Zend/zend_closures.c @@ -467,11 +467,6 @@ ZEND_API zend_function *zend_get_closure_invoke_method(zend_object *object) /* { ZEND_ACC_RETURN_REFERENCE | ZEND_ACC_VARIADIC | ZEND_ACC_HAS_RETURN_TYPE; invoke->common = closure->func.common; - - if (invoke->common.attributes) { - GC_TRY_ADDREF(invoke->common.attributes); - } - /* We return ZEND_INTERNAL_FUNCTION, but arg_info representation is the * same as for ZEND_USER_FUNCTION (uses zend_string* instead of char*). * This is not a problem, because ZEND_ACC_HAS_TYPE_HINTS is never set, diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c index 277430ce1029..efcb7fd5ed3d 100644 --- a/Zend/zend_object_handlers.c +++ b/Zend/zend_object_handlers.c @@ -1618,12 +1618,7 @@ ZEND_API zend_function *zend_get_call_trampoline_func(const zend_class_entry *ce | ZEND_ACC_PUBLIC | ZEND_ACC_VARIADIC | (fbc->common.fn_flags & (ZEND_ACC_RETURN_REFERENCE|ZEND_ACC_ABSTRACT|ZEND_ACC_DEPRECATED)); - if (fbc->common.attributes) { - func->attributes = fbc->common.attributes; - GC_TRY_ADDREF(func->attributes); - } else { - func->attributes = NULL; - } + func->attributes = fbc->common.attributes; if (is_static) { func->fn_flags |= ZEND_ACC_STATIC; } diff --git a/Zend/zend_object_handlers.h b/Zend/zend_object_handlers.h index 2fc59d5020c2..c7348a742455 100644 --- a/Zend/zend_object_handlers.h +++ b/Zend/zend_object_handlers.h @@ -339,10 +339,6 @@ ZEND_API bool ZEND_FASTCALL zend_asymmetric_property_has_set_access(const zend_p } while (0) #define zend_free_trampoline(func) do { \ - HashTable *attributes = (func)->common.attributes; \ - if (attributes && !(GC_FLAGS(attributes) & GC_IMMUTABLE) && !GC_DELREF(attributes)) { \ - zend_array_destroy(attributes); \ - } \ if ((func) == &EG(trampoline)) { \ EG(trampoline).common.attributes = NULL; \ EG(trampoline).common.function_name = NULL; \ From bfdfa9d276920f25a270e64778aa755cfa778176 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 24 Feb 2025 20:34:44 +0100 Subject: [PATCH 4/5] Keep ZEND_ACC_DEPRECATED --- Zend/tests/attributes/deprecated/functions/gh17866.phpt | 2 ++ Zend/zend_closures.c | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Zend/tests/attributes/deprecated/functions/gh17866.phpt b/Zend/tests/attributes/deprecated/functions/gh17866.phpt index d1e9171cc832..8786b49c602f 100644 --- a/Zend/tests/attributes/deprecated/functions/gh17866.phpt +++ b/Zend/tests/attributes/deprecated/functions/gh17866.phpt @@ -16,6 +16,7 @@ $test = $closure->__invoke(...); $rc = new ReflectionMethod($test, '__invoke'); var_dump($rc->getAttributes()); +var_dump($rc->isDeprecated()); $test(); @@ -28,6 +29,7 @@ array(1) { string(10) "Deprecated" } } +bool(true) Deprecated: Method Foo::__invoke() is deprecated, xyzzy in %s on line %d In __invoke diff --git a/Zend/zend_closures.c b/Zend/zend_closures.c index 629aa7d51a84..049c026845b1 100644 --- a/Zend/zend_closures.c +++ b/Zend/zend_closures.c @@ -464,7 +464,7 @@ ZEND_API zend_function *zend_get_closure_invoke_method(zend_object *object) /* { zend_closure *closure = (zend_closure *)object; zend_function *invoke = (zend_function*)emalloc(sizeof(zend_function)); const uint32_t keep_flags = - ZEND_ACC_RETURN_REFERENCE | ZEND_ACC_VARIADIC | ZEND_ACC_HAS_RETURN_TYPE; + ZEND_ACC_RETURN_REFERENCE | ZEND_ACC_VARIADIC | ZEND_ACC_HAS_RETURN_TYPE | ZEND_ACC_DEPRECATED; invoke->common = closure->func.common; /* We return ZEND_INTERNAL_FUNCTION, but arg_info representation is the From 7aba100d1b72766220253cc89d98b8ef04c56bde Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 24 Feb 2025 20:35:23 +0100 Subject: [PATCH 5/5] Add comment --- Zend/zend_object_handlers.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c index efcb7fd5ed3d..5a4e4b3ea3a1 100644 --- a/Zend/zend_object_handlers.c +++ b/Zend/zend_object_handlers.c @@ -1618,6 +1618,7 @@ ZEND_API zend_function *zend_get_call_trampoline_func(const zend_class_entry *ce | ZEND_ACC_PUBLIC | ZEND_ACC_VARIADIC | (fbc->common.fn_flags & (ZEND_ACC_RETURN_REFERENCE|ZEND_ACC_ABSTRACT|ZEND_ACC_DEPRECATED)); + /* Attributes outlive the trampoline because they are created by the compiler. */ func->attributes = fbc->common.attributes; if (is_static) { func->fn_flags |= ZEND_ACC_STATIC;