Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Segfault with array_multisort + array_shift (packed arrays?) #9244

Closed
chschneider opened this issue Aug 3, 2022 · 0 comments
Closed

Segfault with array_multisort + array_shift (packed arrays?) #9244

chschneider opened this issue Aug 3, 2022 · 0 comments

Comments

@chschneider
Copy link

Description

The following code:

<?php
$items = ['foo' => 1, 'bar' => 2];
$order = [4, 3];
array_multisort($order, $items);
var_dump(array_shift($items));

Resulted in this output:

Segmentation fault (core dumped)

But I expected this output instead:

int(2)

After bisecting the relevant commit seems to be

commit 90b7bde61507cee1c6b37f153909d72f5b203b8c
Author: Dmitry Stogov <dmitry@zend.com>
Date:   Wed Nov 3 15:18:26 2021 +0300

    Use more compact representation for packed arrays.
    
    - for packed arrays we store just an array of zvals without keys.
    - the elements of packed array are accessible throuf as ht->arPacked[i]
      instead of ht->arData[i]
    - in addition to general ZEND_HASH_FOREACH_* macros, we introduced similar
      familied for packed (ZEND_HASH_PACKED_FORECH_*) and real hashes
      (ZEND_HASH_MAP_FOREACH_*)
    - introduced an additional family of macros to access elements of array
      (packed or real hashes) ZEND_ARRAY_ELEMET_SIZE, ZEND_ARRAY_ELEMET_EX,
      ZEND_ARRAY_ELEMET, ZEND_ARRAY_NEXT_ELEMENT, ZEND_ARRAY_PREV_ELEMENT
    - zend_hash_minmax() prototype was changed to compare only values
    
    Because of smaller data set, this patch may show performance improvement
    on some apps and benchmarks that use packed arrays. (~1% on PHP-Parser)
    
    TODO:
        - sapi/phpdbg needs special support for packed arrays (WATCH_ON_BUCKET).
        - zend_hash_sort_ex() may require converting packed arrays to hash. 

This commit is too complex for me to come up with a fix. But the last TODO mentioned in the commit message could be an indication of what is going wrong.

PHP Version

PHP 8.2.0-dev

Operating System

openSUSE Leap 15.3

cmb69 added a commit to cmb69/php-src that referenced this issue Aug 4, 2022
After restructuring non-packed arrays, we either need to pack them if
possible, or to rehash them.
@cmb69 cmb69 linked a pull request Aug 4, 2022 that will close this issue
cmb69 added a commit to cmb69/php-src that referenced this issue Aug 4, 2022
After restructuring non-packed arrays, we either need to pack them if
possible, or to rehash them.
@cmb69 cmb69 closed this as completed in ad04345 Aug 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants