From 1728fefffcd48c15340eacddad118bde6f53169b Mon Sep 17 00:00:00 2001 From: PurHur Date: Tue, 16 Jun 2026 02:12:15 +0000 Subject: [PATCH] Fix ?? on property hooks to read backing without get hook (#8902) Zend checks hooked-property backing for ?? / ??= before invoking get hooks. Wire compiler coalesce lowering and VM backing fetch; resolve php-cfg call-arg temp clones so var_dump($obj->hook ?? default) uses the coalesce result slot. Co-authored-by: Cursor --- lib/Compiler.php | 271 +++++++++++++++++- lib/OpCode.php | 4 +- lib/VM.php | 39 ++- .../language/property_hook_coalesce.phpt | 31 ++ test/repro/issue_property_hook_coalesce.php | 23 ++ 5 files changed, 358 insertions(+), 10 deletions(-) create mode 100644 test/compliance/cases/language/property_hook_coalesce.phpt create mode 100644 test/repro/issue_property_hook_coalesce.php diff --git a/lib/Compiler.php b/lib/Compiler.php index 78649911249..85808bdfb4a 100755 --- a/lib/Compiler.php +++ b/lib/Compiler.php @@ -1467,6 +1467,38 @@ protected function compileOps(array $ops, Block $block): void { } elseif ($this->isNullsafeChainArrayDimFetch($ops, $i)) { /** @var Op\Expr\ArrayDimFetch $child */ $block = $this->compileNullsafeArrayDimFetch($child, $block); + } elseif ( + $child instanceof Op\Expr\PropertyFetch + && $i + 1 < $opCount + && ($ops[$i + 1] instanceof Op\Expr\FuncCall || $ops[$i + 1] instanceof Op\Expr\NsFuncCall) + && $this->isPropertyFetchOnlyCoalesceFuncCallArg($child, $ops[$i + 1], $block) + ) { + break; + } elseif ( + $child instanceof Op\Expr\PropertyFetch + && null !== ($coalesceMatch = $this->findCoalesceUsingPropertyFetchLeft($child, $ops, $i)) + ) { + /** @var Op\Expr\BinaryOp\Coalesce $coalesce */ + [$coalesce, $coalesceIndex] = $coalesceMatch; + $resultOverride = null; + if ( + $coalesceIndex + 1 < $opCount + && $ops[$coalesceIndex + 1] instanceof Op\Expr\Assign + && $this->isCoalesceAssignTail($ops[$coalesceIndex + 1], $coalesce) + && $this->operandsChainEqual($ops[$coalesceIndex + 1]->var, $child->result) + ) { + /** @var Op\Expr\Assign $tailAssign */ + $tailAssign = $ops[$coalesceIndex + 1]; + $resultOverride = $tailAssign->var; + } + $block = null !== $resultOverride + ? $this->compileCoalesceForAssign($coalesce, $block, $resultOverride) + : $this->compileCoalesce($coalesce, $block); + $i = $coalesceIndex; + if (null !== $resultOverride) { + ++$i; + } + break; } elseif ( $child instanceof Op\Expr\ArrayDimFetch && null !== ($coalesceMatch = $this->findCoalesceUsingArrayDimFetchLeft($child, $ops, $i)) @@ -2038,6 +2070,80 @@ private function isArrayDimFetchOnlyCoalesceLeft( return $left === $fetch->result; } + private function isPropertyFetchOnlyCoalesceLeft( + Op\Expr\PropertyFetch $fetch, + Op $next + ): bool { + if (!$next instanceof Op\Expr\BinaryOp\Coalesce) { + return false; + } + $left = $next->left; + while ($left instanceof Temporary) { + if ($left === $fetch->result) { + return true; + } + if (null === $left->original) { + break; + } + $left = $left->original; + } + + return $left === $fetch->result; + } + + /** + * php-cfg may emit RHS expr stmts between PropertyFetch and Coalesce (#8902). + * + * @param Op[] $ops + * + * @return ?array{0: Op\Expr\BinaryOp\Coalesce, 1: int} + */ + private function findCoalesceUsingPropertyFetchLeft( + Op\Expr\PropertyFetch $fetch, + array $ops, + int $index + ): ?array { + $count = count($ops); + for ($j = $index + 1; $j < $count; ++$j) { + $next = $ops[$j]; + if ($next instanceof Op\Expr\BinaryOp\Coalesce) { + if (!$this->isPropertyFetchOnlyCoalesceLeft($fetch, $next)) { + return null; + } + + return [$next, $j]; + } + if ($this->isLoweredByFollowingCoalesce($next, $ops, $j)) { + continue; + } + + return null; + } + + return null; + } + + private function isPropertyFetchOnlyCoalesceFuncCallArg( + Op\Expr\PropertyFetch $fetch, + Op $call, + Block $block + ): bool { + if (!$call instanceof Op\Expr\FuncCall && !$call instanceof Op\Expr\NsFuncCall) { + return false; + } + if (!property_exists($call, 'args') || !is_array($call->args)) { + return false; + } + foreach ($call->args as $arg) { + $coalesce = $this->findCoalesceStmtForCallArg($arg, $block); + if (null !== $coalesce && $this->findCoalescePropertyFetch($coalesce->left, $block) === $fetch) { + return true; + } + } + + return false; + } + /** * php-cfg may emit RHS expr stmts (FuncCall, …) between ArrayDimFetch and Coalesce (#4416). * @@ -2280,6 +2386,10 @@ private function findEmbeddedCoalesces(Operand $operand): array if (null !== $coalesce) { $found[] = $coalesce; } + $root = $this->unwrapOperandChain($operand); + if ($root instanceof Op\Expr\BinaryOp\Coalesce) { + $found[] = $root; + } $concat = $this->unwrapConcatListExpr($operand); if (null !== $concat) { foreach ($concat->list as $part) { @@ -7086,8 +7196,6 @@ protected function compileCoalesce( ); if ( null !== $propFetch - && null !== $resultOverride - && $this->operandsChainEqual($resultOverride, $propFetch->result) ) { $issetOp->issetForCoalesceAssign = true; } @@ -7164,7 +7272,7 @@ protected function compileCoalesce( )); } } elseif (null !== $propFetch) { - $this->compilePropertyFetchRead($propFetch, $leftBlock); + $this->compilePropertyFetchRead($propFetch, $leftBlock, true); $leftSlot = $this->compileOperand($propFetch->result, $leftBlock, true); if (!$this->operandsChainEqual($resultOperand, $expr->left)) { $leftBlock->addOpCode(new OpCode( @@ -7214,14 +7322,21 @@ protected function compileCoalesce( /** * Emit a read fetch in $block (used by ?? left branch when the stmt fetch was skipped). */ - private function compilePropertyFetchRead(Op\Expr\PropertyFetch $fetch, Block $block): void - { - $block->addOpCode(new OpCode( + private function compilePropertyFetchRead( + Op\Expr\PropertyFetch $fetch, + Block $block, + bool $propertyHookCoalesceRead = false + ): void { + $op = new OpCode( OpCode::TYPE_PROPERTY_FETCH, $this->compileOperand($fetch->result, $block, false), $this->compileOperand($fetch->var, $block, true), $this->compileOperand($fetch->name, $block, true) - )); + ); + if ($propertyHookCoalesceRead) { + $op->propertyHookCoalesceRead = true; + } + $block->addOpCode($op); } /** @@ -8715,6 +8830,123 @@ private function findInlineArrayProducerForCallArg(Operand $arg, Block $block): return null; } + /** + * @return ?Op\Expr\BinaryOp\Coalesce + */ + private function findCoalesceStmtForCallArg(Operand $arg, Block $block): ?Op\Expr\BinaryOp\Coalesce + { + $coalesce = $this->unwrapCoalesceExpr($arg); + if (null !== $coalesce) { + return $coalesce; + } + if (null === $block->orig) { + return null; + } + foreach ($block->orig->children as $child) { + if ( + $child instanceof Op\Expr\BinaryOp\Coalesce + && ($child->result === $arg || $this->operandsReferToSameVariable($child->result, $arg)) + ) { + return $child; + } + } + // php-cfg clones call-arg temps from stmt Coalesce result (#8766, #8902). + foreach ($block->orig->children as $i => $child) { + if ( + !($child instanceof Op\Expr\FuncCall || $child instanceof Op\Expr\NsFuncCall) + || !property_exists($child, 'args') + || !is_array($child->args) + ) { + continue; + } + $argMatches = false; + foreach ($child->args as $callArg) { + if ($callArg === $arg || $this->operandsReferToSameVariable($callArg, $arg)) { + $argMatches = true; + break; + } + } + if (!$argMatches) { + continue; + } + for ($j = $i - 1; $j >= 0; --$j) { + $prev = $block->orig->children[$j]; + if ($prev instanceof Op\Expr\BinaryOp\Coalesce) { + return $prev; + } + if (!$prev instanceof Op\Expr || !$this->isInlineExprCallArgProducer($prev)) { + break; + } + } + } + + return null; + } + + private function slotForCoalesceResult(Block $block, Op\Expr\BinaryOp\Coalesce $coalesce): ?int + { + $slot = $block->slotForOperand($coalesce->result); + if (null !== $slot) { + return $slot; + } + $seen = []; + $queue = [$block]; + while ([] !== $queue) { + $current = array_shift($queue); + $id = spl_object_id($current); + if (isset($seen[$id])) { + continue; + } + $seen[$id] = true; + foreach ($current->opCodes as $op) { + if (OpCode::TYPE_COALESCE === $op->type) { + return $op->arg1; + } + } + foreach ($current->parents as $parent) { + $queue[] = $parent; + } + } + + return null; + } + + private function compileCallArgCoalesceSlot(Operand $arg, Block $block): ?int + { + $coalesce = $this->findCoalesceStmtForCallArg($arg, $block); + if (null === $coalesce) { + return null; + } + $coalesceSlot = $this->slotForCoalesceResult($block, $coalesce); + if (null === $coalesceSlot) { + $this->compileCoalesce($coalesce, $block); + $coalesceSlot = $this->slotForCoalesceResult($block, $coalesce); + } + + return $coalesceSlot; + } + + /** + * @param list $args + */ + private function lowerEmbeddedCoalesceCallArgs(array $args, Block $block): void + { + foreach ($args as $arg) { + foreach ($this->findEmbeddedCoalesces($arg) as $coalesce) { + if (null === $block->slotForOperand($coalesce->result)) { + $this->compileCoalesce($coalesce, $block); + } + } + $stmtCoalesce = $this->findCoalesceStmtForCallArg($arg, $block); + if ( + null !== $stmtCoalesce + && null === $block->slotForOperand($stmtCoalesce->result) + ) { + $this->compileCoalesce($stmtCoalesce, $block); + } + } + } + private function findInlineExprCallArgProducerSlot(Operand $arg, Block $block): ?string { if (null === $block->orig) { @@ -8728,6 +8960,13 @@ private function findInlineExprCallArgProducerSlot(Operand $arg, Block $block): if (!property_exists($callOp, 'args') || !is_array($callOp->args)) { return null; } + $coalesceArg = $this->findCoalesceStmtForCallArg($arg, $block); + if (null !== $coalesceArg) { + $coalesceSlot = $this->compileCallArgCoalesceSlot($arg, $block); + if (null !== $coalesceSlot) { + return $coalesceSlot; + } + } $producers = $this->precedingInlineCallArgProducersBeforeCfgOp($block->orig->children, $callOp); $producer = $this->matchInlineCallArgProducer($producers, $callOp->args, $argIndex); if (null === $producer) { @@ -8813,6 +9052,17 @@ private function findInlineExprCallArgProducerSlot(Operand $arg, Block $block): } } if ($producer instanceof Op\Expr\PropertyFetch || $producer instanceof Op\Expr\StaticPropertyFetch) { + $coalesceStmt = $this->findCoalesceStmtForCallArg($arg, $block); + if ( + null !== $coalesceStmt + && $producer instanceof Op\Expr\PropertyFetch + && $this->findCoalescePropertyFetch($coalesceStmt->left, $block) === $producer + ) { + $coalesceSlot = $this->compileCallArgCoalesceSlot($arg, $block); + if (null !== $coalesceSlot) { + return $coalesceSlot; + } + } $callIndex = null; $producerIndex = null; foreach ($block->orig->children as $i => $child) { @@ -10986,7 +11236,10 @@ protected function compileCallArgSends(array $args, Block $block, ?string $calle if ([] !== $prefetchOps) { $valueSlot = $prefetchOps[0]->arg1; } else { - $valueSlot = $this->findInlineExprCallArgProducerSlot($arg, $block); + $valueSlot = $this->compileCallArgCoalesceSlot($arg, $block); + if (null === $valueSlot) { + $valueSlot = $this->findInlineExprCallArgProducerSlot($arg, $block); + } if (null === $valueSlot) { if ( null === $calleeName @@ -11338,6 +11591,8 @@ protected function compileFuncCall(?int $name, array $args, Operand $result, Blo $calleeName = $this->resolveCompileTimeStringSlot($callName, $block) ?? ($name !== null ? $this->resolveCompileTimeStringSlot($name, $block) : null); + $this->lowerEmbeddedCoalesceCallArgs($args, $block); + $return = [new OpCode(OpCode::TYPE_FUNCCALL_INIT, $callName)]; foreach ($this->compileCallArgSends($args, $block, $calleeName) as $send) { $return[] = $send; diff --git a/lib/OpCode.php b/lib/OpCode.php index 89251801811..fda3ed51a4b 100755 --- a/lib/OpCode.php +++ b/lib/OpCode.php @@ -265,8 +265,10 @@ class OpCode { public bool $isIncDec = false; /** isset()/empty() on PropertyFetch, not ArrayDimFetch (issue #5117, zend_hash.c). */ public bool $issetOnProperty = false; - /** ??= on hooked properties: null-check backing storage, not get-hook value (#6472). */ + /** ?? / ??= on hooked properties: null-check backing storage, not get-hook value (#6472, #8902). */ public bool $issetForCoalesceAssign = false; + /** ?? / ??= left branch: read backing storage, not get-hook value (#6472, #8902). */ + public bool $propertyHookCoalesceRead = false; /** TYPE_PROPERTY_FETCH in a ?-> fetch arm must read typed slots (#5361, zend_object_handlers.c). */ public bool $nullsafeFetchPropertyRead = false; /** diff --git a/lib/VM.php b/lib/VM.php index b0463bec30a..c3a5f808236 100755 --- a/lib/VM.php +++ b/lib/VM.php @@ -629,7 +629,7 @@ public function objectPropertyIsSet(ObjectEntry $object, string $propName, ?Fram } /** - * ??= on property hooks — Zend checks backing null/uninit, not get-hook return (#6472). + * ?? / ??= on property hooks — Zend checks backing null/uninit, not get-hook return (#6472, #8902). */ public function objectPropertyIsSetForCoalesceAssign(ObjectEntry $object, string $propName, ?Frame $frame = null): bool { @@ -665,6 +665,38 @@ public function objectPropertyIsSetForCoalesceAssign(ObjectEntry $object, string return $this->objectPropertyIsSet($object, $propName, $frame); } + /** + * ?? / ??= left branch on property hooks — read backing without get hook (#6472, #8902). + */ + public function fetchObjectPropertyForCoalesce(ObjectEntry $object, string $propName, Variable $dst): void + { + $lcClass = strtolower($object->class->name); + $propMeta = $this->context->propertyHookRegistry[$lcClass][$propName] + ?? $this->context->propertyHookRegistry[$lcClass][strtolower($propName)] + ?? null; + if (is_array($propMeta)) { + $backingName = $propMeta['setBacking'] ?? $propMeta['getBacking'] ?? null; + if (null !== $backingName && $object->hasProperty($backingName)) { + $dst->copyFrom($object->getProperty($backingName)); + + return; + } + } + $meta = $this->classPropertyMeta($object, $propName); + if (null !== $meta && (null !== $meta->getHookMethodLc || null !== $meta->setHookMethodLc)) { + if ($object->hasProperty($propName)) { + $dst->copyFrom($object->getProperty($propName)); + + return; + } + } + if ($object->hasProperty($propName)) { + $dst->copyFrom($object->getProperty($propName)); + } else { + $dst->undefined(); + } + } + /** * empty($obj->prop) — uninitialized typed slots are empty without read (#6787, zend_object_handlers.c); * dynamic / __isset-only properties keep isset semantics (#3298). @@ -4888,6 +4920,7 @@ private function runFramesInner(): int } $forWrite = $this->propertyFetchDestUsedAsAssignLvalue($frame, $op); $magicGetForRead = !$forWrite + && !$op->propertyHookCoalesceRead && $this->propertyReadUsesMagicGet($propertyObject, $name, $frame); if (!$magicGetForRead && !$forWrite) { $catchFrame = $this->enforcePropertyVisibilityRead($propertyObject, $name, $frame); @@ -4896,6 +4929,10 @@ private function runFramesInner(): int goto restart; } } + if ($op->propertyHookCoalesceRead && !$forWrite) { + $this->fetchObjectPropertyForCoalesce($propertyObject, $name, $result); + break; + } if ($propertyObject->hasProperty($name) && !$magicGetForRead) { if (!$forWrite) { $this->emitInstancePropertyAccessDeprecation($propertyObject, $name, $frame); diff --git a/test/compliance/cases/language/property_hook_coalesce.phpt b/test/compliance/cases/language/property_hook_coalesce.phpt new file mode 100644 index 00000000000..bd6485c3c3e --- /dev/null +++ b/test/compliance/cases/language/property_hook_coalesce.phpt @@ -0,0 +1,31 @@ +--TEST-- +Property hook ?? — null-check backing without get hook (#8902, zend_property_hooks.c) +--FILE-- + $this->backing = $value; + } + private string $backing = 'a'; +} +$c = new C(); +var_dump($c->x ?? 'default'); +echo "ok\n"; + +class U { + public string $x { + get { throw new Exception('get must not run for unset ??'); } + set => $this->backing = $value; + } + private string $backing; +} +$u = new U(); +unset($u->x); +var_dump($u->x ?? 'default'); +echo "unset ok\n"; +--EXPECT-- +string(1) "a" +ok +string(7) "default" +unset ok diff --git a/test/repro/issue_property_hook_coalesce.php b/test/repro/issue_property_hook_coalesce.php new file mode 100644 index 00000000000..95e8d7149af --- /dev/null +++ b/test/repro/issue_property_hook_coalesce.php @@ -0,0 +1,23 @@ + $this->backing = $value; + } + private string $backing = 'a'; +} +$c = new C(); +var_dump($c->x ?? 'default'); +echo "ok\n"; + +class U { + public string $x { + get { throw new Exception('get must not run for unset ??'); } + set => $this->backing = $value; + } + private string $backing; +} +$u = new U(); +unset($u->x); +var_dump($u->x ?? 'default'); +echo "unset ok\n";