diff --git a/lib/JIT.php b/lib/JIT.php index d8f28301c24..20dbb0ef81e 100644 --- a/lib/JIT.php +++ b/lib/JIT.php @@ -15398,7 +15398,7 @@ private function compileIncDecOp(Block $block, OpCode $op, bool $increment, bool Variable::TYPE_VALUE === $read->type && (Variable::KIND_VARIABLE === $read->kind || $read->functionStaticGlobal) ) { - $this->guardIncDecResourceOperand($read, $increment); + $this->guardIncDecResourceOperand($read, $increment, $readOp); $readPtr = JIT\JitValueBox::valuePtrFromVariable($this->context, $read); $cur = $this->readIncDecValueBoxLong($read, $readPtr, $increment); $one = $cur->typeOf()->constInt(1, false); @@ -15435,7 +15435,7 @@ private function compileIncDecOp(Block $block, OpCode $op, bool $increment, bool } if (Variable::TYPE_NATIVE_LONG === $read->type && Variable::KIND_VARIABLE === $read->kind) { - $this->guardIncDecResourceOperand($read, $increment); + $this->guardIncDecResourceOperand($read, $increment, $readOp); $cur = $this->context->helper->loadValue($read); $one = $cur->typeOf()->constInt(1, false); $newLong = $increment @@ -15528,11 +15528,20 @@ private function readIncDecValueBoxLong( } /** Reject ++/-- on stream/dir handles (issue #6396, zend_operators.c). */ - private function guardIncDecResourceOperand(JIT\Variable $read, bool $increment): void + private function guardIncDecResourceOperand( + JIT\Variable $read, + bool $increment, + ?Operand $readOp = null + ): void { if (JIT\NestedJitCompileScope::isActive()) { return; } + // A value that provably came from a literal or from arithmetic cannot be a resource handle, + // so the guard is dead code — and it is expensive enough to dominate hot loops (#23483). + if (JIT\IncDecResourceProvenance::cannotBeResource($readOp)) { + return; + } $longVal = null; if (JIT\Variable::TYPE_NATIVE_LONG === $read->type) { $longVal = $this->context->helper->loadValue($read); diff --git a/lib/JIT/IncDecResourceProvenance.php b/lib/JIT/IncDecResourceProvenance.php new file mode 100644 index 00000000000..f2d78d37264 --- /dev/null +++ b/lib/JIT/IncDecResourceProvenance.php @@ -0,0 +1,162 @@ + 11ms on build/micro/m_loop.php once elided). It also blocks + * every LLVM optimisation, because an opaque call in the loop body stops the counter allocas from + * being promoted out of memory. + * + * Resources are only ever *introduced* by a small set of builtins (fopen, opendir, proc_open, ...). + * A value flowing from an integer literal or from arithmetic therefore cannot be one, and the guard + * is dead code there. This walks the php-cfg producers of the read operand to prove exactly that. + * + * The analysis is deliberately conservative: anything it does not recognise — a call, a parameter, + * a property or array read — answers "unknown" and keeps the guard. Only the listed + * value-producing ops, which cannot yield a resource under any input, allow it to be dropped. + */ +final class IncDecResourceProvenance +{ + /** + * Ops whose result is a number, string, bool, array or object — never a resource handle. + * + * Coalesce is deliberately absent: `$x ?? fopen(...)` produces whatever the right side does. + * Assign and Phi are handled separately because they forward another operand's provenance. + */ + private const NON_RESOURCE_OPS = [ + Op\Expr\BinaryOp\Plus::class => true, + Op\Expr\BinaryOp\Minus::class => true, + Op\Expr\BinaryOp\Mul::class => true, + Op\Expr\BinaryOp\Div::class => true, + Op\Expr\BinaryOp\Mod::class => true, + Op\Expr\BinaryOp\Pow::class => true, + Op\Expr\BinaryOp\ShiftLeft::class => true, + Op\Expr\BinaryOp\ShiftRight::class => true, + Op\Expr\BinaryOp\BitwiseAnd::class => true, + Op\Expr\BinaryOp\BitwiseOr::class => true, + Op\Expr\BinaryOp\BitwiseXor::class => true, + Op\Expr\BinaryOp\Concat::class => true, + Op\Expr\BinaryOp\Equal::class => true, + Op\Expr\BinaryOp\NotEqual::class => true, + Op\Expr\BinaryOp\Identical::class => true, + Op\Expr\BinaryOp\NotIdentical::class => true, + Op\Expr\BinaryOp\Greater::class => true, + Op\Expr\BinaryOp\GreaterOrEqual::class => true, + Op\Expr\BinaryOp\Smaller::class => true, + Op\Expr\BinaryOp\SmallerOrEqual::class => true, + Op\Expr\BinaryOp\Spaceship::class => true, + Op\Expr\BinaryOp\LogicalXor::class => true, + Op\Expr\UnaryMinus::class => true, + Op\Expr\UnaryPlus::class => true, + Op\Expr\BitwiseNot::class => true, + Op\Expr\BooleanNot::class => true, + Op\Expr\PreInc::class => true, + Op\Expr\PostInc::class => true, + Op\Expr\PreDec::class => true, + Op\Expr\PostDec::class => true, + Op\Expr\ConcatList::class => true, + Op\Expr\Array_::class => true, + Op\Expr\Cast\Int_::class => true, + Op\Expr\Cast\Double::class => true, + Op\Expr\Cast\String_::class => true, + Op\Expr\Cast\Bool_::class => true, + Op\Expr\Cast\Array_::class => true, + ]; + + /** Bound on the producer walk, so a pathological CFG cannot cost more than the guard saves. */ + private const MAX_VISITS = 64; + + public static function cannotBeResource(?Operand $op): bool + { + if (!$op instanceof Operand) { + return false; + } + $seen = []; + $budget = self::MAX_VISITS; + + return self::operandIsSafe($op, $seen, $budget); + } + + /** + * @param array $seen + */ + private static function operandIsSafe(Operand $op, array &$seen, int &$budget): bool + { + if (--$budget < 0) { + return false; + } + $id = spl_object_id($op); + if (isset($seen[$id])) { + // Loop back-edge. Resource-ness can only be *introduced* by a producing op, and a cycle + // introduces nothing, so this edge carries no counter-example: the remaining edges of + // the phi still have to prove the property on their own. + return true; + } + $seen[$id] = true; + + if ($op instanceof Operand\Literal) { + // Literals are parser-level scalars; a resource has no literal syntax. + return true; + } + + $producers = $op->ops ?? []; + if ([] === $producers) { + // No producer in this CFG — a parameter, a bound closure var, or an operand written + // somewhere the analysis cannot see. Keep the guard. + return false; + } + + foreach ($producers as $producer) { + if (!self::producerIsSafe($producer, $seen, $budget)) { + return false; + } + } + + return true; + } + + /** + * @param array $seen + */ + private static function producerIsSafe(Op $producer, array &$seen, int &$budget): bool + { + if (--$budget < 0) { + return false; + } + if (isset(self::NON_RESOURCE_OPS[$producer::class])) { + return true; + } + if ($producer instanceof Op\Phi) { + foreach ($producer->vars as $var) { + if (!$var instanceof Operand || !self::operandIsSafe($var, $seen, $budget)) { + return false; + } + } + + return true; + } + if ($producer instanceof Op\Expr\Assign) { + $expr = $producer->expr ?? null; + + return $expr instanceof Operand && self::operandIsSafe($expr, $seen, $budget); + } + + return false; + } +} diff --git a/test/differential/cases/g07_incdec_resource_provenance.php b/test/differential/cases/g07_incdec_resource_provenance.php new file mode 100644 index 00000000000..2233f4226c8 --- /dev/null +++ b/test/differential/cases/g07_incdec_resource_provenance.php @@ -0,0 +1,55 @@ + phi -> ++ -> phi +function counted(): int +{ + $acc = 0; + for ($i = 0; $i < 5; ++$i) { + ++$acc; + } + + return $acc; +} +echo counted(), "\n"; + +// arithmetic provenance, post/pre forms, and a decrement below zero +$x = 10 - 7; +$x++; +$y = 2 * 3; +$y--; +echo "$x $y\n"; + +$n = 0; +$n--; +$n--; +echo $n, "\n"; + +// resources still usable after all that incrementing +fwrite($fh, 'alpha'); +rewind($fh); +echo fread($fh, 5), "\n"; +fwrite($fh2, 'beta'); +rewind($fh2); +echo fread($fh2, 4), "\n"; +fclose($fh); +fclose($fh2); +echo "done\n";