From 786b14ace79bf58c2ea3da5899a929e1d214115f Mon Sep 17 00:00:00 2001 From: Weilin Du Date: Mon, 7 Sep 2026 20:31:35 +0800 Subject: [PATCH 1/3] Fix iterator relocation at the current element during rehash When one iterator points at a hole and another at the next live bucket, the relocation loop must include the iterator at that bucket's original position. Otherwise it is moved to the following bucket's destination, causing a nested by-reference foreach to skip an element. Add regression coverage for packed-to-hash conversion and compaction of a full mixed table. --- NEWS | 2 ++ Zend/tests/rehash_multiple_iterators.phpt | 40 +++++++++++++++++++++++ Zend/zend_hash.c | 2 +- 3 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 Zend/tests/rehash_multiple_iterators.phpt diff --git a/NEWS b/NEWS index 056f0296b68a..7740192f9c8c 100644 --- a/NEWS +++ b/NEWS @@ -9,6 +9,8 @@ PHP NEWS - Core: . Fixed out-of-bounds reads during automatic UTF-16/32 encoding detection. (Yudai Takada) + . Fixed incorrect foreach iterator positions when compacting arrays with + holes. (Weilin Du) . Fixed bug GH-15375 (Nested "yield from" skips items after a valid() or next() call on the inner generator). (iliaal) . Fixed bug GH-23232 (lone namespace separator asks the autoloader for an diff --git a/Zend/tests/rehash_multiple_iterators.phpt b/Zend/tests/rehash_multiple_iterators.phpt new file mode 100644 index 000000000000..cbbfc75d703a --- /dev/null +++ b/Zend/tests/rehash_multiple_iterators.phpt @@ -0,0 +1,40 @@ +--TEST-- +Rehashing updates iterators at both a hole and the next defined element +--FILE-- + &$outerValue) { + $outerVisits[] = "$outerKey=>$outerValue"; + if ($first) { + $first = false; + foreach ($values as $innerKey => &$innerValue) { + $innerVisits[] = "$innerKey=>$innerValue"; + if ($innerValue === 11) { + // The outer cursor is at a hole, the inner at the next value. + unset($values[$firstKey], $values[$secondKey]); + $values[$newKey] = $newValue; + } + } + unset($innerValue); + } + } + unset($outerValue); + echo 'outer: ', implode(' ', $outerVisits), "\n"; + echo 'inner: ', implode(' ', $innerVisits), "\n"; +} + +// Adding a string key converts packed storage and compacts its holes. +test([10, 11, 12, 13, 14], 0, 1, 'new', 15); + +// Inserting into a full mixed table compacts its holes without growing it. +test(['a' => 10, 'b' => 11, 'c' => 12, 'd' => 13, + 'e' => 14, 'f' => 15, 'g' => 16, 'h' => 17], 'a', 'b', 'i', 18); +?> +--EXPECT-- +outer: 0=>10 2=>12 3=>13 4=>14 new=>15 +inner: 0=>10 1=>11 2=>12 3=>13 4=>14 new=>15 +outer: a=>10 c=>12 d=>13 e=>14 f=>15 g=>16 h=>17 i=>18 +inner: a=>10 b=>11 c=>12 d=>13 e=>14 f=>15 g=>16 h=>17 i=>18 diff --git a/Zend/zend_hash.c b/Zend/zend_hash.c index 82d0318428fa..a1f7b3c1e777 100644 --- a/Zend/zend_hash.c +++ b/Zend/zend_hash.c @@ -1412,7 +1412,7 @@ ZEND_API void ZEND_FASTCALL zend_hash_rehash(HashTable *ht) do { zend_hash_iterators_update(ht, iter_pos, j); iter_pos = zend_hash_iterators_lower_pos(ht, iter_pos + 1); - } while (iter_pos < i); + } while (iter_pos <= i); } q++; j++; From 2e62b97963c86f6f5ef89452e876f158930dbde6 Mon Sep 17 00:00:00 2001 From: Weilin Du Date: Tue, 8 Sep 2026 00:20:19 +0800 Subject: [PATCH 2/3] feedback --- Zend/tests/array_dup_multiple_iterators.phpt | 36 ++++++++++++++++++++ Zend/zend_hash.c | 2 +- 2 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 Zend/tests/array_dup_multiple_iterators.phpt diff --git a/Zend/tests/array_dup_multiple_iterators.phpt b/Zend/tests/array_dup_multiple_iterators.phpt new file mode 100644 index 000000000000..db7dcf74a77a --- /dev/null +++ b/Zend/tests/array_dup_multiple_iterators.phpt @@ -0,0 +1,36 @@ +--TEST-- +Array duplication updates iterators at both a hole and the next defined element +--FILE-- + &$outerValue) { + $outerVisits[] = "$outerKey=>$outerValue"; + if ($first) { + $first = false; + foreach ($values as $innerKey => &$innerValue) { + $innerVisits[] = "$innerKey=>$innerValue"; + if ($innerValue === 11) { + // The outer cursor is at a hole, the inner at the next value. + unset($values['a'], $values['b']); + $copy = $values; + // Trigger copy-on-write duplication, which compacts the holes. + $values['i'] = 18; + } + } + unset($innerValue); + } + } + unset($outerValue); + echo 'outer: ', implode(' ', $outerVisits), "\n"; + echo 'inner: ', implode(' ', $innerVisits), "\n"; +} + +test(['a' => 10, 'b' => 11, 'c' => 12, 'd' => 13, + 'e' => 14, 'f' => 15, 'g' => 16, 'h' => 17]); +?> +--EXPECT-- +outer: a=>10 c=>12 d=>13 e=>14 f=>15 g=>16 h=>17 i=>18 +inner: a=>10 b=>11 c=>12 d=>13 e=>14 f=>15 g=>16 h=>17 i=>18 diff --git a/Zend/zend_hash.c b/Zend/zend_hash.c index a1f7b3c1e777..27ea16f208ab 100644 --- a/Zend/zend_hash.c +++ b/Zend/zend_hash.c @@ -2428,7 +2428,7 @@ static zend_always_inline uint32_t zend_array_dup_elements(HashTable *source, Ha do { zend_hash_iterators_update(target, iter_pos, target_idx); iter_pos = zend_hash_iterators_lower_pos(target, iter_pos + 1); - } while (iter_pos < idx); + } while (iter_pos <= idx); } target_idx++; q++; } From d26d5f2d2695cd631f16d33bf9e8e6cbbfd4b90f Mon Sep 17 00:00:00 2001 From: Weilin Du Date: Tue, 8 Sep 2026 20:32:12 +0800 Subject: [PATCH 3/3] feedback --- NEWS | 4 +- .../array_dup_internal_pointer_hole.phpt | 45 +++++++++++++++++++ Zend/tests/array_dup_iterator_past_end.phpt | 23 ++++++++++ Zend/tests/array_dup_multiple_iterators.phpt | 2 + Zend/zend_hash.c | 6 ++- 5 files changed, 76 insertions(+), 4 deletions(-) create mode 100644 Zend/tests/array_dup_internal_pointer_hole.phpt create mode 100644 Zend/tests/array_dup_iterator_past_end.phpt diff --git a/NEWS b/NEWS index 7740192f9c8c..925339a1a164 100644 --- a/NEWS +++ b/NEWS @@ -9,8 +9,8 @@ PHP NEWS - Core: . Fixed out-of-bounds reads during automatic UTF-16/32 encoding detection. (Yudai Takada) - . Fixed incorrect foreach iterator positions when compacting arrays with - holes. (Weilin Du) + . Fixed incorrect internal pointer and foreach iterator positions when + compacting arrays with holes. (Weilin Du) . Fixed bug GH-15375 (Nested "yield from" skips items after a valid() or next() call on the inner generator). (iliaal) . Fixed bug GH-23232 (lone namespace separator asks the autoloader for an diff --git a/Zend/tests/array_dup_internal_pointer_hole.phpt b/Zend/tests/array_dup_internal_pointer_hole.phpt new file mode 100644 index 000000000000..97fdd40bb1ad --- /dev/null +++ b/Zend/tests/array_dup_internal_pointer_hole.phpt @@ -0,0 +1,45 @@ +--TEST-- +Array duplication relocates an internal pointer on a hole, with and without foreach iterators +--FILE-- +', current($values), "\n"; + next($values); + echo 'next: ', key($values), '=>', current($values), "\n"; + echo 'copy current: ', key($copy), '=>', current($copy), "\n"; + echo 'copy keys: ', implode(' ', array_keys($copy)), "\n"; +} + +foreach ([false, true] as $withIterator) { + echo $withIterator ? "With iterator:\n" : "Without iterator:\n"; + $values = ['a' => 10, 'b' => 11, 'c' => 12, 'd' => 13, + 'e' => 14, 'f' => 15, 'g' => 16, 'h' => 17]; + next($values); + next($values); + // Leave the internal pointer on a hole before several surviving elements. + unset($values['a'], $values['b'], $values['c'], $values['d']); + + if ($withIterator) { + foreach ($values as &$value) { + duplicate($values); + break; + } + unset($value); + } else { + duplicate($values); + } +} +?> +--EXPECT-- +Without iterator: +current: e=>14 +next: f=>15 +copy current: e=>14 +copy keys: e f g h +With iterator: +current: e=>14 +next: f=>15 +copy current: e=>14 +copy keys: e f g h diff --git a/Zend/tests/array_dup_iterator_past_end.phpt b/Zend/tests/array_dup_iterator_past_end.phpt new file mode 100644 index 000000000000..80e52e3e5ab2 --- /dev/null +++ b/Zend/tests/array_dup_iterator_past_end.phpt @@ -0,0 +1,23 @@ +--TEST-- +Array duplication preserves past-the-end iterators when compacting holes +--FILE-- + 10, 'b' => 11, 'c' => 12, 'd' => 13]; +unset($values['a'], $values['b']); + +foreach ($values as $key => &$value) { + echo "$key=>$value\n"; + if ($key === 'd') { + // The iterator is one past the end; COW compacts the preceding holes. + $copy = $values; + $values['e'] = 14; + } +} +unset($value); +echo 'copy: ', implode(' ', array_keys($copy)), "\n"; +?> +--EXPECT-- +c=>12 +d=>13 +e=>14 +copy: c d diff --git a/Zend/tests/array_dup_multiple_iterators.phpt b/Zend/tests/array_dup_multiple_iterators.phpt index db7dcf74a77a..96f2dbb7f8de 100644 --- a/Zend/tests/array_dup_multiple_iterators.phpt +++ b/Zend/tests/array_dup_multiple_iterators.phpt @@ -26,6 +26,7 @@ function test(array $values): void { unset($outerValue); echo 'outer: ', implode(' ', $outerVisits), "\n"; echo 'inner: ', implode(' ', $innerVisits), "\n"; + echo 'copy: ', implode(' ', array_keys($copy)), "\n"; } test(['a' => 10, 'b' => 11, 'c' => 12, 'd' => 13, @@ -34,3 +35,4 @@ test(['a' => 10, 'b' => 11, 'c' => 12, 'd' => 13, --EXPECT-- outer: a=>10 c=>12 d=>13 e=>14 f=>15 g=>16 h=>17 i=>18 inner: a=>10 b=>11 c=>12 d=>13 e=>14 f=>15 g=>16 h=>17 i=>18 +copy: c d e f g h diff --git a/Zend/zend_hash.c b/Zend/zend_hash.c index 27ea16f208ab..b4de0d7b4130 100644 --- a/Zend/zend_hash.c +++ b/Zend/zend_hash.c @@ -2408,7 +2408,7 @@ static zend_always_inline uint32_t zend_array_dup_elements(HashTable *source, Ha if (EXPECTED(!HT_HAS_ITERATORS(target))) { while (p != end) { if (zend_array_dup_element(source, target, target_idx, p, q, 0, static_keys, with_holes)) { - if (source->nInternalPointer == idx) { + if (UNEXPECTED(target->nInternalPointer > target_idx && target->nInternalPointer <= idx)) { target->nInternalPointer = target_idx; } target_idx++; q++; @@ -2421,7 +2421,7 @@ static zend_always_inline uint32_t zend_array_dup_elements(HashTable *source, Ha while (p != end) { if (zend_array_dup_element(source, target, target_idx, p, q, 0, static_keys, with_holes)) { - if (source->nInternalPointer == idx) { + if (UNEXPECTED(target->nInternalPointer > target_idx && target->nInternalPointer <= idx)) { target->nInternalPointer = target_idx; } if (UNEXPECTED(idx >= iter_pos)) { @@ -2434,6 +2434,8 @@ static zend_always_inline uint32_t zend_array_dup_elements(HashTable *source, Ha } idx++; p++; } + /* Move past-the-end iterators so they can pick up newly appended elements. */ + _zend_hash_iterators_update(target, source->nNumUsed, target_idx); } return target_idx; }