Tailcall VM interrupt crash

[morrisonlevi]

In the new tailcall VM, if a call like ZEND_VM_DISPATCH_TO_HELPER(...) uses a helper like zend_is_smaller_helper_SPEC_TAILCALL(), and during that helper EG(vm_interrupt) gets set, then it will return the current opline tagged with ZEND_VM_ENTER_BIT. But the ZEND_VM_DISPATCH_TO_HELPER macro directly dereferenced the tagged pointer.

This PR changes the macro to check for a tagged pointer, and returns instead of doing the tailcall as the outer executor will handle the ZEND_VM_ENTER_BIT.

To reproduce this, I piggy-backed on the vm_interrupt machinery in zend_test I just added in #21910. It uses a compare handler on class VmInterruptComparable to deterministically set the EG(vm_interrupt).

[bwoebi]

the alternative would be returning instead of tail-calling here. But that might be slower.

[bwoebi]

Any thoughts @arnaud-lb ?

[arnaud-lb]

Yes this would work. Possibly this may result in worse branch prediction as we revert to the main loop's central dispatch point.

Another alternative would be to make ZEND_VM_INTERRUPT() return an opline whose handler is zend_interrupt_helper, like we do for halt_op in the HYBRID and TAILCALL VMs: https://github.com/php/php-src/compare/arnaud-lb:vm-interrupt-tailcall-repro-3. This probably requires special handling in zend_jit_trace_execute(), I haven't investigated.

I've benchmarked the 3 approaches:

1. check ENTER_BIT
2. return opline
3. return interrupt_op

Results:

Symfony:

base:  mean:  0.4459;  stddev:  0.0004;  diff:  -0.00%                       
1   :  mean:  0.4439;  stddev:  0.0004;  diff:  -0.45%;  p-value:  0.001000  (strong)
2   :  mean:  0.4471;  stddev:  0.0003;  diff:  +0.25%;  p-value:  0.001000  (strong)
3   :  mean:  0.4436;  stddev:  0.0003;  diff:  -0.52%;  p-value:  0.001000  (strong)

Symfony (valgrind):

base:  diff:  +0.00%
1   :  diff:  +0.12%
2   :  diff:  +0.01%
3   :  diff:  -0.00%

bench.php:

base:  mean:  0.8044;  stddev:  0.0004;  diff:  -0.00%                       
1   :  mean:  0.8161;  stddev:  0.0005;  diff:  +1.45%;  p-value:  0.001000  (strong)
2   :  mean:  0.8142;  stddev:  0.0016;  diff:  +1.22%;  p-value:  0.001000  (strong)
3   :  mean:  0.8043;  stddev:  0.0006;  diff:  -0.01%;  p-value:  0.690382  (weak)

bench.php (valgrind):

base:  diff:  +0.00%
1   :  diff:  +1.03%
2   :  diff:  -0.50%
3   :  diff:  -0.00%

The Symfony results do not make sense to me, but these are stable.

Approach 3 seems better overall, speed-wise, assuming that we don't find issues with it.

[bwoebi]

Uh, that's a nice approach! Love it.

I don't see any issues, that approach will never have &call_interrupt_op show up in EX(opline) or the VM IP register so that's perfect.
I have no idea about the JIT code, so, I'll leave that to you :-D