[php-src] PR #23711: Fix GH-23693: JIT guard branches on stale flags across basic blocks
| From: | iliaal | Date: | Wed, 16 Sep 2026 17:42:27 +0000 |
| Subject: | [php-src] PR #23711: Fix GH-23693: JIT guard branches on stale flags across basic blocks | ||
| Groups: | php.git-pulls | ||
| Request: | Send a blank email to git-pulls+get-38516@lists.php.net to get a copy of this message | ||
Pull Request: https://github.com/php/php-src/pull/23711
Author: iliaal
The x86 instruction matcher folds
v = BINOP(a, b); c = CMP(v, 0); GUARD(c) into a bare
conditional jump on the flags the BINOP left behind, checking only that the BINOP precedes the CMP
in the IR. Once GCM hoists a loop-invariant BINOP into a dominating block, the jump reads flags the
intervening code has clobbered. The reporter's recursive board search loses if ($newY
< 0) continue;, so $newPos reaches the callee as -3 and the function throws
on an argument it has already proved non-negative.
This is already merged upstream as dstogov/ir@51107a3 (dstogov/ir#204) and the hunk here is
byte-identical, so the next IR sync is a no-op against it. Landing it directly because 8.5 ships the
miscompile today.
The fold now requires the BINOP in the guard's own block, with 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(). Across the
reporter's script, the new test, Zend/bench.php and
Zend/micro_bench.php the fold fires four times before and twice after; the two it loses
are the miscompiles. The upstream PR carries the IR-level regression test as
tests/x86_64/guard_001.irt, which php-src does not vendor.
PHP-8.4 compiles the same rule but does not hoist the addition under any of the 20 hot-counter
combinations tried, so the test is green there and this targets 8.5. Unrelated, noticed while
auditing the same family: ext/opcache/jit/ir/ir_gcm.c:598 loops on n <
0 where upstream has n > 0, so the "OVERFLOW must be scheduled into the
same block" pin never runs. That one is stale sync drift, not something this PR touches.
Fixes #23693