Skip to content
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

Fix GH-17307: Internal closure causes JIT failure #17319

Closed
wants to merge 1 commit into from

Conversation

nielsdos
Copy link
Member

@nielsdos nielsdos commented Jan 1, 2025

bcadd(...) is a closure for an internal function, and zend_jit_push_call_frame takes into account both last_var and the difference in argument numbers not only for user code but also for internal code. However, this is inconsistent with
zend_vm_calc_used_stack, causing argument corruption. Making this consistent fixes the issue.

I could only reproduce the assertion failure when using Valgrind.

This needs a backport to 8.3, which I can do if this is right.

@nielsdos nielsdos linked an issue Jan 1, 2025 that may be closed by this pull request
`bcadd(...)` is a closure for an internal function, and
`zend_jit_push_call_frame` takes into account both last_var and the
difference in argument numbers not only for user code but also for
internal code. However, this is inconsistent with
`zend_vm_calc_used_stack`, causing argument corruption.
Making this consistent fixes the issue.

I could only reproduce the assertion failure when using Valgrind.
@nielsdos nielsdos marked this pull request as ready for review January 1, 2025 22:33
@nielsdos nielsdos requested a review from dstogov as a code owner January 1, 2025 22:33
Copy link
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.

It seems like the code was originally written when we didn't have internal closures yet.
You fix looks right. Thanks!

@nielsdos nielsdos closed this in 28b448a Jan 9, 2025
nielsdos added a commit to nielsdos/php-src that referenced this pull request Jan 9, 2025
This is a backport of phpGH-17319 to fix phpGH-17307 on lower branches.
@nielsdos nielsdos mentioned this pull request Jan 9, 2025
nielsdos added a commit to nielsdos/php-src that referenced this pull request Jan 9, 2025
This is a backport of phpGH-17319 to fix phpGH-17307 on lower branches.
nielsdos added a commit that referenced this pull request Jan 10, 2025
This is a backport of GH-17319 to fix GH-17307 on lower branches.

Closes GH-17424.
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.

Internal closure causes JIT failure
2 participants