Skip to content

Commit 9c2acc3

Browse files
committed
Fix GH-23693: JIT guard branches on stale flags across basic blocks
The x86 matcher folds v = BINOP(a, b); c = CMP(v, 0); GUARD(c) into IR_GUARD_JCC_INT, emitting the BINOP, dropping the CMP and branching on the flags the BINOP left, but it only required the BINOP to precede the CMP in the IR. Once GCM hoists a loop-invariant BINOP into a dominating block, the jcc reads flags the intervening code has clobbered and the guard fires on whatever is in EFLAGS. Require the BINOP to sit in the guard's block and allow only snapshots between the comparison and the guard, the way ir_match_fuse_load() pairs ir_in_same_block() with ir_match_has_mem_deps(). The sibling MEM_BINOP fold already checks the block, the IF side folds are pinned by full ref adjacency, and ir_aarch64.dasc has no guard fold. Mirrors the upstream fix dstogov/ir@51107a3. Fixes GH-23693 Closes GH-23711
1 parent 3213c3f commit 9c2acc3

3 files changed

Lines changed: 61 additions & 1 deletion

File tree

‎NEWS‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ PHP NEWS
3737
- Opcache:
3838
. Fixed OSS-Fuzz #546798343 (Heap-buffer-overflow in optimizer with
3939
FCCs and inlining). (ndossche)
40+
. Fixed bug GH-23693 (Tracing JIT produces wrong results for a guard on a
41+
loop-invariant addition). (Ilia Alshanetsky)
4042

4143
- PDO:
4244
. Fixed PDOStatement::getColumnMeta() reading out of bounds for an invalid

‎ext/opcache/jit/ir/ir_x86.dasc‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1923,6 +1923,21 @@ static bool ir_match_has_mem_deps(ir_ctx *ctx, ir_ref ref, ir_ref root)
19231923
return 0;
19241924
}
19251925

1926+
/* A naive check if anything that emits code, and so clobbers the flags, is
1927+
* scheduled between the flags setting instruction and the fusion root */
1928+
static bool ir_match_has_flags_deps(ir_ctx *ctx, ir_ref ref, ir_ref root)
1929+
{
1930+
ir_ref pos = ctx->prev_ref[root];
1931+
1932+
while (pos > ref) {
1933+
if (ctx->ir_base[pos].op != IR_SNAPSHOT) {
1934+
return 1;
1935+
}
1936+
pos = ctx->prev_ref[pos];
1937+
}
1938+
return pos != ref;
1939+
}
1940+
19261941
static void ir_match_fuse_load(ir_ctx *ctx, ir_ref ref, ir_ref root)
19271942
{
19281943
if (ir_in_same_block(ctx, ref) &&
@@ -3089,7 +3104,9 @@ store_int:
30893104
if (IR_IS_CONST_REF(op2_insn->op2)
30903105
&& !IR_IS_SYM_CONST(ctx->ir_base[op2_insn->op2].op)
30913106
&& ctx->ir_base[op2_insn->op2].val.i64 == 0) {
3092-
if (op2_insn->op1 == insn->op2 - 1) { /* previous instruction */
3107+
if (op2_insn->op1 == insn->op2 - 1 /* previous instruction */
3108+
&& ir_in_same_block(ctx, op2_insn->op1)
3109+
&& !ir_match_has_flags_deps(ctx, insn->op2, ref)) {
30933110
ir_insn *op1_insn = &ctx->ir_base[op2_insn->op1];
30943111

30953112
if ((op1_insn->op == IR_OR || op1_insn->op == IR_AND || op1_insn->op == IR_XOR) ||

‎ext/opcache/tests/jit/gh23693.phpt‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
--TEST--
2+
GH-23693: Tracing JIT reads stale flags for a guard on a hoisted addition
3+
--INI--
4+
opcache.enable=1
5+
opcache.enable_cli=1
6+
opcache.jit_buffer_size=64M
7+
opcache.jit=tracing
8+
opcache.jit_hot_func=1
9+
--EXTENSIONS--
10+
opcache
11+
--FILE--
12+
<?php
13+
function f(int $pos, int $n): int {
14+
if ($pos < 0) {
15+
return -1;
16+
}
17+
$y = $pos >> 2;
18+
$s = 0;
19+
for ($dy = -1; $dy <= 1; ++$dy) {
20+
for ($i = 0; $i < $n; ++$i) {
21+
$ny = $y + $dy;
22+
if ($ny < 0) {
23+
continue;
24+
}
25+
if ($ny >= 4) {
26+
continue;
27+
}
28+
$s += ($ny << 2) | ($i & 3);
29+
}
30+
}
31+
return $s;
32+
}
33+
for ($k = 0; $k < 30; ++$k) {
34+
f(16, 200);
35+
}
36+
var_dump(f(16, 200));
37+
var_dump(f(0, 200));
38+
?>
39+
--EXPECT--
40+
int(2700)
41+
int(1400)

0 commit comments

Comments
 (0)