Skip to content

Add EG(bailout) consistency assertion#21667

Open
iluuu1994 wants to merge 2 commits intophp:masterfrom
iluuu1994:bailout-check
Open

Add EG(bailout) consistency assertion#21667
iluuu1994 wants to merge 2 commits intophp:masterfrom
iluuu1994:bailout-check

Conversation

@iluuu1994
Copy link
Copy Markdown
Member

@iluuu1994 iluuu1994 commented Apr 7, 2026

Make sure EG(bailout) is what we expect in zend_end_try(). This will detect faulty jumps into or out of zend_try blocks, as in 38628e8.

This can be merged only after #21666.

This uncovered faulty code in zend_jit_trace(), which should likely be backported.

Copy link
Copy Markdown
Member

@devnexen devnexen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lctm

@iluuu1994
Copy link
Copy Markdown
Member Author

I needed to tweak slightly, because when we hit zend_catch we're already re-setting the EG(bailout). I think we can check at the end of the try block, and the start of catch.

Make sure EG(bailout) is what we expect. This will detect faulty jumps into or
out of zend_try blocks, as in 38628e8.
@iluuu1994 iluuu1994 marked this pull request as ready for review April 8, 2026 00:24
@iluuu1994 iluuu1994 requested a review from dstogov as a code owner April 8, 2026 00:24
@iluuu1994 iluuu1994 changed the title Add zend_end_try() assertion Add EG(bailout) consistency assertion Apr 8, 2026
@devnexen
Copy link
Copy Markdown
Member

devnexen commented Apr 8, 2026

I needed to tweak slightly, because when we hit zend_catch we're already re-setting the EG(bailout). I think we can check at the end of the try block, and the start of catch.

ah yes nice catch :)

Copy link
Copy Markdown
Member

@dstogov dstogov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks right. Thanks!

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