From b15c5972fbe2ffbbe3d203c757101ab0f717fbdc Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Wed, 25 Mar 2026 12:21:12 -0400 Subject: [PATCH 1/4] Fix GH-19983: GC assertion failure with fibers, generators and destructors When GC runs inside a fiber handling an exception (e.g. during zend_fiber_object_destroy), EG(exception) is set. gc_call_destructors_in_fiber() saved and cleared the exception after creating the destructor fiber. Since zend_call_function() returns early when EG(exception) is set, the destructor fiber's handler never ran, leaving DTOR_GARBAGE entries in the root buffer. On the next GC cycle, gc_collect_roots() hit an alignment assertion on these stale entries. Move remember_prev_exception() before the destructor fiber creation/resume so EG(exception) is cleared before zend_call_function() runs inside the fiber. Closes GH-21529 --- NEWS | 3 +++ Zend/tests/fibers/gh19983.phpt | 29 +++++++++++++++++++++++++++++ Zend/zend_gc.c | 4 +++- 3 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 Zend/tests/fibers/gh19983.phpt diff --git a/NEWS b/NEWS index b7cc259571133..e86c5132350d9 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,9 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 8.4.21 +- Core: + . Fixed bug GH-19983 (GC assertion failure with fibers, generators and + destructors). (iliaal) 09 Apr 2026, PHP 8.4.20 diff --git a/Zend/tests/fibers/gh19983.phpt b/Zend/tests/fibers/gh19983.phpt new file mode 100644 index 0000000000000..156edc3918151 --- /dev/null +++ b/Zend/tests/fibers/gh19983.phpt @@ -0,0 +1,29 @@ +--TEST-- +GH-19983 (GC Assertion Failure with fibers, generators and destructors) +--SKIPIF-- + +--INI-- +memory_limit=128M +--FILE-- +current(); + }); + $fiber->start(); + } +} +new a; +?> +--EXPECTF-- +Fatal error: Allowed memory size of %d bytes exhausted%s diff --git a/Zend/zend_gc.c b/Zend/zend_gc.c index 7b2ff49a73632..ffd427eac5bc0 100644 --- a/Zend/zend_gc.c +++ b/Zend/zend_gc.c @@ -1893,13 +1893,15 @@ static zend_never_inline void gc_call_destructors_in_fiber(uint32_t end) GC_G(dtor_idx) = GC_FIRST_ROOT; GC_G(dtor_end) = GC_G(first_unused); + zend_object *exception = NULL; + remember_prev_exception(&exception); + if (UNEXPECTED(!fiber)) { fiber = gc_create_destructor_fiber(); } else { zend_fiber_resume(fiber, NULL, NULL); } - zend_object *exception = NULL; remember_prev_exception(&exception); for (;;) { From 437dce71c0a5e341b5b367261ebf89ab56c5298e Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Wed, 25 Mar 2026 18:32:24 +0000 Subject: [PATCH 2/4] ext/spl: add test for phpinfo() output --- ext/spl/tests/phpinfo_out.phpt | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 ext/spl/tests/phpinfo_out.phpt diff --git a/ext/spl/tests/phpinfo_out.phpt b/ext/spl/tests/phpinfo_out.phpt new file mode 100644 index 0000000000000..8be1a2dead419 --- /dev/null +++ b/ext/spl/tests/phpinfo_out.phpt @@ -0,0 +1,16 @@ +--TEST-- +phpinfo() SPL extension information +--FILE-- + +--EXPECT-- +SPL + +SPL support => enabled +Interfaces => OuterIterator, RecursiveIterator, SeekableIterator, SplObserver, SplSubject +Classes => AppendIterator, ArrayIterator, ArrayObject, BadFunctionCallException, BadMethodCallException, CachingIterator, CallbackFilterIterator, DirectoryIterator, DomainException, EmptyIterator, FilesystemIterator, FilterIterator, GlobIterator, InfiniteIterator, InvalidArgumentException, IteratorIterator, LengthException, LimitIterator, LogicException, MultipleIterator, NoRewindIterator, OutOfBoundsException, OutOfRangeException, OverflowException, ParentIterator, RangeException, RecursiveArrayIterator, RecursiveCachingIterator, RecursiveCallbackFilterIterator, RecursiveDirectoryIterator, RecursiveFilterIterator, RecursiveIteratorIterator, RecursiveRegexIterator, RecursiveTreeIterator, RegexIterator, RuntimeException, SplDoublyLinkedList, SplFileInfo, SplFileObject, SplFixedArray, SplHeap, SplMinHeap, SplMaxHeap, SplObjectStorage, SplPriorityQueue, SplQueue, SplStack, SplTempFileObject, UnderflowException, UnexpectedValueException From 8c37bb953031955369e4dd26bcfd2b793a109160 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Wed, 25 Mar 2026 13:31:36 +0000 Subject: [PATCH 3/4] ext/spl: refactor reflection-like functions Make the helpers private and remove a lot of unnecessary computation and code that was effectively dead. --- ext/spl/php_spl.c | 167 ++++++++++++++++++++++++---------------- ext/spl/spl_functions.c | 55 ------------- ext/spl/spl_functions.h | 10 --- 3 files changed, 102 insertions(+), 130 deletions(-) diff --git a/ext/spl/php_spl.c b/ext/spl/php_spl.c index 0c64d815dc812..a4f3284025f13 100644 --- a/ext/spl/php_spl.c +++ b/ext/spl/php_spl.c @@ -60,6 +60,43 @@ static zend_class_entry * spl_find_ce_by_name(zend_string *name, bool autoload) return ce; } +static void spl_add_class_name(HashTable *list, zend_string *name) +{ + zval t; + ZVAL_STR_COPY(&t, name); + zend_hash_add(list, name, &t); +} + +static void spl_add_interfaces(HashTable *list, const zend_class_entry *pce) +{ + if (pce->num_interfaces) { + ZEND_ASSERT(pce->ce_flags & ZEND_ACC_LINKED); + for (uint32_t num_interfaces = 0; num_interfaces < pce->num_interfaces; num_interfaces++) { + spl_add_class_name(list, pce->interfaces[num_interfaces]->name); + } + } +} + +static void spl_add_traits(HashTable *list, const zend_class_entry *pce) +{ + for (uint32_t num_traits = 0; num_traits < pce->num_traits; num_traits++) { + spl_add_class_name(list, pce->trait_names[num_traits].name); + } +} + +static void spl_add_classes(HashTable *list, const zend_class_entry *pce, bool only_classes, bool only_interfaces) +{ + ZEND_ASSERT(pce); + ZEND_ASSERT(!(only_classes && only_interfaces) && "Cannot have both only classes and only interfaces be enabled"); + if ( + (only_classes && (pce->ce_flags & ZEND_ACC_INTERFACE) == ZEND_ACC_INTERFACE) + || (only_interfaces && (pce->ce_flags & ZEND_ACC_INTERFACE) == 0) + ) { + return; + } + spl_add_class_name(list, pce->name); +} + /* {{{ Return an array containing the names of all parent classes */ PHP_FUNCTION(class_parents) { @@ -88,7 +125,7 @@ PHP_FUNCTION(class_parents) array_init(return_value); const zend_class_entry *parent_class = ce->parent; while (parent_class) { - spl_add_class_name(return_value, parent_class, 0, 0); + spl_add_class_name(Z_ARR_P(return_value), parent_class->name); parent_class = parent_class->parent; } } @@ -119,7 +156,7 @@ PHP_FUNCTION(class_implements) } array_init(return_value); - spl_add_interfaces(return_value, ce, 1, ZEND_ACC_INTERFACE); + spl_add_interfaces(Z_ARR_P(return_value), ce); } /* }}} */ @@ -148,69 +185,69 @@ PHP_FUNCTION(class_uses) } array_init(return_value); - spl_add_traits(return_value, ce, 1, ZEND_ACC_TRAIT); + spl_add_traits(Z_ARR_P(return_value), ce); } /* }}} */ -#define SPL_ADD_CLASS(class_name, z_list, sub, allow, ce_flags) \ - spl_add_classes(spl_ce_ ## class_name, z_list, sub, allow, ce_flags) - -#define SPL_LIST_CLASSES(z_list, sub, allow, ce_flags) \ - SPL_ADD_CLASS(AppendIterator, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(ArrayIterator, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(ArrayObject, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(BadFunctionCallException, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(BadMethodCallException, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(CachingIterator, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(CallbackFilterIterator, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(DirectoryIterator, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(DomainException, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(EmptyIterator, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(FilesystemIterator, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(FilterIterator, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(GlobIterator, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(InfiniteIterator, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(InvalidArgumentException, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(IteratorIterator, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(LengthException, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(LimitIterator, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(LogicException, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(MultipleIterator, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(NoRewindIterator, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(OuterIterator, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(OutOfBoundsException, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(OutOfRangeException, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(OverflowException, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(ParentIterator, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(RangeException, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(RecursiveArrayIterator, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(RecursiveCachingIterator, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(RecursiveCallbackFilterIterator, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(RecursiveDirectoryIterator, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(RecursiveFilterIterator, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(RecursiveIterator, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(RecursiveIteratorIterator, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(RecursiveRegexIterator, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(RecursiveTreeIterator, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(RegexIterator, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(RuntimeException, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(SeekableIterator, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(SplDoublyLinkedList, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(SplFileInfo, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(SplFileObject, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(SplFixedArray, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(SplHeap, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(SplMinHeap, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(SplMaxHeap, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(SplObjectStorage, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(SplObserver, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(SplPriorityQueue, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(SplQueue, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(SplStack, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(SplSubject, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(SplTempFileObject, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(UnderflowException, z_list, sub, allow, ce_flags); \ - SPL_ADD_CLASS(UnexpectedValueException, z_list, sub, allow, ce_flags); \ +#define SPL_ADD_CLASS(class_name, z_list, only_classes, only_interfaces) \ + spl_add_classes(Z_ARR_P(z_list), spl_ce_ ## class_name, only_classes, only_interfaces) + +#define SPL_LIST_CLASSES(z_list, only_classes, only_interfaces) \ + SPL_ADD_CLASS(AppendIterator, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(ArrayIterator, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(ArrayObject, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(BadFunctionCallException, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(BadMethodCallException, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(CachingIterator, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(CallbackFilterIterator, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(DirectoryIterator, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(DomainException, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(EmptyIterator, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(FilesystemIterator, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(FilterIterator, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(GlobIterator, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(InfiniteIterator, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(InvalidArgumentException, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(IteratorIterator, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(LengthException, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(LimitIterator, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(LogicException, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(MultipleIterator, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(NoRewindIterator, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(OuterIterator, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(OutOfBoundsException, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(OutOfRangeException, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(OverflowException, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(ParentIterator, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(RangeException, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(RecursiveArrayIterator, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(RecursiveCachingIterator, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(RecursiveCallbackFilterIterator, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(RecursiveDirectoryIterator, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(RecursiveFilterIterator, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(RecursiveIterator, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(RecursiveIteratorIterator, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(RecursiveRegexIterator, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(RecursiveTreeIterator, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(RegexIterator, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(RuntimeException, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(SeekableIterator, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(SplDoublyLinkedList, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(SplFileInfo, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(SplFileObject, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(SplFixedArray, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(SplHeap, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(SplMinHeap, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(SplMaxHeap, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(SplObjectStorage, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(SplObserver, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(SplPriorityQueue, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(SplQueue, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(SplStack, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(SplSubject, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(SplTempFileObject, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(UnderflowException, z_list, only_classes, only_interfaces); \ + SPL_ADD_CLASS(UnexpectedValueException, z_list, only_classes, only_interfaces); \ /* {{{ Return an array containing the names of all classes and interfaces defined in SPL */ PHP_FUNCTION(spl_classes) @@ -219,7 +256,7 @@ PHP_FUNCTION(spl_classes) array_init(return_value); - SPL_LIST_CLASSES(return_value, 0, 0, 0) + SPL_LIST_CLASSES(return_value, false, false) } /* }}} */ @@ -486,7 +523,7 @@ PHP_MINFO_FUNCTION(spl) php_info_print_table_row(2, "SPL support", "enabled"); array_init(&list); - SPL_LIST_CLASSES(&list, 0, 1, ZEND_ACC_INTERFACE) + SPL_LIST_CLASSES(&list, false, true) strg = estrdup(""); ZEND_HASH_MAP_FOREACH_VAL(Z_ARRVAL_P(&list), zv) { spl_build_class_list_string(zv, &strg); @@ -496,7 +533,7 @@ PHP_MINFO_FUNCTION(spl) efree(strg); array_init(&list); - SPL_LIST_CLASSES(&list, 0, -1, ZEND_ACC_INTERFACE) + SPL_LIST_CLASSES(&list, true, false) strg = estrdup(""); ZEND_HASH_MAP_FOREACH_VAL(Z_ARRVAL_P(&list), zv) { spl_build_class_list_string(zv, &strg); diff --git a/ext/spl/spl_functions.c b/ext/spl/spl_functions.c index 7b7991a121708..e3963aced7ebe 100644 --- a/ext/spl/spl_functions.c +++ b/ext/spl/spl_functions.c @@ -20,61 +20,6 @@ #include "php.h" -/* {{{ spl_add_class_name */ -void spl_add_class_name(zval *list, const zend_class_entry *pce, int allow, int ce_flags) -{ - if (!allow || (allow > 0 && (pce->ce_flags & ce_flags)) || (allow < 0 && !(pce->ce_flags & ce_flags))) { - const zval *tmp = zend_hash_find(Z_ARRVAL_P(list), pce->name); - - if (tmp == NULL) { - zval t; - ZVAL_STR_COPY(&t, pce->name); - zend_hash_add(Z_ARRVAL_P(list), pce->name, &t); - } - } -} -/* }}} */ - -/* {{{ spl_add_interfaces */ -void spl_add_interfaces(zval *list, const zend_class_entry *pce, int allow, int ce_flags) -{ - if (pce->num_interfaces) { - ZEND_ASSERT(pce->ce_flags & ZEND_ACC_LINKED); - for (uint32_t num_interfaces = 0; num_interfaces < pce->num_interfaces; num_interfaces++) { - spl_add_class_name(list, pce->interfaces[num_interfaces], allow, ce_flags); - } - } -} -/* }}} */ - -/* {{{ spl_add_traits */ -void spl_add_traits(zval *list, const zend_class_entry *pce, int allow, int ce_flags) -{ - for (uint32_t num_traits = 0; num_traits < pce->num_traits; num_traits++) { - const zend_class_entry *trait = zend_fetch_class_by_name(pce->trait_names[num_traits].name, - pce->trait_names[num_traits].lc_name, ZEND_FETCH_CLASS_TRAIT); - ZEND_ASSERT(trait); - spl_add_class_name(list, trait, allow, ce_flags); - } -} -/* }}} */ - - -/* {{{ spl_add_classes */ -void spl_add_classes(const zend_class_entry *pce, zval *list, bool sub, int allow, int ce_flags) -{ - ZEND_ASSERT(pce); - spl_add_class_name(list, pce, allow, ce_flags); - if (sub) { - spl_add_interfaces(list, pce, allow, ce_flags); - while (pce->parent) { - pce = pce->parent; - spl_add_classes(pce, list, sub, allow, ce_flags); - } - } -} -/* }}} */ - void spl_set_private_debug_info_property( const zend_class_entry *ce, const char *property, diff --git a/ext/spl/spl_functions.h b/ext/spl/spl_functions.h index ca92a237169ee..409ce9fcada67 100644 --- a/ext/spl/spl_functions.h +++ b/ext/spl/spl_functions.h @@ -19,16 +19,6 @@ #include "php.h" -/* sub: whether to allow subclasses/interfaces - allow = 0: allow all classes and interfaces - allow > 0: allow all that match and mask ce_flags - allow < 0: disallow all that match and mask ce_flags - */ -void spl_add_class_name(zval * list, const zend_class_entry *pce, int allow, int ce_flags); -void spl_add_interfaces(zval * list, const zend_class_entry *pce, int allow, int ce_flags); -void spl_add_traits(zval * list, const zend_class_entry *pce, int allow, int ce_flags); -void spl_add_classes(const zend_class_entry *pce, zval *list, bool sub, int allow, int ce_flags); - void spl_set_private_debug_info_property(const zend_class_entry *ce, const char *property, size_t property_len, HashTable *debug_info, zval *value); #endif /* PHP_FUNCTIONS_H */ From e912c022fdaa43a732143d666e3780f32dec997a Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Wed, 25 Mar 2026 20:09:20 +0000 Subject: [PATCH 4/4] Revert 49b2ff5dbb94b76b265fd5909881997e1d95c6b3 to fix bug GH-21499 The fix for this was to take hold of a pointer of the bucket, something that should not be done as it causes memory corruptions Closes GH-21532, GH-21520, GH-21499 --- NEWS | 4 +++ ext/spl/spl_array.c | 57 +---------------------------------- ext/spl/tests/bug42654.phpt | 12 ++++---- ext/spl/tests/bug42654_2.phpt | 33 -------------------- ext/spl/tests/gh10519.phpt | 2 ++ ext/spl/tests/gh21499.phpt | 17 +++++++++++ 6 files changed, 30 insertions(+), 95 deletions(-) delete mode 100644 ext/spl/tests/bug42654_2.phpt create mode 100644 ext/spl/tests/gh21499.phpt diff --git a/NEWS b/NEWS index e86c5132350d9..dc0e430775f18 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,10 @@ PHP NEWS . Fixed bug GH-19983 (GC assertion failure with fibers, generators and destructors). (iliaal) +- SPL: + . Fixed bug GH-21499 (RecursiveArrayIterator getChildren UAF after parent + free). (Girgias) + 09 Apr 2026, PHP 8.4.20 - Bz2: diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c index 3d9d2a9535cbd..01fdccf251b5b 100644 --- a/ext/spl/spl_array.c +++ b/ext/spl/spl_array.c @@ -44,8 +44,6 @@ typedef struct _spl_array_object { uint32_t ht_iter; int ar_flags; unsigned char nApplyCount; - bool is_child; - Bucket *bucket; zend_function *fptr_offset_get; zend_function *fptr_offset_set; zend_function *fptr_offset_has; @@ -166,8 +164,6 @@ static zend_object *spl_array_object_new_ex(zend_class_entry *class_type, zend_o object_properties_init(&intern->std, class_type); intern->ar_flags = 0; - intern->is_child = false; - intern->bucket = NULL; intern->ce_get_iterator = spl_ce_ArrayIterator; if (orig) { spl_array_object *other = spl_array_from_obj(orig); @@ -464,22 +460,6 @@ static zval *spl_array_read_dimension(zend_object *object, zval *offset, int typ return spl_array_read_dimension_ex(1, object, offset, type, rv); } /* }}} */ -/* - * The assertion(HT_ASSERT_RC1(ht)) failed because the refcount was increased manually when intern->is_child is true. - * We have to set the refcount to 1 to make assertion success and restore the refcount to the original value after - * modifying the array when intern->is_child is true. - */ -static uint32_t spl_array_set_refcount(bool is_child, HashTable *ht, uint32_t refcount) /* {{{ */ -{ - uint32_t old_refcount = 0; - if (is_child) { - old_refcount = GC_REFCOUNT(ht); - GC_SET_REFCOUNT(ht, refcount); - } - - return old_refcount; -} /* }}} */ - static void spl_array_write_dimension_ex(int check_inherited, zend_object *object, zval *offset, zval *value) /* {{{ */ { spl_array_object *intern = spl_array_from_obj(object); @@ -503,19 +483,12 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec } Z_TRY_ADDREF_P(value); - - uint32_t refcount = 0; if (!offset || Z_TYPE_P(offset) == IS_NULL) { ht = spl_array_get_hash_table(intern); if (UNEXPECTED(ht == intern->sentinel_array)) { return; } - refcount = spl_array_set_refcount(intern->is_child, ht, 1); zend_hash_next_index_insert(ht, value); - - if (refcount) { - spl_array_set_refcount(intern->is_child, ht, refcount); - } return; } @@ -530,17 +503,13 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec spl_hash_key_release(&key); return; } - refcount = spl_array_set_refcount(intern->is_child, ht, 1); + if (key.key) { zend_hash_update_ind(ht, key.key, value); spl_hash_key_release(&key); } else { zend_hash_index_update(ht, key.h, value); } - - if (refcount) { - spl_array_set_refcount(intern->is_child, ht, refcount); - } } /* }}} */ static void spl_array_write_dimension(zend_object *object, zval *offset, zval *value) /* {{{ */ @@ -570,8 +539,6 @@ static void spl_array_unset_dimension_ex(int check_inherited, zend_object *objec } ht = spl_array_get_hash_table(intern); - uint32_t refcount = spl_array_set_refcount(intern->is_child, ht, 1); - if (key.key) { zval *data = zend_hash_find(ht, key.key); if (data) { @@ -596,10 +563,6 @@ static void spl_array_unset_dimension_ex(int check_inherited, zend_object *objec } else { zend_hash_index_del(ht, key.h); } - - if (refcount) { - spl_array_set_refcount(intern->is_child, ht, refcount); - } } /* }}} */ static void spl_array_unset_dimension(zend_object *object, zval *offset) /* {{{ */ @@ -973,15 +936,6 @@ static void spl_array_set_array(zval *object, spl_array_object *intern, zval *ar } else { //??? TODO: try to avoid array duplication ZVAL_ARR(&intern->array, zend_array_dup(Z_ARR_P(array))); - - if (intern->is_child) { - Z_TRY_DELREF(intern->bucket->val); - /* - * replace bucket->val with copied array, so the changes between - * parent and child object can affect each other. - */ - ZVAL_COPY(&intern->bucket->val, &intern->array); - } } } else { if (Z_OBJ_HT_P(array) == &spl_handler_ArrayObject) { @@ -1854,15 +1808,6 @@ PHP_METHOD(RecursiveArrayIterator, hasChildren) static void spl_instantiate_child_arg(zend_class_entry *pce, zval *retval, zval *arg1, zval *arg2) /* {{{ */ { object_init_ex(retval, pce); - spl_array_object *new_intern = Z_SPLARRAY_P(retval); - /* - * set new_intern->is_child is true to indicate that the object was created by - * RecursiveArrayIterator::getChildren() method. - */ - new_intern->is_child = true; - - /* find the bucket of parent object. */ - new_intern->bucket = (Bucket *)((char *)(arg1) - XtOffsetOf(Bucket, val));; zend_call_known_instance_method_with_2_params(pce->constructor, Z_OBJ_P(retval), NULL, arg1, arg2); } /* }}} */ diff --git a/ext/spl/tests/bug42654.phpt b/ext/spl/tests/bug42654.phpt index eb72863e08759..20aad74b73423 100644 --- a/ext/spl/tests/bug42654.phpt +++ b/ext/spl/tests/bug42654.phpt @@ -110,11 +110,11 @@ object(RecursiveArrayIterator)#%d (1) { [2]=> array(2) { [2]=> - string(5) "alter" + string(4) "val2" [3]=> array(1) { [3]=> - string(5) "alter" + string(4) "val3" } } [4]=> @@ -129,11 +129,11 @@ object(RecursiveArrayIterator)#%d (1) { [2]=> array(2) { [2]=> - string(5) "alter" + string(4) "val2" [3]=> array(1) { [3]=> - string(5) "alter" + string(4) "val3" } } [4]=> @@ -146,11 +146,11 @@ array(3) { [2]=> array(2) { [2]=> - string(5) "alter" + string(4) "val2" [3]=> array(1) { [3]=> - string(5) "alter" + string(4) "val3" } } [4]=> diff --git a/ext/spl/tests/bug42654_2.phpt b/ext/spl/tests/bug42654_2.phpt deleted file mode 100644 index bd290247dbdc4..0000000000000 --- a/ext/spl/tests/bug42654_2.phpt +++ /dev/null @@ -1,33 +0,0 @@ ---TEST-- -Bug #42654 (RecursiveIteratorIterator modifies only part of leaves) ---FILE-- - 'val1', array('key2' => 'val2'), 'key3' => 'val3'); - -$iterator = new RecursiveIteratorIterator(new RecursiveArrayIterator($data)); -foreach($iterator as $foo) { - $key = $iterator->key(); - switch($key) { - case 'key1': // first level - case 'key2': // recursive level - echo "update $key".PHP_EOL; - $iterator->offsetSet($key, 'alter'); - } -} -$copy = $iterator->getArrayCopy(); -var_dump($copy); -?> ---EXPECT-- -update key1 -update key2 -array(3) { - ["key1"]=> - string(5) "alter" - [0]=> - array(1) { - ["key2"]=> - string(5) "alter" - } - ["key3"]=> - string(4) "val3" -} diff --git a/ext/spl/tests/gh10519.phpt b/ext/spl/tests/gh10519.phpt index 1f7572d6e8ca6..87e7eae0cf0c8 100644 --- a/ext/spl/tests/gh10519.phpt +++ b/ext/spl/tests/gh10519.phpt @@ -1,5 +1,7 @@ --TEST-- Bug GH-10519 (Array Data Address Reference Issue) +--XFAIL-- +The fix for this was bad --FILE-- getChildren(); +unset($it); +$child->__construct([2, 3]); +var_dump($child->getArrayCopy()); +?> +--EXPECT-- +array(2) { + [0]=> + int(2) + [1]=> + int(3) +}