Skip to content

Commit 4792857

Browse files
darkdiclaude
authored andcommitted
Fix GH-23617: Keep a past-the-end internal pointer past the end on compaction
zend_array_dup() reset an exhausted internal pointer to 0, and both it and zend_hash_rehash() left it at the old nNumUsed when holes were compacted, so a later append was either skipped or the pointer jumped back to the first element. Move it to the new end instead, the same way past-the-end iterators are already migrated. Signed-off-by: Dmitry Rantovov <rantovov5@gmail.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Puqi4fj3kWhLHanMyF1UNU
1 parent e79d7e5 commit 4792857

2 files changed

Lines changed: 46 additions & 6 deletions

File tree

‎Zend/tests/gh23617.phpt‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
--TEST--
2+
GH-23617: Past-the-end internal pointer picks up an append after separation and compaction
3+
--FILE--
4+
<?php
5+
$makers = [
6+
'mixed' => function () { $a = ['a' => 1, 'b' => 2, 'c' => 3]; $a['c'] = 3; return $a; },
7+
'mixed with holes' => function () { $a = ['a' => 1, 'b' => 2, 'c' => 3, 'd' => 4, 'e' => 5]; unset($a['b'], $a['c']); return $a; },
8+
'full mixed with holes' => function () { $a = []; for ($i = 1; $i <= 8; $i++) $a["k$i"] = $i; unset($a['k2'], $a['k4'], $a['k6']); return $a; },
9+
'packed' => function () { $a = [1, 2, 3]; $a[] = 4; return $a; },
10+
'packed with holes' => function () { $a = [1, 2, 3, 4]; unset($a[1]); return $a; },
11+
];
12+
foreach ($makers as $name => $make) {
13+
foreach ([false, true] as $separate) {
14+
$a = $make();
15+
end($a);
16+
next($a);
17+
if ($separate) {
18+
$copy = $a;
19+
}
20+
$a[] = 'new';
21+
echo $name, $separate ? ' (separated): ' : ': ';
22+
var_dump(current($a));
23+
unset($copy);
24+
}
25+
}
26+
?>
27+
--EXPECT--
28+
mixed: string(3) "new"
29+
mixed (separated): string(3) "new"
30+
mixed with holes: string(3) "new"
31+
mixed with holes (separated): string(3) "new"
32+
full mixed with holes: string(3) "new"
33+
full mixed with holes (separated): string(3) "new"
34+
packed: string(3) "new"
35+
packed (separated): string(3) "new"
36+
packed with holes: string(3) "new"
37+
packed with holes (separated): string(3) "new"

‎Zend/zend_hash.c‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1438,6 +1438,9 @@ ZEND_API void ZEND_FASTCALL zend_hash_rehash(HashTable *ht)
14381438

14391439
/* Migrate pointer to one past the end of the array to the new one past the end, so that
14401440
* newly inserted elements are picked up correctly. */
1441+
if (ht->nInternalPointer >= old_num_used) {
1442+
ht->nInternalPointer = ht->nNumUsed;
1443+
}
14411444
if (UNEXPECTED(HT_HAS_ITERATORS(ht))) {
14421445
_zend_hash_iterators_update(ht, old_num_used, ht->nNumUsed);
14431446
}
@@ -2445,6 +2448,10 @@ static zend_always_inline uint32_t zend_array_dup_elements(const HashTable *sour
24452448
/* Move past-the-end iterators so they can pick up newly appended elements. */
24462449
_zend_hash_iterators_update(target, source->nNumUsed, target_idx);
24472450
}
2451+
/* Same for a past-the-end internal pointer. */
2452+
if (target->nInternalPointer >= source->nNumUsed) {
2453+
target->nInternalPointer = target_idx;
2454+
}
24482455
return target_idx;
24492456
}
24502457
idx++; p++; q++;
@@ -2500,9 +2507,7 @@ ZEND_API HashTable* ZEND_FASTCALL zend_array_dup(const HashTable *source)
25002507
target->nNextFreeElement = source->nNextFreeElement;
25012508
target->nTableSize = source->nTableSize;
25022509
HT_SET_DATA_ADDR(target, emalloc(HT_PACKED_SIZE_EX(target->nTableSize, HT_MIN_MASK)));
2503-
target->nInternalPointer =
2504-
(source->nInternalPointer < source->nNumUsed) ?
2505-
source->nInternalPointer : 0;
2510+
target->nInternalPointer = source->nInternalPointer;
25062511

25072512
HT_HASH_RESET_PACKED(target);
25082513

@@ -2516,9 +2521,7 @@ ZEND_API HashTable* ZEND_FASTCALL zend_array_dup(const HashTable *source)
25162521
HT_FLAGS(target) = HT_FLAGS(source) & (HASH_FLAG_MASK & ~HASH_FLAG_HAS_EMPTY_IND);
25172522
target->nTableMask = source->nTableMask;
25182523
target->nNextFreeElement = source->nNextFreeElement;
2519-
target->nInternalPointer =
2520-
(source->nInternalPointer < source->nNumUsed) ?
2521-
source->nInternalPointer : 0;
2524+
target->nInternalPointer = source->nInternalPointer;
25222525

25232526
target->nTableSize = source->nTableSize;
25242527
HT_SET_DATA_ADDR(target, emalloc(HT_SIZE(target)));

0 commit comments

Comments
 (0)