From 995d23cfc8284e3ff6ac46a95d0bf9e32a1b051a Mon Sep 17 00:00:00 2001 From: tianfenghan Date: Thu, 9 Jul 2026 15:24:40 +0800 Subject: [PATCH] fix(php): resolve native property assignment type checking and conversion issues - Implement proper type checking for native scalar property assignments from variables - Add exact type conversion helpers for int, float, and bool property types - Enforce compile-time validation of static type mismatches in property assignments - Introduce PHP-style error messages for property type assignment failures - Optimize property fetch operations with zval macro support in loops - Refine compound assignment operations to validate variable types before native writes - Update attribute handling in node parsing to preserve original values correctly - Enhance property assignment type resolution with comprehensive scalar type mapping --- debug/prop.php | 19 ++++ .../native-property-static-type-mismatch.php | 13 +++ .../native-property-this-write-conversion.php | 12 +++ .../code/native-property-write-conversion.php | 13 +++ phpunit/src/NativePropertyTest.php | 35 +++++++ src/Php/CompilerBase.php | 98 ++++++++++++++++++- src/Php/Optimizer/SsaPropOptimizer.php | 7 +- src/Php/Parser/AssignOpTrait.php | 42 ++++++-- src/Php/Resolver/PropertyAssignTypeInfo.php | 29 +++++- .../native-int-property-assign-op-var.phpt | 11 ++- .../native-int-property-string-var.phpt | 15 +-- .../native-scalar-property-assign-op-var.phpt | 51 ++++++++++ .../native-scalar-property-assign-var.phpt | 49 ++++++++++ .../native-typed-read-in-loop.phpt | 44 +++++++++ 14 files changed, 408 insertions(+), 30 deletions(-) create mode 100644 debug/prop.php create mode 100644 phpunit/code/native-property-static-type-mismatch.php create mode 100644 phpunit/code/native-property-this-write-conversion.php create mode 100644 phpunit/code/native-property-write-conversion.php create mode 100644 tests/aot/object_property/native-scalar-property-assign-op-var.phpt create mode 100644 tests/aot/object_property/native-scalar-property-assign-var.phpt create mode 100644 tests/aot/object_property/native-typed-read-in-loop.phpt diff --git a/debug/prop.php b/debug/prop.php new file mode 100644 index 00000000..88959ff8 --- /dev/null +++ b/debug/prop.php @@ -0,0 +1,19 @@ +value = $value; + $o->value += '333'; + var_dump($o->value); + + $o->value = 'str'; + $o->value += 'str'; + var_dump($o->value); +} \ No newline at end of file diff --git a/phpunit/code/native-property-static-type-mismatch.php b/phpunit/code/native-property-static-type-mismatch.php new file mode 100644 index 00000000..3f1e5921 --- /dev/null +++ b/phpunit/code/native-property-static-type-mismatch.php @@ -0,0 +1,13 @@ +value = '123'; +} diff --git a/phpunit/code/native-property-this-write-conversion.php b/phpunit/code/native-property-this-write-conversion.php new file mode 100644 index 00000000..2e889bb7 --- /dev/null +++ b/phpunit/code/native-property-this-write-conversion.php @@ -0,0 +1,12 @@ +value = $dynamicValue; + } +} diff --git a/phpunit/code/native-property-write-conversion.php b/phpunit/code/native-property-write-conversion.php new file mode 100644 index 00000000..6cf766c7 --- /dev/null +++ b/phpunit/code/native-property-write-conversion.php @@ -0,0 +1,13 @@ +value = $nativeValue; + $box->value = $dynamicValue; +} diff --git a/phpunit/src/NativePropertyTest.php b/phpunit/src/NativePropertyTest.php index 8809f60c..47fed0d6 100644 --- a/phpunit/src/NativePropertyTest.php +++ b/phpunit/src/NativePropertyTest.php @@ -58,6 +58,41 @@ class NativePropertyTest extends \BaseTest $this->assertStringNotContainsString('box.attr(php_get_prop(0, _literal_strings[0], 0, _literal_strings[1]), true) +=', $code); } + public function testNativePropertyWriteConvertsOnlyWhenTypesDiffer(): void + { + try { + $outputFile = $this->compileNativeProperty('native-property-write-conversion.php'); + } catch (TestError $e) { + $this->fail($e->getMessage()); + } + + $code = file_get_contents($outputFile); + $this->assertStringContainsString(' = nativeValue;', $code); + $this->assertStringContainsString(' = php::toIntExact(dynamicValue, "NativePropertyWriteConversionBox::$value");', $code); + $this->assertStringNotContainsString(' = php::toInt(nativeValue);', $code); + } + + public function testNativeThisPropertyWriteUsesExactHelperOnNativeReference(): void + { + try { + $outputFile = $this->compileNativeProperty('native-property-this-write-conversion.php'); + } catch (TestError $e) { + $this->fail($e->getMessage()); + } + + $code = file_get_contents($outputFile); + $this->assertStringContainsString('php::Int &_object_prop_this___value = Z_LVAL_P(this_.attr(', $code); + $this->assertStringContainsString('_object_prop_this___value = php::toIntExact(dynamicValue, "NativePropertyThisWriteConversionBox::$value");', $code); + } + + public function testNativePropertyStaticScalarTypeMismatchFailsAtCompileTime(): void + { + $this->exec( + 'Cannot assign string to property NativePropertyStaticTypeMismatchBox::$value of type int', + 'native-property-static-type-mismatch.php' + ); + } + public function testCannotAccessPrivateNativePropertyFromUnrelatedClass(): void { $this->exec('Cannot access private property `value` of class `NativePrivateOwner`', 'native-property-private-other-class.php'); diff --git a/src/Php/CompilerBase.php b/src/Php/CompilerBase.php index 05263242..973cd12f 100644 --- a/src/Php/CompilerBase.php +++ b/src/Php/CompilerBase.php @@ -3104,12 +3104,19 @@ class CompilerBase extends \PhpAot\Core\Translator implements PropertyAccessCont protected function parseNodeWithUpdateAttribute(NodeAbstract $node, string $attribute, bool $update, callable $parser): string { - $attributes = $node->getAttributes(); + $hadAttribute = $node->hasAttribute($attribute); + $previousValue = $node->getAttribute($attribute); $node->setAttribute($attribute, $update); try { return $parser(); } finally { - $node->setAttributes($attributes); + if ($hadAttribute) { + $node->setAttribute($attribute, $previousValue); + } else { + $attributes = $node->getAttributes(); + unset($attributes[$attribute]); + $node->setAttributes($attributes); + } } } @@ -5122,11 +5129,23 @@ class CompilerBase extends \PhpAot\Core\Translator implements PropertyAccessCont return; } + $rightType = $this->detectTypeOfExpr($right); + if ($this->isFixedObjectProp($def) && $rightType !== self::TYPE_VAR) { + if (!$this->canAssignStaticTypeToObjectProperty($def, $rightType)) { + $this->fatalError( + $left, + 'Cannot assign ' . $this->getPropertyAssignmentTypeName($rightType) + . ' to property ' . $this->getObjectPropertyTypeCheckDisplayName($left) + . ' of type ' . $this->getObjectPropertyTypeCheckTypeString($def) + ); + } + return; + } + if ($def->type !== self::TYPE_OBJECT) { return; } - $rightType = $this->detectTypeOfExpr($right); if ($rightType !== self::TYPE_VAR && $rightType !== self::TYPE_OBJECT) { $this->fatalError( $left, @@ -5165,6 +5184,14 @@ class CompilerBase extends \PhpAot\Core\Translator implements PropertyAccessCont return $rightExpr; } + $rightType = $this->detectTypeOfExpr($right); + if ($rightType !== self::TYPE_VAR && $this->canAssignStaticTypeToObjectProperty($def, $rightType)) { + return $rightExpr; + } + if ($rightType === self::TYPE_VAR && ($helper = $this->getNativeScalarPropertyTypeCheckHelper($def)) !== null) { + return $helper . '(' . $rightExpr . ', ' . $this->genCharPtr($this->getObjectPropertyTypeCheckDisplayName($left)) . ')'; + } + $rightClass = $this->detectClassOfExpr($right); if ($rightClass !== '') { return $rightExpr; @@ -5184,8 +5211,14 @@ class CompilerBase extends \PhpAot\Core\Translator implements PropertyAccessCont $propDisplay = $this->getObjectPropertyTypeCheckDisplayName($left); $typeStr = $this->getObjectPropertyTypeCheckTypeString($def); - $msgExpr = 'php::concat(php::concat(php::Str(' . $this->genCharPtr($propDisplay, true) . ' " must be of type " ' - . $this->genCharPtr($typeStr, true) . ' ", "), ' . $tmpVar . '.typeStr()), php::Str(" given"))'; + if ($this->usesPhpStylePropertyAssignTypeError($def)) { + $msgExpr = 'php::concat({php::Str("Cannot assign "), ' . $tmpVar . '.typeStr(), php::Str(" to property "), ' + . 'php::Str(' . $this->genCharPtr($propDisplay, true) . '), php::Str(" of type "), ' + . 'php::Str(' . $this->genCharPtr($typeStr, true) . ')})'; + } else { + $msgExpr = 'php::concat(php::concat(php::Str(' . $this->genCharPtr($propDisplay, true) . ' " must be of type " ' + . $this->genCharPtr($typeStr, true) . ' ", "), ' . $tmpVar . '.typeStr()), php::Str(" given"))'; + } return '([&]() -> ' . self::TYPE_VAR . ' { ' . $tmpVar . ' = ' . $rightExpr . '; ' @@ -5222,6 +5255,52 @@ class CompilerBase extends \PhpAot\Core\Translator implements PropertyAccessCont return (new PropertyAssignTypeInfo())->getTypeString($def); } + private function usesPhpStylePropertyAssignTypeError(PropertyDef $def): bool + { + return empty($def->typeCheck) && $def->class === '' && in_array($def->type, [ + self::TYPE_INT, + self::TYPE_FLOAT, + self::TYPE_BOOL, + self::TYPE_STR, + self::TYPE_ARRAY, + ], true); + } + + protected function getNativeScalarPropertyTypeCheckHelper(PropertyDef $def): ?string + { + if (!empty($def->typeCheck) || $def->class !== '' || $def->nullable) { + return null; + } + + return match ($def->type) { + self::TYPE_INT => 'php::toIntExact', + self::TYPE_FLOAT => 'php::toFloatExact', + self::TYPE_BOOL => 'php::toBoolExact', + default => null, + }; + } + + protected function canAssignStaticTypeToObjectProperty(PropertyDef $def, string $rightType): bool + { + return match ($def->type) { + self::TYPE_FLOAT => $rightType === self::TYPE_FLOAT || $rightType === self::TYPE_INT, + default => $rightType === $def->type, + }; + } + + protected function getPropertyAssignmentTypeName(string $type): string + { + return match ($type) { + self::TYPE_INT => 'int', + self::TYPE_FLOAT => 'float', + self::TYPE_BOOL => 'bool', + self::TYPE_STR => 'string', + self::TYPE_ARRAY => 'array', + self::TYPE_OBJECT => 'object', + default => 'value', + }; + } + protected function parseUnset(Node\Stmt\Unset_ $node): string { $vars = $node->vars; @@ -5397,6 +5476,15 @@ class CompilerBase extends \PhpAot\Core\Translator implements PropertyAccessCont PropertyDef $def, string $getter, ): ?string { + if ($this->isPropertyFetchUpdate($expr) && !in_array($def->type, [self::TYPE_INT, self::TYPE_FLOAT], true)) { + return null; + } + + if ($def->type === self::TYPE_BOOL) { + $this->setNativePropertyValueSource($expr, self::NATIVE_PROPERTY_VALUE_DYNAMIC); + return $this->convertBoolExpr($getter); + } + $propVar = $this->getObjectPropVarName($objectVar, $propName); if ($objectVar === 'this_') { if (!$this->canHoistObjectProp($objectVar, $propName)) { diff --git a/src/Php/Optimizer/SsaPropOptimizer.php b/src/Php/Optimizer/SsaPropOptimizer.php index a3ccceb3..1ef6cc7d 100644 --- a/src/Php/Optimizer/SsaPropOptimizer.php +++ b/src/Php/Optimizer/SsaPropOptimizer.php @@ -812,7 +812,12 @@ trait SsaPropOptimizer } if ($this->context->inLoop || $this->context->scopeLevel > 1) { - return $objName . '.attr(' . $id . ', true)'; + $refGetter = $objName . '.attr(' . $id . ', true)'; + $zvalMacro = $this->getZvalValueMacroForPropType($cType); + if ($zvalMacro !== null) { + return $zvalMacro . '(' . $refGetter . '.unwrap_ptr())'; + } + return $refGetter; } $refGetter = $objName . '.attr(' . $id . ', true)'; diff --git a/src/Php/Parser/AssignOpTrait.php b/src/Php/Parser/AssignOpTrait.php index e8ff7092..fc820e81 100644 --- a/src/Php/Parser/AssignOpTrait.php +++ b/src/Php/Parser/AssignOpTrait.php @@ -309,7 +309,10 @@ trait AssignOpTrait $leftExprType = $this->detectTypeOfExpr($left); $rightExprType = $this->detectTypeOfExpr($right); if ($propertyWriteTarget !== null && ($propertyDef = $this->getNativePropertyDef($left)) !== null) { - return $var . ' = ' . $this->convertExprFromType($propertyDef->type, $rightExpr); + $effectiveRightType = $rightExprType === self::TYPE_VAR && $this->getNativeScalarPropertyTypeCheckHelper($propertyDef) !== null + ? $propertyDef->type + : $rightExprType; + return $var . ' = ' . $this->convertNativePropertyWriteExpr($propertyDef->type, $effectiveRightType, $rightExpr); } if ($finalVarType === self::TYPE_VAR) { return $var . ' = ' . $rightExpr; @@ -329,12 +332,8 @@ trait AssignOpTrait return false; } - if ($rightType === self::TYPE_VAR) { - return true; - } - - return in_array($def->type, [self::TYPE_INT, self::TYPE_FLOAT, self::TYPE_BOOL, self::TYPE_STR], true) - && $rightType !== $def->type; + return !in_array($def->type, [self::TYPE_INT, self::TYPE_FLOAT, self::TYPE_BOOL, self::TYPE_STR, self::TYPE_ARRAY], true) + && $rightType === self::TYPE_VAR; } protected function parseStdContainerCopyAssign(string $leftVar, Expr $right): ?string @@ -487,6 +486,14 @@ trait AssignOpTrait } $rightType = $this->detectTypeOfExpr($node->expr); + if ($this->isFixedObjectProp($def) && $rightType !== self::TYPE_VAR && !$this->canAssignStaticTypeToObjectProperty($def, $rightType)) { + $this->fatalError( + $node->var, + 'Cannot assign ' . $this->getPropertyAssignmentTypeName($rightType) + . ' to property ' . $this->getObjectPropertyTypeCheckDisplayName($node->var) + . ' of type ' . $this->getObjectPropertyTypeCheckTypeString($def) + ); + } if (!$this->canUseNativePropertyAssignOp($def->type, $rightType, $op)) { return null; } @@ -497,12 +504,29 @@ trait AssignOpTrait $var = $helper . '(' . $var . '.unwrap_ptr())'; } - return $var . ' ' . $op . ' (' . $this->convertExprFromType($def->type, $this->parseIdentifier($node->expr)) . ')'; + $rightExpr = $this->parseIdentifier($node->expr); + if ($rightType === self::TYPE_VAR) { + $rightExpr = $this->wrapObjectPropertyAssignTypeCheck($node->var, $node->expr, $rightExpr); + } + $effectiveRightType = $rightType === self::TYPE_VAR && $this->getNativeScalarPropertyTypeCheckHelper($def) !== null + ? $def->type + : $rightType; + + return $var . ' ' . $op . ' (' . $this->convertNativePropertyWriteExpr($def->type, $effectiveRightType, $rightExpr) . ')'; + } + + protected function convertNativePropertyWriteExpr(string $propertyType, string $rightType, string $rightExpr): string + { + if ($propertyType === $rightType) { + return $rightExpr; + } + + return $this->convertExprFromType($propertyType, $rightExpr); } protected function canUseNativePropertyAssignOp(string $propertyType, string $rightType, string $op): bool { - if ($propertyType !== $rightType) { + if ($rightType !== self::TYPE_VAR && !($propertyType === $rightType || ($propertyType === self::TYPE_FLOAT && $rightType === self::TYPE_INT))) { return false; } diff --git a/src/Php/Resolver/PropertyAssignTypeInfo.php b/src/Php/Resolver/PropertyAssignTypeInfo.php index 50bb4189..954ec5ce 100644 --- a/src/Php/Resolver/PropertyAssignTypeInfo.php +++ b/src/Php/Resolver/PropertyAssignTypeInfo.php @@ -41,14 +41,25 @@ final class PropertyAssignTypeInfo if (!empty($def->typeCheck)) { return $def->typeCheck; } - if ($def->type !== CompilerBase::TYPE_OBJECT || $def->class === '') { - return []; - } - $check = []; if ($def->nullable) { $check[] = ['kind' => 'isNull']; } + $scalarCheck = match ($def->type) { + CompilerBase::TYPE_INT => [['kind' => 'isInt']], + CompilerBase::TYPE_FLOAT => [['kind' => 'isFloat'], ['kind' => 'isInt']], + CompilerBase::TYPE_BOOL => [['kind' => 'isBool']], + CompilerBase::TYPE_STR => [['kind' => 'isString']], + CompilerBase::TYPE_ARRAY => [['kind' => 'isArray']], + default => null, + }; + if ($scalarCheck !== null) { + return array_merge($check, $scalarCheck); + } + if ($def->type !== CompilerBase::TYPE_OBJECT || $def->class === '') { + return []; + } + $check[] = ['kind' => 'instanceof', 'class' => $def->class]; return $check; } @@ -61,6 +72,14 @@ final class PropertyAssignTypeInfo if ($def->class !== '') { return ($def->nullable ? '?' : '') . $def->class; } - return $def->type; + return match ($def->type) { + CompilerBase::TYPE_INT => 'int', + CompilerBase::TYPE_FLOAT => 'float', + CompilerBase::TYPE_BOOL => 'bool', + CompilerBase::TYPE_STR => 'string', + CompilerBase::TYPE_ARRAY => 'array', + CompilerBase::TYPE_OBJECT => 'object', + default => $def->type, + }; } } diff --git a/tests/aot/object_property/native-int-property-assign-op-var.phpt b/tests/aot/object_property/native-int-property-assign-op-var.phpt index a2e34b34..da908531 100644 --- a/tests/aot/object_property/native-int-property-assign-op-var.phpt +++ b/tests/aot/object_property/native-int-property-assign-op-var.phpt @@ -20,8 +20,11 @@ function main(): void var_dump($box->value); $text = any("3"); - $box->value += $text; - var_dump($box->value); + try { + $box->value += $text; + } catch (TypeError $e) { + var_dump($e->getMessage()); + } $bad = any("abc"); try { @@ -38,6 +41,6 @@ function main(): void ?> --EXPECT-- int(3) -int(6) -string(39) "Unsupported operand types: int + string" +string(73) "Cannot assign string to property NativeIntAssignOpBox::$value of type int" +string(73) "Cannot assign string to property NativeIntAssignOpBox::$value of type int" int(6) diff --git a/tests/aot/object_property/native-int-property-string-var.phpt b/tests/aot/object_property/native-int-property-string-var.phpt index 94d13a1b..169ac5f9 100644 --- a/tests/aot/object_property/native-int-property-string-var.phpt +++ b/tests/aot/object_property/native-int-property-string-var.phpt @@ -1,5 +1,5 @@ --TEST-- -Native int property assignment from string var uses setProperty fallback +Native int property assignment rejects string var in strict mode --FILE-- value = $numeric; - var_dump($box->value); + $numeric = any("123"); + try { + $box->value = $numeric; + } catch (TypeError $e) { + var_dump($e->getMessage()); + } - $bad = "abc"; + $bad = any("abc"); try { $box->value = $bad; } catch (TypeError $e) { @@ -24,5 +27,5 @@ function main(): void } ?> --EXPECT-- -int(123) +string(74) "Cannot assign string to property NativeIntStringVarBox::$value of type int" string(74) "Cannot assign string to property NativeIntStringVarBox::$value of type int" diff --git a/tests/aot/object_property/native-scalar-property-assign-op-var.phpt b/tests/aot/object_property/native-scalar-property-assign-op-var.phpt new file mode 100644 index 00000000..a7e53a71 --- /dev/null +++ b/tests/aot/object_property/native-scalar-property-assign-op-var.phpt @@ -0,0 +1,51 @@ +--TEST-- +Native scalar object property compound assignment checks var RHS before native write +--FILE-- +intValue += $intDelta; + $this->floatValue += $floatDelta; + } +} + +function main(): void +{ + $box = new NativeScalarAssignOpVarBox(); + + $intDelta = any(2); + $box->intValue += $intDelta; + + $floatDelta = any(2.25); + $box->floatValue += $floatDelta; + + var_dump($box->intValue); + var_dump($box->floatValue); + + $methodIntDelta = any(3); + $methodFloatDelta = any(0.25); + $box->addInside($methodIntDelta, $methodFloatDelta); + var_dump($box->intValue); + var_dump($box->floatValue); + + try { + $badIntDelta = any("4"); + $box->intValue += $badIntDelta; + } catch (TypeError $e) { + var_dump($e->getMessage()); + } +} +?> +--EXPECT-- +int(3) +float(3.75) +int(6) +float(4) +string(82) "Cannot assign string to property NativeScalarAssignOpVarBox::$intValue of type int" diff --git a/tests/aot/object_property/native-scalar-property-assign-var.phpt b/tests/aot/object_property/native-scalar-property-assign-var.phpt new file mode 100644 index 00000000..f19a147e --- /dev/null +++ b/tests/aot/object_property/native-scalar-property-assign-var.phpt @@ -0,0 +1,49 @@ +--TEST-- +Native scalar object property assignment checks var RHS before native write +--FILE-- +intValue = $intValue; + + $floatValue = any(3.5); + $box->floatValue = $floatValue; + + $boolValue = any(false); + $box->boolValue = $boolValue; + + $stringValue = any("123"); + $box->stringValue = $stringValue; + + var_dump($box->intValue); + var_dump($box->floatValue); + var_dump($box->boolValue); + var_dump($box->stringValue); + + try { + $badIntValue = any("12"); + $box->intValue = $badIntValue; + } catch (TypeError $e) { + var_dump($e->getMessage()); + } +} +?> +--EXPECT-- +int(12) +float(3.5) +bool(false) +string(3) "123" +string(80) "Cannot assign string to property NativeScalarAssignVarBox::$intValue of type int" diff --git a/tests/aot/object_property/native-typed-read-in-loop.phpt b/tests/aot/object_property/native-typed-read-in-loop.phpt new file mode 100644 index 00000000..31307861 --- /dev/null +++ b/tests/aot/object_property/native-typed-read-in-loop.phpt @@ -0,0 +1,44 @@ +--TEST-- +Native typed object property read inside loop falls back to typed zval value +--FILE-- +val = $i; + $gi->result = $gi->val + 1; + + $r1 = $lookup[$gi->val]; + $r2 = $gi->val < 10 ? $lookup[$gi->val] : 99; + } + + return [$gi->result, $r1, $r2]; +} + +function main(): void +{ + $r = process([10, 20, 30]); + var_dump($r); +} +?> +--EXPECT-- +array(3) { + [0]=> + int(1) + [1]=> + int(10) + [2]=> + int(10) +}