Skip to content

Conversation

@MaxKellermann
Copy link
Contributor

Commit 6c25413 added the flag ZEND_JIT_EXIT_INVALIDATE which resets the trace handlers in zend_jit_trace_exit(), but forgot to consider that on ZEND_JIT_TRACE_STOP_LINK, this changed handler gets passed to zend_jit_find_trace(), causing it to fail, either by returning 0 (results in bogus data) or by aborting due to ZEND_UNREACHABLE(). In either case, this crashes the PHP process.

I'm not quite sure how to fix this multi-threading problem properly; my suggestion is to just fail the zend_jit_trace() call. After all, the whole ZEND_JIT_EXIT_INVALIDATE fix was about reloading modified scripts, so there's probably no point in this pending zend_jit_trace() call.

Commit 6c25413 added the flag ZEND_JIT_EXIT_INVALIDATE which resets
the trace handlers in zend_jit_trace_exit(), but forgot to consider
that on ZEND_JIT_TRACE_STOP_LINK, this changed handler gets passed to
zend_jit_find_trace(), causing it to fail, either by returning 0
(results in bogus data) or by aborting due to ZEND_UNREACHABLE().  In
either case, this crashes the PHP process.

I'm not quite sure how to fix this multi-threading problem properly;
my suggestion is to just fail the zend_jit_trace() call.  After all,
the whole ZEND_JIT_EXIT_INVALIDATE fix was about reloading modified
scripts, so there's probably no point in this pending zend_jit_trace()
call.
@MaxKellermann
Copy link
Contributor Author

@dstogov, please review.
This fix is related to #10138 and I think both are necessary.

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.

The patch looks fine. Thanks.

@devnexen
Copy link
Member

Merged with b26b7589 thanks

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants