From 987426489d515c80d3b95548bd0529a93bb789e1 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Wed, 30 Jul 2025 18:44:31 +0200 Subject: [PATCH 1/2] Fix GH-19303: Unpacking empty packed array into uninitialized array causes assertion failure Having an empty result array is not a problem, because zend_hash_extend() will initialize it. Except it does not when the number of elements to add equals 0, which leaves the array uninitialized and therefore does not set the packed flag, causing the assertion failure. Technically, removing the assert would also work and save a check. On the other hand, this check could also prevent some real work to be done and should be relatively cheap as we already have to compute the sum anyway. --- Zend/tests/array_unpack/gh19303.phpt | 11 +++++++++++ Zend/zend_vm_def.h | 24 +++++++++++++----------- Zend/zend_vm_execute.h | 24 +++++++++++++----------- 3 files changed, 37 insertions(+), 22 deletions(-) create mode 100644 Zend/tests/array_unpack/gh19303.phpt diff --git a/Zend/tests/array_unpack/gh19303.phpt b/Zend/tests/array_unpack/gh19303.phpt new file mode 100644 index 0000000000000..af594c3740c2d --- /dev/null +++ b/Zend/tests/array_unpack/gh19303.phpt @@ -0,0 +1,11 @@ +--TEST-- +GH-19303 (Unpacking empty packed array into uninitialized array causes assertion failure) +--FILE-- + +--EXPECT-- +array(0) { +} diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h index 7c9f151134fe2..a23860e16b86b 100644 --- a/Zend/zend_vm_def.h +++ b/Zend/zend_vm_def.h @@ -6162,17 +6162,19 @@ ZEND_VM_C_LABEL(add_unpack_again): zval *val; if (HT_IS_PACKED(ht) && (zend_hash_num_elements(result_ht) == 0 || HT_IS_PACKED(result_ht))) { - zend_hash_extend(result_ht, result_ht->nNumUsed + zend_hash_num_elements(ht), 1); - 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(); + if (result_ht->nNumUsed + zend_hash_num_elements(ht) > 0) { + zend_hash_extend(result_ht, result_ht->nNumUsed + zend_hash_num_elements(ht), 1); + 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(); + } } else { zend_string *key; diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h index 9daa00d97c418..96e66bcdc577e 100644 --- a/Zend/zend_vm_execute.h +++ b/Zend/zend_vm_execute.h @@ -2657,17 +2657,19 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_ADD_ARRAY_UNPACK_SPEC_HANDLER( zval *val; if (HT_IS_PACKED(ht) && (zend_hash_num_elements(result_ht) == 0 || HT_IS_PACKED(result_ht))) { - zend_hash_extend(result_ht, result_ht->nNumUsed + zend_hash_num_elements(ht), 1); - 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(); + if (result_ht->nNumUsed + zend_hash_num_elements(ht) > 0) { + zend_hash_extend(result_ht, result_ht->nNumUsed + zend_hash_num_elements(ht), 1); + 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(); + } } else { zend_string *key; From 54b0929de37f897f149152edd272919f13523703 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Wed, 30 Jul 2025 22:46:14 +0200 Subject: [PATCH 2/2] [ci skip] comment --- Zend/zend_vm_def.h | 3 +++ Zend/zend_vm_execute.h | 3 +++ 2 files changed, 6 insertions(+) diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h index a23860e16b86b..f1d4f7448ce1e 100644 --- a/Zend/zend_vm_def.h +++ b/Zend/zend_vm_def.h @@ -6162,6 +6162,9 @@ ZEND_VM_C_LABEL(add_unpack_again): zval *val; if (HT_IS_PACKED(ht) && (zend_hash_num_elements(result_ht) == 0 || HT_IS_PACKED(result_ht))) { + /* zend_hash_extend() skips initialization when the number of elements is 0, + * but the code below expects that result_ht is initialized as packed. + * We can just skip the work in that case. */ if (result_ht->nNumUsed + zend_hash_num_elements(ht) > 0) { zend_hash_extend(result_ht, result_ht->nNumUsed + zend_hash_num_elements(ht), 1); ZEND_HASH_FILL_PACKED(result_ht) { diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h index 96e66bcdc577e..5f7f4997ce8d7 100644 --- a/Zend/zend_vm_execute.h +++ b/Zend/zend_vm_execute.h @@ -2657,6 +2657,9 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_ADD_ARRAY_UNPACK_SPEC_HANDLER( zval *val; if (HT_IS_PACKED(ht) && (zend_hash_num_elements(result_ht) == 0 || HT_IS_PACKED(result_ht))) { + /* zend_hash_extend() skips initialization when the number of elements is 0, + * but the code below expects that result_ht is initialized as packed. + * We can just skip the work in that case. */ if (result_ht->nNumUsed + zend_hash_num_elements(ht) > 0) { zend_hash_extend(result_ht, result_ht->nNumUsed + zend_hash_num_elements(ht), 1); ZEND_HASH_FILL_PACKED(result_ht) {