Make the call VM read the opline back after interrupts #19890
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
This happened implicitly in the past due to EX(opline) being used - or in the hybrid VM case, the implicit LOAD_OPLINE() happening as part of ZEND_VM_ENTER(). With 76d7c61 opline is used standalone and the return value of zend_interrupt_helper_SPEC ignored. Make use of it... Signed-off-by: Bob Weinand <bobwei9@hotmail.com>
arnaud-lb reviewed Sep 19, 2025
| out($f,"#ifdef ZEND_VM_FP_GLOBAL_REG\n"); | ||
| out($f,"#define ZEND_VM_LOOP_INTERRUPT() zend_interrupt_helper".($spec?"_SPEC":"")."(ZEND_OPCODE_HANDLER_ARGS_PASSTHRU);\n"); | ||
| out($f,"#else\n"); | ||
| out($f,"#define ZEND_VM_LOOP_INTERRUPT() opline = (zend_op*)((uintptr_t)zend_interrupt_helper".($spec?"_SPEC":"")."(ZEND_OPCODE_HANDLER_ARGS_PASSTHRU) & ~ZEND_VM_ENTER_BIT);\n"); |
There was a problem hiding this comment.
I think that we should reload ex from EG(current_execute_data) as well, because the interrupt handler may have changed it
There was a problem hiding this comment.
I don't object to that, though that hasn't been working since PHP 8.0. Would simplify my extension code a bit :-D
Also ZEND_VM_FCALL_INTERRUPT_CHECK() doesn't read execute_data back. Possibly both should.
There was a problem hiding this comment.
Ah indeed. The HYBRID VM ignores execute_data changes too, as it will assign ex to it.
arnaud-lb approved these changes Sep 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
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.
This happened implicitly in the past due to EX(opline) being used - or in the hybrid VM case, the implicit LOAD_OPLINE() happening as part of ZEND_VM_ENTER().
With 76d7c61 opline is used standalone and the return value of zend_interrupt_helper_SPEC ignored. Make use of it...