Skip to content

Commit 4c11168

Browse files
dstogovdstogov
authored
Fix phpGH-15656: php8.4beta4 JIT erronous results (php#15732)
* Improve trace SSA construction and type inference * Fix incorrect abstract stack maintenance * Add missing register store * Avoid IR binding for the dangerous case * Fix access to possibly uninitilezed variable * Improve trace SSA construction and type inference * Fix IR constuction Force load values into regesters before any branches to guarantee SSA dominance property
1 parent 837a8b6 commit 4c11168

File tree

2 files changed

+93
-9
lines changed

2 files changed

+93
-9
lines changed

‎ext/opcache/jit/zend_jit_ir.c

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1289,6 +1289,16 @@ static bool zend_jit_spilling_may_cause_conflict(zend_jit_ctx *jit, int var, ir_
12891289
// }
12901290
if (jit->ssa->vars[var].var < jit->current_op_array->last_var) {
12911291
/* IS_CV */
1292+
if (jit->ctx.ir_base[val].op == IR_LOAD
1293+
&& jit->ctx.ir_base[jit->ctx.ir_base[val].op2].op == IR_ADD
1294+
&& jit->ctx.ir_base[jit->ctx.ir_base[jit->ctx.ir_base[val].op2].op1].op == IR_RLOAD
1295+
&& jit->ctx.ir_base[jit->ctx.ir_base[jit->ctx.ir_base[val].op2].op1].op2 == ZREG_FP
1296+
&& IR_IS_CONST_REF(jit->ctx.ir_base[jit->ctx.ir_base[val].op2].op2)
1297+
&& jit->ctx.ir_base[jit->ctx.ir_base[jit->ctx.ir_base[val].op2].op2].val.addr != (uintptr_t)EX_NUM_TO_VAR(jit->ssa->vars[var].var)
1298+
&& EX_VAR_TO_NUM(jit->ctx.ir_base[jit->ctx.ir_base[jit->ctx.ir_base[val].op2].op2].val.addr) < jit->current_op_array->last_var) {
1299+
/* binding between different CVs may cause spill conflict */
1300+
return 1;
1301+
}
12921302
return 0;
12931303
}
12941304
return 1;
@@ -5563,6 +5573,19 @@ static int zend_jit_long_math_helper(zend_jit_ctx *jit,
55635573

55645574
ir_refs_init(res_inputs, 2);
55655575

5576+
if (Z_MODE(op1_addr) == IS_REG
5577+
&& Z_LOAD(op1_addr)
5578+
&& jit->ra[Z_SSA_VAR(op1_addr)].ref == IR_NULL) {
5579+
/* Force load */
5580+
zend_jit_use_reg(jit, op1_addr);
5581+
}
5582+
if (Z_MODE(op2_addr) == IS_REG
5583+
&& Z_LOAD(op2_addr)
5584+
&& jit->ra[Z_SSA_VAR(op2_addr)].ref == IR_NULL) {
5585+
/* Force load */
5586+
zend_jit_use_reg(jit, op2_addr);
5587+
}
5588+
55665589
if (op1_info & ((MAY_BE_ANY|MAY_BE_UNDEF)-MAY_BE_LONG)) {
55675590
if_long1 = jit_if_Z_TYPE(jit, op1_addr, IS_LONG);
55685591
ir_IF_TRUE(if_long1);
@@ -6090,6 +6113,10 @@ static int zend_jit_assign_op(zend_jit_ctx *jit,
60906113
ZEND_UNREACHABLE();
60916114
}
60926115

6116+
if (!zend_jit_store_var_if_necessary_ex(jit, opline->op1.var, op1_def_addr, op1_def_info, op1_addr, op1_info)) {
6117+
return 0;
6118+
}
6119+
60936120
if (op1_info & MAY_BE_REF) {
60946121
ir_MERGE_WITH(slow_path);
60956122
}

‎ext/opcache/jit/zend_jit_trace.c

Lines changed: 66 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -805,7 +805,12 @@ static bool zend_jit_trace_is_false_loop(const zend_op_array *op_array, const ze
805805
}
806806
}
807807

808-
static int zend_jit_trace_copy_ssa_var_info(const zend_op_array *op_array, const zend_ssa *ssa, const zend_op **tssa_opcodes, zend_ssa *tssa, int ssa_var)
808+
static int zend_jit_trace_copy_ssa_var_info(const zend_op_array *op_array,
809+
const zend_ssa *ssa,
810+
const zend_op **tssa_opcodes,
811+
zend_ssa *tssa,
812+
int ssa_var,
813+
const zend_op *opline)
809814
{
810815
int var, use, def, src;
811816
zend_ssa_op *op;
@@ -913,6 +918,54 @@ static int zend_jit_trace_copy_ssa_var_info(const zend_op_array *op_array, const
913918
assert(0);
914919
return 0;
915920
}
921+
if (opline) {
922+
/* Try to find a difinition in SSA dominators tree */
923+
var = tssa->vars[ssa_var].var;
924+
uint32_t op_num = opline - op_array->opcodes;
925+
uint32_t b = ssa->cfg.map[op_num];
926+
zend_basic_block *bb = ssa->cfg.blocks + b;
927+
zend_ssa_phi *pi, *phi;
928+
929+
while (1) {
930+
while (op_num > bb->start) {
931+
op_num--;
932+
op = ssa->ops + op_num;
933+
if (op->result_def >= 0 && ssa->vars[op->result_def].var == var) {
934+
src = op->result_def;
935+
goto copy_info;
936+
} else if (op->op2_def >= 0 && ssa->vars[op->op2_def].var == var) {
937+
src = op->op2_def;
938+
goto copy_info;
939+
} else if (op->op1_def >= 0 && ssa->vars[op->op1_def].var == var) {
940+
src = op->op1_def;
941+
goto copy_info;
942+
}
943+
}
944+
phi = ssa->blocks[b].phis;
945+
pi = NULL;
946+
while (phi) {
947+
if (ssa->vars[phi->ssa_var].var == var) {
948+
if (phi->pi >= 0) {
949+
pi = phi;
950+
} else {
951+
src = phi->ssa_var;
952+
goto copy_info;
953+
}
954+
}
955+
phi = phi->next;
956+
}
957+
if (pi) {
958+
src = pi->ssa_var;
959+
goto copy_info;
960+
}
961+
if (bb->idom < 0) {
962+
break;
963+
}
964+
b = bb->idom;
965+
bb = ssa->cfg.blocks + b;
966+
op_num = bb->start + bb->len;
967+
}
968+
}
916969
goto copy_info;
917970
}
918971
return 0;
@@ -1586,6 +1639,7 @@ static zend_ssa *zend_jit_trace_build_tssa(zend_jit_trace_rec *trace_buffer, uin
15861639

15871640
/* 4. Type inference */
15881641
op_array = trace_buffer->op_array;
1642+
opline = trace_buffer[1].opline;
15891643
jit_extension =
15901644
(zend_jit_op_array_trace_extension*)ZEND_FUNC_INFO(op_array);
15911645
ssa = &jit_extension->func_info.ssa;
@@ -1597,7 +1651,7 @@ static zend_ssa *zend_jit_trace_build_tssa(zend_jit_trace_rec *trace_buffer, uin
15971651
while (i < op_array->last_var) {
15981652
if (i < op_array->num_args) {
15991653
if (ssa->var_info
1600-
&& zend_jit_trace_copy_ssa_var_info(op_array, ssa, ssa_opcodes, tssa, i)) {
1654+
&& zend_jit_trace_copy_ssa_var_info(op_array, ssa, ssa_opcodes, tssa, i, NULL)) {
16011655
/* pass */
16021656
} else {
16031657
if (ssa->vars) {
@@ -1652,7 +1706,7 @@ static zend_ssa *zend_jit_trace_build_tssa(zend_jit_trace_rec *trace_buffer, uin
16521706
}
16531707
while (i < op_array->last_var + op_array->T) {
16541708
if (!ssa->var_info
1655-
|| !zend_jit_trace_copy_ssa_var_info(op_array, ssa, ssa_opcodes, tssa, i)) {
1709+
|| !zend_jit_trace_copy_ssa_var_info(op_array, ssa, ssa_opcodes, tssa, i, opline)) {
16561710
if (ssa->vars && i < ssa->vars_count) {
16571711
ssa_vars[i].alias = ssa->vars[i].alias;
16581712
} else {
@@ -1690,7 +1744,7 @@ static zend_ssa *zend_jit_trace_build_tssa(zend_jit_trace_rec *trace_buffer, uin
16901744

16911745
while (phi) {
16921746
if (!ssa->var_info
1693-
|| !zend_jit_trace_copy_ssa_var_info(op_array, ssa, ssa_opcodes, tssa, phi->ssa_var)) {
1747+
|| !zend_jit_trace_copy_ssa_var_info(op_array, ssa, ssa_opcodes, tssa, phi->ssa_var, NULL)) {
16941748
ssa_vars[phi->ssa_var].alias = ssa_vars[phi->sources[0]].alias;
16951749
ssa_var_info[phi->ssa_var].type = ssa_var_info[phi->sources[0]].type;
16961750
}
@@ -2409,7 +2463,7 @@ static zend_ssa *zend_jit_trace_build_tssa(zend_jit_trace_rec *trace_buffer, uin
24092463
ssa_vars[v].var = i;
24102464
if (i < op_array->num_args) {
24112465
if (ssa->var_info
2412-
&& zend_jit_trace_copy_ssa_var_info(op_array, ssa, ssa_opcodes, tssa, v)) {
2466+
&& zend_jit_trace_copy_ssa_var_info(op_array, ssa, ssa_opcodes, tssa, v, NULL)) {
24132467
/* pass */
24142468
} else {
24152469
ssa_vars[v].alias = zend_jit_var_may_alias(op_array, ssa, i);
@@ -2460,7 +2514,7 @@ static zend_ssa *zend_jit_trace_build_tssa(zend_jit_trace_rec *trace_buffer, uin
24602514
while (i < op_array->last_var) {
24612515
ssa_vars[v].var = i;
24622516
if (!ssa->var_info
2463-
|| !zend_jit_trace_copy_ssa_var_info(op_array, ssa, ssa_opcodes, tssa, v)) {
2517+
|| !zend_jit_trace_copy_ssa_var_info(op_array, ssa, ssa_opcodes, tssa, v, NULL)) {
24642518
ssa_var_info[v].type = MAY_BE_UNDEF | MAY_BE_RC1 | MAY_BE_RCN | MAY_BE_REF | MAY_BE_ANY | MAY_BE_ARRAY_KEY_ANY | MAY_BE_ARRAY_OF_ANY | MAY_BE_ARRAY_OF_REF;
24652519
}
24662520
i++;
@@ -2469,7 +2523,7 @@ static zend_ssa *zend_jit_trace_build_tssa(zend_jit_trace_rec *trace_buffer, uin
24692523
while (i < op_array->last_var + op_array->T) {
24702524
ssa_vars[v].var = i;
24712525
if (!ssa->var_info
2472-
|| !zend_jit_trace_copy_ssa_var_info(op_array, ssa, ssa_opcodes, tssa, v)) {
2526+
|| !zend_jit_trace_copy_ssa_var_info(op_array, ssa, ssa_opcodes, tssa, v, NULL)) {
24732527
ssa_var_info[v].type = MAY_BE_RC1 | MAY_BE_RCN | MAY_BE_REF | MAY_BE_ANY | MAY_BE_ARRAY_KEY_ANY | MAY_BE_ARRAY_OF_ANY | MAY_BE_ARRAY_OF_REF;
24742528
}
24752529
i++;
@@ -3308,13 +3362,16 @@ static void zend_jit_trace_cleanup_stack(zend_jit_ctx *jit, zend_jit_trace_stack
33083362
CLEAR_STACK_REF(stack, EX_VAR_TO_NUM(opline->op1.var));
33093363
}
33103364
if (ssa_op->op2_use >= 0
3365+
&& ssa_op->op2_use != ssa_op->op1_use
33113366
&& jit->ra[ssa_op->op2_use].ref
33123367
&& (jit->ra[ssa_op->op2_use].flags & ZREG_LAST_USE)
33133368
&& (ssa_op->op2_use_chain == -1
33143369
|| zend_ssa_is_no_val_use(ssa_opcodes[ssa_op->op2_use_chain], ssa->ops + ssa_op->op2_use_chain, ssa_op->op2_use))) {
33153370
CLEAR_STACK_REF(stack, EX_VAR_TO_NUM(opline->op2.var));
33163371
}
33173372
if (ssa_op->result_use >= 0
3373+
&& ssa_op->result_use != ssa_op->op1_use
3374+
&& ssa_op->result_use != ssa_op->op2_use
33183375
&& jit->ra[ssa_op->result_use].ref
33193376
&& (jit->ra[ssa_op->result_use].flags & ZREG_LAST_USE)
33203377
&& (ssa_op->res_use_chain == -1
@@ -4143,8 +4200,8 @@ static const void *zend_jit_trace(zend_jit_trace_rec *trace_buffer, uint32_t par
41434200
} else if (i < parent_vars_count
41444201
&& STACK_TYPE(parent_stack, i) != IS_UNKNOWN) {
41454202
/* This must be already handled by trace type inference */
4146-
ZEND_UNREACHABLE();
4147-
// SET_STACK_TYPE(stack, i, STACK_TYPE(parent_stack, i));
4203+
ZEND_ASSERT(ssa->vars[i].use_chain < 0 && !ssa->vars[i].phi_use_chain);
4204+
SET_STACK_TYPE(stack, i, STACK_TYPE(parent_stack, i), 1);
41484205
} else if ((info & MAY_BE_GUARD) != 0
41494206
&& (trace_buffer->stop == ZEND_JIT_TRACE_STOP_LOOP
41504207
|| trace_buffer->stop == ZEND_JIT_TRACE_STOP_RECURSIVE_CALL

0 commit comments

Comments
 (0)