From bd1f9b25300429b648ddfa592d61c9d1c2a079b8 Mon Sep 17 00:00:00 2001 From: PurHur Date: Sun, 26 Jul 2026 00:36:37 +0000 Subject: [PATCH] Stdlib: substr_replace string/replace/offset/length named params (#23183) Override InternalArgInfo str/repl/start with Zend stub names, and fix JitSubstrReplace PHI predecessors after jitCopySlice/concat so AOT verifies. Co-authored-by: Cursor --- ext/standard/JitSubstrReplace.php | 13 +++++++++---- lib/BuiltinParamNames.php | 3 +++ .../stdlib/named_args_substr_replace.phpt | 19 +++++++++++++++++++ .../stdlib/named_args_substr_replace_jit.phpt | 11 +++++++++++ .../cases/substr_replace_named_params.phpt | 9 +++++++++ .../issue_23183_substr_replace_named.php | 13 +++++++++++++ test/unit/BuiltinParamNamesAliasTest.php | 15 +++++++++++++++ 7 files changed, 79 insertions(+), 4 deletions(-) create mode 100644 test/compliance/cases/stdlib/named_args_substr_replace.phpt create mode 100644 test/compliance/cases/stdlib/named_args_substr_replace_jit.phpt create mode 100644 test/fixtures/aot/cases/substr_replace_named_params.phpt create mode 100644 test/repro/issue_23183_substr_replace_named.php diff --git a/ext/standard/JitSubstrReplace.php b/ext/standard/JitSubstrReplace.php index 671d457166..95733ce111 100644 --- a/ext/standard/JitSubstrReplace.php +++ b/ext/standard/JitSubstrReplace.php @@ -110,16 +110,19 @@ public static function replace( $context->builder->positionAtEnd($prefixBlock); $prefixSlice = string_trim::jitCopySlice($context, $string, $strPtr, $zero64, $normOff, 'pre'); + // jitCopySlice ends in a fresh continue block — use that as the PHI predecessor. + $prefixEndBlock = $context->builder->getInsertBlock(); $context->builder->branch($afterPrefixBlock); $context->builder->positionAtEnd($skipPrefixBlock); $emptyPrefix = $context->builder->call($context->lookupFunction('__string__alloc'), $zero64); + $skipPrefixEndBlock = $context->builder->getInsertBlock(); $context->builder->branch($afterPrefixBlock); $context->builder->positionAtEnd($afterPrefixBlock); $prefixPhi = $context->builder->phi($context->getTypeFromString('__string__*')); - $prefixPhi->addIncoming($prefixSlice, $prefixBlock); - $prefixPhi->addIncoming($emptyPrefix, $skipPrefixBlock); + $prefixPhi->addIncoming($prefixSlice, $prefixEndBlock); + $prefixPhi->addIncoming($emptyPrefix, $skipPrefixEndBlock); $withReplace = JitStringConcat::concat($context, $prefixPhi, $replace); @@ -132,15 +135,17 @@ public static function replace( $context->builder->positionAtEnd($tailBlock); $tailSlice = string_trim::jitCopySlice($context, $string, $strPtr, $tailStart, $tailLen, 'tail'); $result = JitStringConcat::concat($context, $withReplace, $tailSlice); + $tailEndBlock = $context->builder->getInsertBlock(); $context->builder->branch($doneBlock); $context->builder->positionAtEnd($skipTailBlock); + $skipTailEndBlock = $context->builder->getInsertBlock(); $context->builder->branch($doneBlock); $context->builder->positionAtEnd($doneBlock); $resultPhi = $context->builder->phi($context->getTypeFromString('__string__*')); - $resultPhi->addIncoming($result, $tailBlock); - $resultPhi->addIncoming($withReplace, $skipTailBlock); + $resultPhi->addIncoming($result, $tailEndBlock); + $resultPhi->addIncoming($withReplace, $skipTailEndBlock); return $resultPhi; } diff --git a/lib/BuiltinParamNames.php b/lib/BuiltinParamNames.php index 4d0b5af413..6b8efe3427 100644 --- a/lib/BuiltinParamNames.php +++ b/lib/BuiltinParamNames.php @@ -557,6 +557,9 @@ public static function forFunction(string $name): ?array return ['pattern', 'flags']; case 'substr_compare': return ['haystack', 'needle', 'offset', 'length', 'case_insensitive']; + // php-src ext/standard/string.stub.php — InternalArgInfo still says str/repl/start (#23183) + case 'substr_replace': + return ['string', 'replace', 'offset', 'length']; case 'file_exists': case 'filesize': case 'filemtime': diff --git a/test/compliance/cases/stdlib/named_args_substr_replace.phpt b/test/compliance/cases/stdlib/named_args_substr_replace.phpt new file mode 100644 index 0000000000..2a22fa7f50 --- /dev/null +++ b/test/compliance/cases/stdlib/named_args_substr_replace.phpt @@ -0,0 +1,19 @@ +--TEST-- +substr_replace named string/replace/offset/length arguments (VM, issue #23183) +--FILE-- +getParameters() as $p) { + echo $p->getName(), PHP_EOL; +} +--EXPECT-- +'abXdef' +'abX' +string +replace +offset +length diff --git a/test/compliance/cases/stdlib/named_args_substr_replace_jit.phpt b/test/compliance/cases/stdlib/named_args_substr_replace_jit.phpt new file mode 100644 index 0000000000..98e2848638 --- /dev/null +++ b/test/compliance/cases/stdlib/named_args_substr_replace_jit.phpt @@ -0,0 +1,11 @@ +--TEST-- +substr_replace named string/replace/offset/length arguments (JIT, issue #23183) +--FILE-- +getParameters() as $p) { + $names[] = $p->getName(); +} +$named = substr_replace(string: 'abcdef', replace: 'X', offset: 2, length: 1); +$positional = substr_replace('abcdef', 'X', 2, 1); +$ok = ['string', 'replace', 'offset', 'length'] === $names + && 'abXdef' === $named + && $named === $positional; +echo $ok ? "ok\n" : "fail\n"; diff --git a/test/unit/BuiltinParamNamesAliasTest.php b/test/unit/BuiltinParamNamesAliasTest.php index ad668e7cb2..0c6f494dc8 100644 --- a/test/unit/BuiltinParamNamesAliasTest.php +++ b/test/unit/BuiltinParamNamesAliasTest.php @@ -799,4 +799,19 @@ public function testStrtotimeZendStubNamedParams(): void self::assertFalse(BuiltinParamNames::lookupNamedParamIndex($names, 'time', 'strtotime')); self::assertFalse(BuiltinParamNames::lookupNamedParamIndex($names, 'now', 'strtotime')); } + + /** @covers issue #23183 */ + public function testSubstrReplaceZendStubNamedParams(): void + { + $names = BuiltinParamNames::forFunction('substr_replace'); + self::assertSame(['string', 'replace', 'offset', 'length'], $names); + self::assertSame(0, BuiltinParamNames::lookupNamedParamIndex($names, 'string', 'substr_replace')); + self::assertSame(1, BuiltinParamNames::lookupNamedParamIndex($names, 'replace', 'substr_replace')); + self::assertSame(2, BuiltinParamNames::lookupNamedParamIndex($names, 'offset', 'substr_replace')); + self::assertSame(3, BuiltinParamNames::lookupNamedParamIndex($names, 'length', 'substr_replace')); + // Legacy InternalArgInfo names must not resolve (Zend rejects $str / $repl / $start) + self::assertFalse(BuiltinParamNames::lookupNamedParamIndex($names, 'str', 'substr_replace')); + self::assertFalse(BuiltinParamNames::lookupNamedParamIndex($names, 'repl', 'substr_replace')); + self::assertFalse(BuiltinParamNames::lookupNamedParamIndex($names, 'start', 'substr_replace')); + } }