fix(type): read opline operands off the znode_op union instead of casting it - #229
Merged
Conversation
…ting it
OpLine::getValuePointer() reached the operand through Core::cast('znode_op *',
$node). $node IS the znode_op union already, so that asks FFI to reinterpret a
4-byte union VALUE as an 8-byte pointer, and FFI answers "attempt to cast to
larger type" - for every IS_CV/IS_VAR/IS_TMP_VAR operand and, through
getRuntimeConstant(), for every runtime constant. Reading an opline operand
from a user opcode handler has been impossible since the struct-stub migration
(#190). The fields are now read straight off the union.
Consumers meet that failure as silence rather than as an error, which is what
made it expensive to find: a user opcode handler runs inside an FFI callback,
where an escaping throw is a fatal engine abort, so every serious consumer
wraps its handler in a catch-all. The exception disappears there and the
symptom is an instruction that never seems to arrive. Downstream in ZDebug it
looked like exception breakpoints and return-value debugging had silently
stopped firing, with nothing in any log.
The rest of the change is what sharper types then exposed. The redundant
'zend_op *' cast on an already-typed opline is gone, the two operand params
carry their generated stub views, and the zval cast uses the stub-class form
that types the handle for analysis. Two baseline entries went with it: the
'mixed' they recorded came from the casts this removes.
Covered by two tests on OpCodeHookTest - reading a compiled-variable operand
and reading a constant one - both of which fail on master with the cast error.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JCNB8eDgS6NEnfCFw7tPjU
Reading the operand off the union types $constantOffset as int, which turns
OpLine's pointer arithmetic into the same "Binary operation + between FFI\CData
and int" that Compiler.php and ExecutionData.php already carry for the identical
Core::cast('char *', ...) + int construct. Recorded the same way they are, and
the "and mixed" entry it replaces is dropped.
Not verifiable outside CI: the analyser available in this environment infers
Core::cast() differently and reports neither message, so CI's PHPStan leg is the
only thing that can confirm the entry matches.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JCNB8eDgS6NEnfCFw7tPjU
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
Ports #227 to the
8.4line.OpLine::getValuePointer()reached the operandfields by re-casting the
znode_opit had already been handed:$nodeis already aznode_op(a 4-byte union), not a pointer to one, soCore::cast('znode_op *', …)asks FFI to widen a 4-byte value into an 8-bytepointer and gets
FFI\Exception: attempt to cast to larger type. Every read ofan
IS_CV,IS_VAR,IS_TMP_VARorIS_CONSToperand from a user opcodehandler therefore threw. The fields are read directly now, and the same
already-typed-value mistake in the
zend_opandzvalcasts alongside them isremoved.
Two regression tests cover the paths that were broken — reading a compiled
variable operand and reading a constant operand from inside an installed
handler. Both fail on
8.4before this change with the cast exception above.The PHPStan baseline drops the two
OpLineentries the old casts produced andgains a
Binary operation "+" between FFI\CData and intentry, matching howCompiler.phpandExecutionData.phpalready baseline the identicalchar* + intconstruct.Found downstream: zdebug's
mainwent red on both minors, with return-valuedebugging and exception breakpoints failing because
ReturnHookcould no longerread the
RETURNopline's op1.Note on the branch-flow rule below: this landed on
masterfirst (#227) and isbeing cascaded down, which is the wrong direction. The bug is identical on both
lines and the diff cherry-picked cleanly, so the outcome is the same, but the
8.4branch should have been the base.Environment it was verified on
php -v):PHP 8.4.19 (cli) (built: Mar 30 2026 19:28:35) (NTS)--enable-debug)? noVerified in both directions, with
opcache.enable_cli=0so no stale bytecodecould mask the change:
tests/System/Hook/OpCodeHookTest(--group internal): 10/10 pass with thefix; the two new cases fail with
FFI\Exception: attempt to cast to larger typewithout it.the fix; without it exactly the four failures CI reports on zdebug
maincome back (
testExceptionBreakpointFiresBeforeTheThrow,testSteppingOffAReturnStopsAgainWithTheValue,testContainerAndVoidReturns,testAReturnBreakpointCarriesTheValueWhenTheFeatureIsOn).Checklist
is the cascade of fix(type): read opline operands off the znode_op union instead of casting it #227 rather than the origin of the fix
composer testpasses on the matching PHP minorcomposer phpstan(level max) andcomposer cs:checkare green — noPHPStan binary is installable in this environment (the egress proxy blocks
api.github.com), so CI is the verifier for these twolayout_structsis unchangedtools/generator/symbols.phpunchangedGenerated by Claude Code