Author: Arnaud Le Blanc (arnaud-lb)
Date: 2025-05-14T12:27:57+02:00
Commit: https://github.com/php/php-src/commit/18276a8b42727fc9601c156fa57936d85aaad9d7
Raw diff: https://github.com/php/php-src/commit/18276a8b42727fc9601c156fa57936d85aaad9d7.diff
Snapshotted poly_func / poly_this may be spilled
Polymorphic calls pass this and the function to side traces via snapshotting.
However, we assume that this/func are in registers, when in fact they may be
spilled.
Here I update snapshotting of poly_func/poly_this to support spilling:
- In zend_jit_snapshot_handler, keep track of the C stack offset
of the spilled register, in a way similar to how stack variables.
- In zend_jit_start, do not pre-load the registers if they were spilled.
- In zend_jit_trace_exit / zend_jit_trace_deoptimization, load from the
stack if the register was spilled.
- Store a reference to poly_func/poly_this in zend_jit_ctx so we can use that
directly in the side trace.
Closes GH-18408
Changed paths:
M NEWS
M ext/opcache/jit/zend_jit_internal.h
M ext/opcache/jit/zend_jit_ir.c
M ext/opcache/jit/zend_jit_trace.c
Diff:
diff --git a/NEWS b/NEWS
index 737e352b2d6e6..608880d61fadd 100644
--- a/NEWS
+++ b/NEWS
@@ -29,6 +29,7 @@ PHP NEWS
memory_consumption or jit_buffer_size). (nielsdos)
. Fixed bug GH-18297 (Exception not handled when jit guard is triggered).
(Arnaud)
+ . Fixed bug GH-18408 (Snapshotted poly_func / poly_this may be spilled).
- SPL:
. Fixed bug GH-18421 (Integer overflow with large numbers in LimitIterator).
diff --git a/ext/opcache/jit/zend_jit_internal.h b/ext/opcache/jit/zend_jit_internal.h
index 1bf62e1f91452..f320c51c78b2a 100644
--- a/ext/opcache/jit/zend_jit_internal.h
+++ b/ext/opcache/jit/zend_jit_internal.h
@@ -421,16 +421,22 @@ struct _zend_jit_trace_rec {
#define ZEND_JIT_TRACE_START_REC_SIZE 2
+typedef struct _zend_jit_ref_snapshot {
+ union {
+ int32_t ref; /* While generating code: The ir_ref to snapshot */
+ int32_t offset; /* After compilation / during deopt: C stack offset if 'reg' is
spilled */
+ };
+ int8_t reg; /* Set after compilation by zend_jit_snapshot_handler() */
+} zend_jit_ref_snapshot;
+
typedef struct _zend_jit_trace_exit_info {
- const zend_op *opline; /* opline where VM should continue execution */
- const zend_op_array *op_array;
- uint32_t flags; /* set of ZEND_JIT_EXIT_... */
- uint32_t stack_size;
- uint32_t stack_offset;
- int32_t poly_func_ref;
- int32_t poly_this_ref;
- int8_t poly_func_reg;
- int8_t poly_this_reg;
+ const zend_op *opline; /* opline where VM should continue execution */
+ const zend_op_array *op_array;
+ uint32_t flags; /* set of ZEND_JIT_EXIT_... */
+ uint32_t stack_size;
+ uint32_t stack_offset;
+ zend_jit_ref_snapshot poly_func;
+ zend_jit_ref_snapshot poly_this;
} zend_jit_trace_exit_info;
typedef struct _zend_jit_trace_stack {
diff --git a/ext/opcache/jit/zend_jit_ir.c b/ext/opcache/jit/zend_jit_ir.c
index 064e88c39c14b..1f833ad8f19c5 100644
--- a/ext/opcache/jit/zend_jit_ir.c
+++ b/ext/opcache/jit/zend_jit_ir.c
@@ -277,6 +277,8 @@ typedef struct _zend_jit_ctx {
ir_ref tls;
#endif
ir_ref fp;
+ ir_ref poly_func_ref; /* restored from parent trace snapshot */
+ ir_ref poly_this_ref; /* restored from parent trace snapshot */
ir_ref trace_loop_ref;
ir_ref return_inputs;
const zend_op_array *op_array;
@@ -624,12 +626,12 @@ static void jit_SNAPSHOT(zend_jit_ctx *jit, ir_ref addr)
uint32_t exit_point = 0, n = 0;
if (addr < 0) {
- if (t->exit_count > 0
- && jit->ctx.ir_base[addr].val.u64 ==
(uintptr_t)zend_jit_trace_get_exit_addr(t->exit_count - 1)) {
- exit_point = t->exit_count - 1;
- if (t->exit_info[exit_point].flags & ZEND_JIT_EXIT_METHOD_CALL) {
- n = 2;
- }
+ /* addr is not always the address of the *last* exit point,
+ * so we can not optimize this to 'exit_point = t->exit_count-1' */
+ exit_point = zend_jit_exit_point_by_addr(ptr);
+ ZEND_ASSERT(exit_point != -1);
+ if (t->exit_info[exit_point].flags & ZEND_JIT_EXIT_METHOD_CALL) {
+ n = 2;
}
}
@@ -660,8 +662,8 @@ static void jit_SNAPSHOT(zend_jit_ctx *jit, ir_ref addr)
ir_SNAPSHOT_SET_OP(snapshot, i + 1, ref);
}
if (n) {
- ir_SNAPSHOT_SET_OP(snapshot, snapshot_size + 1, t->exit_info[exit_point].poly_func_ref);
- ir_SNAPSHOT_SET_OP(snapshot, snapshot_size + 2, t->exit_info[exit_point].poly_this_ref);
+ ir_SNAPSHOT_SET_OP(snapshot, snapshot_size + 1, t->exit_info[exit_point].poly_func.ref);
+ ir_SNAPSHOT_SET_OP(snapshot, snapshot_size + 2, t->exit_info[exit_point].poly_this.ref);
}
}
}
@@ -710,6 +712,31 @@ uint32_t zend_jit_duplicate_exit_point(ir_ctx *ctx, zend_jit_trace_info *t,
uint
return new_exit_point;
}
+static void zend_jit_resolve_ref_snapshot(zend_jit_ref_snapshot *dest, ir_ctx *ctx, ir_ref
snapshot_ref, ir_insn *snapshot, int op)
+{
+ int8_t *reg_ops = ctx->regs[snapshot_ref];
+ ZEND_ASSERT(reg_ops[op] != ZREG_NONE);
+
+ int8_t reg = reg_ops[op];
+ int32_t offset;
+
+ if (IR_REG_SPILLED(reg)) {
+ reg = ((ctx->flags & IR_USE_FRAME_POINTER) ? IR_REG_FP : IR_REG_SP) | IR_REG_SPILL_LOAD;
+ offset = ir_get_spill_slot_offset(ctx, ir_insn_op(snapshot, op));
+ } else {
+ offset = 0;
+ }
+
+ dest->reg = reg;
+ dest->offset = offset;
+}
+
+static bool zend_jit_ref_snapshot_equals(const zend_jit_ref_snapshot *a, const
zend_jit_ref_snapshot *b)
+{
+ return a->reg == b->reg
+ && (!IR_REG_SPILLED(a->reg) || (a->offset == b->offset));
+}
+
void *zend_jit_snapshot_handler(ir_ctx *ctx, ir_ref snapshot_ref, ir_insn *snapshot, void *addr)
{
zend_jit_trace_info *t = ((zend_jit_ctx*)ctx)->trace;
@@ -722,18 +749,19 @@ void *zend_jit_snapshot_handler(ir_ctx *ctx, ir_ref snapshot_ref, ir_insn
*snaps
exit_flags = t->exit_info[exit_point].flags;
if (exit_flags & ZEND_JIT_EXIT_METHOD_CALL) {
- int8_t *reg_ops = ctx->regs[snapshot_ref];
+ zend_jit_ref_snapshot func, this;
+ zend_jit_resolve_ref_snapshot(&func, ctx, snapshot_ref, snapshot, n - 1);
+ zend_jit_resolve_ref_snapshot(&this, ctx, snapshot_ref, snapshot, n);
- ZEND_ASSERT(reg_ops[n - 1] != -1 && reg_ops[n] != -1);
if ((exit_flags & ZEND_JIT_EXIT_FIXED)
- && (t->exit_info[exit_point].poly_func_reg != reg_ops[n - 1]
- || t->exit_info[exit_point].poly_this_reg != reg_ops[n])) {
+ && (!zend_jit_ref_snapshot_equals(&t->exit_info[exit_point].poly_func, &func)
+ || !zend_jit_ref_snapshot_equals(&t->exit_info[exit_point].poly_this, &this))) {
exit_point = zend_jit_duplicate_exit_point(ctx, t, exit_point, snapshot_ref);
addr = (void*)zend_jit_trace_get_exit_addr(exit_point);
exit_flags &= ~ZEND_JIT_EXIT_FIXED;
}
- t->exit_info[exit_point].poly_func_reg = reg_ops[n - 1];
- t->exit_info[exit_point].poly_this_reg = reg_ops[n];
+ t->exit_info[exit_point].poly_func = func;
+ t->exit_info[exit_point].poly_this = this;
n -= 2;
}
@@ -2751,6 +2779,8 @@ static void zend_jit_init_ctx(zend_jit_ctx *jit, uint32_t flags)
jit->tls = IR_UNUSED;
#endif
jit->fp = IR_UNUSED;
+ jit->poly_func_ref = IR_UNUSED;
+ jit->poly_this_ref = IR_UNUSED;
jit->trace_loop_ref = IR_UNUSED;
jit->return_inputs = IR_UNUSED;
jit->bb_start_ref = NULL;
@@ -4423,6 +4453,18 @@ static ir_ref zend_jit_deopt_rload(zend_jit_ctx *jit, ir_type type, int32_t
reg)
return ir_RLOAD(type, reg);
}
+/* Same as zend_jit_deopt_rload(), but 'reg' may be spilled on C stack */
+static ir_ref zend_jit_deopt_rload_spilled(zend_jit_ctx *jit, ir_type type, int8_t reg, int32_t
offset)
+{
+ ZEND_ASSERT(reg >= 0);
+
+ if (IR_REG_SPILLED(reg)) {
+ return ir_LOAD(type, ir_ADD_OFFSET(zend_jit_deopt_rload(jit, type, IR_REG_NUM(reg)), offset));
+ } else {
+ return zend_jit_deopt_rload(jit, type, reg);
+ }
+}
+
static int zend_jit_store_const_long(zend_jit_ctx *jit, int var, zend_long val)
{
zend_jit_addr dst = ZEND_ADDR_MEM_ZVAL(ZREG_FP, EX_NUM_TO_VAR(var));
@@ -8477,10 +8519,9 @@ static int zend_jit_stack_check(zend_jit_ctx *jit, const zend_op *opline,
uint32
return 1;
}
-static int zend_jit_free_trampoline(zend_jit_ctx *jit, int8_t func_reg)
+static int zend_jit_free_trampoline(zend_jit_ctx *jit, ir_ref func)
{
// JIT: if (UNEXPECTED(func->common.fn_flags & ZEND_ACC_CALL_VIA_TRAMPOLINE))
- ir_ref func = ir_RLOAD_A(func_reg);
ir_ref if_trampoline = ir_IF(ir_AND_U32(
ir_LOAD_U32(ir_ADD_OFFSET(func, offsetof(zend_function, common.fn_flags))),
ir_CONST_U32(ZEND_ACC_CALL_VIA_TRAMPOLINE)));
@@ -8962,15 +9003,15 @@ static int zend_jit_init_method_call(zend_jit_ctx *jit,
zend_class_entry *trace_ce,
zend_jit_trace_rec *trace,
int checked_stack,
- int8_t func_reg,
- int8_t this_reg,
+ ir_ref func_ref,
+ ir_ref this_ref,
bool polymorphic_side_trace)
{
zend_func_info *info = ZEND_FUNC_INFO(op_array);
zend_call_info *call_info = NULL;
zend_function *func = NULL;
zval *function_name;
- ir_ref if_static = IR_UNUSED, cold_path, this_ref = IR_NULL, func_ref = IR_NULL;
+ ir_ref if_static = IR_UNUSED, cold_path;
ZEND_ASSERT(opline->op2_type == IS_CONST);
ZEND_ASSERT(op1_info & MAY_BE_OBJECT);
@@ -8988,10 +9029,8 @@ static int zend_jit_init_method_call(zend_jit_ctx *jit,
}
if (polymorphic_side_trace) {
- /* function is passed in r0 from parent_trace */
- ZEND_ASSERT(func_reg >= 0 && this_reg >= 0);
- func_ref = zend_jit_deopt_rload(jit, IR_ADDR, func_reg);
- this_ref = zend_jit_deopt_rload(jit, IR_ADDR, this_reg);
+ /* function is passed from parent snapshot */
+ ZEND_ASSERT(func_ref != IR_UNUSED && this_ref != IR_UNUSED);
} else {
ir_ref ref, ref2, if_found, fast_path, run_time_cache, this_ref2;
@@ -9137,8 +9176,8 @@ static int zend_jit_init_method_call(zend_jit_ctx *jit,
return 0;
}
- jit->trace->exit_info[exit_point].poly_func_ref = func_ref;
- jit->trace->exit_info[exit_point].poly_this_ref = this_ref;
+ jit->trace->exit_info[exit_point].poly_func.ref = func_ref;
+ jit->trace->exit_info[exit_point].poly_this.ref = this_ref;
func = (zend_function*)trace->func;
@@ -16991,9 +17030,13 @@ static int zend_jit_trace_start(zend_jit_ctx *jit,
}
if (parent && parent->exit_info[exit_num].flags & ZEND_JIT_EXIT_METHOD_CALL) {
- ZEND_ASSERT(parent->exit_info[exit_num].poly_func_reg >= 0 &&
parent->exit_info[exit_num].poly_this_reg >= 0);
- ir_RLOAD_A(parent->exit_info[exit_num].poly_func_reg);
- ir_RLOAD_A(parent->exit_info[exit_num].poly_this_reg);
+ ZEND_ASSERT(parent->exit_info[exit_num].poly_func.reg >= 0 &&
parent->exit_info[exit_num].poly_this.reg >= 0);
+ if (!IR_REG_SPILLED(parent->exit_info[exit_num].poly_func.reg)) {
+ ir_RLOAD_A(parent->exit_info[exit_num].poly_func.reg);
+ }
+ if (!IR_REG_SPILLED(parent->exit_info[exit_num].poly_this.reg)) {
+ ir_RLOAD_A(parent->exit_info[exit_num].poly_this.reg);
+ }
}
ir_STORE(jit_EG(jit_trace_num), ir_CONST_U32(trace_num));
diff --git a/ext/opcache/jit/zend_jit_trace.c b/ext/opcache/jit/zend_jit_trace.c
index c0ee09e6e6315..2502b22a608f4 100644
--- a/ext/opcache/jit/zend_jit_trace.c
+++ b/ext/opcache/jit/zend_jit_trace.c
@@ -198,10 +198,8 @@ static uint32_t zend_jit_trace_get_exit_point(const zend_op *to_opline,
uint32_t
t->exit_info[exit_point].flags = flags;
t->exit_info[exit_point].stack_size = stack_size;
t->exit_info[exit_point].stack_offset = stack_offset;
- t->exit_info[exit_point].poly_func_ref = 0;
- t->exit_info[exit_point].poly_this_ref = 0;
- t->exit_info[exit_point].poly_func_reg = ZREG_NONE;
- t->exit_info[exit_point].poly_this_reg = ZREG_NONE;
+ t->exit_info[exit_point].poly_func = (zend_jit_ref_snapshot){.reg = ZREG_NONE};
+ t->exit_info[exit_point].poly_this = (zend_jit_ref_snapshot){.reg = ZREG_NONE};
}
return exit_point;
@@ -3484,17 +3482,18 @@ static int zend_jit_trace_exit_needs_deoptimization(uint32_t trace_num,
uint32_t
}
static int zend_jit_trace_deoptimization(
- zend_jit_ctx *jit,
- uint32_t flags,
- const zend_op *opline,
- zend_jit_trace_stack *parent_stack,
- int parent_vars_count,
- zend_ssa *ssa,
- zend_jit_trace_stack *stack,
- zend_jit_exit_const *constants,
- int8_t func_reg,
- bool polymorphic_side_trace)
+ zend_jit_ctx *jit,
+ const zend_jit_trace_exit_info *exit_info,
+ zend_jit_trace_stack *parent_stack,
+ int parent_vars_count,
+ zend_ssa *ssa,
+ zend_jit_trace_stack *stack,
+ zend_jit_exit_const *constants,
+ bool polymorphic_side_trace)
{
+ uint32_t flags = exit_info->flags;
+ const zend_op *opline = exit_info->opline;
+
int i;
int check2 = -1;
@@ -3638,9 +3637,16 @@ static int zend_jit_trace_deoptimization(
zend_jit_check_exception(jit);
}
- if ((flags & ZEND_JIT_EXIT_METHOD_CALL) && !polymorphic_side_trace) {
- if (!zend_jit_free_trampoline(jit, func_reg)) {
- return 0;
+ if (flags & ZEND_JIT_EXIT_METHOD_CALL) {
+ jit->poly_func_ref = zend_jit_deopt_rload_spilled(jit, IR_ADDR,
+ exit_info->poly_func.reg, exit_info->poly_func.offset);
+ jit->poly_this_ref = zend_jit_deopt_rload_spilled(jit, IR_ADDR,
+ exit_info->poly_this.reg, exit_info->poly_this.offset);
+
+ if (!polymorphic_side_trace) {
+ if (!zend_jit_free_trampoline(jit, jit->poly_func_ref)) {
+ return 0;
+ }
}
}
@@ -4235,11 +4241,9 @@ static const void *zend_jit_trace(zend_jit_trace_rec *trace_buffer, uint32_t
par
if (parent_trace) {
/* Deoptimization */
if (!zend_jit_trace_deoptimization(&ctx,
- zend_jit_traces[parent_trace].exit_info[exit_num].flags,
- zend_jit_traces[parent_trace].exit_info[exit_num].opline,
+ &zend_jit_traces[parent_trace].exit_info[exit_num],
parent_stack, parent_vars_count, ssa, stack,
zend_jit_traces[parent_trace].constants,
- zend_jit_traces[parent_trace].exit_info[exit_num].poly_func_reg,
polymorphic_side_trace)) {
goto jit_failure;
}
@@ -6323,8 +6327,8 @@ static const void *zend_jit_trace(zend_jit_trace_rec *trace_buffer, uint32_t
par
op_array, ssa, ssa_op, frame->call_level,
op1_info, op1_addr, ce, ce_is_instanceof, on_this, delayed_fetch_this, op1_ce,
p + 1, peek_checked_stack - checked_stack,
- polymorphic_side_trace ? zend_jit_traces[parent_trace].exit_info[exit_num].poly_func_reg :
-1,
- polymorphic_side_trace ? zend_jit_traces[parent_trace].exit_info[exit_num].poly_this_reg :
-1,
+ polymorphic_side_trace ? jit->poly_func_ref : -1,
+ polymorphic_side_trace ? jit->poly_this_ref : -1,
polymorphic_side_trace)) {
goto jit_failure;
}
@@ -7348,11 +7352,9 @@ static const void *zend_jit_trace_exit_to_vm(uint32_t trace_num, uint32_t
exit_n
NULL;
if (!zend_jit_trace_deoptimization(&ctx,
- zend_jit_traces[trace_num].exit_info[exit_num].flags,
- zend_jit_traces[trace_num].exit_info[exit_num].opline,
+ &zend_jit_traces[trace_num].exit_info[exit_num],
stack, stack_size, NULL, NULL,
zend_jit_traces[trace_num].constants,
- zend_jit_traces[trace_num].exit_info[exit_num].poly_func_reg,
0)) {
goto jit_failure;
}
@@ -7902,6 +7904,17 @@ static void zend_jit_dump_trace(zend_jit_trace_rec *trace_buffer, zend_ssa
*tssa
}
}
+static void zend_jit_dump_ref_snapshot(zend_jit_ref_snapshot *rs)
+{
+ if (rs->reg == ZREG_NONE) {
+ fprintf(stderr, "?");
+ } else if (!IR_REG_SPILLED(rs->reg)) {
+ fprintf(stderr, "%s", zend_reg_name(rs->reg));
+ } else {
+ fprintf(stderr, "0x%x(%s)", rs->offset, zend_reg_name(IR_REG_NUM(rs->reg)));
+ }
+}
+
static void zend_jit_dump_exit_info(zend_jit_trace_info *t)
{
int i, j;
@@ -7932,9 +7945,11 @@ static void zend_jit_dump_exit_info(zend_jit_trace_info *t)
if (t->exit_info[i].flags &
(ZEND_JIT_EXIT_POLYMORPHISM|ZEND_JIT_EXIT_METHOD_CALL|ZEND_JIT_EXIT_CLOSURE_CALL)) {
fprintf(stderr, "/POLY");
if (t->exit_info[i].flags & ZEND_JIT_EXIT_METHOD_CALL) {
- fprintf(stderr, "(%s, %s)",
- t->exit_info[i].poly_func_reg != ZREG_NONE ?
zend_reg_name(t->exit_info[i].poly_func_reg) : "?",
- t->exit_info[i].poly_this_reg != ZREG_NONE ?
zend_reg_name(t->exit_info[i].poly_this_reg) : "?");
+ fprintf(stderr, "(");
+ zend_jit_dump_ref_snapshot(&t->exit_info[i].poly_func);
+ fprintf(stderr, ", ");
+ zend_jit_dump_ref_snapshot(&t->exit_info[i].poly_this);
+ fprintf(stderr, ")");
}
}
if (t->exit_info[i].flags & ZEND_JIT_EXIT_FREE_OP1) {
@@ -8668,9 +8683,15 @@ int ZEND_FASTCALL zend_jit_trace_exit(uint32_t exit_num,
zend_jit_registers_buf
}
}
if (t->exit_info[exit_num].flags & ZEND_JIT_EXIT_METHOD_CALL) {
- ZEND_ASSERT(t->exit_info[exit_num].poly_func_reg >= 0);
- zend_function *func = (zend_function*)regs->gpr[t->exit_info[exit_num].poly_func_reg];
+ zend_jit_ref_snapshot *func_snapshot = &t->exit_info[exit_num].poly_func;
+ ZEND_ASSERT(func_snapshot->reg >= 0);
+ zend_function *func;
+ if (IR_REG_SPILLED(func_snapshot->reg)) {
+ func = *(zend_function**)(regs->gpr[IR_REG_NUM(func_snapshot->reg)] +
func_snapshot->offset);
+ } else {
+ func = (zend_function*)regs->gpr[func_snapshot->reg];
+ }
if (UNEXPECTED(func->common.fn_flags & ZEND_ACC_CALL_VIA_TRAMPOLINE)) {
zend_string_release_ex(func->common.function_name, 0);
zend_free_trampoline(func);