Fix GH-12826: Weird pointers issue in nested loops

[nielsdos]

This regressed in cd53ce8.
The loop with zend_hash_iterators_update hangs forever because iter_pos can't advance to idx. This is because the zend_hash_iterators_lower_pos upper bound is target->nNumUsed, but that is set to source->nNumOfElements.

php-src/Zend/zend_hash.c

Lines 2419 to 2422 in e3de478

	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);

php-src/Zend/zend_hash.c

Line 2410 in e3de478

target->nNumUsed = source->nNumOfElements;

That means that if there are holes in the array, we still loop over all the buckets but the number of bucket slots will not match. Fix it by changing the assignment.

I'm not entirely sure this is the whole story though.

[bwoebi]

I suppose this is now similar to what zend_hash_rehash is doing (it also starts of with the original ht->nNumUsed). So looks correct to me.

However I'm seeing:

php-src/Zend/zend_hash.c

Lines 1407 to 1411 in e3de478

	/* Migrate pointer to one past the end of the array to the new one past the end, so that
	* newly inserted elements are picked up correctly. */
	if (UNEXPECTED(HT_HAS_ITERATORS(ht))) {
	_zend_hash_iterators_update(ht, old_num_used, ht->nNumUsed);
	}

at the end of zend_hash_rehash. Does it need something similar here? I'm not really that deep into the logic here right now.

[dstogov]

I'll need to take a careful look. I don't understand this code in all details...
I'll try to review this on Monday/Tuesday.

[dstogov]

Yeah. This may be used as a fix.

[nielsdos]

@bwoebi I don't think _zend_hash_iterators_update is necessary. I suppose you're suggesting using it on the target HashTable. What _zend_hash_iterators_update does is fixing the iteration index of a HashTable for all its iterators.
However, target is a new HashTable because we're in array separation logic here. So there are no iterators associated with target. The reason it has to happen for rehashing is because the contents of the array may change places, so the index for its iterators needs to change places as well.