[morrisonlevi]

This contains two JIT fixes for vm_interrupts on TAILCALL VMs (and the 2nd also affects the CALL VM too).

---

On the TAILCALL VM, the JIT (both function and tracing) can resume at the wrong opline after it handles a VM interrupt. This re-runs an opcode that has already executed. Depending on that opcode, you get a wrong result or a crash; for example, re-running a call opcode whose call frame has already been used up.

This is a follow-up to #21922.

---

In the CALL and TAILCALL VMs, opcode handlers called by function JIT can return the next opline tagged with ZEND_VM_ENTER_BIT after invoking a VM interrupt callback. Function JIT used that result directly as an opcode pointer, causing invalid accesses when execution continued.

[bwoebi]

These fixes look right to me. Thanks!
Not very nice to have to &~ZEND_VM_ENTER bit in every JIT handler call, but I guess nothing we can avoid here.

[arnaud-lb]

Alternatively we can handle this in zend_interrupt_helper:

@@ -10538,8 +10538,11 @@ ZEND_VM_HELPER(zend_interrupt_helper, ANY, ANY)
 {
        zend_atomic_bool_store_ex(&EG(vm_interrupt), false);
 #if ZEND_VM_KIND == ZEND_VM_KIND_TAILCALL
-       /* opline is &call_interrupt_op. Load orig opline. */
-       LOAD_OPLINE();
+       if (opline == &call_interrupt_op) {
+               LOAD_OPLINE();
+       } else {
+               SAVE_OPLINE();
+       }
 #else
        SAVE_OPLINE();
 #endif

[bwoebi]

Does that win us anything? The current patch only adds the unconditional save when it's anyway tailcalling to the interrupt helper. And here you introduce a branch.

[arnaud-lb]

It doesn't win us much, it just reduces code size slightly.

[arnaud-lb]

Not very nice to have to &~ZEND_VM_ENTER bit in every JIT handler call, but I guess nothing we can avoid here.

Yes it's sad that we have to do that. The bit is set in case the interrupt function switches frame, but I don't think we ever do that, and the JIT would break if we did. Maybe we could set the flag only when the interrupt function actually did switch frame? The JIT assumes it never does, so we could also assume that the flag is never set.

Not a change for this branch however.

Edit: could we at least do that only when there is an interrupt function?

[morrisonlevi]

@arnaud-lb and @bwoebi please take another look! It now does a if (zend_interrupt_function) check at compile-time and avoids the ptr & ~ZEND_VM_ENTER unless an interrupt function has been set.

[arnaud-lb]

Thank you!