Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,10 @@ PHP NEWS
. Fixed JIT segfault on switch over a string reference holding a
non-string value. (jorgsowa)
. Fixed AArch64 JIT encoding of invalid logical immediates. (jorgsowa)
. Fixed JIT crashes when handling a VM interrupt with a stale saved opline
in the tailcall VM. (Levi Morrison)
. Fixed function JIT misinterpreting VM-enter results after an interrupt
callback in the CALL and TAILCALL VMs. (Levi Morrison)

- PDO:
. Fixed invalid ATTR_ORACLE_NULLS values being accepted and truncated.
Expand Down
2 changes: 1 addition & 1 deletion Zend/zend_vm_execute.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Zend/zend_vm_gen.php
Original file line number Diff line number Diff line change
Expand Up @@ -2045,7 +2045,7 @@ function gen_executor($f, $skl, $spec, $kind, $executor_name, $initializer_name)
out($f,"# define ZEND_VM_ENTER() execute_data = EG(current_execute_data); LOAD_OPLINE(); ZEND_VM_ENTER_EX()\n");
out($f,"# define ZEND_VM_LEAVE() return (zend_op*)((uintptr_t)opline | ZEND_VM_ENTER_BIT)\n");
out($f,"#endif\n");
out($f,"#define ZEND_VM_INTERRUPT() ZEND_VM_TAIL_CALL(zend_interrupt_helper".($spec?"_SPEC":"")."(ZEND_OPCODE_HANDLER_ARGS_PASSTHRU));\n");
out($f,"#define ZEND_VM_INTERRUPT() ZEND_VM_KIND_TAILCALL_SAVE_OPLINE(); ZEND_VM_TAIL_CALL(zend_interrupt_helper".($spec?"_SPEC":"")."(ZEND_OPCODE_HANDLER_ARGS_PASSTHRU));\n");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

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");
Expand Down
6 changes: 6 additions & 0 deletions ext/opcache/jit/zend_jit_ir.c
Original file line number Diff line number Diff line change
Expand Up @@ -4165,10 +4165,16 @@ static int zend_jit_handler(zend_jit_ctx *jit, const zend_op *opline, int may_th
} else if (ZEND_VM_KIND == ZEND_VM_KIND_TAILCALL) {
zend_vm_opcode_handler_func_t handler = (zend_vm_opcode_handler_func_t)zend_get_opcode_handler_func(opline);
ir_ref ip = ir_CALL_2(IR_ADDR, ir_CONST_FC_FUNC(handler), jit_FP(jit), jit_IP(jit));
if (zend_interrupt_function) {
ip = ir_AND_A(ip, ir_CONST_ADDR(~ZEND_VM_ENTER_BIT));
}
jit_STORE_IP(jit, ip);
} else {
zend_vm_opcode_handler_t handler = opline->handler;
ir_ref ip = ir_CALL_2(IR_ADDR, ir_CONST_FC_FUNC(handler), jit_FP(jit), jit_IP(jit));
if (zend_interrupt_function) {
ip = ir_AND_A(ip, ir_CONST_ADDR(~ZEND_VM_ENTER_BIT));
}
jit_STORE_IP(jit, ip);
}
if (may_throw) {
Expand Down
25 changes: 25 additions & 0 deletions ext/opcache/tests/jit/function_vm_interrupt_enter_001.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
--TEST--
Function JIT: decode the VM-enter bit after an interrupt callback
--EXTENSIONS--
opcache
zend_test
--INI--
opcache.enable=1
opcache.enable_cli=1
opcache.file_update_protection=0
opcache.jit=1201
opcache.jit_buffer_size=64M
zend_test.observer.set_vm_interrupt_on_begin=1
--FILE--
<?php
function compare($left, $right) {
if ($left == $right) {
return 11;
}
return 22;
}

var_dump(compare(new VmInterruptComparable(1), new VmInterruptComparable(2)));
?>
--EXPECT--
int(22)
39 changes: 39 additions & 0 deletions ext/opcache/tests/jit/tailcall_vm_interrupt_opline_001.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
--TEST--
Tracing JIT: preserve the next opline when handling a VM interrupt
--DESCRIPTION--
The object comparison raises an interrupt; the observer INI setting installs
an interrupt callback without enabling observer events. While recording the
caller, the backward jump must save its target before the interrupt helper
reloads the opline. Otherwise it revisits the already executed call opcode,
whose pending call frame has been consumed.

At the time of writing this was failing on the TAILCALL VM, but this should
pass on all VMs.
--EXTENSIONS--
opcache
zend_test
--INI--
opcache.enable=1
opcache.enable_cli=1
opcache.file_update_protection=0
opcache.jit=tracing
opcache.jit_buffer_size=64M
opcache.jit_hot_func=1
opcache.jit_hot_loop=1
zend_test.observer.set_vm_interrupt_on_begin=1
--FILE--
<?php
function trigger(VmInterruptComparable $left, VmInterruptComparable $right): void {
// Raise an interrupt without an internal call that would handle it immediately.
$result = $left == $right;
}

$left = new VmInterruptComparable(1);
$right = new VmInterruptComparable(2);
foreach ([1, 2, 3] as $i) {
trigger($left, $right);
}
echo "Done\n";
?>
--EXPECT--
Done
29 changes: 29 additions & 0 deletions ext/opcache/tests/jit/tailcall_vm_interrupt_opline_002.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
--TEST--
Function JIT: preserve the next opline when handling a VM interrupt
--DESCRIPTION--
The object comparison raises an interrupt while the minimal function JIT
executes a callable opcode handler. The interrupt helper must resume at the
branch target, not reload the saved comparison opcode. No interrupt callback
is installed, isolating the saved-opline bug from callback resume-tag handling.
--EXTENSIONS--
opcache
zend_test
--INI--
opcache.enable=1
opcache.enable_cli=1
opcache.file_update_protection=0
opcache.jit=1201
opcache.jit_buffer_size=64M
--FILE--
<?php
function compare($left, $right) {
if ($left == $right) {
return 11;
}
return 22;
}

var_dump(compare(new VmInterruptComparable(1), new VmInterruptComparable(2)));
?>
--EXPECT--
int(22)
Loading