Skip to content

Remove unnecessary ast eval bailout #9805

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
wants to merge 1 commit into from

Conversation

iluuu1994
Copy link
Member

We can just reset the filename_override to NULL in php_request_shutdown.

This was only added to PHP-8.2. So I hope it's fine to remove it at this point.

We can just reset the filename_override to NULL in php_request_shutdown.
@TysonAndre
Copy link
Contributor

TysonAndre commented Oct 23, 2022

We can just reset the filename_override to NULL in php_request_shutdown.

This was only added to PHP-8.2. So I hope it's fine to remove it at this point.

This seems like the cleanest fix

  • I can't envision anything inside or outside of php depending on those constants being null - they're only for use by php's ast parser and tokenizer
  • This also avoids any similar fuzzing ASSERT failures in the future
  • the zend_try/zend_catch/zend_bailout apis seem to be catching the bailout (setjmp/longjmp) and rethrowing a bailout

(I started looking at this PR because I'm one of the leads of https://pecl.php.net/package/ast which uses the internal ast APIs meant for external use)

@iluuu1994 iluuu1994 closed this in 1d6b32f Oct 27, 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 this pull request may close these issues.

3 participants