php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #77533 Found crash when appending element to pass-by-ref array with []
Submitted: 2019-01-28 02:43 UTC Modified: 2019-01-29 15:53 UTC
From: timandes@php.net Assigned: timandes (profile)
Status: Not a bug Package: Reproducible crash
PHP Version: 7.3.1 OS: CentOS Linux release 7.6.1810 (C
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: timandes@php.net
New email:
PHP Version: OS:

 

 [2019-01-28 02:43 UTC] timandes@php.net
Description:
------------
Last week, I found a CI failure[1] of pecl/zookeeper. I thought it would be compatibility problem because I turned on PHP-7.3 in .travis.yml.

When I looked into that issue, I found it's weird because the crash point was in the PHP src, and it only crashed when I passed an empty array to the function provided pass-by-ref var.

So I wrote a small ext to reproduce that:

```c
PHP_FUNCTION(caseone_test1)
{
        zval *param;

        if (zend_parse_parameters(ZEND_NUM_ARGS(), "z", &param) == FAILURE) {
                return;
        }

        if (param) {
                ZVAL_DEREF(param);
        }

        if (Z_TYPE_P(param) != IS_ARRAY) {
                array_init(param);
        }

        add_assoc_long_ex(param, ZEND_STRL("foo"), 0);
}
```

```php
$param = [];
caseone_test1($param);
```

```gdb
#0  0x000056193020c64e in zend_hash_real_init_mixed_ex (ht=ht@entry=0x5619305fe220 <zend_empty_array>) at /usr/src/debug/php-7.3.1/Zend/zend_hash.c:131
#1  zend_hash_real_init_mixed (ht=ht@entry=0x5619305fe220 <zend_empty_array>) at /usr/src/debug/php-7.3.1/Zend/zend_hash.c:260
#2  0x000056193020db88 in _zend_hash_str_add_or_update_i (flag=1, pData=0x7fffbb8834d0, h=9223372037048267657, len=3, str=0x7ff29e4b7d93 "foo", ht=0x5619305fe220 <zend_empty_array>)
    at /usr/src/debug/php-7.3.1/Zend/zend_hash.c:746
#3  zend_hash_str_update (ht=ht@entry=0x5619305fe220 <zend_empty_array>, str=str@entry=0x7ff29e4b7d93 "foo", len=len@entry=3, pData=pData@entry=0x7fffbb8834d0)
    at /usr/src/debug/php-7.3.1/Zend/zend_hash.c:854
#4  0x00005619302021a8 in zend_symtable_str_update (pData=0x7fffbb8834d0, len=3, str=0x7ff29e4b7d93 "foo", ht=0x5619305fe220 <zend_empty_array>)
    at /usr/src/debug/php-7.3.1/Zend/zend_hash.h:498
#5  add_assoc_long_ex (arg=<optimized out>, key=key@entry=0x7ff29e4b7d93 "foo", key_len=key_len@entry=3, n=n@entry=0) at /usr/src/debug/php-7.3.1/Zend/zend_API.c:1359
#6  0x00007ff29e4b7c7c in zif_caseone_test1 (execute_data=<optimized out>, return_value=<optimized out>) at /root/php-7.3.1/ext/caseone/caseone.c:29
#7  0x000056193028b2f9 in ZEND_DO_ICALL_SPEC_RETVAL_UNUSED_HANDLER () at /usr/src/debug/php-7.3.1/Zend/zend_vm_execute.h:645
#8  execute_ex (ex=0x5619305fe220 <zend_empty_array>) at /usr/src/debug/php-7.3.1/Zend/zend_vm_execute.h:55414
#9  0x000056193028f3a3 in zend_execute (op_array=op_array@entry=0x7ff2a267d2a0, return_value=0x0, return_value@entry=0x7ff2a267d3e0)
    at /usr/src/debug/php-7.3.1/Zend/zend_vm_execute.h:60834
#10 0x0000561930200622 in zend_execute_scripts (type=type@entry=8, retval=0x7ff2a267d3e0, retval@entry=0x0, file_count=-1570648016, file_count@entry=3)
    at /usr/src/debug/php-7.3.1/Zend/zend.c:1568
#11 0x00005619301a05d0 in php_execute_script (primary_file=primary_file@entry=0x7fffbb885b80) at /usr/src/debug/php-7.3.1/main/main.c:2630
#12 0x0000561930291892 in do_cli (argc=3, argv=0x561930f3f960) at /usr/src/debug/php-7.3.1/sapi/cli/php_cli.c:997
#13 0x000056193000121b in main (argc=3, argv=0x561930f3f960) at /usr/src/debug/php-7.3.1/sapi/cli/php_cli.c:1389
```

When I changed the former PHP script to :
```php
$param = '';
caseone_test1($param);
```
or
```php
$param = [0];
caseone_test1($param);
```
, they worked fine.

This issue can only be reproduced under Linux.
I'm not sure it's a bug or not, so I turned to you guys for help.

System info:
```shell
uname -a
Linux 93a912e38b29 4.9.125-linuxkit #1 SMP Fri Sep 7 08:20:28 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
```
```shell
cat /etc/redhat-release
CentOS Linux release 7.6.1810 (Core)
```


[1] https://api.travis-ci.org/v3/job/484167428/log.txt


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2019-01-28 08:14 UTC] nikic@php.net
-Status: Open +Status: Not a bug
 [2019-01-28 08:14 UTC] nikic@php.net
Should be:

    if (Z_TYPE_P(param) != IS_ARRAY) {
        zval_ptr_dtor(param);
        array_init(param);
    } else {
        SEPARATE_ARRAY(param);
    }

You are trying to modify a potentially shared array without separating it first. In this case it is the empty array, which lives in read-only memory.
 [2019-01-28 08:22 UTC] timandes@php.net
Thank you for your help.

I have another question to ask:
How can I know when to separate an array?
 [2019-01-28 08:36 UTC] nikic@php.net
You should always separate the array before modifying it, unless you know that you are the sole owner of the array (for example, because you only created it just now, already separated it, etc.)

I would recommend compiling PHP with --enable-debug when developing extensions, as this will (usually) trigger assertion failures if you forget to separate something.
 [2019-01-29 02:29 UTC] timandes@php.net
I got this message when debug mode enabled:

```
php: /root/php-7.3.1/Zend/zend_hash.c:743: _zend_hash_str_add_or_update_i: Assertion `(zend_gc_refcount(&(ht)->gc) == 1) || ((ht)->u.flags & (1<<6))' failed.
Aborted
```

Thank you very much.
 [2019-01-29 02:32 UTC] timandes@php.net
-Status: Not a bug +Status: Closed -Assigned To: +Assigned To: timandes
 [2019-01-29 15:53 UTC] cmb@php.net
-Status: Closed +Status: Not a bug
 
PHP Copyright © 2001-2021 The PHP Group
All rights reserved.
Last updated: Sat Apr 10 12:01:23 2021 UTC