Fix GH-23617: Keep a past-the-end internal pointer past the end on compaction

[darkdi]

An exhausted internal pointer now moves to the new end whenever holes get compacted, in zend_array_dup() and in zend_hash_rehash(), the same way past-the-end iterators already do. The test covers packed and mixed, with and without holes, full table included, each with and without separation.

Fixes GH-23617.

[devnexen]

Hi @darkdi thanks for the patch but I m afraid the fix is really incorrect, I ll let ilaal review tough.

[iliaal]

Direction looks right: matching the immutable path lines up with the MIN(nInternalPointer, nNumUsed) clamp tail deletion already uses, and an exhausted cursor picking up a later append is what the same code does with no separation at all. Two problems though.

The .phpt expectation is wrong and the test fails on this branch. end(); next(); then an append gives "d" at refcount 1 with no copy involved, so NULL is not a behaviour being restored. The test body prints "d", 4, 3, 4 here.

And it is incomplete for mixed arrays with holes, where zend_array_dup_elements() compacts and target->nNumUsed ends up below the preserved pointer:

array	no separation	separated, this PR
['a'=>1,'b'=>2,'c'=>3]	'zz'	'zz'
same, two keys unset	'zz'	NULL
8 keys, 3 interior unset	NULL	NULL

Setting target->nInternalPointer = idx fixes row 2 but breaks row 3, since a full table rehashes on the append and zend_hash_rehash() does not remap an exhausted pointer either. So no single copied value agrees with the middle column everywhere; normalising the exhausted cursor wherever compaction happens, rehash included, looks like the actual fix.

The NULL expectation came from my issue text, which was wrong. I have corrected GH-23617.

[darkdi]

Thanks, you were right on both. Reworked it the way you suggested: the exhausted cursor now goes to the new nNumUsed after compaction in both zend_array_dup_elements() and zend_hash_rehash(), not just copied. Built locally this time, your three rows plus packed with and without holes all give the appended element now, separated or not, and the new test fails on master for the holes cases. Zend/tests and ext/standard/tests/array pass

[darkdi]

@iliaal gentle ping on the Sep 11 revision. Does normalizing the exhausted cursor in both compaction paths address the cases you found?

[iliaal]

Yes, that covers them. Please add a NEWS entry for the behaviour change.

[darkdi]

AI-assisted reply:

Added the NEWS entry in e15bff3 and rebased onto current master. The code fix is unchanged. The PHP 8.7 debug+ZTS build passes Zend/tests and ext/standard/tests/array: 6,273 passed, 188 skipped, one expected failure, no unexpected failures.