Skip to content

segfault in Zend/tests/assign_coalesce_001.phpt #13323

Open

Description

@divinity76

Description

Doing the following:

hans@DESKTOP-EE15SLU:~/projects/php-src$ git log -1
commit da6a4e799aafdf33c8701fcdd632d646f61e612e (HEAD -> master, origin/master, origin/HEAD)
Merge: 1cc0a16752 ae44ab47a7
Author: Jakub Zelenka <bukka@php.net>
Date:   Sun Feb 4 12:01:09 2024 +0000

    Merge branch 'PHP-8.3'
hans@DESKTOP-EE15SLU:~/projects/php-src$ git status
On branch master
Your branch is up to date with 'origin/master'.

nothing to commit, working tree clean
hans@DESKTOP-EE15SLU:~/projects/php-src$ ./buildconf >/dev/null
hans@DESKTOP-EE15SLU:~/projects/php-src$ ./configure --disable-all --disable-cgi --enable-cli >/dev/null
hans@DESKTOP-EE15SLU:~/projects/php-src$ make clean >/dev/null;
hans@DESKTOP-EE15SLU:~/projects/php-src$ make -j$(nproc) >/dev/null
/home/hans/projects/php-src/ext/pcre/pcre2lib/pcre2_compile.c: In function ‘compile_regex’:
/home/hans/projects/php-src/ext/pcre/pcre2lib/pcre2_compile.c:8169:17: warning: storing the address of local variable ‘capitem’ in ‘*cb.open_caps’ [-Wdangling-pointer=]
 8169 |   cb->open_caps = &capitem;
      |   ~~~~~~~~~~~~~~^~~~~~~~~~
/home/hans/projects/php-src/ext/pcre/pcre2lib/pcre2_compile.c:8106:14: note: ‘capitem’ declared here
 8106 | open_capitem capitem;
      |              ^~~~~~~
/home/hans/projects/php-src/ext/pcre/pcre2lib/pcre2_compile.c:8100:39: note: ‘cb’ declared here
 8100 |   branch_chain *bcptr, compile_block *cb, PCRE2_SIZE *lengthptr)
In file included from /home/hans/projects/php-src/Zend/zend.h:32,
                 from /home/hans/projects/php-src/Zend/zend_API.c:22:
In function ‘zend_check_magic_method_implementation’,
    inlined from ‘zend_register_functions’ at /home/hans/projects/php-src/Zend/zend_API.c:3035:4:
/home/hans/projects/php-src/Zend/zend_API.c:2636:34: warning: array subscript ‘zend_function {aka const union _zend_function}[0]’ is partly outside array bounds of ‘unsigned char[136]’ [-Warray-bounds=]
 2636 |         if (ZSTR_VAL(fptr->common.function_name)[0] != '_'
/home/hans/projects/php-src/Zend/zend_string.h:68:26: note: in definition of macro ‘ZSTR_VAL’
   68 | #define ZSTR_VAL(zstr)  (zstr)->val
      |                          ^~~~
/home/hans/projects/php-src/Zend/zend_API.c: In function ‘zend_register_functions’:
/home/hans/projects/php-src/Zend/zend_API.c:2926:32: note: object of size 136 allocated by ‘malloc’
 2926 |                 reg_function = malloc(sizeof(zend_internal_function));
      |                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
hans@DESKTOP-EE15SLU:~/projects/php-src$ ./sapi/cli/php ./run-tests.php Zend/tests/assign_coalesce_001.phpt --show-diff

resulted in:

=====================================================================
PHP         : /home/hans/projects/php-src/sapi/cli/php
PHP_SAPI    : cli
PHP_VERSION : 8.4.0-dev
ZEND_VERSION: 4.4.0-dev
PHP_OS      : Linux - Linux DESKTOP-EE15SLU 5.15.133.1-microsoft-standard-WSL2 #1 SMP Thu Oct 5 21:02:42 UTC 2023 x86_64
INI actual  : /home/hans/projects/php-src
More .INIs  :
---------------------------------------------------------------------
PHP         : /home/hans/projects/php-src/sapi/phpdbg/phpdbg
PHP_SAPI    : phpdbg
PHP_VERSION : 8.4.0-dev
ZEND_VERSION: 4.4.0-dev
PHP_OS      : Linux - Linux DESKTOP-EE15SLU 5.15.133.1-microsoft-standard-WSL2 #1 SMP Thu Oct 5 21:02:42 UTC 2023 x86_64
INI actual  : /home/hans/projects/php-src
More .INIs  :
---------------------------------------------------------------------
CWD         : /home/hans/projects/php-src
Extra dirs  :
VALGRIND    : Not used
=====================================================================
Running selected tests.
TEST 1/1 [Zend/tests/assign_coalesce_001.phpt]
========DIFF========
--
     }

     Static props:
042- int(123)
043- string(3) "foo"
044- id(foo)
045- id(foo)
046- id(bar)
047- int(123)
048- string(3) "foo"
042+ Segmentation fault
043+
044+ Termsig=11
========DONE========
FAIL Coalesce assign (??=): Basic behavior [Zend/tests/assign_coalesce_001.phpt]
=====================================================================
Number of tests :     1                 1
Tests skipped   :     0 (  0.0%) --------
Tests warned    :     0 (  0.0%) (  0.0%)
Tests failed    :     1 (100.0%) (100.0%)
Tests passed    :     0 (  0.0%) (  0.0%)
---------------------------------------------------------------------
Time taken      : 0.003 seconds
=====================================================================

=====================================================================
FAILED TEST SUMMARY
---------------------------------------------------------------------
Coalesce assign (??=): Basic behavior [Zend/tests/assign_coalesce_001.phpt]
=====================================================================

but I expected the test to pass.

PHP Version

8.4.0-dev commit da6a4e7

Operating System

WSL2 Ubuntu 24.04-dev

Activity

  1. iluuu1994 commented on Feb 4, 2024

    @iluuu1994
    Member

    Can you try a debug build and the address/undefined sanitizers with USE_ZEND_ALLOC=0? Is the GCC version of 24.04-dev stable?

  2. devnexen commented on Feb 4, 2024

    @devnexen
    Member
    Static props:
    AddressSanitizer:DEADLYSIGNAL
    =================================================================
    ==206313==ERROR: AddressSanitizer: SEGV on unknown address 0x001d7fff80c3 (pc 0x55d437cbda4a bp 0x7ffe4bfe0fc0 sp 0x7ffe4bfe0fa0 T0)
    ==206313==The signal is caused by a READ memory access.
        #0 0x55d437cbda4a in zend_string_hash_val /home/dcarlier/Contribs/php-src/Zend/zend_string.h:141
        #1 0x55d437cbda4a in zend_hash_find /home/dcarlier/Contribs/php-src/Zend/zend_hash.c:2666
        #2 0x55d437ecf1c9 in zend_hash_find_ptr /home/dcarlier/Contribs/php-src/Zend/zend_hash.h:869
        #3 0x55d437ecf1c9 in zend_std_get_static_property_with_info /home/dcarlier/Contribs/php-src/Zend/zend_object_handlers.c:1583
        #4 0x55d437d09f53 in zend_fetch_static_property_address_ex /home/dcarlier/Contribs/php-src/Zend/zend_execute.c:3399
        #5 0x55d437e0f754 in zend_fetch_static_property_address /home/dcarlier/Contribs/php-src/Zend/zend_execute.c:3452
        #6 0x55d437e0f754 in zend_fetch_static_prop_helper_SPEC /home/dcarlier/Contribs/php-src/Zend/zend_vm_execute.h:861
        #7 0x55d437e2d50a in ZEND_FETCH_STATIC_PROP_IS_SPEC_HANDLER /home/dcarlier/Contribs/php-src/Zend/zend_vm_execute.h:910
        #8 0x55d437e2d50a in execute_ex /home/dcarlier/Contribs/php-src/Zend/zend_vm_execute.h:57065
        #9 0x55d437e608d5 in zend_execute /home/dcarlier/Contribs/php-src/Zend/zend_vm_execute.h:62621
        #10 0x55d437c70067 in zend_execute_scripts /home/dcarlier/Contribs/php-src/Zend/zend.c:1885
        #11 0x55d437b35cb6 in php_execute_script /home/dcarlier/Contribs/php-src/main/main.c:2504
        #12 0x55d437fdfc31 in do_cli /home/dcarlier/Contribs/php-src/sapi/cli/php_cli.c:966
        #13 0x55d437698540 in main /home/dcarlier/Contribs/php-src/sapi/cli/php_cli.c:1340
        #14 0x7f50f742814f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
        #15 0x7f50f7428208 in __libc_start_main_impl ../csu/libc-start.c:360
    
  3. divinity76 commented on Feb 4, 2024

    @divinity76
    ContributorAuthor

    @iluuu1994 annoyingly, no: the segfault does not happen in debug builds, the test pass with ./configure --enable-debug
    luckily @devnexen is able to reproduce with address sanitizer (valgrind?)

    Is the GCC version of 24.04-dev stable?

    should be, based on gcc release version 13.2.0 with some ubuntu patches

  4. devnexen commented on Feb 4, 2024

    @devnexen
    Member

    with a pure debug build, the segfault does not occur (zeroed field ?) however with asan you get the report.

  5. iluuu1994 commented on Feb 4, 2024

    @iluuu1994
    Member

    Thank you. I'm using version 13.2.1 20231205 (Red Hat 13.2.1-6) (GCC). I suppose it's possible this is a miscompilation on the Ubuntu version, or something that was fixed in version 13.2.1.

    @devnexen You might also try memory sanitizer, if you suspect this could be due to an uninitialized memory read.

  6. devnexen commented on Feb 4, 2024

    @devnexen
    Member

    might be not it, works fine with msan.

  7. divinity76 commented on Feb 4, 2024

    @divinity76
    ContributorAuthor

    doing run-tests.php with -m --show-mem with a release-build gave me a slightly different output from @devnexen

    $ ./sapi/cli/php ./run-tests.php Zend/tests/assign_coalesce_001.phpt --show-mem -m
    
    =====================================================================
    PHP         : /home/hans/projects/php-src/sapi/cli/php
    PHP_SAPI    : cli
    PHP_VERSION : 8.4.0-dev
    ZEND_VERSION: 4.4.0-dev
    PHP_OS      : Linux - Linux DESKTOP-EE15SLU 5.15.133.1-microsoft-standard-WSL2 #1 SMP Thu Oct 5 21:02:42 UTC 2023 x86_64INI actual  : /home/hans/projects/php-src
    More .INIs  :
    ---------------------------------------------------------------------
    PHP         : /home/hans/projects/php-src/sapi/phpdbg/phpdbg
    PHP_SAPI    : phpdbg
    PHP_VERSION : 8.4.0-dev
    ZEND_VERSION: 4.4.0-dev
    PHP_OS      : Linux - Linux DESKTOP-EE15SLU 5.15.133.1-microsoft-standard-WSL2 #1 SMP Thu Oct 5 21:02:42 UTC 2023 x86_64INI actual  : /home/hans/projects/php-src
    More .INIs  :
    ---------------------------------------------------------------------
    CWD         : /home/hans/projects/php-src
    Extra dirs  :
    VALGRIND    : valgrind-3.22.0 (memcheck)
    =====================================================================
    Running selected tests.
    TEST 1/1 [Zend/tests/assign_coalesce_001.phpt]
    ========MEM========
    ==1046412== Invalid read of size 8
    ==1046412==    at 0x56006B: zend_string_hash_val (zend_string.h:141)
    ==1046412==    by 0x56006B: zend_hash_find (zend_hash.c:2666)
    ==1046412==    by 0x5DFE50: zend_hash_find_ptr (zend_hash.h:869)
    ==1046412==    by 0x5DFE50: zend_std_get_static_property_with_info (zend_object_handlers.c:1583)
    ==1046412==    by 0x572523: zend_fetch_static_property_address_ex (zend_execute.c:3399)
    ==1046412==    by 0x5AC945: zend_fetch_static_property_address (zend_execute.c:3452)
    ==1046412==    by 0x5AC945: zend_fetch_static_prop_helper_SPEC (zend_vm_execute.h:861)
    ==1046412==    by 0x5B3510: ZEND_FETCH_STATIC_PROP_IS_SPEC_HANDLER (zend_vm_execute.h:910)
    ==1046412==    by 0x5B3510: execute_ex (zend_vm_execute.h:57065)
    ==1046412==    by 0x5BD2BF: zend_execute (zend_vm_execute.h:62621)
    ==1046412==    by 0x54A771: zend_execute_scripts (zend.c:1885)
    ==1046412==    by 0x4DDC11: php_execute_script (main.c:2504)
    ==1046412==    by 0x6343BB: do_cli (php_cli.c:966)
    ==1046412==    by 0x33FC93: main (php_cli.c:1340)
    ==1046412==  Address 0xe800000618 is not stack'd, malloc'd or (recently) free'd
    ==1046412==
    ==1046412==
    ==1046412== Process terminating with default action of signal 11 (SIGSEGV)
    ==1046412==  Access not within mapped region at address 0xE800000618
    ==1046412==    at 0x56006B: zend_string_hash_val (zend_string.h:141)
    ==1046412==    by 0x56006B: zend_hash_find (zend_hash.c:2666)
    ==1046412==    by 0x5DFE50: zend_hash_find_ptr (zend_hash.h:869)
    ==1046412==    by 0x5DFE50: zend_std_get_static_property_with_info (zend_object_handlers.c:1583)
    ==1046412==    by 0x572523: zend_fetch_static_property_address_ex (zend_execute.c:3399)
    ==1046412==    by 0x5AC945: zend_fetch_static_property_address (zend_execute.c:3452)
    ==1046412==    by 0x5AC945: zend_fetch_static_prop_helper_SPEC (zend_vm_execute.h:861)
    ==1046412==    by 0x5B3510: ZEND_FETCH_STATIC_PROP_IS_SPEC_HANDLER (zend_vm_execute.h:910)
    ==1046412==    by 0x5B3510: execute_ex (zend_vm_execute.h:57065)
    ==1046412==    by 0x5BD2BF: zend_execute (zend_vm_execute.h:62621)
    ==1046412==    by 0x54A771: zend_execute_scripts (zend.c:1885)
    ==1046412==    by 0x4DDC11: php_execute_script (main.c:2504)
    ==1046412==    by 0x6343BB: do_cli (php_cli.c:966)
    ==1046412==    by 0x33FC93: main (php_cli.c:1340)
    ==1046412==  If you believe this happened as a result of a stack
    ==1046412==  overflow in your program's main thread (unlikely but
    ==1046412==  possible), you can try to increase the size of the
    ==1046412==  main thread stack using the --main-stacksize= flag.
    ==1046412==  The main thread stack size used in this run was 8388608.
    ========DONE========
    LEAK&FAIL Coalesce assign (??=): Basic behavior [Zend/tests/assign_coalesce_001.phpt]
    =====================================================================
    Number of tests :     1                 1
    Tests skipped   :     0 (  0.0%) --------
    Tests warned    :     0 (  0.0%) (  0.0%)
    Tests failed    :     0 (  0.0%) (  0.0%)
    Tests leaked    :     1 (100.0%) (100.0%)
    Tests passed    :     0 (  0.0%) (  0.0%)
    ---------------------------------------------------------------------
    Time taken      : 0.856 seconds
    =====================================================================
    
    =====================================================================
    FAILED TEST SUMMARY
    ---------------------------------------------------------------------
    Coalesce assign (??=): Basic behavior [Zend/tests/assign_coalesce_001.phpt]
    =====================================================================
    
    =====================================================================
    LEAKED TEST SUMMARY
    ---------------------------------------------------------------------
    Coalesce assign (??=): Basic behavior [Zend/tests/assign_coalesce_001.phpt]
    =====================================================================
  8. divinity76 commented on Feb 4, 2024

    @divinity76
    ContributorAuthor

    seems when the issue does happen, Z_STR_P returns a bogus pointer in

    name = Z_STR_P(RT_CONSTANT(opline, opline->op1));

    and opline->op1.var and opline->op1.num are both 80, whatever that means - but in debug mode, where it doesn't happen, opline->op1.var and opline->op1.num are both 2544

  9. divinity76 commented on Feb 4, 2024

    @divinity76
    ContributorAuthor

    @iluuu1994 it's all your fault! semi-kidding, seems the guilty commit is bf4ec8b, doing

    git revert bf4ec8bd9d9d30713a9d3294b946084a6e9e088a
    

    fixes it?
    @devnexen can you confirm?

  10. iluuu1994 commented on Feb 4, 2024

    @iluuu1994
    Member

    @divinity76 Thanks for the analysis. It makes sense for this to fix the release build if any of the assumptions are wrong. It seems odd for this to fix debug builds, given that they are usually compiled to asserts, unless ZEND_ASSUME is used explicitly. Can you try converting the ZEND_ASSUMEs in zend_hash.h to ZEND_ASSERTs and see if any of them fail? You'll need a debug build for this.

  11. 4 remaining items

  12. iluuu1994 commented on Feb 4, 2024

    @iluuu1994
    Member

    @divinity76 David mentioned that the issue is reproducible with address sanitizer in debug builds, which is why I suggested converting the remaining assumes to asserts. I was hoping this would reveal some invalid assertion. Can you try that, with address sanitizer?

    Alternatively I can try it in a docker container tomorrow.

    yeah seems likely, also seems gcc 13.2.1 isn't even released yet, it's a dev-version of gcc

    Hmm, not sure why Fedora ships with 13.2.1 then.

  13. divinity76 commented on Feb 4, 2024

    @divinity76
    ContributorAuthor

    Can you try that, with address sanitizer?

    No, i don't know how to use it, and don't want to study it today. Hope someone else can do it 👍

    Hmm, not sure why Fedora ships with 13.2.1 then.

    I guess a combo of Fedora being bleeding-edge and multiple Fedora developers also being gcc developers

  14. iluuu1994 commented on Feb 4, 2024

    @iluuu1994
    Member

    No, i don't know how to use it, and don't want to study it today. Hope someone else can do it 👍

    Fair enough. I'll try it tomorrow.

  15. divinity76 commented on Feb 6, 2024

    @divinity76
    ContributorAuthor

    @iluuu1994

    Alternatively I can try it in a docker container tomorrow.

    fwiw if testing it through a Docker container is troublesome, the instructions for building gcc13.2.0 from source is roughly (or with a bit of luck, exactly)

    mkdir /tmp/gcc;
    cd /tmp/gcc;
    wget 'https://gcc.gnu.org/pub/gcc/releases/gcc-13.2.0/gcc-13.2.0.tar.xz' -O- | tar xfvJ -;
    cd gcc-13.2.0;
    ./contrib/download_prerequisites;
    ./configure --disable-multilib;
    make -j$(nproc);
    mkdir installdir;
    make install DESTDIR=/tmp/gcc/gcc-13.2.0/installdir;
    export PATH=/tmp/gcc/gcc-13.2.0/installdir/usr/local/bin:$PATH;
    $ type gcc
    gcc is /tmp/gcc/gcc-13.2.0/installdir/usr/local/bin/gcc
    $ gcc -v
    Using built-in specs.
    COLLECT_GCC=gcc
    COLLECT_LTO_WRAPPER=/tmp/gcc/gcc-13.2.0/installdir/usr/local/bin/../libexec/gcc/x86_64-pc-linux-gnu/13.2.0/lto-wrapper
    Target: x86_64-pc-linux-gnu
    Configured with: ./configure --disable-multilib
    Thread model: posix
    Supported LTO compression algorithms: zlib
    gcc version 13.2.0 (GCC)
    
  16. iluuu1994 commented on Feb 6, 2024

    @iluuu1994
    Member

    @divinity76 It's not too bad, I just haven't gotten around to it yet. However, building GCC locally may be easier. Thank you!

  17. iluuu1994 commented on Feb 7, 2024

    @iluuu1994
    Member

    It turns out I can reproduce this on 13.2.1 after all. I'll revert the commit tomorrow, and see if I can identify the assumption that's responsible, and create a reproducer to submit upstream.

  18. divinity76 commented on Feb 7, 2024

    @divinity76
    ContributorAuthor

    @iluuu1994 GLHF, I suspect it won't be easy 🥲

  19. divinity76 commented on Feb 8, 2024

    @divinity76
    ContributorAuthor

    I suggest we revert bf4ec8b from master for the duration of the investigation, and perhaps re-introduce it when it's ready. having master segfault on run-tests is a burden

    (for example, I thought it was something I did when I got segfaults testing an unrelated pull request #13194 )

  20. self-assigned this
    on Feb 8, 2024
  21. iluuu1994 commented on Feb 8, 2024

    @iluuu1994
    Member

    Reverted here: 2f89438

  22. divinity76 commented on Feb 9, 2024

    @divinity76
    ContributorAuthor

    @iluuu1994 just curious, any luck tracking down the exact assume responsible? How about minimal reproducible sample?

  23. iluuu1994 commented on Feb 9, 2024

    @iluuu1994
    Member

    Did not get to it yet.

  24. kamil-tekiela commented on Nov 8, 2026

    @kamil-tekiela
    Member

    It looks to me like it was a GCC bug. It seems fixed in 13.5.0 and higher. This is the line that seems to trigger it:

    ZEND_ASSERT(op1_type != IS_CONST || CACHED_PTR(cache_slot) == NULL);
    
  25. iluuu1994 commented on Nov 8, 2026

    @iluuu1994
    Member

    @kamil-tekiela Feel free to propose this again for master. Would also be interested if binary size differs in a meaningful way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions