Skip to content

Commit 18276a8

Browse files
arnaud-lbarnaud-lb
committed
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 phpGH-18408
1 parent 40edd58 commit 18276a8

File tree

4 files changed

+138
-67
lines changed

4 files changed

+138
-67
lines changed

‎NEWS‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ PHP NEWS
2929
memory_consumption or jit_buffer_size). (nielsdos)
3030
. Fixed bug GH-18297 (Exception not handled when jit guard is triggered).
3131
(Arnaud)
32+
. Fixed bug GH-18408 (Snapshotted poly_func / poly_this may be spilled).
3233

3334
- SPL:
3435
. Fixed bug GH-18421 (Integer overflow with large numbers in LimitIterator).

‎ext/opcache/jit/zend_jit_internal.h‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -421,16 +421,22 @@ struct _zend_jit_trace_rec {
421421

422422
#define ZEND_JIT_TRACE_START_REC_SIZE 2
423423

424+
typedef struct _zend_jit_ref_snapshot {
425+
union {
426+
int32_t ref; /* While generating code: The ir_ref to snapshot */
427+
int32_t offset; /* After compilation / during deopt: C stack offset if 'reg' is spilled */
428+
};
429+
int8_t reg; /* Set after compilation by zend_jit_snapshot_handler() */
430+
} zend_jit_ref_snapshot;
431+
424432
typedef struct _zend_jit_trace_exit_info {
425-
const zend_op *opline; /* opline where VM should continue execution */
426-
const zend_op_array *op_array;
427-
uint32_t flags; /* set of ZEND_JIT_EXIT_... */
428-
uint32_t stack_size;
429-
uint32_t stack_offset;
430-
int32_t poly_func_ref;
431-
int32_t poly_this_ref;
432-
int8_t poly_func_reg;
433-
int8_t poly_this_reg;
433+
const zend_op *opline; /* opline where VM should continue execution */
434+
const zend_op_array *op_array;
435+
uint32_t flags; /* set of ZEND_JIT_EXIT_... */
436+
uint32_t stack_size;
437+
uint32_t stack_offset;
438+
zend_jit_ref_snapshot poly_func;
439+
zend_jit_ref_snapshot poly_this;
434440
} zend_jit_trace_exit_info;
435441

436442
typedef struct _zend_jit_trace_stack {

‎ext/opcache/jit/zend_jit_ir.c‎

Lines changed: 71 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,8 @@ typedef struct _zend_jit_ctx {
277277
ir_ref tls;
278278
#endif
279279
ir_ref fp;
280+
ir_ref poly_func_ref; /* restored from parent trace snapshot */
281+
ir_ref poly_this_ref; /* restored from parent trace snapshot */
280282
ir_ref trace_loop_ref;
281283
ir_ref return_inputs;
282284
const zend_op_array *op_array;
@@ -624,12 +626,12 @@ static void jit_SNAPSHOT(zend_jit_ctx *jit, ir_ref addr)
624626
uint32_t exit_point = 0, n = 0;
625627

626628
if (addr < 0) {
627-
if (t->exit_count > 0
628-
&& jit->ctx.ir_base[addr].val.u64 == (uintptr_t)zend_jit_trace_get_exit_addr(t->exit_count - 1)) {
629-
exit_point = t->exit_count - 1;
630-
if (t->exit_info[exit_point].flags & ZEND_JIT_EXIT_METHOD_CALL) {
631-
n = 2;
632-
}
629+
/* addr is not always the address of the *last* exit point,
630+
* so we can not optimize this to 'exit_point = t->exit_count-1' */
631+
exit_point = zend_jit_exit_point_by_addr(ptr);
632+
ZEND_ASSERT(exit_point != -1);
633+
if (t->exit_info[exit_point].flags & ZEND_JIT_EXIT_METHOD_CALL) {
634+
n = 2;
633635
}
634636
}
635637

@@ -660,8 +662,8 @@ static void jit_SNAPSHOT(zend_jit_ctx *jit, ir_ref addr)
660662
ir_SNAPSHOT_SET_OP(snapshot, i + 1, ref);
661663
}
662664
if (n) {
663-
ir_SNAPSHOT_SET_OP(snapshot, snapshot_size + 1, t->exit_info[exit_point].poly_func_ref);
664-
ir_SNAPSHOT_SET_OP(snapshot, snapshot_size + 2, t->exit_info[exit_point].poly_this_ref);
665+
ir_SNAPSHOT_SET_OP(snapshot, snapshot_size + 1, t->exit_info[exit_point].poly_func.ref);
666+
ir_SNAPSHOT_SET_OP(snapshot, snapshot_size + 2, t->exit_info[exit_point].poly_this.ref);
665667
}
666668
}
667669
}
@@ -710,6 +712,31 @@ uint32_t zend_jit_duplicate_exit_point(ir_ctx *ctx, zend_jit_trace_info *t, uint
710712
return new_exit_point;
711713
}
712714

715+
static void zend_jit_resolve_ref_snapshot(zend_jit_ref_snapshot *dest, ir_ctx *ctx, ir_ref snapshot_ref, ir_insn *snapshot, int op)
716+
{
717+
int8_t *reg_ops = ctx->regs[snapshot_ref];
718+
ZEND_ASSERT(reg_ops[op] != ZREG_NONE);
719+
720+
int8_t reg = reg_ops[op];
721+
int32_t offset;
722+
723+
if (IR_REG_SPILLED(reg)) {
724+
reg = ((ctx->flags & IR_USE_FRAME_POINTER) ? IR_REG_FP : IR_REG_SP) | IR_REG_SPILL_LOAD;
725+
offset = ir_get_spill_slot_offset(ctx, ir_insn_op(snapshot, op));
726+
} else {
727+
offset = 0;
728+
}
729+
730+
dest->reg = reg;
731+
dest->offset = offset;
732+
}
733+
734+
static bool zend_jit_ref_snapshot_equals(const zend_jit_ref_snapshot *a, const zend_jit_ref_snapshot *b)
735+
{
736+
return a->reg == b->reg
737+
&& (!IR_REG_SPILLED(a->reg) || (a->offset == b->offset));
738+
}
739+
713740
void *zend_jit_snapshot_handler(ir_ctx *ctx, ir_ref snapshot_ref, ir_insn *snapshot, void *addr)
714741
{
715742
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
722749
exit_flags = t->exit_info[exit_point].flags;
723750

724751
if (exit_flags & ZEND_JIT_EXIT_METHOD_CALL) {
725-
int8_t *reg_ops = ctx->regs[snapshot_ref];
752+
zend_jit_ref_snapshot func, this;
753+
zend_jit_resolve_ref_snapshot(&func, ctx, snapshot_ref, snapshot, n - 1);
754+
zend_jit_resolve_ref_snapshot(&this, ctx, snapshot_ref, snapshot, n);
726755

727-
ZEND_ASSERT(reg_ops[n - 1] != -1 && reg_ops[n] != -1);
728756
if ((exit_flags & ZEND_JIT_EXIT_FIXED)
729-
&& (t->exit_info[exit_point].poly_func_reg != reg_ops[n - 1]
730-
|| t->exit_info[exit_point].poly_this_reg != reg_ops[n])) {
757+
&& (!zend_jit_ref_snapshot_equals(&t->exit_info[exit_point].poly_func, &func)
758+
|| !zend_jit_ref_snapshot_equals(&t->exit_info[exit_point].poly_this, &this))) {
731759
exit_point = zend_jit_duplicate_exit_point(ctx, t, exit_point, snapshot_ref);
732760
addr = (void*)zend_jit_trace_get_exit_addr(exit_point);
733761
exit_flags &= ~ZEND_JIT_EXIT_FIXED;
734762
}
735-
t->exit_info[exit_point].poly_func_reg = reg_ops[n - 1];
736-
t->exit_info[exit_point].poly_this_reg = reg_ops[n];
763+
t->exit_info[exit_point].poly_func = func;
764+
t->exit_info[exit_point].poly_this = this;
737765
n -= 2;
738766
}
739767

@@ -2751,6 +2779,8 @@ static void zend_jit_init_ctx(zend_jit_ctx *jit, uint32_t flags)
27512779
jit->tls = IR_UNUSED;
27522780
#endif
27532781
jit->fp = IR_UNUSED;
2782+
jit->poly_func_ref = IR_UNUSED;
2783+
jit->poly_this_ref = IR_UNUSED;
27542784
jit->trace_loop_ref = IR_UNUSED;
27552785
jit->return_inputs = IR_UNUSED;
27562786
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)
44234453
return ir_RLOAD(type, reg);
44244454
}
44254455

4456+
/* Same as zend_jit_deopt_rload(), but 'reg' may be spilled on C stack */
4457+
static ir_ref zend_jit_deopt_rload_spilled(zend_jit_ctx *jit, ir_type type, int8_t reg, int32_t offset)
4458+
{
4459+
ZEND_ASSERT(reg >= 0);
4460+
4461+
if (IR_REG_SPILLED(reg)) {
4462+
return ir_LOAD(type, ir_ADD_OFFSET(zend_jit_deopt_rload(jit, type, IR_REG_NUM(reg)), offset));
4463+
} else {
4464+
return zend_jit_deopt_rload(jit, type, reg);
4465+
}
4466+
}
4467+
44264468
static int zend_jit_store_const_long(zend_jit_ctx *jit, int var, zend_long val)
44274469
{
44284470
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
84778519
return 1;
84788520
}
84798521

8480-
static int zend_jit_free_trampoline(zend_jit_ctx *jit, int8_t func_reg)
8522+
static int zend_jit_free_trampoline(zend_jit_ctx *jit, ir_ref func)
84818523
{
84828524
// JIT: if (UNEXPECTED(func->common.fn_flags & ZEND_ACC_CALL_VIA_TRAMPOLINE))
8483-
ir_ref func = ir_RLOAD_A(func_reg);
84848525
ir_ref if_trampoline = ir_IF(ir_AND_U32(
84858526
ir_LOAD_U32(ir_ADD_OFFSET(func, offsetof(zend_function, common.fn_flags))),
84868527
ir_CONST_U32(ZEND_ACC_CALL_VIA_TRAMPOLINE)));
@@ -8962,15 +9003,15 @@ static int zend_jit_init_method_call(zend_jit_ctx *jit,
89629003
zend_class_entry *trace_ce,
89639004
zend_jit_trace_rec *trace,
89649005
int checked_stack,
8965-
int8_t func_reg,
8966-
int8_t this_reg,
9006+
ir_ref func_ref,
9007+
ir_ref this_ref,
89679008
bool polymorphic_side_trace)
89689009
{
89699010
zend_func_info *info = ZEND_FUNC_INFO(op_array);
89709011
zend_call_info *call_info = NULL;
89719012
zend_function *func = NULL;
89729013
zval *function_name;
8973-
ir_ref if_static = IR_UNUSED, cold_path, this_ref = IR_NULL, func_ref = IR_NULL;
9014+
ir_ref if_static = IR_UNUSED, cold_path;
89749015

89759016
ZEND_ASSERT(opline->op2_type == IS_CONST);
89769017
ZEND_ASSERT(op1_info & MAY_BE_OBJECT);
@@ -8988,10 +9029,8 @@ static int zend_jit_init_method_call(zend_jit_ctx *jit,
89889029
}
89899030

89909031
if (polymorphic_side_trace) {
8991-
/* function is passed in r0 from parent_trace */
8992-
ZEND_ASSERT(func_reg >= 0 && this_reg >= 0);
8993-
func_ref = zend_jit_deopt_rload(jit, IR_ADDR, func_reg);
8994-
this_ref = zend_jit_deopt_rload(jit, IR_ADDR, this_reg);
9032+
/* function is passed from parent snapshot */
9033+
ZEND_ASSERT(func_ref != IR_UNUSED && this_ref != IR_UNUSED);
89959034
} else {
89969035
ir_ref ref, ref2, if_found, fast_path, run_time_cache, this_ref2;
89979036

@@ -9137,8 +9176,8 @@ static int zend_jit_init_method_call(zend_jit_ctx *jit,
91379176
return 0;
91389177
}
91399178

9140-
jit->trace->exit_info[exit_point].poly_func_ref = func_ref;
9141-
jit->trace->exit_info[exit_point].poly_this_ref = this_ref;
9179+
jit->trace->exit_info[exit_point].poly_func.ref = func_ref;
9180+
jit->trace->exit_info[exit_point].poly_this.ref = this_ref;
91429181

91439182
func = (zend_function*)trace->func;
91449183

@@ -16991,9 +17030,13 @@ static int zend_jit_trace_start(zend_jit_ctx *jit,
1699117030
}
1699217031

1699317032
if (parent && parent->exit_info[exit_num].flags & ZEND_JIT_EXIT_METHOD_CALL) {
16994-
ZEND_ASSERT(parent->exit_info[exit_num].poly_func_reg >= 0 && parent->exit_info[exit_num].poly_this_reg >= 0);
16995-
ir_RLOAD_A(parent->exit_info[exit_num].poly_func_reg);
16996-
ir_RLOAD_A(parent->exit_info[exit_num].poly_this_reg);
17033+
ZEND_ASSERT(parent->exit_info[exit_num].poly_func.reg >= 0 && parent->exit_info[exit_num].poly_this.reg >= 0);
17034+
if (!IR_REG_SPILLED(parent->exit_info[exit_num].poly_func.reg)) {
17035+
ir_RLOAD_A(parent->exit_info[exit_num].poly_func.reg);
17036+
}
17037+
if (!IR_REG_SPILLED(parent->exit_info[exit_num].poly_this.reg)) {
17038+
ir_RLOAD_A(parent->exit_info[exit_num].poly_this.reg);
17039+
}
1699717040
}
1699817041

1699917042
ir_STORE(jit_EG(jit_trace_num), ir_CONST_U32(trace_num));

‎ext/opcache/jit/zend_jit_trace.c‎

Lines changed: 51 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -198,10 +198,8 @@ static uint32_t zend_jit_trace_get_exit_point(const zend_op *to_opline, uint32_t
198198
t->exit_info[exit_point].flags = flags;
199199
t->exit_info[exit_point].stack_size = stack_size;
200200
t->exit_info[exit_point].stack_offset = stack_offset;
201-
t->exit_info[exit_point].poly_func_ref = 0;
202-
t->exit_info[exit_point].poly_this_ref = 0;
203-
t->exit_info[exit_point].poly_func_reg = ZREG_NONE;
204-
t->exit_info[exit_point].poly_this_reg = ZREG_NONE;
201+
t->exit_info[exit_point].poly_func = (zend_jit_ref_snapshot){.reg = ZREG_NONE};
202+
t->exit_info[exit_point].poly_this = (zend_jit_ref_snapshot){.reg = ZREG_NONE};
205203
}
206204

207205
return exit_point;
@@ -3484,17 +3482,18 @@ static int zend_jit_trace_exit_needs_deoptimization(uint32_t trace_num, uint32_t
34843482
}
34853483

34863484
static int zend_jit_trace_deoptimization(
3487-
zend_jit_ctx *jit,
3488-
uint32_t flags,
3489-
const zend_op *opline,
3490-
zend_jit_trace_stack *parent_stack,
3491-
int parent_vars_count,
3492-
zend_ssa *ssa,
3493-
zend_jit_trace_stack *stack,
3494-
zend_jit_exit_const *constants,
3495-
int8_t func_reg,
3496-
bool polymorphic_side_trace)
3485+
zend_jit_ctx *jit,
3486+
const zend_jit_trace_exit_info *exit_info,
3487+
zend_jit_trace_stack *parent_stack,
3488+
int parent_vars_count,
3489+
zend_ssa *ssa,
3490+
zend_jit_trace_stack *stack,
3491+
zend_jit_exit_const *constants,
3492+
bool polymorphic_side_trace)
34973493
{
3494+
uint32_t flags = exit_info->flags;
3495+
const zend_op *opline = exit_info->opline;
3496+
34983497
int i;
34993498
int check2 = -1;
35003499

@@ -3638,9 +3637,16 @@ static int zend_jit_trace_deoptimization(
36383637
zend_jit_check_exception(jit);
36393638
}
36403639

3641-
if ((flags & ZEND_JIT_EXIT_METHOD_CALL) && !polymorphic_side_trace) {
3642-
if (!zend_jit_free_trampoline(jit, func_reg)) {
3643-
return 0;
3640+
if (flags & ZEND_JIT_EXIT_METHOD_CALL) {
3641+
jit->poly_func_ref = zend_jit_deopt_rload_spilled(jit, IR_ADDR,
3642+
exit_info->poly_func.reg, exit_info->poly_func.offset);
3643+
jit->poly_this_ref = zend_jit_deopt_rload_spilled(jit, IR_ADDR,
3644+
exit_info->poly_this.reg, exit_info->poly_this.offset);
3645+
3646+
if (!polymorphic_side_trace) {
3647+
if (!zend_jit_free_trampoline(jit, jit->poly_func_ref)) {
3648+
return 0;
3649+
}
36443650
}
36453651
}
36463652

@@ -4235,11 +4241,9 @@ static const void *zend_jit_trace(zend_jit_trace_rec *trace_buffer, uint32_t par
42354241
if (parent_trace) {
42364242
/* Deoptimization */
42374243
if (!zend_jit_trace_deoptimization(&ctx,
4238-
zend_jit_traces[parent_trace].exit_info[exit_num].flags,
4239-
zend_jit_traces[parent_trace].exit_info[exit_num].opline,
4244+
&zend_jit_traces[parent_trace].exit_info[exit_num],
42404245
parent_stack, parent_vars_count, ssa, stack,
42414246
zend_jit_traces[parent_trace].constants,
4242-
zend_jit_traces[parent_trace].exit_info[exit_num].poly_func_reg,
42434247
polymorphic_side_trace)) {
42444248
goto jit_failure;
42454249
}
@@ -6323,8 +6327,8 @@ static const void *zend_jit_trace(zend_jit_trace_rec *trace_buffer, uint32_t par
63236327
op_array, ssa, ssa_op, frame->call_level,
63246328
op1_info, op1_addr, ce, ce_is_instanceof, on_this, delayed_fetch_this, op1_ce,
63256329
p + 1, peek_checked_stack - checked_stack,
6326-
polymorphic_side_trace ? zend_jit_traces[parent_trace].exit_info[exit_num].poly_func_reg : -1,
6327-
polymorphic_side_trace ? zend_jit_traces[parent_trace].exit_info[exit_num].poly_this_reg : -1,
6330+
polymorphic_side_trace ? jit->poly_func_ref : -1,
6331+
polymorphic_side_trace ? jit->poly_this_ref : -1,
63286332
polymorphic_side_trace)) {
63296333
goto jit_failure;
63306334
}
@@ -7348,11 +7352,9 @@ static const void *zend_jit_trace_exit_to_vm(uint32_t trace_num, uint32_t exit_n
73487352
NULL;
73497353

73507354
if (!zend_jit_trace_deoptimization(&ctx,
7351-
zend_jit_traces[trace_num].exit_info[exit_num].flags,
7352-
zend_jit_traces[trace_num].exit_info[exit_num].opline,
7355+
&zend_jit_traces[trace_num].exit_info[exit_num],
73537356
stack, stack_size, NULL, NULL,
73547357
zend_jit_traces[trace_num].constants,
7355-
zend_jit_traces[trace_num].exit_info[exit_num].poly_func_reg,
73567358
0)) {
73577359
goto jit_failure;
73587360
}
@@ -7902,6 +7904,17 @@ static void zend_jit_dump_trace(zend_jit_trace_rec *trace_buffer, zend_ssa *tssa
79027904
}
79037905
}
79047906

7907+
static void zend_jit_dump_ref_snapshot(zend_jit_ref_snapshot *rs)
7908+
{
7909+
if (rs->reg == ZREG_NONE) {
7910+
fprintf(stderr, "?");
7911+
} else if (!IR_REG_SPILLED(rs->reg)) {
7912+
fprintf(stderr, "%s", zend_reg_name(rs->reg));
7913+
} else {
7914+
fprintf(stderr, "0x%x(%s)", rs->offset, zend_reg_name(IR_REG_NUM(rs->reg)));
7915+
}
7916+
}
7917+
79057918
static void zend_jit_dump_exit_info(zend_jit_trace_info *t)
79067919
{
79077920
int i, j;
@@ -7932,9 +7945,11 @@ static void zend_jit_dump_exit_info(zend_jit_trace_info *t)
79327945
if (t->exit_info[i].flags & (ZEND_JIT_EXIT_POLYMORPHISM|ZEND_JIT_EXIT_METHOD_CALL|ZEND_JIT_EXIT_CLOSURE_CALL)) {
79337946
fprintf(stderr, "/POLY");
79347947
if (t->exit_info[i].flags & ZEND_JIT_EXIT_METHOD_CALL) {
7935-
fprintf(stderr, "(%s, %s)",
7936-
t->exit_info[i].poly_func_reg != ZREG_NONE ? zend_reg_name(t->exit_info[i].poly_func_reg) : "?",
7937-
t->exit_info[i].poly_this_reg != ZREG_NONE ? zend_reg_name(t->exit_info[i].poly_this_reg) : "?");
7948+
fprintf(stderr, "(");
7949+
zend_jit_dump_ref_snapshot(&t->exit_info[i].poly_func);
7950+
fprintf(stderr, ", ");
7951+
zend_jit_dump_ref_snapshot(&t->exit_info[i].poly_this);
7952+
fprintf(stderr, ")");
79387953
}
79397954
}
79407955
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
86688683
}
86698684
}
86708685
if (t->exit_info[exit_num].flags & ZEND_JIT_EXIT_METHOD_CALL) {
8671-
ZEND_ASSERT(t->exit_info[exit_num].poly_func_reg >= 0);
8672-
zend_function *func = (zend_function*)regs->gpr[t->exit_info[exit_num].poly_func_reg];
8686+
zend_jit_ref_snapshot *func_snapshot = &t->exit_info[exit_num].poly_func;
8687+
ZEND_ASSERT(func_snapshot->reg >= 0);
86738688

8689+
zend_function *func;
8690+
if (IR_REG_SPILLED(func_snapshot->reg)) {
8691+
func = *(zend_function**)(regs->gpr[IR_REG_NUM(func_snapshot->reg)] + func_snapshot->offset);
8692+
} else {
8693+
func = (zend_function*)regs->gpr[func_snapshot->reg];
8694+
}
86748695
if (UNEXPECTED(func->common.fn_flags & ZEND_ACC_CALL_VIA_TRAMPOLINE)) {
86758696
zend_string_release_ex(func->common.function_name, 0);
86768697
zend_free_trampoline(func);

0 commit comments

Comments
 (0)