Fix number of elements after packed hash filling

[nielsdos]

After a hash filling routine the number of elements are set to the fill index. However, if the fill index is larger than the number of elements, the number of elements are no longer correct. This is observable at least via count() and var_dump(). E.g. the attached test case would incorrectly show int(17) instead of int(11).

Solve this by only increasing the number of elements by the actual number that got added. Instead of adding a variable that increments per iteration, I wanted to save some cycles in the iteration and simply compute the number of added elements at the end.

I removed the assignment in ZEND_HASH_FILL_GROW() because the number of elements is set anyway at the end of the hash function and this avoids an unnecessary computation.

I discovered this behaviour while fixing GH-11016, where this filling routine is easily exposed to userland via a specialised VM path [1]. Since this seems to be more a general problem with the macros, and may be triggered outside of the VM handlers, I fixed it in the macros instead of modifying the VM to fixup the number of elements.

[1]

php-src/Zend/zend_vm_def.h

Lines 6132 to 6141 in b2c5acb

	ZEND_HASH_FILL_PACKED(result_ht) {
	ZEND_HASH_PACKED_FOREACH_VAL(ht, val) {
	if (UNEXPECTED(Z_ISREF_P(val)) &&
	UNEXPECTED(Z_REFCOUNT_P(val) == 1)) {
	val = Z_REFVAL_P(val);
	}
	Z_TRY_ADDREF_P(val);
	ZEND_HASH_FILL_ADD(val);
	} ZEND_HASH_FOREACH_END();
	} ZEND_HASH_FILL_END();

[iluuu1994]

LGTM! But let's hear what Dmitry has to say 🙂

[dstogov]

The problem was introduced by #9796 that incorrectly used ZEND_HASH_FILL_PACKED() macro.
The comment above ZEND_HASH_FILL_PACKED() tells - "HashTable must have enough free buckets", but zend_hash_extend(result_ht, zend_hash_num_elements(result_ht) + zend_hash_num_elements(ht), 1); doesn't take into account possible holes.

The fix proposed in #11016 looks right and I would add it to this PR.

Actually ZEND_HASH_FILL_PACKED() was useful to fill new tables.
This patch should extend the usability of ZEND_HASH_FILL_PACKED().

In general this looks good, but let me play with this a bit (in worse case by Monday).

[dstogov]

Please update the fix according to suggestions and include the fix for #11016

[nielsdos]

Made the changes like requested. Thanks. As per #11021 (comment) I'll keep the PRs separate, and make sure to commit them in the order Dmitry said.

[dstogov]

Looks fine