Fix number of elements after packed hash filling (php#11022)

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 discovered this behaviour while fixing phpGH-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] https://github.com/php/php-src/blob/b2c5acbb010f4bbc7ea9b53ba9bc81d672dd0f34/Zend/zend_vm_def.h#L6132-L6141

Author: nielsdos

Zend/zend_hash.h

@@ -1517,8 +1517,8 @@ static zend_always_inline void *zend_hash_get_current_data_ptr_ex(HashTable *ht,
 
 #define ZEND_HASH_FILL_GROW() do { \
 		if (UNEXPECTED(__fill_idx >= __fill_ht->nTableSize)) { \
+			__fill_ht->nNumOfElements += __fill_idx - __fill_ht->nNumUsed; \
 			__fill_ht->nNumUsed = __fill_idx; \
-			__fill_ht->nNumOfElements = __fill_idx; \
 			__fill_ht->nNextFreeElement = __fill_idx; \
 			zend_hash_packed_grow(__fill_ht); \
 			__fill_val = __fill_ht->arPacked + __fill_idx; \
@@ -1557,8 +1557,8 @@ static zend_always_inline void *zend_hash_get_current_data_ptr_ex(HashTable *ht,
 	} while (0)
 
 #define ZEND_HASH_FILL_FINISH() do { \
+		__fill_ht->nNumOfElements += __fill_idx - __fill_ht->nNumUsed; \
 		__fill_ht->nNumUsed = __fill_idx; \
-		__fill_ht->nNumOfElements = __fill_idx; \
 		__fill_ht->nNextFreeElement = __fill_idx; \
 		__fill_ht->nInternalPointer = 0; \
 	} while (0)

ext/zend_test/test.c

@@ -533,6 +533,29 @@ static ZEND_FUNCTION(zend_get_map_ptr_last)
 	RETURN_LONG(CG(map_ptr_last));
 }
 
+static ZEND_FUNCTION(zend_test_fill_packed_array)
+{
+	HashTable *parameter;
+
+	ZEND_PARSE_PARAMETERS_START(1, 1)
+		Z_PARAM_ARRAY_HT_EX(parameter, 0, 1)
+	ZEND_PARSE_PARAMETERS_END();
+
+	if (!HT_IS_PACKED(parameter)) {
+		zend_argument_value_error(1, "must be a packed array");
+		RETURN_THROWS();
+	}
+
+	zend_hash_extend(parameter, parameter->nNumUsed + 10, true);
+	ZEND_HASH_FILL_PACKED(parameter) {
+		for (int i = 0; i < 10; i++) {
+			zval value;
+			ZVAL_LONG(&value, i);
+			ZEND_HASH_FILL_ADD(&value);
+		}
+	} ZEND_HASH_FILL_END();
+}
+
 static zend_object *zend_test_class_new(zend_class_entry *class_type)
 {
 	zend_object *obj = zend_objects_new(class_type);

ext/zend_test/test.stub.php

@@ -192,6 +192,8 @@ function zend_test_zend_call_stack_use_all(): int {}
     function zend_test_is_string_marked_as_valid_utf8(string $string): bool {}
 
     function zend_get_map_ptr_last(): int {}
+
+    function zend_test_fill_packed_array(array &$array): void {}
 }
 
 namespace ZendTestNS {

ext/zend_test/test_arginfo.h

@@ -0,0 +1,44 @@
+--TEST--
+Test hash packed fill number of elements
+--EXTENSIONS--
+zend_test
+--FILE--
+<?php
+
+function number() {
+    return 6;
+}
+
+$my_array = [number() => 0];
+zend_test_fill_packed_array($my_array);
+
+var_dump($my_array);
+var_dump(count($my_array));
+
+?>
+--EXPECT--
+array(11) {
+  [6]=>
+  int(0)
+  [7]=>
+  int(0)
+  [8]=>
+  int(1)
+  [9]=>
+  int(2)
+  [10]=>
+  int(3)
+  [11]=>
+  int(4)
+  [12]=>
+  int(5)
+  [13]=>
+  int(6)
+  [14]=>
+  int(7)
+  [15]=>
+  int(8)
+  [16]=>
+  int(9)
+}
+int(11)