Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions lib/JIT.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
162 changes: 162 additions & 0 deletions lib/JIT/IncDecResourceProvenance.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
<?php

declare(strict_types=1);

namespace PHPCompiler\JIT;

use PHPCfg\Op;
use PHPCfg\Operand;

/**
* Prove that a native long cannot be a resource handle, so ++/-- can skip its guard (#23483).
*
* This compiler stores resource handles *as* native longs, and php-types has no resource type at
* all — `Type::TYPE_LONG` covers both an `int` and an open stream handle. So `++$x` on a
* TYPE_NATIVE_LONG genuinely cannot tell a loop counter from the result of `fopen()`, and
* {@see \PHPCompiler\JIT::guardIncDecResourceOperand()} conservatively guards every single ++/--
* with `__compiler_is_resource`.
*
* That guard is not cheap: it calls into {@see \PHPCompiler\ext\standard\StreamLifecycleJitHelper}
* which walks up to four separate handle registries. In
* `for ($i = 0; $i < 1000000; ++$i) { ++$a; }` that is two such calls per iteration, and measurably
* ~92% of the loop's runtime (135ms -> 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<int, true> $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<int, true> $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;
}
}
55 changes: 55 additions & 0 deletions test/differential/cases/g07_incdec_resource_provenance.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
<?php
// #23483: ++/-- guards every native long with __compiler_is_resource, because resource handles
// ARE native longs here and php-types has no resource type. IncDecResourceProvenance elides that
// guard when the value provably comes from a literal or from arithmetic.
//
// The way that elision could go wrong is by dropping the guard on something that really is a
// resource, so this keeps live handles open while incrementing integers whose values collide with
// plausible handle ids (1, 2, 3 ...), and also increments across loop phis and function scope.
$fh = fopen('php://memory', 'r+');
$fh2 = fopen('php://memory', 'r+');

$a = 1;
++$a;
$b = 2;
++$b;
$c = 3;
++$c;
$d = 4;
--$d;
echo "$a $b $c $d\n";

// loop phi: counter and accumulator both flow literal -> 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";