Fix number of elements after packed hash filling

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

Author: nielsdos

Zend/zend_hash.h

@@ -1175,12 +1175,12 @@ static zend_always_inline void *zend_hash_get_current_data_ptr_ex(HashTable *ht,
 		HashTable *__fill_ht = (ht); \
 		Bucket *__fill_bkt = __fill_ht->arData + __fill_ht->nNumUsed; \
 		uint32_t __fill_idx = __fill_ht->nNumUsed; \
+		uint32_t __fill_old_used = __fill_idx; \
 		ZEND_ASSERT(HT_FLAGS(__fill_ht) & HASH_FLAG_PACKED);
 
 #define ZEND_HASH_FILL_GROW() do { \
 		if (UNEXPECTED(__fill_idx >= __fill_ht->nTableSize)) { \
 			__fill_ht->nNumUsed = __fill_idx; \
-			__fill_ht->nNumOfElements = __fill_idx; \
 			__fill_ht->nNextFreeElement = __fill_idx; \
 			zend_hash_packed_grow(__fill_ht); \
 			__fill_bkt = __fill_ht->arData + __fill_idx; \
@@ -1222,7 +1222,7 @@ static zend_always_inline void *zend_hash_get_current_data_ptr_ex(HashTable *ht,
 
 #define ZEND_HASH_FILL_FINISH() do { \
 		__fill_ht->nNumUsed = __fill_idx; \
-		__fill_ht->nNumOfElements = __fill_idx; \
+		__fill_ht->nNumOfElements += __fill_idx - __fill_old_used; \
 		__fill_ht->nNextFreeElement = __fill_idx; \
 		__fill_ht->nInternalPointer = 0; \
 	} while (0)

ext/zend_test/test.c

@@ -327,6 +327,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

@@ -119,6 +119,8 @@ function zend_get_current_func_name(): string {}
     function zend_call_method(string $class, string $method, mixed $arg1 = UNKNOWN, mixed $arg2 = UNKNOWN): mixed {}
 
     function zend_get_map_ptr_last(): int {}
+
+    function zend_test_fill_packed_array(array &$array): void {}
 }
 
 namespace ZendTestNS {

ext/zend_test/test_arginfo.h

@@ -1,5 +1,5 @@
 /* This is a generated file, edit the .stub.php file instead.
- * Stub hash: 614310958c6e2acde46c9b7932ba894caf72d6df */
+ * Stub hash: 7e9e4384787822de32a284d45cf9d1355391e848 */
 
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_test_array_return, 0, 0, IS_ARRAY, 0)
 ZEND_END_ARG_INFO()
@@ -82,6 +82,10 @@ ZEND_END_ARG_INFO()
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_get_map_ptr_last, 0, 0, IS_LONG, 0)
 ZEND_END_ARG_INFO()
 
+ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_test_fill_packed_array, 0, 1, IS_VOID, 0)
+	ZEND_ARG_TYPE_INFO(1, array, IS_ARRAY, 0)
+ZEND_END_ARG_INFO()
+
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_ZendTestNS2_ZendSubNS_namespaced_func, 0, 0, _IS_BOOL, 0)
 ZEND_END_ARG_INFO()
 
@@ -143,6 +147,7 @@ static ZEND_FUNCTION(zend_test_parameter_with_attribute);
 static ZEND_FUNCTION(zend_get_current_func_name);
 static ZEND_FUNCTION(zend_call_method);
 static ZEND_FUNCTION(zend_get_map_ptr_last);
+static ZEND_FUNCTION(zend_test_fill_packed_array);
 static ZEND_FUNCTION(namespaced_func);
 static ZEND_METHOD(_ZendTestClass, is_object);
 static ZEND_METHOD(_ZendTestClass, __toString);
@@ -182,6 +187,7 @@ static const zend_function_entry ext_functions[] = {
 	ZEND_FE(zend_get_current_func_name, arginfo_zend_get_current_func_name)
 	ZEND_FE(zend_call_method, arginfo_zend_call_method)
 	ZEND_FE(zend_get_map_ptr_last, arginfo_zend_get_map_ptr_last)
+	ZEND_FE(zend_test_fill_packed_array, arginfo_zend_test_fill_packed_array)
 	ZEND_NS_FE("ZendTestNS2\\ZendSubNS", namespaced_func, arginfo_ZendTestNS2_ZendSubNS_namespaced_func)
 	ZEND_FE_END
 };

ext/zend_test/tests/hash_fill_packed_nr_elements.phpt

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