diff --git a/docs/bootstrap-inventory.md b/docs/bootstrap-inventory.md index efcbb166676..9ec4fb0e2b1 100644 --- a/docs/bootstrap-inventory.md +++ b/docs/bootstrap-inventory.md @@ -8,11 +8,11 @@ Regenerate: `php script/bootstrap-inventory.php` | Metric | Count | |--------|------:| -| PHP files on vm.php path | 7579 | -| Phase A inventory files (M2 ratio SSOT) | 7579 | +| PHP files on vm.php path | 7580 | +| Phase A inventory files (M2 ratio SSOT) | 7580 | | Phase A ratio-deferred paths | 0 | | Source constructs flagged (blockers) | 0 | -| Source constructs flagged (warnings) | 24238 | +| Source constructs flagged (warnings) | 24242 | ## Compiler CFG gaps (`lib/Compiler.php`) @@ -5262,6 +5262,7 @@ Rank live CFG gaps across inventory files: `php script/bootstrap-inventory-triag | `lib/Ast/AbstractEnumMarker.php` | 0 | 1 | | `lib/Ast/AssertExprMessageRewriter.php` | 0 | 7 | | `lib/Ast/AsymmetricVisibilityRewriter.php` | 0 | 11 | +| `lib/Ast/BreakContinueOperandCompileCheck.php` | 0 | 3 | | `lib/Ast/CatchIntersectionAttacher.php` | 0 | 1 | | `lib/Ast/CatchIntersectionSupport.php` | 0 | 1 | | `lib/Ast/CloneWithDesugar.php` | 0 | 1 | @@ -6760,7 +6761,7 @@ Rank live CFG gaps across inventory files: `php script/bootstrap-inventory-triag | `lib/ReadonlyFunctionRejector.php` | 0 | 2 | | `lib/ReadonlyMethodModifierRejector.php` | 0 | 2 | | `lib/ReferenceProfileTokenScan.php` | 0 | 1 | -| `lib/Runtime.php` | 0 | 42 | +| `lib/Runtime.php` | 0 | 43 | | `lib/RuntimeStrictness.php` | 0 | 1 | | `lib/SourceBareThrowRewriter.php` | 0 | 1 | | `lib/SourcePreprocessor/PropertyHooks.php` | 0 | 19 | @@ -46352,6 +46353,13 @@ Rank live CFG gaps across inventory files: `php script/bootstrap-inventory-triag - 48 class method(s) - 8 closure(s) +### `lib/Ast/BreakContinueOperandCompileCheck.php` + +**Warnings** (review for bootstrap subset): +- new CompileFatal (line 61) +- new CompileFatal (line 83) +- 6 class method(s) + ### `lib/Ast/CatchIntersectionAttacher.php` **Warnings** (review for bootstrap subset): @@ -55910,47 +55918,48 @@ Rank live CFG gaps across inventory files: `php script/bootstrap-inventory-triag ### `lib/Runtime.php` **Warnings** (review for bootstrap subset): -- new NodeTraverser (line 148) -- new Ast\ConfusableBuiltinTypeHintCheck (line 150) -- new MultiBlockNameResolver (line 152) -- new Ast\EnumCaseImportRewriter (line 153) -- new GroupUseStripper (line 155) -- new Ast\AbstractEnumMarker (line 156) -- new SealedClassAnnotator (line 158) -- new StaticClassAnnotator (line 160) -- new Ast\EnumPropertyCompileCheck (line 162) -- new Ast\ReadonlyClassTraitPropertyCompileCheck (line 163) -- new Ast\GeneratorYieldSourceMarker (line 164) -- new Ast\AssertExprMessageRewriter (line 166) -- new ReadonlyFunctionAnnotator (line 167) -- new TryCatchElseAttacher (line 168) -- new CatchIntersectionAttacher (line 169) -- new Parser (line 170) -- new ParserFactory (line 171) -- new Traverser (line 175) -- new InOperatorResolver (line 176) -- new ExitFunctionResolver (line 177) -- new VoidCastResolver (line 178) -- new Visitor\Simplifier (line 179) -- new Visitor\DeadBlockEliminator (line 180) -- new Traverser (line 181) -- new Visitor\PhiResolver (line 182) -- new NullSafeLivenessDetector (line 183) -- new Optimizer\AssignOp (line 184) -- new CompilerTypeReconstructor (line 186) -- new Compiler (line 204) -- new VMContext (line 209) -- new VM (line 215) -- new JIT (line 325) -- new JITContext (line 339) -- new SealedClassPreprocessor (line 412) -- new StaticClassPreprocessor (line 415) -- new SourcePreprocessor\PropertyHooks (line 418) -- new State (line 575) -- new ReflectionProperty (line 609) -- new ReflectionProperty (line 612) -- new LintCompiler (line 1044) -- new Variable (line 1175) +- new NodeTraverser (line 149) +- new Ast\ConfusableBuiltinTypeHintCheck (line 151) +- new MultiBlockNameResolver (line 153) +- new Ast\EnumCaseImportRewriter (line 154) +- new GroupUseStripper (line 156) +- new Ast\AbstractEnumMarker (line 157) +- new Ast\BreakContinueOperandCompileCheck (line 159) +- new SealedClassAnnotator (line 161) +- new StaticClassAnnotator (line 163) +- new Ast\EnumPropertyCompileCheck (line 165) +- new Ast\ReadonlyClassTraitPropertyCompileCheck (line 166) +- new Ast\GeneratorYieldSourceMarker (line 167) +- new Ast\AssertExprMessageRewriter (line 169) +- new ReadonlyFunctionAnnotator (line 170) +- new TryCatchElseAttacher (line 171) +- new CatchIntersectionAttacher (line 172) +- new Parser (line 173) +- new ParserFactory (line 174) +- new Traverser (line 178) +- new InOperatorResolver (line 179) +- new ExitFunctionResolver (line 180) +- new VoidCastResolver (line 181) +- new Visitor\Simplifier (line 182) +- new Visitor\DeadBlockEliminator (line 183) +- new Traverser (line 184) +- new Visitor\PhiResolver (line 185) +- new NullSafeLivenessDetector (line 186) +- new Optimizer\AssignOp (line 187) +- new CompilerTypeReconstructor (line 189) +- new Compiler (line 207) +- new VMContext (line 212) +- new VM (line 218) +- new JIT (line 328) +- new JITContext (line 342) +- new SealedClassPreprocessor (line 415) +- new StaticClassPreprocessor (line 418) +- new SourcePreprocessor\PropertyHooks (line 421) +- new State (line 588) +- new ReflectionProperty (line 622) +- new ReflectionProperty (line 625) +- new LintCompiler (line 1057) +- new Variable (line 1188) - 53 class method(s) ### `lib/RuntimeStrictness.php` diff --git a/lib/Ast/BreakContinueOperandCompileCheck.php b/lib/Ast/BreakContinueOperandCompileCheck.php new file mode 100644 index 00000000000..9becee46012 --- /dev/null +++ b/lib/Ast/BreakContinueOperandCompileCheck.php @@ -0,0 +1,106 @@ + 0}. + * + * php-src: Zend/zend_compile.c — zend_compile_break_continue(); depth must be {@code > 0} + */ +final class BreakContinueOperandCompileCheck extends NodeVisitorAbstract +{ + public const CONTINUE_MESSAGE = "'continue' operator accepts only positive integers"; + + public const BREAK_MESSAGE = "'break' operator accepts only positive integers"; + + private string $sourceFile = 'unknown'; + + public function setSourceFile(string $sourceFile): void + { + $this->sourceFile = '' !== $sourceFile ? $sourceFile : 'unknown'; + } + + public function enterNode(Node $node) + { + if ($node instanceof Continue_) { + $this->rejectNonPositive($node, $node->num, self::CONTINUE_MESSAGE); + } elseif ($node instanceof Break_) { + $this->rejectNonPositive($node, $node->num, self::BREAK_MESSAGE); + } + + return null; + } + + /** + * Map LoopResolver LogicException into Zend-shaped CompileFatal (JIT must not echo on stdout). + * + * @return never + */ + public static function rethrowAsCompileFatal(\LogicException $e, string $filename): never + { + $msg = $e->getMessage(); + if (!self::isBreakContinueCompileMessage($msg)) { + throw $e; + } + $file = '' !== $filename ? $filename : 'unknown'; + throw new CompileFatal($file, 1, $msg); + } + + public static function isBreakContinueCompileMessage(string $message): bool + { + return str_contains($message, "operator accepts only positive integers") + || str_starts_with($message, "Cannot 'continue' ") + || str_starts_with($message, "Cannot 'break' "); + } + + private function rejectNonPositive(Node $stmt, ?Node $num, string $message): void + { + $value = self::integerLiteralValue($num); + if (null === $value || $value >= 1) { + return; + } + + $file = $stmt->getAttribute('fileName'); + if (!is_string($file) || '' === $file) { + $file = $this->sourceFile; + } + + throw new CompileFatal($file, max(1, $stmt->getStartLine()), $message); + } + + private static function integerLiteralValue(?Node $expr): ?int + { + if (null === $expr) { + return null; + } + if ($expr instanceof LNumber) { + return $expr->value; + } + if ($expr instanceof UnaryMinus && $expr->expr instanceof LNumber) { + return -$expr->expr->value; + } + if ($expr instanceof UnaryPlus && $expr->expr instanceof LNumber) { + return $expr->expr->value; + } + if (class_exists(\PhpParser\Node\Expr\Paren::class) && $expr instanceof \PhpParser\Node\Expr\Paren) { + return self::integerLiteralValue($expr->expr); + } + + return null; + } +} diff --git a/lib/Runtime.php b/lib/Runtime.php index b0c4a143ca5..de67eb02eea 100755 --- a/lib/Runtime.php +++ b/lib/Runtime.php @@ -83,6 +83,7 @@ class Runtime { public Traverser $preprocessor; public Traverser $postprocessor; private Ast\AbstractEnumMarker $abstractEnumMarker; + private ?Ast\BreakContinueOperandCompileCheck $breakContinueOperandCompileCheck = null; private Ast\ConfusableBuiltinTypeHintCheck $confusableBuiltinTypeHintCheck; public CfgLivenessDetector $detector; public Optimizer $assignOpResolver; @@ -155,6 +156,8 @@ private function initParsePipeline(): void { $astTraverser->addVisitor(new GroupUseStripper()); $this->abstractEnumMarker = new Ast\AbstractEnumMarker(); $astTraverser->addVisitor($this->abstractEnumMarker); + $this->breakContinueOperandCompileCheck = new Ast\BreakContinueOperandCompileCheck(); + $astTraverser->addVisitor($this->breakContinueOperandCompileCheck); $this->sealedClassAnnotator = new SealedClassAnnotator(); $astTraverser->addVisitor($this->sealedClassAnnotator); $this->staticClassAnnotator = new StaticClassAnnotator(); @@ -504,8 +507,13 @@ public function parseForIncludeDiscovery(string $code, string $filename): Script $this->compiler->setBareRethrowLines($bareRethrowLines); } $this->resetParserNameResolverState(); + if (null !== $this->breakContinueOperandCompileCheck) { + $this->breakContinueOperandCompileCheck->setSourceFile($filename); + } try { $script = $this->parser->parse($code, $filename); + } catch (\LogicException $e) { + Ast\BreakContinueOperandCompileCheck::rethrowAsCompileFatal($e, $filename); } finally { $this->abstractEnumMarker->clear(); } @@ -562,9 +570,14 @@ public function parse(string $code, string $filename): Script { } $fileStrictTypes = $this->detectFileStrictTypes($code, $filename); $this->resetParserNameResolverState(); + if (null !== $this->breakContinueOperandCompileCheck) { + $this->breakContinueOperandCompileCheck->setSourceFile($filename); + } try { $script = $this->parser->parse($code, $filename); $this->confusableBuiltinTypeHintCheck->emitPending($this->vmContext, $filename); + } catch (\LogicException $e) { + Ast\BreakContinueOperandCompileCheck::rethrowAsCompileFatal($e, $filename); } finally { $this->abstractEnumMarker->clear(); $this->confusableBuiltinTypeHintCheck->clearPending(); diff --git a/patches/php-cfg-loop-resolver-break-continue-positive.patch b/patches/php-cfg-loop-resolver-break-continue-positive.patch new file mode 100644 index 00000000000..127e80e52d3 --- /dev/null +++ b/patches/php-cfg-loop-resolver-break-continue-positive.patch @@ -0,0 +1,21 @@ +diff --git vendor/ircmaxell/php-cfg/lib/PHPCfg/AstVisitor/LoopResolver.php vendor/ircmaxell/php-cfg/lib/PHPCfg/AstVisitor/LoopResolver.php +--- vendor/ircmaxell/php-cfg/lib/PHPCfg/AstVisitor/LoopResolver.php ++++ vendor/ircmaxell/php-cfg/lib/PHPCfg/AstVisitor/LoopResolver.php +@@ -97,10 +97,14 @@ + if (! $node->num) { + return new Goto_(end($stack), $node->getAttributes()); + } + if ($node->num instanceof LNumber) { + $num = $node->num->value; +- if ($num < 1 || $num > \count($stack)) { +- throw new \LogicException('Too high of a count for '.$node->getType()); ++ $keyword = 'Stmt_Break' === $node->getType() ? 'break' : 'continue'; ++ if ($num < 1) { ++ throw new \LogicException(sprintf("'%s' operator accepts only positive integers", $keyword)); ++ } ++ if ($num > \count($stack)) { ++ throw new \LogicException(sprintf("Cannot '%s' %d levels", $keyword, $num)); + } + $loc = array_slice($stack, -$num, 1); + + return new Goto_($loc[0], $node->getAttributes()); diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 92ca8d763b8..717bb2a3655 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -146,6 +146,8 @@ ./test/compliance/TraitInsteadofParentJITTest.php ./test/compliance/ClosureUseThis32152VMTest.php ./test/compliance/ClosureUseThis32152JITTest.php + ./test/compliance/ContinueZero32155VMTest.php + ./test/compliance/ContinueZero32155JITTest.php ./test/compliance/UninitTypedArrayDimAssignVMTest.php ./test/compliance/UninitTypedArrayDimAssignJITTest.php ./test/compliance/UninitTypedArrayDimRwVMTest.php diff --git a/script/apply-patches.sh b/script/apply-patches.sh index 56064511f67..fa93f13fe5c 100755 --- a/script/apply-patches.sh +++ b/script/apply-patches.sh @@ -358,6 +358,9 @@ patch_already_applied() { php-cfg-loop-resolver-break-outside-context.patch) grep -q "not in the 'loop' or 'switch' context" "$ROOT/vendor/ircmaxell/php-cfg/lib/PHPCfg/AstVisitor/LoopResolver.php" 2>/dev/null ;; + php-cfg-loop-resolver-break-continue-positive.patch) + grep -q "operator accepts only positive integers" "$ROOT/vendor/ircmaxell/php-cfg/lib/PHPCfg/AstVisitor/LoopResolver.php" 2>/dev/null + ;; php-cfg-no-arrow-function.patch) ! grep -q 'fn (Op\\Type $t) => ' "$ROOT/vendor/ircmaxell/php-cfg/lib/PHPCfg/Printer.php" 2>/dev/null ;; @@ -737,6 +740,42 @@ PY echo "Applied php-cfg-loop-resolver-continue-switch-warning.patch (level overlay)" } +apply_php_cfg_loop_resolver_break_continue_positive_overlay() { + local target="$ROOT/vendor/ircmaxell/php-cfg/lib/PHPCfg/AstVisitor/LoopResolver.php" + if grep -q "operator accepts only positive integers" "$target" 2>/dev/null; then + echo "Skip php-cfg-loop-resolver-break-continue-positive overlay (already applied)" + return 0 + fi + python3 - "$target" <<'PY' +import sys +from pathlib import Path + +path = Path(sys.argv[1]) +text = path.read_text() +if "operator accepts only positive integers" in text: + raise SystemExit(0) +old = """ $num = $node->num->value; + if ($num < 1 || $num > \\count($stack)) { + throw new \\LogicException('Too high of a count for '.$node->getType()); + } +""" +new = """ $num = $node->num->value; + $keyword = 'Stmt_Break' === $node->getType() ? 'break' : 'continue'; + if ($num < 1) { + throw new \\LogicException(sprintf(\"'%s' operator accepts only positive integers\", $keyword)); + } + if ($num > \\count($stack)) { + throw new \\LogicException(sprintf(\"Cannot '%s' %d levels\", $keyword, $num)); + } +""" +if old not in text: + sys.stderr.write("php-cfg-loop-resolver-break-continue-positive: LoopResolver anchor not found\n") + raise SystemExit(1) +path.write_text(text.replace(old, new, 1)) +PY + echo "Applied php-cfg-loop-resolver-break-continue-positive.patch (overlay)" +} + apply_php_cfg_arrow_function_overlay() { local parser="$ROOT/vendor/ircmaxell/php-cfg/lib/PHPCfg/Parser.php" local op="$ROOT/vendor/ircmaxell/php-cfg/lib/PHPCfg/Op/Expr/ArrowFunction.php" @@ -6561,6 +6600,11 @@ apply_patch() { return 0 fi fi + if [[ "$(basename "$patch")" == "php-cfg-loop-resolver-break-continue-positive.patch" ]]; then + if apply_php_cfg_loop_resolver_break_continue_positive_overlay; then + return 0 + fi + fi if [[ "$(basename "$patch")" == "php-cfg-yield-keyed.patch" ]]; then apply_php_cfg_yield_keyed_overlay return $? @@ -6979,6 +7023,7 @@ if [[ -d "$ROOT/vendor/ircmaxell/php-cfg" ]]; then apply_patch "$PATCH_DIR/php-cfg-loop-resolver-nested.patch" apply_patch "$PATCH_DIR/php-cfg-loop-resolver-continue-switch-warning.patch" apply_patch "$PATCH_DIR/php-cfg-loop-resolver-break-outside-context.patch" + apply_patch "$PATCH_DIR/php-cfg-loop-resolver-break-continue-positive.patch" apply_patch "$PATCH_DIR/php-cfg-no-arrow-function.patch" apply_patch "$PATCH_DIR/php-cfg-no-closure-preg-replace-callback.patch" apply_patch "$PATCH_DIR/php-cfg-property-type.patch" diff --git a/test/compliance/ContinueZero32155JITTest.php b/test/compliance/ContinueZero32155JITTest.php new file mode 100644 index 00000000000..a83d8940ad6 --- /dev/null +++ b/test/compliance/ContinueZero32155JITTest.php @@ -0,0 +1,36 @@ + self::parsePHPT( + __DIR__.'/cases/language/continue_zero.phpt', + 'continue_zero.phpt' + ); + yield 'break_zero.phpt' => self::parsePHPT( + __DIR__.'/cases/language/break_zero.phpt', + 'break_zero.phpt' + ); + } + + public function setUp(): void + { + $this->BIN = realpath(__DIR__.'/../../bin/jit.php'); + } +} diff --git a/test/compliance/ContinueZero32155VMTest.php b/test/compliance/ContinueZero32155VMTest.php new file mode 100644 index 00000000000..a8529b8ac06 --- /dev/null +++ b/test/compliance/ContinueZero32155VMTest.php @@ -0,0 +1,34 @@ + self::parsePHPT( + __DIR__.'/cases/language/continue_zero.phpt', + 'continue_zero.phpt' + ); + yield 'break_zero.phpt' => self::parsePHPT( + __DIR__.'/cases/language/break_zero.phpt', + 'break_zero.phpt' + ); + } + + public function setUp(): void + { + $this->BIN = realpath(__DIR__.'/../../bin/vm.php'); + } +} diff --git a/test/compliance/cases/language/break_zero.phpt b/test/compliance/cases/language/break_zero.phpt new file mode 100644 index 00000000000..65635a5ab2a --- /dev/null +++ b/test/compliance/cases/language/break_zero.phpt @@ -0,0 +1,12 @@ +--TEST-- +Language: break 0 is compile-time fatal (#32155, Zend/zend_compile.c) +--FILE-- +expectException(CompileFatal::class); + $this->expectExceptionMessage(BreakContinueOperandCompileCheck::CONTINUE_MESSAGE); + $this->traverse(<<<'PHP' +expectException(CompileFatal::class); + $this->expectExceptionMessage(BreakContinueOperandCompileCheck::BREAK_MESSAGE); + $this->traverse(<<<'PHP' +expectException(CompileFatal::class); + $this->expectExceptionMessage(BreakContinueOperandCompileCheck::CONTINUE_MESSAGE); + $this->traverse(<<<'PHP' +traverse(<<<'PHP' +addToAssertionCount(1); + } + + public function testBareContinueDoesNotUsePositiveIntegerRule(): void + { + $this->traverse("addToAssertionCount(1); + } + + private function traverse(string $code): void + { + $parser = (new ParserFactory())->create(ParserFactory::ONLY_PHP7); + $ast = $parser->parse($code); + $this->assertNotNull($ast); + $visitor = new BreakContinueOperandCompileCheck(); + $visitor->setSourceFile('continue_zero.php'); + $traverser = new NodeTraverser(); + $traverser->addVisitor($visitor); + $traverser->traverse($ast); + } +}