Fix incorrect opline after deoptimization #19535
Open
+44
−10
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes GH-19486 introduced by 76d7c61.
Blacklisted side traces (aka JIT'ed exits) may return the previous opline after calling the original op handler. As a result, the op handler is called again by the VM.
Fix this by always returning the opline returned by the original op handler.
Always use
zend_jit_vm_enter(jit, ref)
to signal the VM that it must reloadEG(current_execute_data)
as it may have changed during the execution of the trace.This is consistent with non-JIT'ed exits:
php-src/ext/opcache/jit/zend_jit_ir.c
Lines 2488 to 2489 in 0bf2959
And with the previous version of this code (
ir_RETURN(ref)
andir_RETURN(ir_CONST_I32(2))
have the same effect in this case) :php-src/ext/opcache/jit/zend_jit_ir.c
Lines 17108 to 17119 in bc05bfe
Analysis of the reproducer code in GH-19486:
The function floodFill() performs a deep recursion, exhausting the VM stack, causing a side exit in zend_jit_push_call_frame(). The JIT'ed code of this side exit mistakenly returns the previous opline to the VM, causing the same opline (INIT_FCALL) to be executed twice:
The first frame never make it to the call stack. Returning from the second frame to its parent creates the inconsistency that causes the assertion failure (the frame doesn't have ZEND_CALL_ALLOCATED, so EG(vm_stack) is not freed and set to EG(vm_stack)->prev).