From 47c7da9ecf5aef8a72268589807ec48e03ff21ec Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Thu, 12 Nov 2026 18:34:58 +0100 Subject: [PATCH 1/3] Fix GH-17935: Separate ArrayObject and ArrayIterator storage on write ArrayObject and ArrayIterator assumed they were the only owner of their storage, but __serialize(), __debugInfo(), var_export(), json_encode() and the sort methods hand it out or borrow it without copying it. Writing to the object then modified the array that was handed out, and failed the refcount assertion of zend_hash.c in debug builds. The storage is now separated before any write. When this happens while it is iterated, iterators move to the copy of their position that zend_array_dup() registers on the new table. When the storage is replaced, they keep their position and get registered on the new table. An error handler can now separate the storage by writing to the object, so the table is fetched after the code that can call one: get_hash_key() on read paths, and argument parsing in ArrayIterator::seek(). This also fixes use-after-frees when a handler calls exchangeArray() there, or while an element is being created. --- NEWS | 2 + ext/spl/spl_array.c | 94 ++++++++-- ...bject_exchange_array_in_error_handler.phpt | 21 +++ .../ArrayObject_separate_on_write.phpt | 82 +++++++++ ...bject_separate_on_write_error_handler.phpt | 52 ++++++ ...rayObject_separate_on_write_iterators.phpt | 164 ++++++++++++++++++ ...ArrayObject_separate_on_write_reentry.phpt | 63 +++++++ .../ArrayObject_separate_on_write_twice.phpt | 29 ++++ ...storage_changed_in_error_handler_read.phpt | 82 +++++++++ ext/spl/tests/ArrayObject/gh17935.phpt | 27 +++ 10 files changed, 605 insertions(+), 11 deletions(-) create mode 100644 ext/spl/tests/ArrayObject/ArrayObject_exchange_array_in_error_handler.phpt create mode 100644 ext/spl/tests/ArrayObject/ArrayObject_separate_on_write.phpt create mode 100644 ext/spl/tests/ArrayObject/ArrayObject_separate_on_write_error_handler.phpt create mode 100644 ext/spl/tests/ArrayObject/ArrayObject_separate_on_write_iterators.phpt create mode 100644 ext/spl/tests/ArrayObject/ArrayObject_separate_on_write_reentry.phpt create mode 100644 ext/spl/tests/ArrayObject/ArrayObject_separate_on_write_twice.phpt create mode 100644 ext/spl/tests/ArrayObject/ArrayObject_storage_changed_in_error_handler_read.phpt create mode 100644 ext/spl/tests/ArrayObject/gh17935.phpt diff --git a/NEWS b/NEWS index a0bce4ed6f43..b96f6f774919 100644 --- a/NEWS +++ b/NEWS @@ -152,6 +152,8 @@ PHP NEWS (Ilia Alshanetsky) - SPL: + . Fixed bug GH-17935 (Assertion failure Zend/zend_hash.c when appending to + serialized ArrayObject). (Nicolas Grekas) . Fixed bug GH-23385 (SplDoublyLinkedList::serialize() use-after-free when __serialize() removes an element). (David Carlier) diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c index 9ca034e72abb..d6d0b219ea98 100644 --- a/ext/spl/spl_array.c +++ b/ext/spl/spl_array.c @@ -108,9 +108,22 @@ static inline bool spl_array_is_object(spl_array_object *intern) /* {{{ */ } /* }}} */ +/* Returns the storage, separated from its other holders so that it can be written to. */ +static HashTable *spl_array_get_hash_table_for_write(spl_array_object *intern) +{ + while (intern->ar_flags & SPL_ARRAY_USE_OTHER) { + intern = Z_SPLARRAY_P(&intern->array); + } + if (!(intern->ar_flags & SPL_ARRAY_IS_SELF) && Z_TYPE(intern->array) == IS_ARRAY) { + SEPARATE_ARRAY(&intern->array); + return Z_ARRVAL(intern->array); + } + return spl_array_get_hash_table(intern); +} + static zend_result spl_array_skip_protected(spl_array_object *intern, HashTable *aht); -static zend_never_inline void spl_array_create_ht_iter(HashTable *ht, spl_array_object* intern) /* {{{ */ +static void spl_array_create_ht_iter(HashTable *ht, spl_array_object* intern) /* {{{ */ { intern->ht_iter = zend_hash_iterator_add(ht, zend_hash_get_current_pos(ht)); zend_hash_internal_pointer_reset_ex(ht, &EG(ht_iterators)[intern->ht_iter].pos); @@ -118,10 +131,52 @@ static zend_never_inline void spl_array_create_ht_iter(HashTable *ht, spl_array_ } /* }}} */ -static zend_always_inline uint32_t *spl_array_get_pos_ptr(HashTable *ht, spl_array_object* intern) /* {{{ */ +static void spl_array_update_ht_iter(HashTable *ht, spl_array_object* intern) { - if (UNEXPECTED(intern->ht_iter == (uint32_t)-1)) { + uint32_t idx = intern->ht_iter; + HashPosition pos = EG(ht_iterators)[idx].pos; + bool has_copy = false; + + /* When the storage is separated, zend_array_dup() registers a copy of the + * iterator on the new table, with its position adjusted to it. */ + for (uint32_t i = EG(ht_iterators)[idx].next_copy; i != idx; i = EG(ht_iterators)[i].next_copy) { + if (EG(ht_iterators)[i].ht == ht) { + has_copy = true; + break; + } + } + + /* Moves the registration to the new table, so that the previous one stops + * tracking this iterator, and takes the position of the copy if there is one. */ + zend_hash_iterator_pos(idx, ht); + if (!has_copy) { + /* The storage was replaced: the position is kept as is */ + EG(ht_iterators)[idx].pos = pos; + } +} + +static zend_always_inline bool spl_array_ht_iter_is_on(const spl_array_object *intern, const HashTable *ht) +{ + /* The registered table is compared rather than a cached pointer: it is + * poisoned when the table is destroyed, so a new table allocated at the + * same address cannot be mistaken for it. */ + return intern->ht_iter != (uint32_t)-1 && EG(ht_iterators)[intern->ht_iter].ht == ht; +} + +static zend_never_inline uint32_t *spl_array_get_pos_ptr_slow(HashTable *ht, spl_array_object* intern) +{ + if (intern->ht_iter == (uint32_t)-1) { spl_array_create_ht_iter(ht, intern); + } else { + spl_array_update_ht_iter(ht, intern); + } + return &EG(ht_iterators)[intern->ht_iter].pos; +} + +static zend_always_inline uint32_t *spl_array_get_pos_ptr(HashTable *ht, spl_array_object* intern) /* {{{ */ +{ + if (UNEXPECTED(!spl_array_ht_iter_is_on(intern, ht))) { + return spl_array_get_pos_ptr_slow(ht, intern); } return &EG(ht_iterators)[intern->ht_iter].pos; } @@ -321,9 +376,9 @@ static zval *spl_array_get_dimension_ptr(bool check_inherited, spl_array_object { zval *retval; spl_hash_key key; - HashTable *ht = spl_array_get_hash_table(intern); + HashTable *ht; - if (!offset || Z_ISUNDEF_P(offset) || !ht) { + if (!offset || Z_ISUNDEF_P(offset)) { return &EG(uninitialized_zval); } @@ -338,6 +393,13 @@ static zval *spl_array_get_dimension_ptr(bool check_inherited, spl_array_object &EG(error_zval) : &EG(uninitialized_zval); } + /* The table is fetched after get_hash_key(), whose errors may call a handler + * that shares or replaces the storage. In write context, the returned zval + * may be turned into a reference or written to. */ + ht = (type != BP_VAR_R && type != BP_VAR_IS) + ? spl_array_get_hash_table_for_write(intern) + : spl_array_get_hash_table(intern); + if (key.key) { retval = zend_hash_find(ht, key.key); if (retval) { @@ -372,6 +434,8 @@ static zval *spl_array_get_dimension_ptr(bool check_inherited, spl_array_object break; case BP_VAR_RW: zend_error(E_WARNING,"Undefined array key \"%pS\"", key.key); + /* The error handler may have shared or replaced the storage */ + ht = spl_array_get_hash_table_for_write(intern); ZEND_FALLTHROUGH; case BP_VAR_W: { zval value; @@ -393,6 +457,8 @@ static zval *spl_array_get_dimension_ptr(bool check_inherited, spl_array_object break; case BP_VAR_RW: zend_error(E_WARNING, "Undefined array key " ZEND_LONG_FMT, key.h); + /* The error handler may have shared or replaced the storage */ + ht = spl_array_get_hash_table_for_write(intern); ZEND_FALLTHROUGH; case BP_VAR_W: { zval value; @@ -480,7 +546,7 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec Z_TRY_ADDREF_P(value); if (!offset || Z_TYPE_P(offset) == IS_NULL) { - ht = spl_array_get_hash_table(intern); + ht = spl_array_get_hash_table_for_write(intern); if (UNEXPECTED(ht == intern->sentinel_array)) { return; } @@ -494,7 +560,7 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec return; } - ht = spl_array_get_hash_table(intern); + ht = spl_array_get_hash_table_for_write(intern); if (UNEXPECTED(ht == intern->sentinel_array)) { spl_hash_key_release(&key); return; @@ -534,7 +600,7 @@ static void spl_array_unset_dimension_ex(int check_inherited, zend_object *objec return; } - ht = spl_array_get_hash_table(intern); + ht = spl_array_get_hash_table_for_write(intern); if (key.key) { zval *data = zend_hash_find(ht, key.key); if (data) { @@ -592,7 +658,7 @@ static bool spl_array_has_dimension_ex(bool check_inherited, zend_object *object } if (!value) { - HashTable *ht = spl_array_get_hash_table(intern); + HashTable *ht; spl_hash_key key; if (get_hash_key(&key, intern, offset) == FAILURE) { @@ -600,6 +666,9 @@ static bool spl_array_has_dimension_ex(bool check_inherited, zend_object *object return false; } + /* Fetched after get_hash_key(), whose errors may call a handler that replaces the storage */ + ht = spl_array_get_hash_table(intern); + if (key.key) { tmp = zend_hash_find(ht, key.key); spl_hash_key_release(&key); @@ -1562,7 +1631,9 @@ static zval *spl_array_it_get_current_data(zend_object_iterator *iter) /* {{{ */ { spl_array_iterator *array_iter = (spl_array_iterator*)iter; spl_array_object *object = Z_SPLARRAY_P(&iter->data); - HashTable *aht = spl_array_get_hash_table(object); + HashTable *aht = array_iter->by_ref + ? spl_array_get_hash_table_for_write(object) + : spl_array_get_hash_table(object); zval *data = zend_hash_get_current_data_ex(aht, spl_array_get_pos_ptr(aht, object)); if (data && Z_TYPE_P(data) == IS_INDIRECT) { data = Z_INDIRECT_P(data); @@ -1705,7 +1776,7 @@ PHP_METHOD(ArrayIterator, seek) zend_long opos, position; zval *object = ZEND_THIS; spl_array_object *intern = Z_SPLARRAY_P(object); - HashTable *aht = spl_array_get_hash_table(intern); + HashTable *aht; int result; if (zend_parse_parameters(ZEND_NUM_ARGS(), "l", &position) == FAILURE) { @@ -1720,6 +1791,7 @@ PHP_METHOD(ArrayIterator, seek) while (position-- > 0 && (result = spl_array_next(intern)) == SUCCESS); + aht = spl_array_get_hash_table(intern); if (result == SUCCESS && zend_hash_has_more_elements_ex(aht, spl_array_get_pos_ptr(aht, intern)) == SUCCESS) { return; /* ok */ } diff --git a/ext/spl/tests/ArrayObject/ArrayObject_exchange_array_in_error_handler.phpt b/ext/spl/tests/ArrayObject/ArrayObject_exchange_array_in_error_handler.phpt new file mode 100644 index 000000000000..8f249c291899 --- /dev/null +++ b/ext/spl/tests/ArrayObject/ArrayObject_exchange_array_in_error_handler.phpt @@ -0,0 +1,21 @@ +--TEST-- +ArrayObject storage exchanged by an error handler while creating an element +--FILE-- + 1], ArrayObject::ARRAY_AS_PROPS); +set_error_handler(function ($errno, $errstr) use ($ao) { + echo $errstr, "\n"; + $ao->exchangeArray(['z' => str_repeat('z', 3)]); +}); +$ao->b .= 'x'; +restore_error_handler(); +var_dump($ao->getArrayCopy()); +?> +--EXPECT-- +Undefined array key "b" +array(2) { + ["z"]=> + string(3) "zzz" + ["b"]=> + string(1) "x" +} diff --git a/ext/spl/tests/ArrayObject/ArrayObject_separate_on_write.phpt b/ext/spl/tests/ArrayObject/ArrayObject_separate_on_write.phpt new file mode 100644 index 000000000000..79fe9c1cbabe --- /dev/null +++ b/ext/spl/tests/ArrayObject/ArrayObject_separate_on_write.phpt @@ -0,0 +1,82 @@ +--TEST-- +ArrayObject and ArrayIterator separate their storage from the arrays they hand out before writing to it +--FILE-- + 1, 'b' => ['x' => 1], 'c' => 3]; + $object = new $class($expected, ArrayObject::ARRAY_AS_PROPS); + $serialized = $object->__serialize()[1]; + $debug = array_values($object->__debugInfo())[0]; + $write($object); + if ($serialized !== $expected || $debug !== $expected) { + echo "$class $label: FAIL\n"; + var_dump($serialized, $debug); + } + echo "$class $label: ", json_encode($object->getArrayCopy()), "\n"; + } +} + +check('offsetSet', function ($o) { $o['a'] = 2; }); +check('offsetSet()', function ($o) { $o->offsetSet('a', 2); }); +check('append', function ($o) { $o[] = 4; }); +check('append()', function ($o) { $o->append(4); }); +check('offsetUnset', function ($o) { unset($o['a']); }); +check('offsetUnset() missing', function ($o) { $o->offsetUnset('z'); }); +check('compound assign', function ($o) { $o['a'] += 10; }); +check('increment', function ($o) { $o['a']++; }); +check('nested write', function ($o) { $o['b']['y'] = 2; }); +check('nested append', function ($o) { $o['b'][] = 2; }); +check('nested unset', function ($o) { unset($o['b']['x']); }); +check('reference', function ($o) { $r = &$o['a']; $r = 5; }); +check('property', function ($o) { $o->a = 6; }); +check('property compound assign', function ($o) { $o->a .= 'x'; }); +check('property nested write', function ($o) { $o->b['z'] = 3; }); +check('property reference', function ($o) { $r = &$o->c; $r = 7; }); +check('property unset', function ($o) { unset($o->c); }); +check('foreach by reference', function ($o) { foreach ($o as &$v) { $v = 8; } }); +check('uasort', function ($o) { $o->uasort(fn ($a, $b) => is_array($a) <=> is_array($b)); }, ['ArrayObject']); +check('ksort', function ($o) { $o->ksort(); $o['d'] = 0; }, ['ArrayObject']); +?> +--EXPECT-- +ArrayIterator offsetSet: {"a":2,"b":{"x":1},"c":3} +ArrayObject offsetSet: {"a":2,"b":{"x":1},"c":3} +ArrayIterator offsetSet(): {"a":2,"b":{"x":1},"c":3} +ArrayObject offsetSet(): {"a":2,"b":{"x":1},"c":3} +ArrayIterator append: {"a":1,"b":{"x":1},"c":3,"0":4} +ArrayObject append: {"a":1,"b":{"x":1},"c":3,"0":4} +ArrayIterator append(): {"a":1,"b":{"x":1},"c":3,"0":4} +ArrayObject append(): {"a":1,"b":{"x":1},"c":3,"0":4} +ArrayIterator offsetUnset: {"b":{"x":1},"c":3} +ArrayObject offsetUnset: {"b":{"x":1},"c":3} +ArrayIterator offsetUnset() missing: {"a":1,"b":{"x":1},"c":3} +ArrayObject offsetUnset() missing: {"a":1,"b":{"x":1},"c":3} +ArrayIterator compound assign: {"a":11,"b":{"x":1},"c":3} +ArrayObject compound assign: {"a":11,"b":{"x":1},"c":3} +ArrayIterator increment: {"a":2,"b":{"x":1},"c":3} +ArrayObject increment: {"a":2,"b":{"x":1},"c":3} +ArrayIterator nested write: {"a":1,"b":{"x":1,"y":2},"c":3} +ArrayObject nested write: {"a":1,"b":{"x":1,"y":2},"c":3} +ArrayIterator nested append: {"a":1,"b":{"x":1,"0":2},"c":3} +ArrayObject nested append: {"a":1,"b":{"x":1,"0":2},"c":3} +ArrayIterator nested unset: {"a":1,"b":[],"c":3} +ArrayObject nested unset: {"a":1,"b":[],"c":3} +ArrayIterator reference: {"a":5,"b":{"x":1},"c":3} +ArrayObject reference: {"a":5,"b":{"x":1},"c":3} +ArrayIterator property: {"a":6,"b":{"x":1},"c":3} +ArrayObject property: {"a":6,"b":{"x":1},"c":3} +ArrayIterator property compound assign: {"a":"1x","b":{"x":1},"c":3} +ArrayObject property compound assign: {"a":"1x","b":{"x":1},"c":3} +ArrayIterator property nested write: {"a":1,"b":{"x":1,"z":3},"c":3} +ArrayObject property nested write: {"a":1,"b":{"x":1,"z":3},"c":3} +ArrayIterator property reference: {"a":1,"b":{"x":1},"c":7} +ArrayObject property reference: {"a":1,"b":{"x":1},"c":7} +ArrayIterator property unset: {"a":1,"b":{"x":1}} +ArrayObject property unset: {"a":1,"b":{"x":1}} +ArrayIterator foreach by reference: {"a":8,"b":8,"c":8} +ArrayObject foreach by reference: {"a":8,"b":8,"c":8} +ArrayObject uasort: {"a":1,"c":3,"b":{"x":1}} +ArrayObject ksort: {"a":1,"b":{"x":1},"c":3,"d":0} diff --git a/ext/spl/tests/ArrayObject/ArrayObject_separate_on_write_error_handler.phpt b/ext/spl/tests/ArrayObject/ArrayObject_separate_on_write_error_handler.phpt new file mode 100644 index 000000000000..5144dda3d04b --- /dev/null +++ b/ext/spl/tests/ArrayObject/ArrayObject_separate_on_write_error_handler.phpt @@ -0,0 +1,52 @@ +--TEST-- +ArrayObject storage shared by an error handler while creating an element +--FILE-- + 1], ArrayObject::ARRAY_AS_PROPS); +set_error_handler(function ($errno, $errstr) use ($ao) { + echo $errstr, "\n"; + $GLOBALS['copies'][] = $ao->__serialize()[1]; +}); +$ao->b .= 'x'; +$ao->c++; +$ao['d'] .= 'y'; +restore_error_handler(); +var_dump($ao->getArrayCopy(), $copies); +?> +--EXPECT-- +Undefined array key "b" +Undefined array key "c" +Undefined array key "d" +array(4) { + ["a"]=> + int(1) + ["b"]=> + string(1) "x" + ["c"]=> + int(1) + ["d"]=> + string(1) "y" +} +array(3) { + [0]=> + array(1) { + ["a"]=> + int(1) + } + [1]=> + array(2) { + ["a"]=> + int(1) + ["b"]=> + string(1) "x" + } + [2]=> + array(3) { + ["a"]=> + int(1) + ["b"]=> + string(1) "x" + ["c"]=> + int(1) + } +} diff --git a/ext/spl/tests/ArrayObject/ArrayObject_separate_on_write_iterators.phpt b/ext/spl/tests/ArrayObject/ArrayObject_separate_on_write_iterators.phpt new file mode 100644 index 000000000000..5d0058974a8d --- /dev/null +++ b/ext/spl/tests/ArrayObject/ArrayObject_separate_on_write_iterators.phpt @@ -0,0 +1,164 @@ +--TEST-- +ArrayObject and ArrayIterator keep their iteration position when their storage is separated +--FILE-- + 0, 'a' => 1, 'b' => 2, 'c' => 3, 'd' => 4]); + unset($object['z']); + return $object; +} + +echo "-- write during foreach\n"; +$it = make('ArrayIterator'); +foreach ($it as $k => $v) { + echo "$k=$v "; + $data = $it->__serialize(); + $it[$k] = $v * 10; +} +echo "\n", json_encode($it->getArrayCopy()), ' ', json_encode($data[1]), "\n"; + +echo "-- write during manual iteration\n"; +$it = make('ArrayIterator'); +$it->rewind(); +$it->next(); +$data = $it->__serialize(); +$it['e'] = 5; +for (; $it->valid(); $it->next()) { + echo $it->key(), '=', $it->current(), ' '; +} +echo "\n", json_encode($data[1]), "\n"; + +echo "-- unset during foreach\n"; +$it = make('ArrayIterator'); +foreach ($it as $k => $v) { + echo "$k=$v "; + if ($k === 'b') { + $data = $it->__debugInfo(); + unset($it['c']); + } +} +echo "\n", json_encode($it->getArrayCopy()), ' ', json_encode(array_values($data)[0]), "\n"; + +echo "-- ArrayObject iterator, write through the ArrayObject\n"; +$ao = make('ArrayObject'); +foreach ($ao as $k => $v) { + echo "$k=$v "; + $data = $ao->__serialize(); + $ao[$k] = -$v; +} +echo "\n", json_encode($ao->getArrayCopy()), ' ', json_encode($data[1]), "\n"; + +echo "-- two ArrayObject iterators\n"; +$ao = make('ArrayObject'); +$it1 = $ao->getIterator(); +$it2 = $ao->getIterator(); +$it1->next(); +$it2->next(); +$it2->next(); +$data = $ao->__serialize(); +$ao['f'] = 6; +echo $it1->key(), ' ', $it2->key(), "\n"; +$it1->next(); +$it2->next(); +echo $it1->key(), ' ', $it2->key(), "\n"; + +echo "-- iterator created after the separation\n"; +$ao = make('ArrayObject'); +$it1 = $ao->getIterator(); +$it1->next(); +$data = $ao->__serialize(); +unset($ao['a']); +$it2 = $ao->getIterator(); +echo $it1->key(), ' ', $it2->key(), "\n"; +$data = $ao->__serialize(); +$ao['g'] = 7; +$it1->next(); +$it2->next(); +echo $it1->key(), ' ', $it2->key(), "\n"; + +echo "-- foreach by reference\n"; +$ao = make('ArrayObject'); +$data = $ao->__serialize(); +foreach ($ao as $k => &$v) { + echo "$k=$v "; + $v *= 2; +} +unset($v); +echo "\n", json_encode($ao->getArrayCopy()), ' ', json_encode($data[1]), "\n"; + +echo "-- nested foreach with writes\n"; +$ao = make('ArrayObject'); +foreach ($ao as $k1 => $v1) { + foreach ($ao as $k2 => $v2) { + if ($k2 === 'b') { + $data = $ao->__serialize(); + $ao['b'] = $v2 + 1; + } + } + echo "$k1=$v1 "; +} +echo "\n", json_encode($ao->getArrayCopy()), "\n"; + +echo "-- sort during foreach\n"; +$ao = make('ArrayObject'); +foreach ($ao as $k => $v) { + echo "$k=$v "; + if ($k === 'b') { + $ao->ksort(); + } +} +echo "\n"; + +echo "-- storage replaced during iteration\n"; +$ao = make('ArrayObject'); +$it = $ao->getIterator(); +$it->next(); +$it->next(); +$new = []; +foreach (['p', 'q', 'r', 's', 't'] as $k) { + $new[$k] = $k; +} +$ao->exchangeArray($new); +// The position is kept as is +echo $it->key(), "\n"; +// It then follows the new storage, here when filling it compacts it +unset($ao['p'], $ao['q']); +for ($i = 0; $i < 8; $i++) { + $ao["k$i"] = $i; +} +echo $it->key(), "\n"; +?> +--EXPECT-- +-- write during foreach +a=1 b=2 c=3 d=4 +{"a":10,"b":20,"c":30,"d":40} {"a":10,"b":20,"c":30,"d":4} +-- write during manual iteration +b=2 c=3 d=4 e=5 +{"a":1,"b":2,"c":3,"d":4} +-- unset during foreach +a=1 b=2 d=4 +{"a":1,"b":2,"d":4} {"a":1,"b":2,"c":3,"d":4} +-- ArrayObject iterator, write through the ArrayObject +a=1 b=2 c=3 d=4 +{"a":-1,"b":-2,"c":-3,"d":-4} {"a":-1,"b":-2,"c":-3,"d":4} +-- two ArrayObject iterators +b c +c d +-- iterator created after the separation +b b +c c +-- foreach by reference +a=1 b=2 c=3 d=4 +{"a":2,"b":4,"c":6,"d":8} {"a":1,"b":2,"c":3,"d":4} +-- nested foreach with writes +a=1 b=3 c=3 d=4 +{"a":1,"b":6,"c":3,"d":4} +-- sort during foreach +a=1 b=2 c=3 d=4 +-- storage replaced during iteration +s +s diff --git a/ext/spl/tests/ArrayObject/ArrayObject_separate_on_write_reentry.phpt b/ext/spl/tests/ArrayObject/ArrayObject_separate_on_write_reentry.phpt new file mode 100644 index 000000000000..21ba4d484a6c --- /dev/null +++ b/ext/spl/tests/ArrayObject/ArrayObject_separate_on_write_reentry.phpt @@ -0,0 +1,63 @@ +--TEST-- +ArrayObject and ArrayIterator written to while their storage is being encoded, sorted or compared +--FILE-- +target["new$i"] = $i; + } + unset($this->target['b']); + $ref = &$this->target['ref']; + $ref = 'ref'; + + return 'written'; + } + + public function __toString(): string + { + $this->target[] = 'appended'; + + return 'x'; + } +} + +echo "-- json_encode()\n"; +foreach (['ArrayObject', 'ArrayIterator'] as $class) { + $object = new $class(['a' => null, 'b' => 2, 'c' => 3]); + $object['a'] = new Writer($object); + echo json_encode($object), ' ', count($object), "\n"; +} + +echo "-- sort\n"; +$ao = new ArrayObject(['b' => 2, 'a' => 1, 'c' => 3]); +$it = $ao->getIterator(); +$ao->uasort(function ($x, $y) use ($it) { + // Written to a copy that the sorted array replaces + $it['d'] = 4; + unset($it['c']); + + return $x <=> $y; +}); +echo json_encode($ao->getArrayCopy()), "\n"; + +echo "-- comparison\n"; +$a = new ArrayObject(); +$a[] = new Writer($a); +$a[] = 'y'; +$b = new ArrayObject(['x', 'y']); +var_dump($a == $b, count($a)); +?> +--EXPECT-- +-- json_encode() +{"a":"written","b":2,"c":3} 23 +{"a":"written","b":2,"c":3} 23 +-- sort +{"a":1,"b":2,"c":3} +-- comparison +bool(true) +int(3) diff --git a/ext/spl/tests/ArrayObject/ArrayObject_separate_on_write_twice.phpt b/ext/spl/tests/ArrayObject/ArrayObject_separate_on_write_twice.phpt new file mode 100644 index 000000000000..df3a405cd31b --- /dev/null +++ b/ext/spl/tests/ArrayObject/ArrayObject_separate_on_write_twice.phpt @@ -0,0 +1,29 @@ +--TEST-- +ArrayObject iterator keeps its position when the storage is separated twice between two steps +--SKIPIF-- + +--FILE-- + 1, 'b' => 2, 'c' => 3, 'd' => 4, 'e' => 5]); +foreach ($ao as $k => $v) { + echo "$k "; + if ($k === 'b') { + // A hole before the current position, which a copy compacts + unset($ao['a']); + $data = $ao->__serialize(); + $ao['x'] = 1; + unset($data); + $data = $ao->__serialize(); + $ao['y'] = 1; + } +} +echo "\n"; +?> +--EXPECT-- +a b c d e x y diff --git a/ext/spl/tests/ArrayObject/ArrayObject_storage_changed_in_error_handler_read.phpt b/ext/spl/tests/ArrayObject/ArrayObject_storage_changed_in_error_handler_read.phpt new file mode 100644 index 000000000000..5f290eb80f87 --- /dev/null +++ b/ext/spl/tests/ArrayObject/ArrayObject_storage_changed_in_error_handler_read.phpt @@ -0,0 +1,82 @@ +--TEST-- +ArrayObject storage separated or replaced by an error handler while reading an element +--FILE-- + function ($ao) { + $ao->exchangeArray(['' => 'other empty', 1 => 'other one']); + }, + 'separated' => function ($ao) { + $data = $ao->__serialize(); + $ao['new'] = 1; + // Frees the table that the storage was separated from + unset($data); + }, +]; +$reads = [ + '$ao[null]' => fn ($ao) => $ao[null], + 'isset($ao[null])' => fn ($ao) => isset($ao[null]), + 'empty($ao[null])' => fn ($ao) => empty($ao[null]), + '$ao->offsetGet(null)' => fn ($ao) => $ao->offsetGet(null), + '$ao->offsetExists(null)' => fn ($ao) => $ao->offsetExists(null), + '$ao[1.5]' => fn ($ao) => $ao[1.5], + 'isset($ao[1.5])' => fn ($ao) => isset($ao[1.5]), + 'seek(1.5)' => function ($ao) { + $it = $ao->getIterator(); + $it->seek(1.5); + return $it->current(); + }, +]; + +foreach ($handlers as $how => $handler) { + echo "-- $how\n"; + foreach ($reads as $label => $read) { + $ao = new ArrayObject(['' => 'empty', 1 => 'one', 2 => 'two']); + set_error_handler(function ($errno, $errstr) use ($ao, $handler) { + echo $errstr, "\n"; + $handler($ao); + }); + $result = $read($ao); + restore_error_handler(); + echo $label, ': ', var_export($result, true), "\n"; + } +} +?> +--EXPECT-- +-- replaced +Using null as an array offset is deprecated, use an empty string instead +$ao[null]: 'other empty' +Using null as an array offset is deprecated, use an empty string instead +isset($ao[null]): true +Using null as an array offset is deprecated, use an empty string instead +empty($ao[null]): false +Using null as an array offset is deprecated, use an empty string instead +$ao->offsetGet(null): 'other empty' +Using null as an array offset is deprecated, use an empty string instead +$ao->offsetExists(null): true +Implicit conversion from float 1.5 to int loses precision +$ao[1.5]: 'other one' +Implicit conversion from float 1.5 to int loses precision +isset($ao[1.5]): true +Implicit conversion from float 1.5 to int loses precision +seek(1.5): 'other one' +-- separated +Using null as an array offset is deprecated, use an empty string instead +$ao[null]: 'empty' +Using null as an array offset is deprecated, use an empty string instead +isset($ao[null]): true +Using null as an array offset is deprecated, use an empty string instead +empty($ao[null]): false +Using null as an array offset is deprecated, use an empty string instead +$ao->offsetGet(null): 'empty' +Using null as an array offset is deprecated, use an empty string instead +$ao->offsetExists(null): true +Implicit conversion from float 1.5 to int loses precision +$ao[1.5]: 'one' +Implicit conversion from float 1.5 to int loses precision +isset($ao[1.5]): true +Implicit conversion from float 1.5 to int loses precision +seek(1.5): 'one' diff --git a/ext/spl/tests/ArrayObject/gh17935.phpt b/ext/spl/tests/ArrayObject/gh17935.phpt new file mode 100644 index 000000000000..123226208d1c --- /dev/null +++ b/ext/spl/tests/ArrayObject/gh17935.phpt @@ -0,0 +1,27 @@ +--TEST-- +GH-17935 (Assertion failure Zend/zend_hash.c when appending to serialized ArrayObject) +--FILE-- +__serialize(); +$o['a'] = 'b'; +var_dump($data[1], $o->getArrayCopy()); + +$o = new ArrayIterator(['a' => 1]); +$data = $o->__serialize(); +unset($o['a']); +var_dump($data[1], $o->getArrayCopy()); +?> +--EXPECT-- +array(0) { +} +array(1) { + ["a"]=> + string(1) "b" +} +array(1) { + ["a"]=> + int(1) +} +array(0) { +} From 7d3e22622df9dd5f1efffe4650709e17ee1127b1 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Thu, 12 Nov 2026 18:37:32 +0100 Subject: [PATCH 2/3] ext/spl: Share arrays copy-on-write in ArrayObject and ArrayIterator ArrayObject and ArrayIterator duplicated the array they were given when its refcount was above 1, and the arrays they hand out through getArrayCopy(), exchangeArray(), clone and array casts. They now share these arrays, and rely on the separation of their storage before writes. Arrays holding references are still duplicated, since zend_array_dup() unwraps references with refcount 1. Immutable arrays are still duplicated too, since iterators cannot be registered on them. exchangeArray() still returns a duplicate when iterators are registered on the storage, as they would keep following the returned array. While an iterator has a position, it is registered on the table shared with the caller's array. --- NEWS | 2 + UPGRADING | 5 + ext/spl/spl_array.c | 68 ++++++-- .../ArrayObject_copy_on_write_copies.phpt | 123 +++++++++++++ ...rayObject_copy_on_write_error_handler.phpt | 52 ++++++ ...ject_copy_on_write_error_handler_read.phpt | 56 ++++++ .../ArrayObject_copy_on_write_iterators.phpt | 163 ++++++++++++++++++ .../ArrayObject_copy_on_write_misc.phpt | 77 +++++++++ .../ArrayObject_copy_on_write_references.phpt | 78 +++++++++ .../ArrayObject_copy_on_write_twice.phpt | 34 ++++ .../ArrayObject_copy_on_write_writes.phpt | 86 +++++++++ 11 files changed, 730 insertions(+), 14 deletions(-) create mode 100644 ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_copies.phpt create mode 100644 ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_error_handler.phpt create mode 100644 ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_error_handler_read.phpt create mode 100644 ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_iterators.phpt create mode 100644 ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_misc.phpt create mode 100644 ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_references.phpt create mode 100644 ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_twice.phpt create mode 100644 ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_writes.phpt diff --git a/NEWS b/NEWS index b96f6f774919..fc467ae48fb5 100644 --- a/NEWS +++ b/NEWS @@ -152,6 +152,8 @@ PHP NEWS (Ilia Alshanetsky) - SPL: + . ArrayObject and ArrayIterator now share the arrays they are given or hand + out until one side modifies them. (Nicolas Grekas) . Fixed bug GH-17935 (Assertion failure Zend/zend_hash.c when appending to serialized ArrayObject). (Nicolas Grekas) . Fixed bug GH-23385 (SplDoublyLinkedList::serialize() use-after-free when diff --git a/UPGRADING b/UPGRADING index d9242ecb3c17..56b87f673659 100644 --- a/UPGRADING +++ b/UPGRADING @@ -205,6 +205,11 @@ PHP 8.7 UPGRADE NOTES target does not keep is now released at that assignment instead of after the last one. +- SPL: + . ArrayObject and ArrayIterator no longer copy the arrays they are given or + hand out (constructor, exchangeArray(), getArrayCopy(), clone, array cast) + until one side modifies them, unless these arrays hold references. + - Standard: . Improved performance of strcspn() and strpbrk() when searching longer strings with a single-byte character mask. diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c index d6d0b219ea98..f8056d9fe5ea 100644 --- a/ext/spl/spl_array.c +++ b/ext/spl/spl_array.c @@ -56,7 +56,6 @@ typedef struct _spl_array_object { #define Z_SPLARRAY_P(zv) spl_array_from_obj(Z_OBJ_P((zv))) static inline HashTable **spl_array_get_hash_table_ptr(spl_array_object* intern) { /* {{{ */ - //??? TODO: Delay duplication for arrays; only duplicate for write operations if (intern->ar_flags & SPL_ARRAY_IS_SELF) { /* rebuild properties */ zend_std_get_properties_ex(&intern->std); @@ -108,6 +107,26 @@ static inline bool spl_array_is_object(spl_array_object *intern) /* {{{ */ } /* }}} */ +/* Arrays are shared with their other holders when doing so is indistinguishable + * from zend_array_dup(), which unwraps references with refcount 1. */ +static bool spl_array_can_share(const HashTable *ht) +{ + const zval *zv; + + if (GC_FLAGS(ht) & IS_ARRAY_IMMUTABLE) { + /* Immutable arrays cannot hold iterators */ + return false; + } + + ZEND_HASH_FOREACH_VAL(ht, zv) { + if (Z_ISREF_P(zv)) { + return false; + } + } ZEND_HASH_FOREACH_END(); + + return true; +} + /* Returns the storage, separated from its other holders so that it can be written to. */ static HashTable *spl_array_get_hash_table_for_write(spl_array_object *intern) { @@ -121,6 +140,22 @@ static HashTable *spl_array_get_hash_table_for_write(spl_array_object *intern) return spl_array_get_hash_table(intern); } +/* Copies the storage into zv, sharing it when possible. */ +static void spl_array_copy_storage(zval *zv, spl_array_object *intern) +{ + spl_array_object *owner = intern; + + while (owner->ar_flags & SPL_ARRAY_USE_OTHER) { + owner = Z_SPLARRAY_P(&owner->array); + } + if (!(owner->ar_flags & SPL_ARRAY_IS_SELF) && Z_TYPE(owner->array) == IS_ARRAY + && spl_array_can_share(Z_ARRVAL(owner->array))) { + ZVAL_COPY(zv, &owner->array); + } else { + ZVAL_ARR(zv, zend_array_dup(spl_array_get_hash_table(intern))); + } +} + static zend_result spl_array_skip_protected(spl_array_object *intern, HashTable *aht); static void spl_array_create_ht_iter(HashTable *ht, spl_array_object* intern) /* {{{ */ @@ -225,8 +260,7 @@ static zend_object *spl_array_object_new_ex(zend_class_entry *class_type, zend_o if (other->ar_flags & SPL_ARRAY_IS_SELF) { ZVAL_UNDEF(&intern->array); } else if (instanceof_function(class_type, spl_ce_ArrayObject)) { - ZVAL_ARR(&intern->array, - zend_array_dup(spl_array_get_hash_table(other))); + spl_array_copy_storage(&intern->array, other); } else { #if ZEND_DEBUG /* This is because the call to instanceof_function will remain because @@ -783,7 +817,7 @@ PHP_METHOD(ArrayObject, getArrayCopy) ZEND_PARSE_PARAMETERS_NONE(); - RETURN_ARR(zend_array_dup(spl_array_get_hash_table(intern))); + spl_array_copy_storage(return_value, intern); } /* }}} */ static HashTable *spl_array_get_properties_for(zend_object *object, zend_prop_purpose purpose) /* {{{ */ @@ -796,9 +830,8 @@ static HashTable *spl_array_get_properties_for(zend_object *object, zend_prop_pu return zend_std_get_properties_for(object, purpose); } - /* We are supposed to be the only owner of the internal hashtable. - * The "dup" flag decides whether this is a "long-term" use where - * we need to duplicate, or a "temporary" one, where we can expect + /* The "dup" flag decides whether this is a "long-term" use where + * we need a copy, or a "temporary" one, where we can expect * that no operations on the ArrayObject will be performed in the * meantime. */ switch (purpose) { @@ -813,12 +846,14 @@ static HashTable *spl_array_get_properties_for(zend_object *object, zend_prop_pu return zend_std_get_properties_for(object, purpose); } - ht = spl_array_get_hash_table(intern); if (dup) { - ht = zend_array_dup(ht); - } else { - GC_ADDREF(ht); + zval copy; + spl_array_copy_storage(©, intern); + return Z_ARR(copy); } + + ht = spl_array_get_hash_table(intern); + GC_ADDREF(ht); return ht; } /* }}} */ @@ -999,10 +1034,9 @@ static void spl_array_set_array(zval *object, spl_array_object *intern, zval *ar ZVAL_UNDEF(&garbage); if (Z_TYPE_P(array) == IS_ARRAY) { ZVAL_COPY_VALUE(&garbage, &intern->array); - if (Z_REFCOUNT_P(array) == 1) { + if (Z_REFCOUNT_P(array) == 1 || spl_array_can_share(Z_ARR_P(array))) { ZVAL_COPY(&intern->array, array); } else { - //??? TODO: try to avoid array duplication ZVAL_ARR(&intern->array, zend_array_dup(Z_ARR_P(array))); } } else { @@ -1159,7 +1193,13 @@ PHP_METHOD(ArrayObject, exchangeArray) RETURN_THROWS(); } - RETVAL_ARR(zend_array_dup(spl_array_get_hash_table(intern))); + HashTable *ht = spl_array_get_hash_table(intern); + if (UNEXPECTED(HT_HAS_ITERATORS(ht))) { + /* Iterators of the storage would keep following the returned array */ + RETVAL_ARR(zend_array_dup(ht)); + } else { + spl_array_copy_storage(return_value, intern); + } spl_array_set_array(object, intern, array, 0L, true); } /* }}} */ diff --git a/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_copies.phpt b/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_copies.phpt new file mode 100644 index 000000000000..b9205e52303b --- /dev/null +++ b/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_copies.phpt @@ -0,0 +1,123 @@ +--TEST-- +ArrayObject and ArrayIterator share arrays they hand out copy-on-write +--FILE-- + 1, 'b' => 2]); +$copy = $ao->getArrayCopy(); +$ao['a'] = 10; +$copy['b'] = 20; +var_dump($ao->getArrayCopy(), $copy); + +$ao = new ArrayObject(['a' => 1]); +$clone = clone $ao; +$clone['a'] = 2; +$ao['b'] = 3; +var_dump($ao->getArrayCopy(), $clone->getArrayCopy()); + +$ao = new ArrayObject(['a' => 1]); +$cast = (array) $ao; +$ao['a'] = 2; +$cast['b'] = 3; +var_dump($ao->getArrayCopy(), $cast); + +$source = ['a' => 1]; +$ao = new ArrayObject($source); +$old = $ao->exchangeArray($source); +$old['b'] = 2; +$ao['c'] = 3; +var_dump($source, $old, $ao->getArrayCopy()); + +$it = new ArrayIterator(['a' => 1]); +$copy = $it->getArrayCopy(); +$it2 = new ArrayIterator($copy); +$it2['a'] = 2; +$copy['a'] = 3; +var_dump($it['a'], $it2['a'], $copy['a']); + +$ao = new ArrayObject(['a' => 1]); +$it = $ao->getIterator(); +$copy = $it->getArrayCopy(); +$it['a'] = 2; +$copy['b'] = 3; +var_dump($ao->getArrayCopy(), $copy); + +// Several objects built from the same array +$source = [1, 2, 3]; +$objects = [new ArrayIterator($source), new ArrayObject($source), new ArrayObject($source)]; +$objects[0][0] = 'it'; +$objects[1][] = 'ao'; +unset($objects[2][2]); +foreach ($objects as $object) { + echo json_encode($object->getArrayCopy()), "\n"; +} +echo json_encode($source), "\n"; +?> +--EXPECT-- +array(2) { + ["a"]=> + int(10) + ["b"]=> + int(2) +} +array(2) { + ["a"]=> + int(1) + ["b"]=> + int(20) +} +array(2) { + ["a"]=> + int(1) + ["b"]=> + int(3) +} +array(1) { + ["a"]=> + int(2) +} +array(1) { + ["a"]=> + int(2) +} +array(2) { + ["a"]=> + int(1) + ["b"]=> + int(3) +} +array(1) { + ["a"]=> + int(1) +} +array(2) { + ["a"]=> + int(1) + ["b"]=> + int(2) +} +array(2) { + ["a"]=> + int(1) + ["c"]=> + int(3) +} +int(1) +int(2) +int(3) +array(1) { + ["a"]=> + int(2) +} +array(2) { + ["a"]=> + int(1) + ["b"]=> + int(3) +} +["it",2,3] +[1,2,3,"ao"] +[1,2] +[1,2,3] diff --git a/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_error_handler.phpt b/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_error_handler.phpt new file mode 100644 index 000000000000..26c89727f63d --- /dev/null +++ b/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_error_handler.phpt @@ -0,0 +1,52 @@ +--TEST-- +ArrayObject copied by an error handler while creating an element +--FILE-- + 1], ArrayObject::ARRAY_AS_PROPS); +set_error_handler(function ($errno, $errstr) use ($ao) { + echo $errstr, "\n"; + $GLOBALS['copies'][] = $ao->getArrayCopy(); +}); +$ao->b .= 'x'; +$ao->c++; +$ao['d'] .= 'y'; +restore_error_handler(); +var_dump($ao->getArrayCopy(), $copies); +?> +--EXPECT-- +Undefined array key "b" +Undefined array key "c" +Undefined array key "d" +array(4) { + ["a"]=> + int(1) + ["b"]=> + string(1) "x" + ["c"]=> + int(1) + ["d"]=> + string(1) "y" +} +array(3) { + [0]=> + array(1) { + ["a"]=> + int(1) + } + [1]=> + array(2) { + ["a"]=> + int(1) + ["b"]=> + string(1) "x" + } + [2]=> + array(3) { + ["a"]=> + int(1) + ["b"]=> + string(1) "x" + ["c"]=> + int(1) + } +} diff --git a/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_error_handler_read.phpt b/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_error_handler_read.phpt new file mode 100644 index 000000000000..e612e460671c --- /dev/null +++ b/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_error_handler_read.phpt @@ -0,0 +1,56 @@ +--TEST-- +ArrayObject storage separated from the array it was built from by an error handler while reading an element +--FILE-- + fn ($ao) => $ao[null], + 'isset($ao[null])' => fn ($ao) => isset($ao[null]), + 'empty($ao[null])' => fn ($ao) => empty($ao[null]), + '$ao->offsetGet(null)' => fn ($ao) => $ao->offsetGet(null), + '$ao->offsetExists(null)' => fn ($ao) => $ao->offsetExists(null), + '$ao[1.5]' => fn ($ao) => $ao[1.5], + 'isset($ao[1.5])' => fn ($ao) => isset($ao[1.5]), + 'seek(1.5)' => function ($ao) { + $it = $ao->getIterator(); + $it->seek(1.5); + return $it->current(); + }, +]; + +foreach ($reads as $label => $read) { + $source = []; + foreach (['' => 'empty', 1 => 'one', 2 => 'two'] as $k => $v) { + $source[$k] = $v; + } + $ao = new ArrayObject($source); + set_error_handler(function ($errno, $errstr) use ($ao, &$source) { + echo $errstr, "\n"; + $ao['new'] = 1; + $source = null; + }); + $result = $read($ao); + restore_error_handler(); + echo $label, ': ', var_export($result, true), "\n"; + unset($source); +} +?> +--EXPECT-- +Using null as an array offset is deprecated, use an empty string instead +$ao[null]: 'empty' +Using null as an array offset is deprecated, use an empty string instead +isset($ao[null]): true +Using null as an array offset is deprecated, use an empty string instead +empty($ao[null]): false +Using null as an array offset is deprecated, use an empty string instead +$ao->offsetGet(null): 'empty' +Using null as an array offset is deprecated, use an empty string instead +$ao->offsetExists(null): true +Implicit conversion from float 1.5 to int loses precision +$ao[1.5]: 'one' +Implicit conversion from float 1.5 to int loses precision +isset($ao[1.5]): true +Implicit conversion from float 1.5 to int loses precision +seek(1.5): 'one' diff --git a/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_iterators.phpt b/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_iterators.phpt new file mode 100644 index 000000000000..625159b13c49 --- /dev/null +++ b/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_iterators.phpt @@ -0,0 +1,163 @@ +--TEST-- +ArrayObject and ArrayIterator keep their iteration position when separating a shared array +--FILE-- + 0, 'a' => 1, 'b' => 2, 'c' => 3, 'd' => 4]; + unset($a['z']); + return $a; +} + +echo "-- write during foreach\n"; +$source = source(); +$it = new ArrayIterator($source); +foreach ($it as $k => $v) { + echo "$k=$v "; + $it[$k] = $v * 10; +} +echo "\n", json_encode($it->getArrayCopy()), ' ', json_encode($source), "\n"; + +echo "-- write during manual iteration\n"; +$source = source(); +$it = new ArrayIterator($source); +$it->rewind(); +$it->next(); +$it['e'] = 5; +for (; $it->valid(); $it->next()) { + echo $it->key(), '=', $it->current(), ' '; +} +echo "\n"; + +echo "-- unset during foreach\n"; +$source = source(); +$it = new ArrayIterator($source); +foreach ($it as $k => $v) { + echo "$k=$v "; + if ($k === 'b') { + unset($it['c']); + } +} +echo "\n", json_encode($it->getArrayCopy()), "\n"; + +echo "-- ArrayObject iterator, write through the ArrayObject\n"; +$source = source(); +$ao = new ArrayObject($source); +foreach ($ao as $k => $v) { + echo "$k=$v "; + $ao[$k] = -$v; +} +echo "\n", json_encode($ao->getArrayCopy()), ' ', json_encode($source), "\n"; + +echo "-- two ArrayObject iterators\n"; +$source = source(); +$ao = new ArrayObject($source); +$it1 = $ao->getIterator(); +$it2 = $ao->getIterator(); +$it1->next(); +$it2->next(); +$it2->next(); +$ao['f'] = 6; +echo $it1->key(), ' ', $it2->key(), "\n"; +$it1->next(); +$it2->next(); +echo $it1->key(), ' ', $it2->key(), "\n"; + +echo "-- source modified during foreach\n"; +$source = source(); +$it = new ArrayIterator($source); +foreach ($it as $k => $v) { + echo "$k=$v "; + unset($source['c']); + $source['g'] = 7; +} +echo "\n", json_encode($it->getArrayCopy()), ' ', json_encode($source), "\n"; + +echo "-- foreach by reference\n"; +$source = source(); +$ao = new ArrayObject($source); +foreach ($ao as $k => &$v) { + echo "$k=$v "; + $v *= 2; +} +unset($v); +echo "\n", json_encode($ao->getArrayCopy()), ' ', json_encode($source), "\n"; + +echo "-- nested foreach with writes\n"; +$source = source(); +$ao = new ArrayObject($source); +foreach ($ao as $k1 => $v1) { + foreach ($ao as $k2 => $v2) { + if ($k2 === 'b') { + $ao['b'] = $v2 + 1; + } + } + echo "$k1=$v1 "; +} +echo "\n", json_encode($ao->getArrayCopy()), "\n"; + +echo "-- getArrayCopy() then write during foreach\n"; +$it = new ArrayIterator(source()); +foreach ($it as $k => $v) { + $copy = $it->getArrayCopy(); + $it[$k] = $v + 100; + echo "$k=$v "; +} +echo "\n", json_encode($it->getArrayCopy()), ' ', json_encode($copy), "\n"; + +echo "-- sort during foreach\n"; +$source = [3 => 'c', 1 => 'a', 2 => 'b']; +$ao = new ArrayObject($source); +foreach ($ao as $k => $v) { + echo "$k=$v "; + if ($k === 3) { + $ao->ksort(); + } +} +echo "\n", json_encode($ao->getArrayCopy()), ' ', json_encode($source), "\n"; + +echo "-- RecursiveArrayIterator\n"; +$source = ['a' => [1, 2], 'b' => ['c' => [3]]]; +$it = new RecursiveArrayIterator($source); +$children = $it->getChildren(); +$children[] = 'x'; +foreach (new RecursiveIteratorIterator($it) as $k => $v) { + echo "$k=$v "; +} +echo "\n", json_encode($children->getArrayCopy()), ' ', json_encode($source), "\n"; +?> +--EXPECT-- +-- write during foreach +a=1 b=2 c=3 d=4 +{"a":10,"b":20,"c":30,"d":40} {"a":1,"b":2,"c":3,"d":4} +-- write during manual iteration +b=2 c=3 d=4 e=5 +-- unset during foreach +a=1 b=2 d=4 +{"a":1,"b":2,"d":4} +-- ArrayObject iterator, write through the ArrayObject +a=1 b=2 c=3 d=4 +{"a":-1,"b":-2,"c":-3,"d":-4} {"a":1,"b":2,"c":3,"d":4} +-- two ArrayObject iterators +b c +c d +-- source modified during foreach +a=1 b=2 c=3 d=4 +{"a":1,"b":2,"c":3,"d":4} {"a":1,"b":2,"d":4,"g":7} +-- foreach by reference +a=1 b=2 c=3 d=4 +{"a":2,"b":4,"c":6,"d":8} {"a":1,"b":2,"c":3,"d":4} +-- nested foreach with writes +a=1 b=3 c=3 d=4 +{"a":1,"b":6,"c":3,"d":4} +-- getArrayCopy() then write during foreach +a=1 b=2 c=3 d=4 +{"a":101,"b":102,"c":103,"d":104} {"a":101,"b":102,"c":103,"d":4} +-- sort during foreach +3=c 2=b 3=c +{"1":"a","2":"b","3":"c"} {"3":"c","1":"a","2":"b"} +-- RecursiveArrayIterator +0=1 1=2 0=3 +[1,2,"x"] {"a":[1,2],"b":{"c":[3]}} diff --git a/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_misc.phpt b/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_misc.phpt new file mode 100644 index 000000000000..4ee505ad1cde --- /dev/null +++ b/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_misc.phpt @@ -0,0 +1,77 @@ +--TEST-- +ArrayObject and ArrayIterator sharing an array: serialization and dumps +--FILE-- + 1, 'b' => [2]]; +$ao = new ArrayObject($source); +$it = new ArrayIterator($source); +echo serialize([$source, $ao, $it]), "\n"; +[$s, $ao2, $it2] = unserialize(serialize([$source, $ao, $it])); +$ao2['a'] = 2; +$it2['b'][] = 3; +var_dump($s, $ao2->getArrayCopy(), $it2->getArrayCopy()); + +echo "-- var_dump, var_export, json_encode, count, ==\n"; +$ao = new ArrayObject($source); +var_dump($ao); +var_export($ao); echo "\n"; +echo json_encode($ao), "\n"; +var_dump(count($ao), $ao == new ArrayObject($source), $ao == new ArrayObject([])); +?> +--EXPECT-- +-- serialize +a:3:{i:0;a:2:{s:1:"a";i:1;s:1:"b";a:1:{i:0;i:2;}}i:1;O:11:"ArrayObject":4:{i:0;i:0;i:1;a:2:{s:1:"a";i:1;s:1:"b";a:1:{i:0;i:2;}}i:2;a:0:{}i:3;N;}i:2;O:13:"ArrayIterator":4:{i:0;i:0;i:1;a:2:{s:1:"a";i:1;s:1:"b";a:1:{i:0;i:2;}}i:2;a:0:{}i:3;N;}} +array(2) { + ["a"]=> + int(1) + ["b"]=> + array(1) { + [0]=> + int(2) + } +} +array(2) { + ["a"]=> + int(2) + ["b"]=> + array(1) { + [0]=> + int(2) + } +} +array(2) { + ["a"]=> + int(1) + ["b"]=> + array(2) { + [0]=> + int(2) + [1]=> + int(3) + } +} +-- var_dump, var_export, json_encode, count, == +object(ArrayObject)#5 (1) { + ["storage":"ArrayObject":private]=> + array(2) { + ["a"]=> + int(1) + ["b"]=> + array(1) { + [0]=> + int(2) + } + } +} +\ArrayObject::__set_state(array( + 'a' => 1, + 'b' => + array ( + 0 => 2, + ), +)) +{"a":1,"b":[2]} +int(2) +bool(true) +bool(false) diff --git a/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_references.phpt b/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_references.phpt new file mode 100644 index 000000000000..2405a578849c --- /dev/null +++ b/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_references.phpt @@ -0,0 +1,78 @@ +--TEST-- +ArrayObject and ArrayIterator copy arrays holding references +--FILE-- + null, 'b' => 2]; +foreach ($source as &$v); +unset($v); +foreach ([new ArrayIterator($source), new ArrayObject($source)] as $object) { + var_dump(isset($object['a'])); + $array = iterator_to_array($object); + $array['b'] = 3; + $cache = new CachingIterator(new IteratorIterator($object), CachingIterator::FULL_CACHE); + foreach ($cache as $v); + $source['b'] = 4; + var_dump($object['b'], $source['b']); + $source['b'] = 2; +} + +echo "-- shared references in the source\n"; +$x = 1; +$source = ['x' => &$x]; +$it = new ArrayIterator($source); +$x = 2; +var_dump($it['x']); +unset($x); +$source['x'] = 3; +var_dump($it['x']); + +echo "-- dangling reference in the storage\n"; +$ao = new ArrayObject(['x' => 1]); +$r = &$ao['x']; +unset($r); +$copy = $ao->getArrayCopy(); +$array = iterator_to_array($ao); +$copy['x'] = 2; +$clone = clone $ao; +$array['x'] = 3; +var_dump($copy['x'], $clone['x'], $ao['x']); + +echo "-- built inside foreach by reference over the source\n"; +$source = [1, 2, 3]; +foreach ($source as $k => &$v) { + if ($k === 0) { + $it = new ArrayIterator($source); + } + $v *= 10; +} +unset($v); +var_dump($it->getArrayCopy()); +?> +--EXPECT-- +-- dangling references in the source +bool(false) +int(2) +int(4) +bool(false) +int(2) +int(4) +-- shared references in the source +int(2) +int(3) +-- dangling reference in the storage +int(2) +int(3) +int(3) +-- built inside foreach by reference over the source +array(3) { + [0]=> + &int(10) + [1]=> + int(2) + [2]=> + int(3) +} diff --git a/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_twice.phpt b/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_twice.phpt new file mode 100644 index 000000000000..272447470fe8 --- /dev/null +++ b/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_twice.phpt @@ -0,0 +1,34 @@ +--TEST-- +ArrayObject iterator keeps its position when a shared storage is separated twice between two steps +--SKIPIF-- + +--FILE-- + $k) { + $source[$k] = $i + 1; +} +$ao = new ArrayObject($source); +unset($source); +foreach ($ao as $k => $v) { + echo "$k "; + if ($k === 'b') { + // A hole before the current position, which a copy compacts + unset($ao['a']); + $copy = $ao->getArrayCopy(); + $ao['x'] = 1; + unset($copy); + $copy = $ao->getArrayCopy(); + $ao['y'] = 1; + } +} +echo "\n"; +?> +--EXPECT-- +a b c d e x y diff --git a/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_writes.phpt b/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_writes.phpt new file mode 100644 index 000000000000..6cd8f26e07d0 --- /dev/null +++ b/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_writes.phpt @@ -0,0 +1,86 @@ +--TEST-- +ArrayObject and ArrayIterator separate a shared array before writing to it +--FILE-- + 1, 'b' => ['x' => 1], 'c' => 3]; + $object = new $class($source, ArrayObject::ARRAY_AS_PROPS); + $copy = $object->getArrayCopy(); + $write($object); + if ($source !== ['a' => 1, 'b' => ['x' => 1], 'c' => 3] || $copy !== $source) { + echo "$class $label: FAIL\n"; + var_dump($source, $copy); + } + echo "$class $label: ", json_encode($object->getArrayCopy()), "\n"; + } +} + +check('offsetSet', function ($o) { $o['a'] = 2; }); +check('offsetSet()', function ($o) { $o->offsetSet('a', 2); }); +check('append', function ($o) { $o[] = 4; }); +check('append()', function ($o) { $o->append(4); }); +check('offsetUnset', function ($o) { unset($o['a']); }); +check('offsetUnset() missing', function ($o) { $o->offsetUnset('z'); }); +check('compound assign', function ($o) { $o['a'] += 10; }); +check('increment', function ($o) { $o['a']++; }); +check('nested write', function ($o) { $o['b']['y'] = 2; }); +check('nested append', function ($o) { $o['b'][] = 2; }); +check('nested unset', function ($o) { unset($o['b']['x']); }); +check('reference', function ($o) { $r = &$o['a']; $r = 5; }); +check('property', function ($o) { $o->a = 6; }); +check('property compound assign', function ($o) { $o->a .= 'x'; }); +check('property nested write', function ($o) { $o->b['z'] = 3; }); +check('property reference', function ($o) { $r = &$o->c; $r = 7; }); +check('property unset', function ($o) { unset($o->c); }); +check('foreach by reference', function ($o) { foreach ($o as &$v) { $v = 8; } }); +check('uasort', function ($o) { $o->uasort(fn ($a, $b) => is_array($a) <=> is_array($b)); }, ['ArrayObject']); +check('ksort', function ($o) { $o->ksort(); $o['d'] = 0; }, ['ArrayObject']); +check('exchangeArray', function ($o) { if ($o instanceof ArrayObject) { $o->exchangeArray(['e' => 5]); } else { $o->__construct(['e' => 5]); } }); +check('unserialize', function ($o) { $o->__unserialize([0, ['f' => 6], [], null]); }); +?> +--EXPECT-- +ArrayIterator offsetSet: {"a":2,"b":{"x":1},"c":3} +ArrayObject offsetSet: {"a":2,"b":{"x":1},"c":3} +ArrayIterator offsetSet(): {"a":2,"b":{"x":1},"c":3} +ArrayObject offsetSet(): {"a":2,"b":{"x":1},"c":3} +ArrayIterator append: {"a":1,"b":{"x":1},"c":3,"0":4} +ArrayObject append: {"a":1,"b":{"x":1},"c":3,"0":4} +ArrayIterator append(): {"a":1,"b":{"x":1},"c":3,"0":4} +ArrayObject append(): {"a":1,"b":{"x":1},"c":3,"0":4} +ArrayIterator offsetUnset: {"b":{"x":1},"c":3} +ArrayObject offsetUnset: {"b":{"x":1},"c":3} +ArrayIterator offsetUnset() missing: {"a":1,"b":{"x":1},"c":3} +ArrayObject offsetUnset() missing: {"a":1,"b":{"x":1},"c":3} +ArrayIterator compound assign: {"a":11,"b":{"x":1},"c":3} +ArrayObject compound assign: {"a":11,"b":{"x":1},"c":3} +ArrayIterator increment: {"a":2,"b":{"x":1},"c":3} +ArrayObject increment: {"a":2,"b":{"x":1},"c":3} +ArrayIterator nested write: {"a":1,"b":{"x":1,"y":2},"c":3} +ArrayObject nested write: {"a":1,"b":{"x":1,"y":2},"c":3} +ArrayIterator nested append: {"a":1,"b":{"x":1,"0":2},"c":3} +ArrayObject nested append: {"a":1,"b":{"x":1,"0":2},"c":3} +ArrayIterator nested unset: {"a":1,"b":[],"c":3} +ArrayObject nested unset: {"a":1,"b":[],"c":3} +ArrayIterator reference: {"a":5,"b":{"x":1},"c":3} +ArrayObject reference: {"a":5,"b":{"x":1},"c":3} +ArrayIterator property: {"a":6,"b":{"x":1},"c":3} +ArrayObject property: {"a":6,"b":{"x":1},"c":3} +ArrayIterator property compound assign: {"a":"1x","b":{"x":1},"c":3} +ArrayObject property compound assign: {"a":"1x","b":{"x":1},"c":3} +ArrayIterator property nested write: {"a":1,"b":{"x":1,"z":3},"c":3} +ArrayObject property nested write: {"a":1,"b":{"x":1,"z":3},"c":3} +ArrayIterator property reference: {"a":1,"b":{"x":1},"c":7} +ArrayObject property reference: {"a":1,"b":{"x":1},"c":7} +ArrayIterator property unset: {"a":1,"b":{"x":1}} +ArrayObject property unset: {"a":1,"b":{"x":1}} +ArrayIterator foreach by reference: {"a":8,"b":8,"c":8} +ArrayObject foreach by reference: {"a":8,"b":8,"c":8} +ArrayObject uasort: {"a":1,"c":3,"b":{"x":1}} +ArrayObject ksort: {"a":1,"b":{"x":1},"c":3,"d":0} +ArrayIterator exchangeArray: {"e":5} +ArrayObject exchangeArray: {"e":5} +ArrayIterator unserialize: {"f":6} +ArrayObject unserialize: {"f":6} From d01b76f7989e01ef37dbbffd850b9292101bf5c9 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Thu, 12 Nov 2026 18:39:06 +0100 Subject: [PATCH 3/3] ext/spl: Share immutable arrays in ArrayObject and ArrayIterator Immutable arrays, that is opcache literals and the empty array, were still duplicated. They are now stored as is. Since iterators cannot be registered on them, the storage is separated when a position is first needed in it, unless it is empty: positions in an empty array need no iterator. The sort methods separate it first too, as the sorted array replaces the storage. spl_array_method() no longer writes the sorted array through a pointer kept across the call. When sorting through an iterator, the callback can replace the storage of the underlying ArrayObject by an immutable array, and writing the HashTable alone would leave the type flags of that array on a refcounted one. --- ext/spl/spl_array.c | 50 ++- .../ArrayObject_copy_on_write_immutable.phpt | 361 ++++++++++++++++++ ...copy_on_write_immutable_sort_callback.phpt | 44 +++ .../ArrayObject_copy_on_write_misc.phpt | 35 +- 4 files changed, 484 insertions(+), 6 deletions(-) create mode 100644 ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_immutable.phpt create mode 100644 ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_immutable_sort_callback.phpt diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c index f8056d9fe5ea..de424f71b1dc 100644 --- a/ext/spl/spl_array.c +++ b/ext/spl/spl_array.c @@ -42,6 +42,7 @@ typedef struct _spl_array_object { uint32_t ht_iter; int ar_flags; unsigned char nApplyCount; + HashPosition empty_pos; /* The position in an empty immutable storage */ zend_function *fptr_offset_get; zend_function *fptr_offset_set; zend_function *fptr_offset_has; @@ -114,8 +115,8 @@ static bool spl_array_can_share(const HashTable *ht) const zval *zv; if (GC_FLAGS(ht) & IS_ARRAY_IMMUTABLE) { - /* Immutable arrays cannot hold iterators */ - return false; + /* Immutable arrays hold no references */ + return true; } ZEND_HASH_FOREACH_VAL(ht, zv) { @@ -160,6 +161,7 @@ static zend_result spl_array_skip_protected(spl_array_object *intern, HashTable static void spl_array_create_ht_iter(HashTable *ht, spl_array_object* intern) /* {{{ */ { + ZEND_ASSERT(!(GC_FLAGS(ht) & IS_ARRAY_IMMUTABLE)); intern->ht_iter = zend_hash_iterator_add(ht, zend_hash_get_current_pos(ht)); zend_hash_internal_pointer_reset_ex(ht, &EG(ht_iterators)[intern->ht_iter].pos); spl_array_skip_protected(intern, ht); @@ -172,6 +174,8 @@ static void spl_array_update_ht_iter(HashTable *ht, spl_array_object* intern) HashPosition pos = EG(ht_iterators)[idx].pos; bool has_copy = false; + ZEND_ASSERT(!(GC_FLAGS(ht) & IS_ARRAY_IMMUTABLE)); + /* When the storage is separated, zend_array_dup() registers a copy of the * iterator on the new table, with its position adjusted to it. */ for (uint32_t i = EG(ht_iterators)[idx].next_copy; i != idx; i = EG(ht_iterators)[i].next_copy) { @@ -200,6 +204,21 @@ static zend_always_inline bool spl_array_ht_iter_is_on(const spl_array_object *i static zend_never_inline uint32_t *spl_array_get_pos_ptr_slow(HashTable *ht, spl_array_object* intern) { + if (GC_FLAGS(ht) & IS_ARRAY_IMMUTABLE) { + if (!ht->nNumUsed) { + /* No iterator is needed as anything added to the storage separates it first */ + intern->empty_pos = 0; + return &intern->empty_pos; + } + /* Iterators cannot be registered on immutable arrays, so the storage is separated. + * zend_array_dup() copies immutable arrays as is: positions are the same in both + * and callers can complete their read on the immutable array. */ + ht = spl_array_get_hash_table_for_write(intern); + if (spl_array_ht_iter_is_on(intern, ht)) { + return &EG(ht_iterators)[intern->ht_iter].pos; + } + } + if (intern->ht_iter == (uint32_t)-1) { spl_array_create_ht_iter(ht, intern); } else { @@ -853,7 +872,7 @@ static HashTable *spl_array_get_properties_for(zend_object *object, zend_prop_pu } ht = spl_array_get_hash_table(intern); - GC_ADDREF(ht); + GC_TRY_ADDREF(ht); return ht; } /* }}} */ @@ -1268,6 +1287,11 @@ PHP_METHOD(ArrayObject, count) static void spl_array_method(zval *return_value, spl_array_object *intern, const char *fname, size_t fname_len, const zval *extra_arg) /* {{{ */ { + /* The sorted array replaces the storage, which cannot be done for immutable arrays */ + if (GC_FLAGS(spl_array_get_hash_table(intern)) & IS_ARRAY_IMMUTABLE) { + spl_array_get_hash_table_for_write(intern); + } + HashTable **ht_ptr = spl_array_get_hash_table_ptr(intern); HashTable *aht = *ht_ptr; zval params[2]; @@ -1292,9 +1316,25 @@ static void spl_array_method(zval *return_value, spl_array_object *intern, const intern->nApplyCount--; zval *ht_zv = Z_REFVAL(params[0]); - zend_array_release(*ht_ptr); SEPARATE_ARRAY(ht_zv); - *ht_ptr = Z_ARRVAL_P(ht_zv); + + /* nApplyCount only guards this object: when it points to another ArrayObject, + * the callback may have replaced the storage of that one, so it is looked up + * again, and an array storage is replaced as a zval to keep its type flags right. */ + spl_array_object *owner = intern; + while (owner->ar_flags & SPL_ARRAY_USE_OTHER) { + owner = Z_SPLARRAY_P(&owner->array); + } + if (!(owner->ar_flags & SPL_ARRAY_IS_SELF) && Z_TYPE(owner->array) == IS_ARRAY) { + zval garbage; + ZVAL_COPY_VALUE(&garbage, &owner->array); + ZVAL_ARR(&owner->array, Z_ARRVAL_P(ht_zv)); + zval_ptr_dtor(&garbage); + } else { + ht_ptr = spl_array_get_hash_table_ptr(intern); + zend_array_release(*ht_ptr); + *ht_ptr = Z_ARRVAL_P(ht_zv); + } ZVAL_NULL(ht_zv); zval_ptr_dtor(¶ms[0]); } /* }}} */ diff --git a/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_immutable.phpt b/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_immutable.phpt new file mode 100644 index 000000000000..00d5effb954e --- /dev/null +++ b/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_immutable.phpt @@ -0,0 +1,361 @@ +--TEST-- +ArrayObject and ArrayIterator share immutable arrays and separate them before iterating or writing +--EXTENSIONS-- +opcache +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.protect_memory=1 +opcache.file_update_protection=0 +--FILE-- + 2, 'a' => 1, 'c' => [3, 4]]; + const NAT = ['b10' => 'x10', 'b9' => 'x9', 'B1' => 'X1']; +} + +function literal() { + return ['x' => 1, 'y' => 2, 'z' => 3]; +} + +echo "-- foreach by value with writes\n"; +foreach (['ArrayIterator', 'ArrayObject'] as $class) { + $o = new $class(Consts::LIST); + foreach ($o as $k => $v) { + echo "$k=$v "; + $o[$k] = $v * 10; + if ($k === 0) { + $o[] = 4; + } + } + echo json_encode($o->getArrayCopy()), "\n"; +} + +echo "-- foreach by reference\n"; +foreach (['ArrayIterator', 'ArrayObject'] as $class) { + $o = new $class(literal()); + foreach ($o as $k => &$v) { + $v = "$k$v"; + } + unset($v); + echo json_encode($o->getArrayCopy()), "\n"; +} + +echo "-- ArrayIterator methods\n"; +$it = new ArrayIterator(literal()); +var_dump($it->valid(), $it->key(), $it->current()); +$it->next(); +var_dump($it->key(), $it->current()); +$it->seek(2); +var_dump($it->key()); +$it->rewind(); +var_dump($it->key()); +$it['w'] = 0; +$it->next(); +$it->next(); +$it->next(); +var_dump($it->key(), $it->current()); +$it = new ArrayIterator(literal()); +$it->seek(1); +unset($it['x']); +var_dump($it->key()); + +echo "-- first call on the iterator\n"; +foreach (['valid', 'current', 'key', 'next', 'rewind'] as $method) { + $it = new ArrayIterator(literal()); + echo $method, ': ', json_encode($it->$method()), ' ', $it->key(), '=', $it->current(), "\n"; + $it = (new ArrayObject(literal()))->getIterator(); + echo $method, ': ', json_encode($it->$method()), ' ', $it->key(), '=', $it->current(), "\n"; +} +$it = new ArrayIterator(literal()); +$it->seek(1); +echo $it->key(), "\n"; +$it = new RecursiveArrayIterator(['a' => [1, 2]]); +var_dump($it->hasChildren()); +$it = new RecursiveArrayIterator(['a' => [1, 2]]); +echo json_encode($it->getChildren()->getArrayCopy()), "\n"; + +echo "-- getIterator()\n"; +$ao = new ArrayObject(literal()); +$it1 = $ao->getIterator(); +$it2 = $ao->getIterator(); +$it1->next(); +foreach ($it2 as $k => $v) { + echo "$k=$v "; + $ao[$k] = -$v; +} +echo $it1->key(), ' ', json_encode($ao->getArrayCopy()), "\n"; +$ao = new ArrayObject(literal()); +$it = $ao->getIterator(); +$it->next(); +$ao->exchangeArray(Consts::MAP); +var_dump($it->key()); + +echo "-- RecursiveArrayIterator\n"; +$it = new RecursiveArrayIterator(['a' => [1, [2, 3]], 'b' => 4]); +foreach (new RecursiveIteratorIterator($it, RecursiveIteratorIterator::SELF_FIRST) as $k => $v) { + echo $k, '=', json_encode($v), ' '; +} +echo "\n"; +$it->rewind(); +$children = $it->getChildren(); +$children[] = 5; +foreach ($children as $v) { + echo json_encode($v), ' '; +} +echo json_encode($it->getArrayCopy()), "\n"; + +echo "-- sort\n"; +foreach (['asort', 'ksort', 'natsort', 'natcasesort'] as $sort) { + $ao = new ArrayObject(Consts::NAT); + $ao->$sort(); + echo json_encode($ao->getArrayCopy()), ' '; +} +$ao = new ArrayObject([]); +$ao->ksort(); +$ao[] = 0; +echo json_encode($ao->getArrayCopy()), ' '; +$ao = new ArrayObject(Consts::LIST); +$ao->uasort(fn ($a, $b) => $b <=> $a); +$ao->uksort(fn ($a, $b) => $a <=> $b); +echo json_encode($ao->getArrayCopy()), "\n"; + +echo "-- copies\n"; +$ao = new ArrayObject(Consts::MAP); +$copy = $ao->getArrayCopy(); +$clone = clone $ao; +$cast = (array) $ao; +$old = $ao->exchangeArray(Consts::LIST); +$ao[] = 4; +$clone['d'] = 5; +$copy['e'] = 6; +$cast['f'] = 7; +$old['g'] = 8; +echo json_encode([$ao->getArrayCopy(), $clone->getArrayCopy(), $copy, $cast, $old]), "\n"; +var_dump($ao->getArrayCopy() === [1, 2, 3, 4], (new ArrayIterator([]))->getArrayCopy() === []); + +echo "-- serialization and dumps\n"; +$ao = new ArrayObject(Consts::MAP); +$it = new ArrayIterator(Consts::LIST); +echo serialize($ao), "\n", serialize($it), "\n", $ao->serialize(), "\n"; +$ao2 = unserialize(serialize($ao)); +$ao2['d'] = 1; +echo json_encode($ao2), ' ', json_encode($ao), ' ', count($ao), "\n"; +var_export($ao); +echo "\n"; +var_dump($it, $ao == new ArrayObject(Consts::MAP), isset($ao['a']), $ao['c'][1]); +print_r($it); +echo "\n"; + +echo "-- empty array\n"; +foreach (['ArrayIterator', 'ArrayObject'] as $class) { + $o = new $class([]); + foreach ($o as $v) { + echo "unreachable\n"; + } + $it = $o instanceof ArrayObject ? $o->getIterator() : $o; + $it->rewind(); + var_dump($it->valid(), $it->key(), $it->current(), count($o)); + $o[] = 'a'; + var_dump($it->valid(), $it->current()); + $e = new $class(Consts::LIST); + $e->__construct([]); + var_dump(iterator_to_array($e), iterator_count($e)); +} +$it = new ArrayIterator([]); +$it->rewind(); +$it->next(); +$it->next(); +$it[] = 'a'; +$it[] = 'b'; +var_dump($it->key(), $it->current()); +$ao = new ArrayObject([]); +$it = $ao->getIterator(); +$it->rewind(); +$ao[] = 'c'; +unset($ao[0]); +$ao[] = 'd'; +var_dump($it->key(), $it->current()); +$ao = new ArrayObject(Consts::LIST); +$it = $ao->getIterator(); +$it->next(); +$ao->exchangeArray([]); +var_dump($it->valid()); +$ao[] = 'e'; +$ao[] = 'f'; +var_dump($it->valid(), $it->current()); +foreach (new ArrayObject([]) as &$v) { + echo "unreachable\n"; +} +$it = new ArrayIterator([]); +unset($it['x']); +var_dump(count($it)); +try { + $it->seek(0); +} catch (OutOfBoundsException $e) { + echo $e->getMessage(), "\n"; +} +$append = new AppendIterator(); +$append->append(new ArrayIterator([])); +$append->append(new ArrayIterator(Consts::LIST)); +echo json_encode(iterator_to_array($append, false)), "\n"; + +echo "-- literals are unchanged\n"; +var_dump(Consts::LIST, Consts::MAP, Consts::NAT, literal(), []); +?> +--EXPECT-- +-- foreach by value with writes +0=1 1=2 2=3 3=4 [10,20,30,40] +0=1 1=2 2=3 3=4 [10,20,30,40] +-- foreach by reference +{"x":"x1","y":"y2","z":"z3"} +{"x":"x1","y":"y2","z":"z3"} +-- ArrayIterator methods +bool(true) +string(1) "x" +int(1) +string(1) "y" +int(2) +string(1) "z" +string(1) "x" +string(1) "w" +int(0) +string(1) "y" +-- first call on the iterator +valid: true x=1 +valid: true x=1 +current: 1 x=1 +current: 1 x=1 +key: "x" x=1 +key: "x" x=1 +next: null y=2 +next: null y=2 +rewind: null x=1 +rewind: null x=1 +y +bool(true) +[1,2] +-- getIterator() +x=1 y=2 z=3 y {"x":-1,"y":-2,"z":-3} +string(1) "a" +-- RecursiveArrayIterator +a=[1,[2,3]] 0=1 1=[2,3] 0=2 1=3 b=4 +1 [2,3] 5 {"a":[1,[2,3]],"b":4} +-- sort +{"B1":"X1","b10":"x10","b9":"x9"} {"B1":"X1","b10":"x10","b9":"x9"} {"B1":"X1","b9":"x9","b10":"x10"} {"B1":"X1","b9":"x9","b10":"x10"} [0] [1,2,3] +-- copies +[[1,2,3,4],{"b":2,"a":1,"c":[3,4],"d":5},{"b":2,"a":1,"c":[3,4],"e":6},{"b":2,"a":1,"c":[3,4],"f":7},{"b":2,"a":1,"c":[3,4],"g":8}] +bool(true) +bool(true) +-- serialization and dumps +O:11:"ArrayObject":4:{i:0;i:0;i:1;a:3:{s:1:"b";i:2;s:1:"a";i:1;s:1:"c";a:2:{i:0;i:3;i:1;i:4;}}i:2;a:0:{}i:3;N;} +O:13:"ArrayIterator":4:{i:0;i:0;i:1;a:3:{i:0;i:1;i:1;i:2;i:2;i:3;}i:2;a:0:{}i:3;N;} +x:i:0;a:3:{s:1:"b";i:2;s:1:"a";i:1;s:1:"c";a:2:{i:0;i:3;i:1;i:4;}};m:a:0:{} +{"b":2,"a":1,"c":[3,4],"d":1} {"b":2,"a":1,"c":[3,4]} 3 +\ArrayObject::__set_state(array( + 'b' => 2, + 'a' => 1, + 'c' => + array ( + 0 => 3, + 1 => 4, + ), +)) +object(ArrayIterator)#7 (1) { + ["storage":"ArrayIterator":private]=> + array(3) { + [0]=> + int(1) + [1]=> + int(2) + [2]=> + int(3) + } +} +bool(true) +bool(true) +int(4) +ArrayIterator Object +( + [storage:ArrayIterator:private] => Array + ( + [0] => 1 + [1] => 2 + [2] => 3 + ) + +) + +-- empty array +bool(false) +NULL +NULL +int(0) +bool(true) +string(1) "a" +array(0) { +} +int(0) +bool(false) +NULL +NULL +int(0) +bool(true) +string(1) "a" +array(0) { +} +int(0) +int(0) +string(1) "a" +int(1) +string(1) "d" +bool(false) +bool(true) +string(1) "f" +int(0) +Seek position 0 is out of range +[1,2,3] +-- literals are unchanged +array(3) { + [0]=> + int(1) + [1]=> + int(2) + [2]=> + int(3) +} +array(3) { + ["b"]=> + int(2) + ["a"]=> + int(1) + ["c"]=> + array(2) { + [0]=> + int(3) + [1]=> + int(4) + } +} +array(3) { + ["b10"]=> + string(3) "x10" + ["b9"]=> + string(2) "x9" + ["B1"]=> + string(2) "X1" +} +array(3) { + ["x"]=> + int(1) + ["y"]=> + int(2) + ["z"]=> + int(3) +} +array(0) { +} diff --git a/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_immutable_sort_callback.phpt b/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_immutable_sort_callback.phpt new file mode 100644 index 000000000000..dac17f6b61ea --- /dev/null +++ b/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_immutable_sort_callback.phpt @@ -0,0 +1,44 @@ +--TEST-- +ArrayObject storage replaced by an immutable array while it is sorted through an iterator +--EXTENSIONS-- +opcache +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.protect_memory=1 +opcache.file_update_protection=0 +--FILE-- +getIterator(); + $replaced = false; + $it->uasort(function ($a, $b) use ($ao, $replace, &$replaced) { + if (!$replaced) { + $replaced = true; + $replace($ao); + } + + return $a <=> $b; + }); + $before = $ao->getArrayCopy(); + $ao[] = 'object'; + $after = $ao->getArrayCopy(); + $after[] = 'copy'; + echo json_encode($before), ' ', json_encode($ao->getArrayCopy()), ' ', json_encode($after), "\n"; +} + +test(fn ($ao) => $ao->exchangeArray([])); +test(fn ($ao) => $ao->exchangeArray([7, 8, 9])); +?> +--EXPECTF-- + +Deprecated: Method ArrayIterator::uasort() is deprecated since 8.6 in %s on line %d +{"1":1,"2":2,"0":3} {"1":1,"2":2,"0":3,"3":"object"} {"1":1,"2":2,"0":3,"3":"object","4":"copy"} + +Deprecated: Method ArrayIterator::uasort() is deprecated since 8.6 in %s on line %d +{"1":1,"2":2,"0":3} {"1":1,"2":2,"0":3,"3":"object"} {"1":1,"2":2,"0":3,"3":"object","4":"copy"} diff --git a/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_misc.phpt b/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_misc.phpt index 4ee505ad1cde..618e35d1e7f8 100644 --- a/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_misc.phpt +++ b/ext/spl/tests/ArrayObject/ArrayObject_copy_on_write_misc.phpt @@ -1,7 +1,20 @@ --TEST-- -ArrayObject and ArrayIterator sharing an array: serialization and dumps +ArrayObject and ArrayIterator with immutable arrays, serialization and dumps --FILE-- $v) { + $it[$k] = $v * 2; +} +$ao = new ArrayObject([]); +$ao[] = 1; +var_dump($it->getArrayCopy(), $ao->getArrayCopy()); +const LIST_CONST = ['a', 'b']; +$ao = new ArrayObject(LIST_CONST); +$ao[] = 'c'; +var_dump(LIST_CONST, count($ao)); + echo "-- serialize\n"; $source = ['a' => 1, 'b' => [2]]; $ao = new ArrayObject($source); @@ -20,6 +33,26 @@ echo json_encode($ao), "\n"; var_dump(count($ao), $ao == new ArrayObject($source), $ao == new ArrayObject([])); ?> --EXPECT-- +-- immutable arrays +array(3) { + [0]=> + int(2) + [1]=> + int(4) + [2]=> + int(6) +} +array(1) { + [0]=> + int(1) +} +array(2) { + [0]=> + string(1) "a" + [1]=> + string(1) "b" +} +int(3) -- serialize a:3:{i:0;a:2:{s:1:"a";i:1;s:1:"b";a:1:{i:0;i:2;}}i:1;O:11:"ArrayObject":4:{i:0;i:0;i:1;a:2:{s:1:"a";i:1;s:1:"b";a:1:{i:0;i:2;}}i:2;a:0:{}i:3;N;}i:2;O:13:"ArrayIterator":4:{i:0;i:0;i:1;a:2:{s:1:"a";i:1;s:1:"b";a:1:{i:0;i:2;}}i:2;a:0:{}i:3;N;}} array(2) {