From 18dd9a8a321fb69d2d45d9a7e6e4a3abdfd7a143 Mon Sep 17 00:00:00 2001 From: tianfenghan Date: Tue, 11 Aug 2026 16:57:22 +0800 Subject: [PATCH] fix(compiler): prevent static method call cache pollution with dynamic trampolines - Add test case for repeated dynamic static calls not reusing released trampoline - Add test case for repeated static calls to runtime-defined magic class - Throw LogicException in getFuncPtr when class method name is detected - Update getFuncPtr to reject class method names with proper error message - Replace getFuncPtr with getMethodPtr for static method calls in method call trait - Update Closure::fromCallable handling to use getMethodPtr instead of getFuncPtr - Fix Python module trait to use getMethodPtr for PyCore methods - Update translator to clear function and class maps on every request shutdown - Refactor property access resolver to use compiled class declaration for parent lookup - Add test to verify subclass lookup uses compiled class declaration when parent index is missing - Update request shutdown test to run in all build modes with proper assertions - Add specific test for function pointer cache rejecting class method names --- phpunit/src/CompilerBaseApiTest.php | 56 ++++++++++++------- phpunit/src/Python/PythonModuleTest.php | 3 +- .../Resolver/PropertyAccessResolverTest.php | 32 +++++++++++ src/CompilerBase.php | 3 + src/Generator/PlaceHolderGenerator.php | 2 +- src/Parser/MethodCallTrait.php | 19 ++----- src/Python/PythonModuleTrait.php | 12 ++-- src/Resolver/PropertyAccessResolver.php | 15 ++++- src/Translator.php | 13 ++--- .../call-static-trampoline-repeat.phpt | 23 ++++++++ .../static-call-runtime-magic-repeat.phpt | 25 +++++++++ 11 files changed, 152 insertions(+), 51 deletions(-) create mode 100644 tests/compiler/dynamic_call/call-static-trampoline-repeat.phpt create mode 100644 tests/compiler/dynamic_call/static-call-runtime-magic-repeat.phpt diff --git a/phpunit/src/CompilerBaseApiTest.php b/phpunit/src/CompilerBaseApiTest.php index a4e6d1e6..2d0dbb7b 100644 --- a/phpunit/src/CompilerBaseApiTest.php +++ b/phpunit/src/CompilerBaseApiTest.php @@ -916,30 +916,44 @@ YAML); $this->assertContains('/usr/local/lib', $libraryPaths); } - public function testExtensionCleanClearsRuntimeMapsWithValidCpp(): void + public function testRequestShutdownClearsRuntimeMapsInEveryBuildMode(): void { global $translator; - $compiler = CompilerTest::create(ROOT_PATH); - $translator = $compiler; - $ref = new \ReflectionClass($compiler); - $buildMode = $ref->getProperty('buildMode'); - $buildMode->setAccessible(true); - $buildMode->setValue($compiler, CompilerBase::BUILD_MODE_EXT); + foreach ([CompilerBase::BUILD_MODE_BIN, CompilerBase::BUILD_MODE_LIB, CompilerBase::BUILD_MODE_EXT] as $mode) { + $compiler = CompilerTest::create(ROOT_PATH); + $translator = $compiler; + $compiler->setBuildMode($mode); + + $testFile = ROOT_PATH . '/phpunit/code/compiler_api/extension_clean_maps.php'; + $compiler->addFiles([$testFile]); + $compiler->prepareFile($testFile); + $compiler->convertFile($testFile); + $code = file_get_contents($compiler->genExtension()); + + $this->assertStringContainsString('#include ', $code, $mode); + $this->assertStringContainsString( + 'std::memset(php_func_map, 0, sizeof(php_func_map));', + $code, + $mode, + ); + $this->assertStringContainsString( + 'std::memset(php_class_map, 0, sizeof(php_class_map));', + $code, + $mode, + ); + $this->assertStringNotContainsString('php_property_map', $code, $mode); + $this->assertStringNotContainsString('func_map = {}', $code, $mode); + $this->assertStringNotContainsString('class_map = {}', $code, $mode); + $this->assertStringNotContainsString('property_map = {}', $code, $mode); + } + } - $testFile = ROOT_PATH . '/phpunit/code/compiler_api/extension_clean_maps.php'; - $compiler->addFiles([$testFile]); - $compiler->prepareFile($testFile); - $compiler->convertFile($testFile); - $extensionFile = $compiler->genExtension(); - $code = file_get_contents($extensionFile); - - $this->assertStringContainsString('#include ', $code); - $this->assertStringContainsString('std::memset(php_func_map, 0, sizeof(php_func_map));', $code); - $this->assertStringContainsString('std::memset(php_class_map, 0, sizeof(php_class_map));', $code); - $this->assertStringNotContainsString('php_property_map', $code); - $this->assertStringNotContainsString('func_map = {}', $code); - $this->assertStringNotContainsString('class_map = {}', $code); - $this->assertStringNotContainsString('property_map = {}', $code); + public function testFunctionPointerCacheRejectsClassMethodNames(): void + { + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('Class methods must be resolved through getMethodPtr()'); + + $this->invokeMethod('getFuncPtr', 'RuntimeClass::method'); } public function testPersistentSymbolCachesAreLazyAndZtsSafe(): void diff --git a/phpunit/src/Python/PythonModuleTest.php b/phpunit/src/Python/PythonModuleTest.php index 8c808fa7..9d1118a6 100644 --- a/phpunit/src/Python/PythonModuleTest.php +++ b/phpunit/src/Python/PythonModuleTest.php @@ -216,7 +216,8 @@ final class PythonModuleTest extends TestCase $this->assertStringContainsString('PyDict', $extension); $this->assertStringContainsString('THREAD_LOCAL zval php_python_module_map[1]', $extension); $this->assertStringContainsString('builtins', $extension); - $this->assertStringContainsString('PyCore::setOptions', $extension); + $this->assertStringContainsString('php_get_persistent_method(', $extension); + $this->assertStringContainsString('setOptions', $extension); $this->assertStringContainsString('return_as_object', $extension); $this->assertStringContainsString('php_python_runtime_configured = false;', $extension); $this->assertStringNotContainsString('python\\\\list', $extension); diff --git a/phpunit/src/Resolver/PropertyAccessResolverTest.php b/phpunit/src/Resolver/PropertyAccessResolverTest.php index b2ef5563..2d63c48b 100644 --- a/phpunit/src/Resolver/PropertyAccessResolverTest.php +++ b/phpunit/src/Resolver/PropertyAccessResolverTest.php @@ -41,4 +41,36 @@ final class PropertyAccessResolverTest extends TestCase $this->assertTrue($resolver->isSameOrSubclassOf('App\\ChildException', 'runtimeexception')); $this->assertTrue($resolver->canAccessProtectedProperty('App\\ChildException', 'RuntimeException')); } + + public function testSubclassLookupUsesCompiledClassDeclarationWhenParentIndexIsMissing(): void + { + $child = new ClassDef('ChildException', 0, 'App'); + $child->extends = 'RuntimeException'; + + $context = new class($child) implements PropertyAccessContext { + public function __construct(private ClassDef $child) + { + } + + public function getClassDef(string $name): ?ClassDef + { + return strtolower(ltrim($name, '\\')) === 'app\\childexception' ? $this->child : null; + } + + public function getParentClass(string $class): string + { + return ''; + } + + public function fatalError(NodeAbstract $node, string $msg): never + { + throw new \LogicException($msg); + } + }; + + $resolver = new PropertyAccessResolver($context); + + $this->assertTrue($resolver->isSameOrSubclassOf('App\\ChildException', 'runtimeexception')); + $this->assertTrue($resolver->canAccessProtectedProperty('App\\ChildException', 'RuntimeException')); + } } diff --git a/src/CompilerBase.php b/src/CompilerBase.php index f48e9d3d..99a0f0f6 100644 --- a/src/CompilerBase.php +++ b/src/CompilerBase.php @@ -1334,6 +1334,9 @@ class CompilerBase implements PropertyAccessContext protected function getFuncPtr(string $funcName): string { + if (str_contains($funcName, '::')) { + throw new \LogicException('Class methods must be resolved through getMethodPtr()'); + } $id = $this->getFuncId($funcName); $helper = isset($this->persistentFuncMap[$funcName]) ? 'php_get_persistent_func' : 'php_get_func'; return $helper . '(' . $id . ', ' . $this->getLiteralString($funcName) . ')'; diff --git a/src/Generator/PlaceHolderGenerator.php b/src/Generator/PlaceHolderGenerator.php index 40e2adab..fe3bb7bf 100644 --- a/src/Generator/PlaceHolderGenerator.php +++ b/src/Generator/PlaceHolderGenerator.php @@ -17,7 +17,7 @@ trait PlaceHolderGenerator } $ce = $this->getClassEntryPtr(\Closure::class); - $fn = $ce . ', ' . $this->getFuncPtr('Closure::fromCallable'); + $fn = $ce . ', ' . $this->getMethodPtr('Closure', 'fromCallable'); return 'php::call(' . $fn . ', {' . $callable . '})'; } } diff --git a/src/Parser/MethodCallTrait.php b/src/Parser/MethodCallTrait.php index ba1384b6..92417ce8 100644 --- a/src/Parser/MethodCallTrait.php +++ b/src/Parser/MethodCallTrait.php @@ -683,7 +683,6 @@ trait MethodCallTrait $method = $this->parseIdentifier($expr->name); $rtFunc = $method; $rtClass = $class; - $dynamicCall = false; $this->context->beforeStmtLines[] = $this->formatCppLineComment( 'Static Method Call: ', $class . '::' . $method . '()' @@ -695,16 +694,11 @@ trait MethodCallTrait if ($callScope) { $nativeFunc = $this->getNativeMethod($expr, $class, $method); - // 存在 Native 类,但是没有找到方法,可能是动态调用 - if (!$nativeFunc and $this->hasClass($class) and $this->getNativeMethod($expr, $class, '__callStatic', false)) { - $dynamicCall = true; - } - if ($nativeFunc) { try { if ($this->shouldUseDynamicCallForNativeArgs($nativeFunc, $expr->args)) { return $this->genRuntimeFunctionCall( - $this->getClassEntryPtr($class) . ', ' . $this->getFuncPtr($class . '::' . $method), + $this->getClassEntryPtr($class) . ', ' . $this->getMethodPtr($class, $method), $expr->args, $method, $class @@ -729,12 +723,11 @@ trait MethodCallTrait } } - if ($dynamicCall) { - $fn = $this->getLiteralString($class . '::' . $method); - } else { - $ce = $this->getClassEntryPtr($class); - $fn = $ce . ', ' . $this->getFuncPtr($class . '::' . $method); - } + // Reaching this fallback means no concrete native method was + // proven above. Keep the callable dynamic so Zend can resolve a + // runtime-defined method or __callStatic(). PHPX only caches real, + // reusable handlers and never stores transient trampolines. + $fn = $this->getLiteralString($class . '::' . $method); $placeHolder = $this->genArray($callScope); } else { $fn = 'php::concat({' . $this->identifierToStr($expr->class) . ', "::", ' . $this->identifierToStr($expr->name) . '})'; diff --git a/src/Python/PythonModuleTrait.php b/src/Python/PythonModuleTrait.php index 00adbc57..0357203c 100644 --- a/src/Python/PythonModuleTrait.php +++ b/src/Python/PythonModuleTrait.php @@ -140,7 +140,7 @@ trait PythonModuleTrait { $this->markPythonRuntimeUsed(); $scalar = 'php::call(' . $this->getClassEntryPtr('PyCore') . ', ' - . $this->getFuncPtr('PyCore::scalar') . ', php::ArgList{' . $expression . '})'; + . $this->getMethodPtr('PyCore', 'scalar') . ', php::ArgList{' . $expression . '})'; return 'php::toBool(' . $scalar . ')'; } @@ -330,7 +330,7 @@ trait PythonModuleTrait $this->markPythonRuntimeUsed(); $this->getFuncId('PyCore::import'); - $this->getLiteralString('PyCore::import'); + $this->getLiteralString('import'); $this->getLiteralString($module); return $id; @@ -348,7 +348,7 @@ trait PythonModuleTrait $this->getClassId('PyCore'); $this->getFuncId('PyCore::setOptions'); $this->getLiteralString('PyCore'); - $this->getLiteralString('PyCore::setOptions'); + $this->getLiteralString('setOptions'); $this->getLiteralString('return_as_object'); } @@ -444,7 +444,7 @@ trait PythonModuleTrait // or (`scalar`) explicitly leave Python's object-preserving rules. if (isset(self::PYTHON_CORE_FUNCTIONS[$builtin])) { $callable = $this->getClassEntryPtr('PyCore') . ', ' - . $this->getFuncPtr('PyCore::' . $builtin); + . $this->getMethodPtr('PyCore', $builtin); if ($expr->args === []) { return $this->withPythonRuntimeConfigured('php::call(' . $callable . ')'); } @@ -648,7 +648,7 @@ trait PythonModuleTrait } $pyCoreClass = $this->getClassEntryPtr('PyCore'); - $setOptionsFunction = $this->getFuncPtr('PyCore::setOptions'); + $setOptionsFunction = $this->getMethodPtr('PyCore', 'setOptions'); $returnAsObject = $this->getLiteralString('return_as_object'); $code = 'void ' . self::PREFIX . 'configure_python_runtime() {' . PHP_EOL @@ -665,7 +665,7 @@ trait PythonModuleTrait return $code; } - $importFunction = $this->getFuncPtr('PyCore::import'); + $importFunction = $this->getMethodPtr('PyCore', 'import'); return $code . 'php::Object ' . self::PREFIX diff --git a/src/Resolver/PropertyAccessResolver.php b/src/Resolver/PropertyAccessResolver.php index c720115c..e53858b1 100644 --- a/src/Resolver/PropertyAccessResolver.php +++ b/src/Resolver/PropertyAccessResolver.php @@ -35,9 +35,15 @@ final class PropertyAccessResolver return true; } + // The conversion snapshot may not retain every user-to-internal + // edge in the parent index. A compiled class still carries its + // declared parent, so prefer that value when it is available. + $classDef = $this->compiler->getClassDef($class); + $next = $classDef?->extends ?: $this->compiler->getParentClass($class); + // Parent names retain declaration casing. Normalize every hop, // not only the initial class name. - $class = strtolower(ltrim($this->compiler->getParentClass($class), '\\')); + $class = strtolower(ltrim($next, '\\')); } return false; } @@ -189,7 +195,12 @@ final class PropertyAccessResolver $declaringClass = $propRef->getDeclaringClass()->getName(); if ($propRef->isProtected()) { - if (!$this->canAccessProtectedProperty($scope, $declaringClass)) { + // Reaching an internal declaration by walking requestedClass's + // compiled parent chain already proves that requestedClass may + // access the inherited protected slot. Avoid rebuilding that + // relationship from a conversion-time parent index here. + if (!$this->isSameClassName($scope, $requestedClass) + && !$this->canAccessProtectedProperty($scope, $declaringClass)) { $displayClass = ltrim($requestedClass, '\\'); $this->fatal($expr, "Cannot access protected property `{$property}` of class `{$displayClass}`"); } diff --git a/src/Translator.php b/src/Translator.php index 68af7914..caa69756 100644 --- a/src/Translator.php +++ b/src/Translator.php @@ -1116,13 +1116,12 @@ CODE; } } - // 扩展模式,需要在 RSHUTDOWN 阶段中清理用户定义的函数、类缓存; - // 内置/编译产物符号在 persistent_*_map 中,跨请求存活,无需清理; - // 无动态 propMap(属性 offset 缓存仅含进程级稳定的声明属性),无需清理 - if ($this->isBuildModeExt()) { - $code .= 'std::memset(' . self::PREFIX . self::FUNC_MAP . ', 0, sizeof(' . self::PREFIX . self::FUNC_MAP . '));' . PHP_EOL; - $code .= 'std::memset(' . self::PREFIX . self::CLASS_MAP . ', 0, sizeof(' . self::PREFIX . self::CLASS_MAP . '));' . PHP_EOL; - } + // User-code symbols have request lifetime regardless of the build mode. + // Embedded/library hosts may start more than one Zend request in the + // same process, so never let these pointers survive RSHUTDOWN. + // Internal/compiled symbols remain in the module-lifetime persistent maps. + $code .= 'std::memset(' . self::PREFIX . self::FUNC_MAP . ', 0, sizeof(' . self::PREFIX . self::FUNC_MAP . '));' . PHP_EOL; + $code .= 'std::memset(' . self::PREFIX . self::CLASS_MAP . ', 0, sizeof(' . self::PREFIX . self::CLASS_MAP . '));' . PHP_EOL; $code .= '}' . PHP_EOL . PHP_EOL; // php_app_clean end diff --git a/tests/compiler/dynamic_call/call-static-trampoline-repeat.phpt b/tests/compiler/dynamic_call/call-static-trampoline-repeat.phpt new file mode 100644 index 00000000..5d7004cf --- /dev/null +++ b/tests/compiler/dynamic_call/call-static-trampoline-repeat.phpt @@ -0,0 +1,23 @@ +--TEST-- +Repeated dynamic static calls do not reuse a released Zend trampoline +--FILE-- + +--EXPECT-- +string(9) "missing:0" +string(9) "missing:1" +string(9) "missing:2" diff --git a/tests/compiler/dynamic_call/static-call-runtime-magic-repeat.phpt b/tests/compiler/dynamic_call/static-call-runtime-magic-repeat.phpt new file mode 100644 index 00000000..b13b3e24 --- /dev/null +++ b/tests/compiler/dynamic_call/static-call-runtime-magic-repeat.phpt @@ -0,0 +1,25 @@ +--TEST-- +Repeated static calls to a runtime-defined magic class use transient callables safely +--FILE-- + +--EXPECT-- +string(9) "missing:0" +string(9) "missing:1" +string(9) "missing:2"