From c94be9c6a93197d3e2bc5f6eacd485b97929d46d Mon Sep 17 00:00:00 2001 From: tianfenghan Date: Sat, 4 Jul 2026 15:06:34 +0800 Subject: [PATCH] =?UTF-8?q?fix(compiler):=E6=B3=A8=E5=85=A5finally?= =?UTF-8?q?=E5=9D=97=E5=88=B0return=E8=B7=AF=E5=BE=84=E5=B9=B6=E4=BF=AE?= =?UTF-8?q?=E5=A4=8D=E5=B1=9E=E6=80=A7=E6=B3=A8=E5=86=8C?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Php/CompilerBase.php | 78 +++++++++++++++++-- src/Php/Translator.php | 12 +-- src/gen_stub.php | 26 ++----- .../anon_class/constructor-state-method.phpt | 32 ++++++++ tests/aot/attribute/function-attributes.phpt | 40 ++++++++++ .../parameter-property-attributes.phpt | 52 +++++++++++++ tests/aot/basic/unset-dim-side-effects.phpt | 27 +++++++ .../new-dynamic-unpack-call-order.phpt | 43 ++++++++++ .../empty/isset-empty-dim-side-effects.phpt | 36 +++++++++ tests/aot/exception/finally-catch-return.phpt | 29 +++++++ .../aot/exception/finally-nested-return.phpt | 33 ++++++++ .../finally-return-throw-side-effects.phpt | 45 +++++++++++ .../array-push-unpack-side-effects.phpt | 31 ++++++++ .../preg-match-output-ref-reuse.phpt | 24 ++++++ 14 files changed, 478 insertions(+), 30 deletions(-) create mode 100644 tests/aot/anon_class/constructor-state-method.phpt create mode 100644 tests/aot/attribute/function-attributes.phpt create mode 100644 tests/aot/attribute/parameter-property-attributes.phpt create mode 100644 tests/aot/basic/unset-dim-side-effects.phpt create mode 100644 tests/aot/dynamic_call/new-dynamic-unpack-call-order.phpt create mode 100644 tests/aot/empty/isset-empty-dim-side-effects.phpt create mode 100644 tests/aot/exception/finally-catch-return.phpt create mode 100644 tests/aot/exception/finally-nested-return.phpt create mode 100644 tests/aot/exception/finally-return-throw-side-effects.phpt create mode 100644 tests/aot/functions/array-push-unpack-side-effects.phpt create mode 100644 tests/aot/functions/preg-match-output-ref-reuse.phpt diff --git a/src/Php/CompilerBase.php b/src/Php/CompilerBase.php index 0fe8d29c..f01faceb 100644 --- a/src/Php/CompilerBase.php +++ b/src/Php/CompilerBase.php @@ -6444,14 +6444,14 @@ class CompilerBase extends \PhpAot\Core\Translator implements PropertyAccessCont { $code = $this->parseBeforeStmtLines() . PHP_EOL; $code .= 'try {'; - $stmts = $v->stmts; + $finally = $v->finally; + $stmts = $finally ? $this->injectFinallyBeforeReturn($v->stmts, $finally->stmts) : $v->stmts; $code .= PHP_EOL; $code .= $this->parseBlockStmts($stmts); $code .= $this->getIndent() . '}' . PHP_EOL; $catches = $v->catches; - $finally = $v->finally; $exVar = $this->genTmpVarName(); $this->addLocalVar($exVar, self::TYPE_VAR); @@ -6461,7 +6461,7 @@ class CompilerBase extends \PhpAot\Core\Translator implements PropertyAccessCont if ($catches) { $this->indentLevel++; foreach ($catches as $catch) { - $code .= $this->parseCatch($catch, $exVar); + $code .= $this->parseCatch($catch, $exVar, $finally?->stmts ?? []); } $this->indentLevel--; } @@ -6476,7 +6476,74 @@ class CompilerBase extends \PhpAot\Core\Translator implements PropertyAccessCont return $code; } - protected function parseCatch(Node\Stmt\Catch_ $catch, string $exVar): string + protected function injectFinallyBeforeReturn(array $stmts, array $finallyStmts): array + { + $result = []; + foreach ($stmts as $stmt) { + if ($stmt instanceof Node\Stmt\Return_) { + if ($stmt->expr) { + $tmpVar = $this->addTmpVar(self::TYPE_VAR); + $result[] = new Node\Stmt\Expression(new Expr\Assign(new Variable($tmpVar), $stmt->expr)); + array_push($result, ...$this->cloneStmtList($finallyStmts)); + $result[] = new Node\Stmt\Return_(new Variable($tmpVar)); + continue; + } + array_push($result, ...$this->cloneStmtList($finallyStmts)); + $result[] = $stmt; + continue; + } + + $result[] = $this->injectFinallyBeforeReturnInStmt($stmt, $finallyStmts); + } + return $result; + } + + protected function injectFinallyBeforeReturnInStmt(Node\Stmt $stmt, array $finallyStmts): Node\Stmt + { + if ($stmt instanceof Node\Stmt\If_) { + $stmt = clone $stmt; + $stmt->stmts = $this->injectFinallyBeforeReturn($stmt->stmts, $finallyStmts); + foreach ($stmt->elseifs as $index => $elseIf) { + $elseIf = clone $elseIf; + $elseIf->stmts = $this->injectFinallyBeforeReturn($elseIf->stmts, $finallyStmts); + $stmt->elseifs[$index] = $elseIf; + } + if ($stmt->else) { + $stmt->else = clone $stmt->else; + $stmt->else->stmts = $this->injectFinallyBeforeReturn($stmt->else->stmts, $finallyStmts); + } + return $stmt; + } + + if ($stmt instanceof Node\Stmt\For_ + || $stmt instanceof Node\Stmt\Foreach_ + || $stmt instanceof Node\Stmt\While_ + || $stmt instanceof Node\Stmt\Do_ + ) { + $stmt = clone $stmt; + $stmt->stmts = $this->injectFinallyBeforeReturn($stmt->stmts, $finallyStmts); + return $stmt; + } + + if ($stmt instanceof Node\Stmt\Switch_) { + $stmt = clone $stmt; + foreach ($stmt->cases as $index => $case) { + $case = clone $case; + $case->stmts = $this->injectFinallyBeforeReturn($case->stmts, $finallyStmts); + $stmt->cases[$index] = $case; + } + return $stmt; + } + + return $stmt; + } + + protected function cloneStmtList(array $stmts): array + { + return array_map(static fn (Node\Stmt $stmt): Node\Stmt => clone $stmt, $stmts); + } + + protected function parseCatch(Node\Stmt\Catch_ $catch, string $exVar, array $finallyStmts = []): string { $types = $catch->types; $var = $catch->var ? $this->parseIdentifier($catch->var) : $this->genTmpVarName(); @@ -6503,7 +6570,8 @@ class CompilerBase extends \PhpAot\Core\Translator implements PropertyAccessCont $code .= ') {' . PHP_EOL; $this->indentLevel++; $code .= $this->getIndent() . "{$exVar} = php::null;" . PHP_EOL; - $code .= $this->parseStmts($catch->stmts); + $stmts = $finallyStmts ? $this->injectFinallyBeforeReturn($catch->stmts, $finallyStmts) : $catch->stmts; + $code .= $this->parseStmts($stmts); $this->indentLevel--; $code .= $this->getIndent() . '}'; diff --git a/src/Php/Translator.php b/src/Php/Translator.php index d16793d7..4148aa7d 100644 --- a/src/Php/Translator.php +++ b/src/Php/Translator.php @@ -2503,11 +2503,13 @@ CODE; $this->climate->info('generate arginfo file: ' . $this->getRelativePath($file)); generateStubFile($file, $this->getIncludeDir() . '/' . $headerFile, true); - if ($this->useRegisterSymbolsFn) { - preg_match('/php_(.*)_arginfo.h/', $headerFile, $matches); - $registerSymbolFn = 'register_' . $matches[1] . '_symbols'; - $registerSymbol = PHP_EOL . 'static void ' . $registerSymbolFn . '(int module_number)' . PHP_EOL; - if (str_contains(file_get_contents($this->getBuildDir() . '/include/' . $headerFile), $registerSymbol)) { + $headerCode = file_get_contents($this->getBuildDir() . '/include/' . $headerFile); + $needsAttributeSymbols = str_contains($headerCode, 'zend_add_function_attribute(') + || str_contains($headerCode, 'zend_add_parameter_attribute(') + || str_contains($headerCode, 'zend_add_global_constant_attribute('); + if ($this->useRegisterSymbolsFn || $needsAttributeSymbols) { + if (preg_match('/\bstatic\s+void\s+(register_[A-Za-z0-9_]+_symbols)\s*\(\s*int\s+module_number\s*\)/', $headerCode, $matches)) { + $registerSymbolFn = $matches[1]; $this->registerSymbols[] = $registerSymbolFn; } } diff --git a/src/gen_stub.php b/src/gen_stub.php index bcf590e9..cdae7643 100755 --- a/src/gen_stub.php +++ b/src/gen_stub.php @@ -5467,20 +5467,9 @@ function generateArgInfoCode( } } - if ($attributeInitializationCode !== "" || !empty($fileInfo->constInfos)) { + if ($attributeInitializationCode !== "") { $code .= "\nstatic void register_{$stubFilenameWithoutExtension}_symbols(int module_number)\n"; $code .= "{\n"; - - $code .= generateCodeWithConditions( - $fileInfo->constInfos, - '', - static fn (ConstInfo $constInfo): string => $constInfo->getDeclaration($allConstInfos) - ); - - if ($attributeInitializationCode !== "" && $fileInfo->constInfos) { - $code .= "\n"; - } - $code .= $attributeInitializationCode; $code .= "}\n"; } @@ -5550,8 +5539,9 @@ function generateFunctionAttributeInitialization(iterable $funcInfos, array $all } foreach ($funcInfo->attributes as $key => $attribute) { + $functionLookup = "(zend_function *) zend_hash_str_find_ptr($functionTable, \"" . $funcInfo->name->getNameForAttributes() . "\", sizeof(\"" . $funcInfo->name->getNameForAttributes() . "\") - 1)"; $code .= $attribute->generateCode( - "zend_add_function_attribute(zend_hash_str_find_ptr($functionTable, \"" . $funcInfo->name->getNameForAttributes() . "\", sizeof(\"" . $funcInfo->name->getNameForAttributes() . "\") - 1)", + "zend_add_function_attribute($functionLookup", "func_" . $funcInfo->name->getNameForAttributes() . "_$key", $allConstInfos, $phpVersionIdMinimumCompatibility, @@ -5561,8 +5551,9 @@ function generateFunctionAttributeInitialization(iterable $funcInfos, array $all foreach ($funcInfo->args as $index => $arg) { foreach ($arg->attributes as $key => $attribute) { + $functionLookup = "(zend_function *) zend_hash_str_find_ptr($functionTable, \"" . $funcInfo->name->getNameForAttributes() . "\", sizeof(\"" . $funcInfo->name->getNameForAttributes() . "\") - 1)"; $code .= $attribute->generateCode( - "zend_add_parameter_attribute(zend_hash_str_find_ptr($functionTable, \"" . $funcInfo->name->getNameForAttributes() . "\", sizeof(\"" . $funcInfo->name->getNameForAttributes() . "\") - 1), $index", + "zend_add_parameter_attribute($functionLookup, $index", "func_{$funcInfo->name->getNameForAttributes()}_arg{$index}_$key", $allConstInfos, $phpVersionIdMinimumCompatibility, @@ -5615,12 +5606,7 @@ function generateGlobalConstantAttributeInitialization( $constName = str_replace('\\', '\\\\', $constInfo->name->__toString()); $constVarName = 'const_' . $constName; - // The entire attribute block will be conditional if PHP < 8.5 is - // supported, but also if PHP < 8.5 is supported we need to search - // for the constant; see GH-19029 - if ($isConditional) { - $code .= "\tzend_constant *$constVarName = zend_hash_str_find_ptr(EG(zend_constants), \"" . $constName . "\", sizeof(\"" . $constName . "\") - 1);\n"; - } + $code .= "\tzend_constant *$constVarName = zend_hash_str_find_ptr(EG(zend_constants), \"" . $constName . "\", sizeof(\"" . $constName . "\") - 1);\n"; foreach ($constInfo->attributes as $key => $attribute) { $code .= $attribute->generateCode( "zend_add_global_constant_attribute($constVarName", diff --git a/tests/aot/anon_class/constructor-state-method.phpt b/tests/aot/anon_class/constructor-state-method.phpt new file mode 100644 index 00000000..c8f06059 --- /dev/null +++ b/tests/aot/anon_class/constructor-state-method.phpt @@ -0,0 +1,32 @@ +--TEST-- +anonymous class stores constructor state and uses it in methods +--FILE-- + 'Node', 'leaf' => 'Leaf']; + + $visitor = new class($prefixes) extends AnonStateVisitor { + public function __construct(private array $prefixes) + { + } + + public function visit(string $name): string + { + return ($this->prefixes[$name] ?? 'Unknown') . ':' . $name; + } + }; + + var_dump($visitor->visit('node')); + var_dump($visitor->visit('missing')); +} +?> +--EXPECT-- +string(9) "Node:node" +string(15) "Unknown:missing" diff --git a/tests/aot/attribute/function-attributes.phpt b/tests/aot/attribute/function-attributes.phpt new file mode 100644 index 00000000..87d3d4bc --- /dev/null +++ b/tests/aot/attribute/function-attributes.phpt @@ -0,0 +1,40 @@ +--TEST-- +function attributes are available through reflection +--FILE-- +getAttributes(AotFunctionMeta::class); + + var_dump($attrs[0]->getName()); + var_dump($attrs[0]->getArguments()); + var_dump($attrs[0]->newInstance()->name); + var_dump(attributed_function()); +} +?> +--EXPECT-- +string(15) "AotFunctionMeta" +array(2) { + [0]=> + string(7) "handler" + [1]=> + int(10) +} +string(7) "handler" +string(2) "ok" diff --git a/tests/aot/attribute/parameter-property-attributes.phpt b/tests/aot/attribute/parameter-property-attributes.phpt new file mode 100644 index 00000000..855a3b41 --- /dev/null +++ b/tests/aot/attribute/parameter-property-attributes.phpt @@ -0,0 +1,52 @@ +--TEST-- +function parameter and property attributes are available through reflection +--FILE-- +getParameters()[0]->getAttributes(AotMeta::class); + var_dump($paramAttrs[0]->getName()); + var_dump($paramAttrs[0]->getArguments()); + + $prop = new ReflectionProperty(AttributeHolder::class, 'value'); + $propAttrs = $prop->getAttributes(AotMeta::class); + var_dump($propAttrs[0]->getName()); + var_dump($propAttrs[0]->getArguments()); +} +?> +--EXPECT-- +string(7) "AotMeta" +array(2) { + [0]=> + string(9) "parameter" + [1]=> + int(2) +} +string(7) "AotMeta" +array(2) { + [0]=> + string(8) "property" + [1]=> + int(1) +} diff --git a/tests/aot/basic/unset-dim-side-effects.phpt b/tests/aot/basic/unset-dim-side-effects.phpt new file mode 100644 index 00000000..4022c7e6 --- /dev/null +++ b/tests/aot/basic/unset-dim-side-effects.phpt @@ -0,0 +1,27 @@ +--TEST-- +unset evaluates array dimension expressions left to right +--FILE-- + 1, 'b' => 2, 'c' => 3]; + + unset($items[unset_key('a')], $items[unset_key('c')]); + + var_dump($items); +} +?> +--EXPECT-- +unset-key:a +unset-key:c +array(1) { + ["b"]=> + int(2) +} diff --git a/tests/aot/dynamic_call/new-dynamic-unpack-call-order.phpt b/tests/aot/dynamic_call/new-dynamic-unpack-call-order.phpt new file mode 100644 index 00000000..72ccb5f5 --- /dev/null +++ b/tests/aot/dynamic_call/new-dynamic-unpack-call-order.phpt @@ -0,0 +1,43 @@ +--TEST-- +dynamic new with unpacked constructor args then method call preserves order +--FILE-- +name . ':' . $suffix; + } +} + +function make_new_args(): array +{ + echo "new-args\n"; + return ['object']; +} + +function make_call_arg(): string +{ + echo "call-arg\n"; + return 'method'; +} + +function main(): void +{ + $class = DynamicNewOrder::class; + var_dump((new $class(...make_new_args()))->run(make_call_arg())); +} +?> +--EXPECT-- +new-args +ctor:object +call-arg +run:method +string(13) "object:method" diff --git a/tests/aot/empty/isset-empty-dim-side-effects.phpt b/tests/aot/empty/isset-empty-dim-side-effects.phpt new file mode 100644 index 00000000..4f1ff949 --- /dev/null +++ b/tests/aot/empty/isset-empty-dim-side-effects.phpt @@ -0,0 +1,36 @@ +--TEST-- +isset and empty evaluate array dimension expressions in order +--FILE-- + ['name' => 'Alice'], + 'zero' => 0, + ]; + + var_dump(isset($data[dim_key('user')][dim_key('name')])); + var_dump(isset($data[dim_key('missing')][dim_key('nested')])); + var_dump(empty($data[dim_key('zero')])); + var_dump(empty($data[dim_key('missing')][dim_key('empty-nested')])); +} +?> +--EXPECT-- +key:user +key:name +bool(true) +key:missing +key:nested +bool(false) +key:zero +bool(true) +key:missing +key:empty-nested +bool(true) diff --git a/tests/aot/exception/finally-catch-return.phpt b/tests/aot/exception/finally-catch-return.phpt new file mode 100644 index 00000000..43498881 --- /dev/null +++ b/tests/aot/exception/finally-catch-return.phpt @@ -0,0 +1,29 @@ +--TEST-- +finally runs for return inside catch without changing return value +--FILE-- +getMessage(); + return $state; + } finally { + echo "finally:$state\n"; + $state = "finally"; + } +} + +function main(): void +{ + var_dump(catch_finally_return()); +} +?> +--EXPECT-- +finally:catch:failure +string(13) "catch:failure" diff --git a/tests/aot/exception/finally-nested-return.phpt b/tests/aot/exception/finally-nested-return.phpt new file mode 100644 index 00000000..b4c95c2a --- /dev/null +++ b/tests/aot/exception/finally-nested-return.phpt @@ -0,0 +1,33 @@ +--TEST-- +finally runs for return nested in if/else branches without changing return value +--FILE-- + 0) { + $state .= ":positive"; + return $state; + } else { + $state .= ":negative"; + return $state; + } + } finally { + echo "finally:$state\n"; + $state .= ":finally"; + } +} + +function main(): void +{ + var_dump(nested_finally_return(1)); + var_dump(nested_finally_return(-1)); +} +?> +--EXPECT-- +finally:start:positive +string(14) "start:positive" +finally:start:negative +string(14) "start:negative" diff --git a/tests/aot/exception/finally-return-throw-side-effects.phpt b/tests/aot/exception/finally-return-throw-side-effects.phpt new file mode 100644 index 00000000..943e9b0d --- /dev/null +++ b/tests/aot/exception/finally-return-throw-side-effects.phpt @@ -0,0 +1,45 @@ +--TEST-- +finally side effects run before return and throw leave the frame +--FILE-- +getMessage()); + } +} +?> +--EXPECT-- +try-return +finally-return +string(8) "returned" +try-throw +finally-throw +string(6) "thrown" diff --git a/tests/aot/functions/array-push-unpack-side-effects.phpt b/tests/aot/functions/array-push-unpack-side-effects.phpt new file mode 100644 index 00000000..7ca6065e --- /dev/null +++ b/tests/aot/functions/array-push-unpack-side-effects.phpt @@ -0,0 +1,31 @@ +--TEST-- +array_push with unpacked values mutates first argument once +--FILE-- + +--EXPECT-- +make-values +int(3) +array(3) { + [0]=> + int(1) + [1]=> + int(2) + [2]=> + int(3) +} diff --git a/tests/aot/functions/preg-match-output-ref-reuse.phpt b/tests/aot/functions/preg-match-output-ref-reuse.phpt new file mode 100644 index 00000000..4de9da59 --- /dev/null +++ b/tests/aot/functions/preg-match-output-ref-reuse.phpt @@ -0,0 +1,24 @@ +--TEST-- +preg_match output array can be reused through references +--FILE-- + +--EXPECT-- +array(3) { + [0]=> + string(6) "foobar" + [1]=> + &string(3) "FOO" + [2]=> + string(3) "bar" +}