Nothing Special   »   [go: up one dir, main page]
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.

Sign up for GitHub

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

Jump to bottom

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
Labels
Bug Extension: standard Status: Verified

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
@cmb69
Fix phpGH-9244: Segfault with array_multisort + array_shift
43d9f71 
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
Fix GH-9244: Segfault with array_multisort + array_shift #9247
Closed
cmb69 added a commit to cmb69/php-src that referenced this issue Aug 4, 2022
@cmb69
Fix phpGH-9244: Segfault with array_multisort + array_shift
d37c328 
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
Assignees
No one assigned
Labels
Bug Extension: standard Status: Verified
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix GH-9244: Segfault with array_multisort + array_shift cmb69/php-src
2 participants
@chschneider @cmb69