diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 2583439..b0e4f10 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -871,17 +871,11 @@ parameters: path: src/Type/ObjectEntry.php - - message: '#^Binary operation "\+" between FFI\\CData and mixed results in an error\.$#' + message: '#^Binary operation "\+" between FFI\\CData and int results in an error\.$#' identifier: binaryOp.invalid count: 1 path: src/Type/OpLine.php - - - message: '#^Parameter \#1 \$variableOffset of method ZEngine\\System\\ExecutionData\:\:getCallVariable\(\) expects int, mixed given\.$#' - identifier: argument.type - count: 1 - path: src/Type/OpLine.php - - message: '#^Parameter \#2 \$pointer of static method ZEngine\\Core\:\:cast\(\) expects object, mixed given\.$#' identifier: argument.type diff --git a/src/Type/OpLine.php b/src/Type/OpLine.php index 52f48c2..124e263 100644 --- a/src/Type/OpLine.php +++ b/src/Type/OpLine.php @@ -16,6 +16,8 @@ use FFI\CData; use ZEngine\Core; use ZEngine\Generated\zend_op; +use ZEngine\Generated\znode_op; +use ZEngine\Generated\zval; use ZEngine\Reflection\ReflectionValue; use ZEngine\System\ExecutionData; use ZEngine\System\OpCode; @@ -222,8 +224,8 @@ public function __debugInfo(): array /** * This utility function returns a pointer to value for given op_node and it's type * - * @param CData|object $node Instance of op1/op2/result node (znode_op union view) - * @param int $opType operation code type, eg IS_CONST, IS_CV... + * @param znode_op $node Typed view of the op1/op2/result node; the runtime value is raw CData + * @param int $opType operation code type, eg IS_CONST, IS_CV... * * @return ReflectionValue|null Extracted value or null, if value could not be resolved (eg. not in runtime) * @@ -231,12 +233,19 @@ public function __debugInfo(): array */ private function getValuePointer(object $node, int $opType): ?ReflectionValue { + // $node is already the znode_op union itself, so its fields are read straight off + // it. Casting it to `znode_op *` first would be asking FFI to reinterpret a 4-byte + // union VALUE as an 8-byte pointer, which it refuses with "attempt to cast to + // larger type" - the operand of every IS_CV/IS_VAR/IS_TMP_VAR opline became + // unreadable, and a caller reading operands from inside an opcode handler (where a + // throw cannot escape) saw the failure only as a silently skipped read. + // // IS_UNUSED is still used by some opcodes, in most cases it points to an IS_UNDEF value $pointer = match ($opType) { - self::IS_CONST => self::getRuntimeConstant(Core::cast('zend_op *', $this->opline), $node), + self::IS_CONST => self::getRuntimeConstant($this->opline, $node), // All these types requires context to be present, otherwise we can't resolve such nodes self::IS_TMP_VAR, self::IS_VAR, self::IS_CV, self::IS_UNUSED => isset($this->context) - ? $this->context->getCallVariable(Core::cast('znode_op *', $node)->var) + ? $this->context->getCallVariable($node->var) : null, default => throw new \InvalidArgumentException('Received invalid opcode type: ' . $opType), }; @@ -251,15 +260,16 @@ private function getValuePointer(object $node, int $opType): ?ReflectionValue * * @see zend_compile.h:RT_CONSTANT macro definition * - * @return CData zval* pointer - * @param \FFI\CData $opline + * @return zval zval* pointer; the runtime value is always raw CData + * @param CData|zend_op $opline Typed view of the opline; the runtime value is raw CData + * @param znode_op $node Typed view of the node; the runtime value is raw CData */ private static function getRuntimeConstant(object $opline, object $node): object { // ((zval*)(((char*)(opline)) + (int32_t)(node).constant)) - $constantOffset = Core::cast('znode_op *', $node)->constant; + $constantOffset = $node->constant; $pointer = Core::cast('char *', $opline) + $constantOffset; - return Core::cast('zval *', $pointer); + return Core::cast(zval::class, $pointer); } } diff --git a/tests/System/Hook/OpCodeHookTest.php b/tests/System/Hook/OpCodeHookTest.php index 505f3c9..a439c62 100644 --- a/tests/System/Hook/OpCodeHookTest.php +++ b/tests/System/Hook/OpCodeHookTest.php @@ -21,6 +21,7 @@ use ZEngine\Core; use ZEngine\System\ExecutionData; use ZEngine\System\OpCode; +use ZEngine\Type\OpLine; /** * Lifecycle of user opcode handlers: install, chaining, guarded uninstall and Core::shutdown @@ -240,6 +241,77 @@ public function testShutdownUninstallsOpCodeHooksAndBlocksNewInstalls(): void * an op_array compiled against a user opcode keeps dispatching through the user * handler table for its whole lifetime. */ + /** + * A handler exists to inspect the instruction it intercepts, and the operands are the + * whole of what there is to inspect - so a handler that cannot read op1 is a handler + * that cannot do its job. + * + * The read used to be routed through a `znode_op *` cast of the operand, which asks + * FFI to reinterpret the 4-byte union VALUE as an 8-byte pointer; it answered "attempt + * to cast to larger type" for every compiled-variable operand. Consumers meet that as + * silence rather than as an error: an opcode handler runs inside an FFI callback, so + * they catch everything, and the failed read simply looked like an instruction that + * never arrived. + */ + public function testHandlerCanReadCompiledVariableOperands(): void + { + $log = new ArrayObject(); + $hook = OpCode::setHandler(OpCode::ADD, static function (ExecutionData $scope) use ($log): int { + try { + $operand = $scope->getOpline()->getOp1(); + $value = null; + $operand?->getNativeValue($value); + $log->append($value); + } catch (\Throwable $error) { + $log->append($error::class . ': ' . $error->getMessage()); + } + + return Core::ZEND_USER_OPCODE_DISPATCH; + }); + + try { + $probe = self::compileProbe('$a + $b'); + $this->assertSame(5, $probe(2, 3)); + } finally { + $hook->uninstall(); + } + + // op1 of `$a + $b` is the compiled variable $a, holding the first argument + $this->assertSame([2], $log->getArrayCopy()); + } + + /** + * The same read for a literal operand, which resolves through the runtime-constant + * offset rather than through a frame variable slot + */ + public function testHandlerCanReadConstantOperands(): void + { + $log = new ArrayObject(); + $hook = OpCode::setHandler(OpCode::ADD, static function (ExecutionData $scope) use ($log): int { + try { + $opline = $scope->getOpline(); + if ($opline->getOp2Type() === OpLine::IS_CONST) { + $value = null; + $opline->getOp2()?->getNativeValue($value); + $log->append($value); + } + } catch (\Throwable $error) { + $log->append($error::class . ': ' . $error->getMessage()); + } + + return Core::ZEND_USER_OPCODE_DISPATCH; + }); + + try { + $probe = self::compileProbe('$a + 40'); + $this->assertSame(42, $probe(2, 0)); + } finally { + $hook->uninstall(); + } + + $this->assertSame([40], $log->getArrayCopy()); + } + private static function compileProbe(string $expression): Closure { $name = str_replace('.', '_', uniqid('zengine_opcode_probe_', true));