Skip to content

Segfault on PHP 8.3 #11406

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

Closed
nicolas-grekas opened this issue Jun 8, 2023 · 4 comments
Closed

Segfault on PHP 8.3 #11406

nicolas-grekas opened this issue Jun 8, 2023 · 4 comments

Comments

@nicolas-grekas
Copy link
Contributor

Description

Sorry I don't have a better reproducer right now.

git clone https://github.com/symfony/symfony/
cd symfony
composer install
./phpunit src/Symfony/Bundle/SecurityBundle

Never completes, eg this job:

...............................................................  63 / 314 ( 20%)
............................................................... 126 / 314 ( 40%)
............................................................... 189 / 314 ( 60%)
.........................................

KO src/Symfony/Bundle/SecurityBundle

PHP Version

PHP 8.3

Operating System

No response

@ramsey
Copy link
Member

ramsey commented Jun 9, 2023

Can you isolate the test where this is occurring?

@nielsdos
Copy link
Member

nielsdos commented Jun 9, 2023

Test that fails is EventAliasTest: ./phpunit src/Symfony/Bundle/SecurityBundle/Tests/Functional/EventAliasTest.php --testdox
Fails at the first $dispatcher->dispatch call.
ASAN report:

==330732==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000010 (pc 0x558a00b040f5 bp 0x7ffcadb71c20 sp 0x7ffcadb71c10 T0)
==330732==The signal is caused by a READ memory access.
==330732==Hint: address points to the zero page.
    #0 0x558a00b040f5 in zend_check_arg_send_type /home/niels/php-src/Zend/zend_compile.h:1061
    #1 0x558a00b37ce8 in ZEND_SEND_UNPACK_SPEC_HANDLER /home/niels/php-src/Zend/zend_vm_execute.h:2285
    #2 0x558a00c88280 in execute_ex /home/niels/php-src/Zend/zend_vm_execute.h:57113
    #3 0x558a00c9a071 in zend_execute /home/niels/php-src/Zend/zend_vm_execute.h:61445
    #4 0x558a00a8e66f in zend_execute_scripts /home/niels/php-src/Zend/zend.c:1859
    #5 0x558a0091145f in php_execute_script /home/niels/php-src/main/main.c:2482
    #6 0x558a00e6f03a in do_cli /home/niels/php-src/sapi/cli/php_cli.c:959
    #7 0x558a00e70d78 in main /home/niels/php-src/sapi/cli/php_cli.c:1328
    #8 0x7f6d55d5484f  (/usr/lib/libc.so.6+0x2384f) (BuildId: 2f005a79cd1a8e385972f5a102f16adba414d75e)
    #9 0x7f6d55d54909 in __libc_start_main (/usr/lib/libc.so.6+0x23909) (BuildId: 2f005a79cd1a8e385972f5a102f16adba414d75e)
    #10 0x558a00005784 in _start (/usr/local/bin/php+0x605784) (BuildId: 2d2cfd4f28feb94b5331e070c74c424d89c4b84a)

zf->common.arg_info is NULL, so the access with arg_num derefs a NULL pointer.
Regressed in 61e1f8a which indeed changed the variadic flag, causing the faulting path to be executed. There used to be a NULL check for arg_info but it was removed a long time ago in fd4844e
Will look tonight.

nielsdos added a commit to nielsdos/php-src that referenced this issue Jun 9, 2023
The magic method trampoline closure may be variadic. However, the
arg_info for the variadic argument was not set, resulting in a crash
both in reflection and in the VM.

Fix it by creating an arg_info containing a single element in case of
the variadic case. The variadic argument is the last one (and in this
case only one) in the arg_info array.

We make sure the argument info is equivalent to the argument info of
`$closure` of the following code snippet:
```
function foo(...$arguments) {}
$closure = foo(...);
```
@nicolas-grekas
Copy link
Contributor Author

Thanks for fixing this @nielsdos!
I was that this was merged on master, so the issue doesn't exist on previous versions right?

@nielsdos
Copy link
Member

nielsdos commented Jun 13, 2023

@nicolas-grekas Indeed, this bug only existed on master.

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.

3 participants