Skip to content

Conversation

bwoebi
Copy link
Member

@bwoebi bwoebi commented Sep 19, 2025

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...

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>
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");
Copy link
Member

Choose a reason for hiding this comment

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

I think that we should reload ex from EG(current_execute_data) as well, because the interrupt handler may have changed it

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

Ah indeed. The HYBRID VM ignores execute_data changes too, as it will assign ex to it.

@bwoebi bwoebi merged commit 0bb146f into php:master Sep 19, 2025
9 checks passed
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.

2 participants