Skip to content

Commit 5bd5f35

Browse files
committed
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. Closes GH-19318.
1 parent bc4b6ce commit 5bd5f35

File tree

4 files changed

+45
-22
lines changed

4 files changed

+45
-22
lines changed

NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ PHP NEWS
1111
binary const expr). (ilutov)
1212
. Fixed bug GH-19305 (Operands may be being released during comparison).
1313
(Arnaud)
14+
. Fixed bug GH-19303 (Unpacking empty packed array into uninitialized array
15+
causes assertion failure). (nielsdos)
1416

1517
- FTP:
1618
. Fix theoretical issues with hrtime() not being available. (nielsdos)

Zend/tests/array_unpack/gh19303.phpt

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
--TEST--
2+
GH-19303 (Unpacking empty packed array into uninitialized array causes assertion failure)
3+
--FILE--
4+
<?php
5+
$a = [0];
6+
unset($a[0]);
7+
var_dump([...$a]);
8+
?>
9+
--EXPECT--
10+
array(0) {
11+
}

Zend/zend_vm_def.h

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6162,17 +6162,22 @@ ZEND_VM_C_LABEL(add_unpack_again):
61626162
zval *val;
61636163

61646164
if (HT_IS_PACKED(ht) && (zend_hash_num_elements(result_ht) == 0 || HT_IS_PACKED(result_ht))) {
6165-
zend_hash_extend(result_ht, result_ht->nNumUsed + zend_hash_num_elements(ht), 1);
6166-
ZEND_HASH_FILL_PACKED(result_ht) {
6167-
ZEND_HASH_PACKED_FOREACH_VAL(ht, val) {
6168-
if (UNEXPECTED(Z_ISREF_P(val)) &&
6169-
UNEXPECTED(Z_REFCOUNT_P(val) == 1)) {
6170-
val = Z_REFVAL_P(val);
6171-
}
6172-
Z_TRY_ADDREF_P(val);
6173-
ZEND_HASH_FILL_ADD(val);
6174-
} ZEND_HASH_FOREACH_END();
6175-
} ZEND_HASH_FILL_END();
6165+
/* zend_hash_extend() skips initialization when the number of elements is 0,
6166+
* but the code below expects that result_ht is initialized as packed.
6167+
* We can just skip the work in that case. */
6168+
if (result_ht->nNumUsed + zend_hash_num_elements(ht) > 0) {
6169+
zend_hash_extend(result_ht, result_ht->nNumUsed + zend_hash_num_elements(ht), 1);
6170+
ZEND_HASH_FILL_PACKED(result_ht) {
6171+
ZEND_HASH_PACKED_FOREACH_VAL(ht, val) {
6172+
if (UNEXPECTED(Z_ISREF_P(val)) &&
6173+
UNEXPECTED(Z_REFCOUNT_P(val) == 1)) {
6174+
val = Z_REFVAL_P(val);
6175+
}
6176+
Z_TRY_ADDREF_P(val);
6177+
ZEND_HASH_FILL_ADD(val);
6178+
} ZEND_HASH_FOREACH_END();
6179+
} ZEND_HASH_FILL_END();
6180+
}
61766181
} else {
61776182
zend_string *key;
61786183

Zend/zend_vm_execute.h

Lines changed: 16 additions & 11 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)