Merge pull request 'fix(compiler): 修复 finally 使用变量' (#12) from fix-finally-use-var into master

Reviewed-on: #12

src/Php/CompilerBase.php

@@ -6566,21 +6566,37 @@ class CompilerBase extends \PhpAot\Core\Translator implements PropertyAccessCont
         $finally = $v->finally;
         $stmts = $finally ? $this->injectFinallyBeforeReturn($v->stmts, $finally->stmts) : $v->stmts;
 
+        $catches = $v->catches;
+
+        // Pre-register catch variables before parsing the try block so that
+        // injected finally code referencing catch variables (e.g. $e) does not
+        // trigger undefined-variable errors.
+        if ($finally) {
+            foreach ($catches as $catch) {
+                if ($catch->var) {
+                    $varName = $this->parseIdentifier($catch->var);
+                    if (!$this->hasVar($varName) && $this->stmtListUsesVariable($finally->stmts, $varName)) {
+                        $this->addLocalVar($varName, self::TYPE_OBJECT);
+                    }
+                }
+            }
+        }
+
         $code .= PHP_EOL;
         $code .= $this->parseBlockStmts($stmts);
         $code .= $this->getIndent() . '}' . PHP_EOL;
 
-        $catches = $v->catches;
-
         $exVar = $this->genTmpVarName();
         $this->addLocalVar($exVar, self::TYPE_VAR);
 
         $code .= 'catch(zend_object *_ex) {' . PHP_EOL;
         $code .= $this->getIndent() . $exVar . ' = php::catchException();' . PHP_EOL;
         if ($catches) {
+            $catchMatched = $this->genTmpVarName();
+            $code .= $this->getIndent() . 'bool ' . $catchMatched . ' = false;' . PHP_EOL;
             $this->indentLevel++;
             foreach ($catches as $catch) {
-                $code .= $this->parseCatch($catch, $exVar, $finally?->stmts ?? []);
+                $code .= $this->parseCatch($catch, $exVar, $catchMatched, $finally?->stmts ?? []);
             }
             $this->indentLevel--;
         }
@@ -6677,35 +6693,59 @@ class CompilerBase extends \PhpAot\Core\Translator implements PropertyAccessCont
         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
+    protected function stmtListUsesVariable(array $stmts, string $name): bool
+    {
+        $nodeFinder = new NodeFinder();
+        foreach ($nodeFinder->findInstanceOf($stmts, Variable::class) as $var) {
+            if (is_string($var->name) && $this->escapeVarName($var->name) === $name) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    protected function parseCatch(Node\Stmt\Catch_ $catch, string $exVar, string $catchMatched, array $finallyStmts = []): string
     {
         $types = $catch->types;
-        $var = $catch->var ? $this->parseIdentifier($catch->var) : $this->genTmpVarName();
-        if (!$this->hasVar($var)) {
+        $var = $catch->var ? $this->parseIdentifier($catch->var) : '';
+        if ($var !== '' && !$this->hasVar($var)) {
             $this->addLocalVar($var, self::TYPE_OBJECT);
         }
-        $code = $this->getIndent() . $var . ' = ' . $exVar . ';' . PHP_EOL;
-
-        $code .= $this->parseBeforeStmtLines() . PHP_EOL;
 
-        $code .= $this->getIndent() . 'if (' . $var . ' && ';
+        $code = $this->parseBeforeStmtLines() . PHP_EOL;
+        $code .= $this->getIndent() . 'if (!' . $catchMatched . ' && ' . $exVar . ' && ';
         $conditions = [];
         foreach ($types as $type) {
             if ($this->isNameExpr($type) or $this->isFullNameExpr($type)) {
                 $class = $this->getNamespacedClassName($this->parseIdentifier($type));
                 $ce = $this->getClassEntryPtr($class);
-                $conditions[] = Symbol::instanceOf() . '(' . $var . ', ' . $ce . ')';
+                $conditions[] = Symbol::instanceOf() . '(' . $exVar . ', ' . $ce . ')';
             } else {
                 $this->fatalError($type, 'Unsupported catch type');
             }
         }
 
-        $code .= implode(' || ', $conditions);
-        $code .= ') {' . PHP_EOL;
+        $code .= '(' . implode(' || ', $conditions) . ')) {' . PHP_EOL;
         $this->indentLevel++;
+        $code .= $this->getIndent() . $catchMatched . ' = true;' . PHP_EOL;
+        if ($var !== '') {
+            $code .= $this->getIndent() . $var . ' = ' . $exVar . ';' . PHP_EOL;
+        }
         $code .= $this->getIndent() . "{$exVar} = php::null;" . PHP_EOL;
         $stmts = $finallyStmts ? $this->injectFinallyBeforeReturn($catch->stmts, $finallyStmts) : $catch->stmts;
-        $code .= $this->parseStmts($stmts);
+        if ($finallyStmts) {
+            $code .= $this->getIndent() . 'try {' . PHP_EOL;
+            $this->indentLevel++;
+            $code .= $this->parseStmts($stmts);
+            $this->indentLevel--;
+            $code .= $this->getIndent() . '} catch(zend_object *_catch_throw_ex) {' . PHP_EOL;
+            $this->indentLevel++;
+            $code .= $this->getIndent() . "{$exVar} = php::catchException();" . PHP_EOL;
+            $this->indentLevel--;
+            $code .= $this->getIndent() . '}' . PHP_EOL;
+        } else {
+            $code .= $this->parseStmts($stmts);
+        }
         $this->indentLevel--;
         $code .= $this->getIndent() . '}';
 

tests/aot/exception/finally-catch-throw-sibling.phpt

@@ -0,0 +1,30 @@
+--TEST--
+exception thrown inside catch is not handled by sibling catch
+--FILE--
+<?php
+
+class FinallySiblingA extends Exception {}
+class FinallySiblingB extends Exception {}
+
+function main(): void
+{
+    try {
+        try {
+            throw new FinallySiblingA("a");
+        } catch (FinallySiblingA $e) {
+            echo "catch-a\n";
+            throw new FinallySiblingB("b");
+        } catch (FinallySiblingB $e) {
+            echo "catch-b\n";
+        } finally {
+            echo "finally\n";
+        }
+    } catch (FinallySiblingB $e) {
+        echo "outer:" . $e->getMessage() . "\n";
+    }
+}
+?>
+--EXPECT--
+catch-a
+finally
+outer:b

tests/aot/exception/finally-unmatched-catch-var.phpt

@@ -0,0 +1,26 @@
+--TEST--
+finally must not see variables from unmatched catch clauses
+--FILE--
+<?php
+
+class FinallyUnmatchedA extends Exception {}
+class FinallyUnmatchedB extends Exception {}
+
+function main(): void
+{
+    try {
+        try {
+            throw new FinallyUnmatchedA("a");
+        } catch (FinallyUnmatchedB $e) {
+            echo "catch-b\n";
+        } finally {
+            echo isset($e) ? "set\n" : "unset\n";
+        }
+    } catch (FinallyUnmatchedA $e) {
+        echo "outer:" . $e->getMessage() . "\n";
+    }
+}
+?>
+--EXPECT--
+unset
+outer:a

tests/aot/exception/finally-use-var.phpt

@@ -0,0 +1,26 @@
+--TEST--
+finally can use caught exception variable when catch rethrows
+--FILE--
+<?php
+
+function main(): void
+{
+    try {
+        $a = 1;
+        throw new RuntimeException('test');
+        return;
+    } catch (Throwable $e) {
+        echo 'Caught exception: ', $e->getMessage(), "\n";
+        throw $e;
+    } finally {
+        var_dump($a);
+        echo 'Finally exception: ', $e->getMessage(), "\n";
+    }
+}
+?>
+--EXPECTF--
+Caught exception: test
+int(1)
+Finally exception: test
+
+Fatal error: Uncaught RuntimeException: test in %A

tests/aot/exception/multi-catch-finally-sibling.phpt

@@ -0,0 +1,31 @@
+--TEST--
+multi-catch conditions respect catch match state with finally
+--FILE--
+<?php
+
+class FinallyMultiCatchA extends Exception {}
+class FinallyMultiCatchB extends Exception {}
+class FinallyMultiCatchC extends Exception {}
+
+function main(): void
+{
+    try {
+        try {
+            throw new FinallyMultiCatchA("a");
+        } catch (FinallyMultiCatchA|FinallyMultiCatchB $e) {
+            echo "catch-ab\n";
+            throw new FinallyMultiCatchC("c");
+        } catch (FinallyMultiCatchC $e) {
+            echo "catch-c\n";
+        } finally {
+            echo "finally\n";
+        }
+    } catch (FinallyMultiCatchC $e) {
+        echo "outer:" . $e->getMessage() . "\n";
+    }
+}
+?>
+--EXPECT--
+catch-ab
+finally
+outer:c