From 922271ae613dfad47eba556a3c3c34b0833f829c Mon Sep 17 00:00:00 2001 From: PurHur Date: Wed, 26 Aug 2026 18:15:41 +0000 Subject: [PATCH] Stdlib: AOT mb_trim() honors runtime encoding (#35199). NestedJIT assertEncodingArgv (peer #35193); preserve isNullConstant on ConstFetch null first-bind so default $characters works. Co-authored-by: Cursor --- ext/mbstring/JitMbTrim.php | 115 +++++++++++--------- ext/mbstring/MbTrimJitHelper.php | 48 ++++++-- lib/JIT.php | 4 + lib/JIT/Builtin/MbTrimRuntime.php | 12 ++ lib/JIT/Variable.php | 8 +- test/repro/aot_mb_trim_runtime_encoding.php | 26 +++++ test/unit/MbTrimRuntimeEncodingAotTest.php | 88 +++++++++++++++ 7 files changed, 236 insertions(+), 65 deletions(-) create mode 100644 test/repro/aot_mb_trim_runtime_encoding.php create mode 100644 test/unit/MbTrimRuntimeEncodingAotTest.php diff --git a/ext/mbstring/JitMbTrim.php b/ext/mbstring/JitMbTrim.php index 7fa683036c8..81f6128b49b 100644 --- a/ext/mbstring/JitMbTrim.php +++ b/ext/mbstring/JitMbTrim.php @@ -10,17 +10,17 @@ use PHPCompiler\JIT\JitStringArg; use PHPCompiler\JIT\JitStringBuiltinArg; use PHPCompiler\JIT\JitValueBox; -use PHPCompiler\JIT\TypeErrorRaise; use PHPCompiler\JIT\Variable as JITVariable; use PHPLLVM\Value; /** * LLVM JIT/AOT helpers for mb_trim() / mb_ltrim() / mb_rtrim() - * (php-src ext/mbstring/mbstring.c; #5957, #9208, #23883, #34379). + * (php-src ext/mbstring/mbstring.c; #5957, #9208, #23883, #34379, #35199). * * Compile-time fold for string literals; runtime haystack via NestedJIT - * {@see MbTrimJitHelper} (peer {@see JitMbScrub}). $characters + encoding stay - * compile-time literals. + * {@see MbTrimJitHelper} (peer {@see JitMbScrub}). Runtime encoding via + * NestedJIT assertEncodingArgv (#35199 leftover of #34379 / peer #35161). + * $characters stays a compile-time string or null. */ final class JitMbTrim { @@ -47,22 +47,6 @@ public static function invoke(Context $context, int $mode, string $function, arr $function.'() characters must be a compile-time string or null in this compiler build' ); } - $encoding = self::runtimeEncodingLiteral($args, $argc); - if (null === $encoding) { - throw new \LogicException( - $function.'() encoding must be a string literal in this compiler build' - ); - } - if (!MbstringEncodingRegistry::isValid($encoding)) { - return self::emitEncodingValueError($context, $function, $encoding); - } - // Mirror VmMbstring::assertTrimEncoding — only UTF-8 / ASCII / 8BIT in this build. - $canonical = self::canonicalTrimEncoding($encoding); - if (null === $canonical) { - throw new \LogicException( - $function.'() requires mbstring for encoding '.$encoding.' in this compiler build' - ); - } // Soft-null DEP+coerce on 8.4 (php-src mbstring.c; peer mb_scrub #21516). $str = JitStringBuiltinArg::lowerTrimFamilyString( @@ -79,6 +63,7 @@ public static function invoke(Context $context, int $mode, string $function, arr return self::materializeOwnedString($context, $str); } + // Link NestedJIT helpers before encoding lower — NestedJIT can invalidate prior IR (#34270 / #35199). $savedInsert = BasicBlockHelper::tryGetInsertBlock($context); MbTrimRuntime::ensureLinked($context); if (null !== $savedInsert) { @@ -87,12 +72,20 @@ public static function invoke(Context $context, int $mode, string $function, arr BasicBlockHelper::ensureOpenInsertBlock($context, $function.'_runtime'); if (null === $what) { + [$encPtr, $needsAssert] = self::encodingPtr($context, $args, $argc, $function); + if ($needsAssert) { + $fnName = $context->builder->load($context->constantStringFromString($function)); + $context->builder->call( + MbTrimRuntime::assertEncodingHelper($context), + $encPtr, + $fnName + ); + } $helper = match ($mode) { 1 => MbTrimRuntime::ltrimDefaultHelper($context), 2 => MbTrimRuntime::rtrimDefaultHelper($context), default => MbTrimRuntime::trimDefaultHelper($context), }; - $encPtr = $context->builder->load($context->constantStringFromString($canonical)); // Two-string ABI like mb_scrub — raw call; callHelper/`__value__` 1-arg SIGSEGVs. $resultStr = $context->builder->call($helper, $str, $encPtr); } else { @@ -101,6 +94,18 @@ public static function invoke(Context $context, int $mode, string $function, arr $function.'() with custom $characters only supports trim (both sides) in this compiler build' ); } + // Custom characters path still validates encoding when passed (#35199). + if ($argc >= 3) { + [$encPtr, $needsAssert] = self::encodingPtr($context, $args, $argc, $function); + if ($needsAssert) { + $fnName = $context->builder->load($context->constantStringFromString($function)); + $context->builder->call( + MbTrimRuntime::assertEncodingHelper($context), + $encPtr, + $fnName + ); + } + } $whatPtr = $context->builder->load($context->constantStringFromString($what)); $resultStr = $context->builder->call( MbTrimRuntime::trimCharsHelper($context), @@ -137,11 +142,7 @@ private static function tryCompileTimeFold(Context $context, int $mode, string $ return null; } - // Unknown encoding → do not throw ValueError during IR fold (breaks try/catch). - // Fall through to runtime invoke which emits catchable ValueError (#23883 / #34379). - if (!MbstringEncodingRegistry::isValid($encoding)) { - return null; - } + // Unknown / unsupported encoding → runtime NestedJIT assert (catchable ValueError) (#35199). if (null === self::canonicalTrimEncoding($encoding)) { return null; } @@ -193,47 +194,53 @@ private static function compileTimeEncoding(array $args, int $index): ?string } /** + * Literal UTF-8/ASCII/8BIT → constant string (no assert); otherwise NestedJIT encoding + assert (#35199). + * * @param list $args + * @return array{0: Value, 1: bool} encoding ptr, needsAssert */ - private static function runtimeEncodingLiteral(array $args, int $argc): ?string + private static function encodingPtr(Context $context, array $args, int $argc, string $function): array { - if ($argc < 3) { - return 'UTF-8'; + if ($argc < 3 || JITVariable::TYPE_NULL === $args[2]->type || ($args[2]->isNullConstant ?? false)) { + return [$context->builder->load($context->constantStringFromString('UTF-8')), false]; } - if (JITVariable::TYPE_NULL === $args[2]->type || ($args[2]->isNullConstant ?? false)) { - return 'UTF-8'; + + $encodingLit = JitStringArg::compileTimeLiteral($args[2]); + if (null !== $encodingLit) { + $canonical = self::canonicalTrimEncoding($encodingLit); + if (null !== $canonical) { + return [$context->builder->load($context->constantStringFromString($canonical)), false]; + } + + return [$context->builder->load($context->constantStringFromString($encodingLit)), true]; } - return JitStringArg::compileTimeLiteral($args[2]); + return [ + JitStringBuiltinArg::lower( + $context, + $args[2], + $function, + 2, + 'encoding' + ), + true, + ]; } private static function canonicalTrimEncoding(string $encoding): ?string { - try { - $valid = MbstringEncodingRegistry::assertValid($encoding, 'mb_trim', 2); - } catch (\ValueError) { - return null; + $upper = \strtoupper($encoding); + if ('UTF-8' === $upper || 'UTF8' === $upper) { + return 'UTF-8'; } - if ('UTF-8' !== $valid && 'ASCII' !== $valid && '8BIT' !== $valid) { - return null; + if ('ASCII' === $upper || 'US-ASCII' === $upper) { + return 'ASCII'; + } + if ('8BIT' === $upper || 'BINARY' === $upper) { + return '8BIT'; } - return $valid; - } - - private static function emitEncodingValueError(Context $context, string $function, string $encoding): Value - { - TypeErrorRaise::emitValueError( - $context, - sprintf( - '%s(): Argument #3 ($encoding) must be a valid encoding, "%s" given', - $function, - $encoding - ) - ); - BasicBlockHelper::ensureOpenInsertBlock($context, $function.'_bad_enc_dead'); - - return JitValueBox::pointer($context, JitValueBox::alloc($context)); + return null; } private static function materializeOwnedString(Context $context, Value $resultStr): Value diff --git a/ext/mbstring/MbTrimJitHelper.php b/ext/mbstring/MbTrimJitHelper.php index 68a0465d668..80ea5c611c4 100644 --- a/ext/mbstring/MbTrimJitHelper.php +++ b/ext/mbstring/MbTrimJitHelper.php @@ -15,33 +15,63 @@ * Rtrim NBSP uses a C2-hold deferral — `$last = $i - 1` / `$last === $n - 1` miscompile * under thin AOT (#34396 leftover). * + * Runtime encoding via {@see assertEncodingArgv} (#35199 leftover of #34379 / peer #35161). + * * Default charset: ASCII ws + U+00A0 (C2 A0). php-src: ext/mbstring/mbstring.c */ final class MbTrimJitHelper { - public static function trimDefault(string $value, string $encoding): string + /** + * Int-returning encoding check — NestedJIT ValueError from string-returning helpers + * SIGSEGVs under thin AOT; int helpers match {@see MbScrubJitHelper::assertEncodingArgv}. + * + * Argument #3 ($encoding) for mb_trim / mb_ltrim / mb_rtrim. + */ + public static function assertEncodingArgv(string $encoding, string $function): int { - if ('8BIT' === $encoding) { - return $value; + $ok = 0; + if ('UTF-8' === $encoding || 'utf-8' === $encoding || 'UTF8' === $encoding || 'utf8' === $encoding) { + $ok = 1; + } + if ( + 'ASCII' === $encoding || 'ascii' === $encoding + || 'US-ASCII' === $encoding || 'us-ascii' === $encoding + ) { + $ok = 1; } + if ('8BIT' === $encoding || '8bit' === $encoding || 'BINARY' === $encoding || 'binary' === $encoding) { + $ok = 1; + } + if (0 === $ok) { + // Concat (not sprintf) — NestedJIT sprintf+throw breaks module verify (#34625). + throw new \ValueError( + $function.'(): Argument #3 ($encoding) must be a valid encoding, "'.$encoding.'" given' + ); + } + + return 1; + } + + public static function trimDefault(string $value, string $encoding): string + { + // Encoding already validated via {@see assertEncodingArgv} (#35199). + // UTF-8 / ASCII / 8BIT share the default trim set for ASCII ws (+ UTF-8 NBSP); + // php-src mb_trim uses single-byte trim for ASCII/8BIT (VmMbstring::trimString). + unset($encoding); return self::trimRightBody(self::trimLeftBody($value)); } public static function ltrimDefault(string $value, string $encoding): string { - if ('8BIT' === $encoding) { - return $value; - } + unset($encoding); return self::trimLeftBody($value); } public static function rtrimDefault(string $value, string $encoding): string { - if ('8BIT' === $encoding) { - return $value; - } + unset($encoding); return self::trimRightBody($value); } diff --git a/lib/JIT.php b/lib/JIT.php index 8243561b1e6..99acdf93939 100644 --- a/lib/JIT.php +++ b/lib/JIT.php @@ -20165,6 +20165,7 @@ private function assignOperand(Operand $resultOp, Variable $value, bool $force = $slot ); $var->addref(); + $this->copyValueBoxJitFlags($var, $value, false); $this->context->setVariableOp($resultOp, $var); $resolved = JIT\OperandName::resolve($resultOp); if (null !== $resolved && '' !== $resolved) { @@ -20181,6 +20182,8 @@ private function assignOperand(Operand $resultOp, Variable $value, bool $force = ) { // HT dim-fetch copies into stack __value__ slots — first-bind must copy the // slot, not loadValue()+makeVariableFromValueOp (AOT abort / empty chain) (#31938). + // Also copy isNullConstant / compile-time flags — ConstFetch null for + // mb_trim($s, null, $enc) otherwise loses the null marker (#35199). $slot = JIT\JitValueBox::alloc($this->context); JIT\JitValueBox::copyFromPointer( $this->context, @@ -20194,6 +20197,7 @@ private function assignOperand(Operand $resultOp, Variable $value, bool $force = $slot ); $var->addref(); + $this->copyValueBoxJitFlags($var, $value, false); $this->context->setVariableOp($resultOp, $var); $resolved = JIT\OperandName::resolve($resultOp); if (null !== $resolved && '' !== $resolved) { diff --git a/lib/JIT/Builtin/MbTrimRuntime.php b/lib/JIT/Builtin/MbTrimRuntime.php index 19b007290a6..e257e20a397 100644 --- a/lib/JIT/Builtin/MbTrimRuntime.php +++ b/lib/JIT/Builtin/MbTrimRuntime.php @@ -11,6 +11,8 @@ /** * JIT/AOT link hook for mb_trim() / mb_ltrim() / mb_rtrim() — MbTrimJitHelper (#34379). * + * Runtime encoding assert: {@see MbTrimJitHelper::assertEncodingArgv} (#35199 leftover of #34379). + * * php-src: ext/mbstring/mbstring.c — PHP_FUNCTION(mb_trim) */ final class MbTrimRuntime @@ -25,12 +27,15 @@ final class MbTrimRuntime private const TRIM_CHARS = 'PHPCompiler\\ext\\mbstring\\MbTrimJitHelper::trimChars'; + private const ASSERT_ENCODING = 'PHPCompiler\\ext\\mbstring\\MbTrimJitHelper::assertEncodingArgv'; + /** @var list */ private const COMPILED_HELPERS = [ self::TRIM_DEFAULT, self::LTRIM_DEFAULT, self::RTRIM_DEFAULT, self::TRIM_CHARS, + self::ASSERT_ENCODING, ]; public static function ensureLinked(Context $context): void @@ -66,6 +71,13 @@ public static function trimCharsHelper(Context $context): LlvmFunction return JitVmHelperLink::lookupCompiled($context, self::TRIM_CHARS, 'mb_trim_chars'); } + public static function assertEncodingHelper(Context $context): LlvmFunction + { + self::ensureJitHelperCompiled($context); + + return JitVmHelperLink::lookupCompiled($context, self::ASSERT_ENCODING, 'mb_trim_encoding'); + } + private static function ensureJitHelperCompiled(Context $context): void { JitVmHelperLink::ensureCompiled( diff --git a/lib/JIT/Variable.php b/lib/JIT/Variable.php index 0cfd1cdb306..3fe9051cd19 100755 --- a/lib/JIT/Variable.php +++ b/lib/JIT/Variable.php @@ -656,14 +656,18 @@ public static function fromOp( } $type = self::getTypeFromType($op->type); if ($type === self::TYPE_NULL) { + // Match fromLiteral TYPE_NULL — keep isNullConstant so builtins can treat + // SSA null temps like literal null (mb_trim $characters, #35199). $slot = JitValueBox::alloc($context); - - return new Variable( + $nullVar = new Variable( $context, self::TYPE_VALUE, self::KIND_VARIABLE, $slot ); + $nullVar->isNullConstant = true; + + return $nullVar; } $stringType = self::getStringType($type); if ($type === self::TYPE_HASHTABLE) { diff --git a/test/repro/aot_mb_trim_runtime_encoding.php b/test/repro/aot_mb_trim_runtime_encoding.php new file mode 100644 index 00000000000..ea11adbc8c2 --- /dev/null +++ b/test/repro/aot_mb_trim_runtime_encoding.php @@ -0,0 +1,26 @@ +getMessage(), "\n"; +} diff --git a/test/unit/MbTrimRuntimeEncodingAotTest.php b/test/unit/MbTrimRuntimeEncodingAotTest.php new file mode 100644 index 00000000000..37b2c1fe24f --- /dev/null +++ b/test/unit/MbTrimRuntimeEncodingAotTest.php @@ -0,0 +1,88 @@ +markTestSkipped('LLVM 9 toolchain not available'); + } + $src = __DIR__.'/../repro/aot_mb_trim_runtime_encoding.php'; + $vm = $this->runVm($src); + $aot = $this->runAot($src); + $this->assertSame($vm, $aot); + $this->assertStringContainsString( + 'bad_enc=mb_trim(): Argument #3 ($encoding) must be a valid encoding, "nope" given', + $vm + ); + } + + public function testHelperAndLoweringPresent(): void + { + $root = dirname(__DIR__, 2); + $helper = (string) file_get_contents($root.'/ext/mbstring/MbTrimJitHelper.php'); + $this->assertStringContainsString('function assertEncodingArgv', $helper); + $this->assertStringContainsString('Argument #3', $helper); + $runtime = (string) file_get_contents($root.'/lib/JIT/Builtin/MbTrimRuntime.php'); + $this->assertStringContainsString('assertEncodingHelper', $runtime); + $this->assertStringContainsString('ASSERT_ENCODING', $runtime); + $jit = (string) file_get_contents($root.'/ext/mbstring/JitMbTrim.php'); + $this->assertStringContainsString('encodingPtr', $jit); + $this->assertStringNotContainsString( + 'encoding must be a string literal in this compiler build', + $jit + ); + $this->assertFileDoesNotExist($root.'/lib/AOT/runtime/mb_trim.c'); + } + + private function runVm(string $src): string + { + $root = dirname(__DIR__, 2); + $cmd = 'env PHP_COMPILER_PROFILE=8.4 ' + .escapeshellarg(PHP_BINARY).' ' + .escapeshellarg($root.'/bin/vm.php').' ' + .escapeshellarg($src).' 2>&1'; + exec($cmd, $out, $rc); + $this->assertSame(0, $rc, implode("\n", $out)); + + return implode("\n", $out); + } + + private function runAot(string $src): string + { + $root = dirname(__DIR__, 2); + $bin = sys_get_temp_dir().'/mb_trim_enc_'.getmypid().'_'.md5($src); + $cmd = 'env PHP_COMPILER_PROFILE=8.4 PHP_COMPILER_HELPER_RUNTIME_O=0 ' + .escapeshellarg(PHP_BINARY).' ' + .escapeshellarg($root.'/bin/compile.php') + .' -o '.escapeshellarg($bin).' '.escapeshellarg($src); + $cwd = getcwd(); + chdir($root); + try { + exec($cmd.' 2>&1', $compOut, $compRc); + $this->assertSame(0, $compRc, implode("\n", $compOut)); + $this->assertFileExists($bin); + exec(escapeshellarg($bin).' 2>&1', $out, $rc); + $this->assertSame(0, $rc, implode("\n", $out)); + + return implode("\n", $out); + } finally { + chdir($cwd); + @unlink($bin); + } + } +}