Fix GH-19303: Unpacking empty packed array into uninitialized array causes assertion failure #19318
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…y 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.
arnaud-lb
approved these changes
Jul 30, 2025
| ZEND_HASH_FILL_ADD(val); | ||
| } ZEND_HASH_FOREACH_END(); | ||
| } ZEND_HASH_FILL_END(); | ||
| if (result_ht->nNumUsed + zend_hash_num_elements(ht) > 0) { |
There was a problem hiding this comment.
This might be worth a comment because it's not obvious why it is needed (without the PR/commit description).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.