From 40c6861c96b4d92d8e939f622ec1cf7c7217b457 Mon Sep 17 00:00:00 2001 From: USAMI Kenta Date: Wed, 23 Sep 2026 11:50:21 +0900 Subject: [PATCH 1/5] Pass immutable array arguments non-refcounted through zv::Args zv::Args stored a HashTable argument with a bare ZVAL_ARR, which types an immutable table (the shared zend_empty_array of a PHP [] literal or zv::Arr::empty()) as refcounted. When the argument vector reached a PHP method, the addref in zend_call_function() wrote into read-only memory (SIGBUS on macOS arm64, SIGSEGV on Linux). ForeachHandler hit this for a constant array without keys: its conditional holder tables stay empty and are handed to a PHP override of MutatingScope::addConditionalExpressions(). Wrap immutable tables as plain IS_ARRAY, like zv::Arr::adoptTable() and copyOfTable() do. Co-authored-by: Claude Opus 5.5 --- build/php-85.neon | 6 ++ .../ConditionalExpressionsRecordingScope.php | 43 +++++++++++++ .../StmtHandler/ForeachHandlerTest.php | 64 +++++++++++++++++++ turbo-ext/src/zv.h | 10 ++- 4 files changed, 122 insertions(+), 1 deletion(-) create mode 100644 tests/PHPStan/Analyser/StmtHandler/ConditionalExpressionsRecordingScope.php create mode 100644 tests/PHPStan/Analyser/StmtHandler/ForeachHandlerTest.php diff --git a/build/php-85.neon b/build/php-85.neon index 6542e75f247..41c64921f3f 100644 --- a/build/php-85.neon +++ b/build/php-85.neon @@ -12,6 +12,12 @@ parameters: count: 1 path: ../src/Testing/TestCaseSourceLocatorFactory.php + - + rawMessage: 'Call to deprecated method setAccessible() of class ReflectionProperty.' + identifier: method.deprecated + count: 1 + path: ../tests/PHPStan/Analyser/StmtHandler/ForeachHandlerTest.php + - rawMessage: 'Call to deprecated method setAccessible() of class ReflectionProperty.' identifier: method.deprecated diff --git a/tests/PHPStan/Analyser/StmtHandler/ConditionalExpressionsRecordingScope.php b/tests/PHPStan/Analyser/StmtHandler/ConditionalExpressionsRecordingScope.php new file mode 100644 index 00000000000..a1d71f8d6db --- /dev/null +++ b/tests/PHPStan/Analyser/StmtHandler/ConditionalExpressionsRecordingScope.php @@ -0,0 +1,43 @@ + */ + public array $added = []; + + public function enterForeach(MutatingScope $originalScope, Expr $iteratee, Type $iterateeType, Type $nativeIterateeType, string $valueName, ?string $keyName, bool $valueByRef): MutatingScope + { + return $this; + } + + public function mergeWith(?MutatingScope $otherScope, bool $preserveVacuousConditionals = false): MutatingScope + { + return $this; + } + + /** + * @param ConditionalExpressionHolder[] $conditionalExpressionHolders + */ + public function addConditionalExpressions(string $exprString, array $conditionalExpressionHolders): MutatingScope + { + $this->added[] = [$exprString, count($conditionalExpressionHolders)]; + + return $this; + } + +} diff --git a/tests/PHPStan/Analyser/StmtHandler/ForeachHandlerTest.php b/tests/PHPStan/Analyser/StmtHandler/ForeachHandlerTest.php new file mode 100644 index 00000000000..e85accfc53e --- /dev/null +++ b/tests/PHPStan/Analyser/StmtHandler/ForeachHandlerTest.php @@ -0,0 +1,64 @@ +getService('typeSpecifier')) + ->create(ScopeContext::create(__FILE__)) + ->assignVariable('a', $iterateeType, $iterateeType, TrinaryLogic::createYes()); + // the scope factory only creates MutatingScope: rebuild the scope as + // the subclass from its constructor arguments + $args = []; + $reflection = new ReflectionClass(MutatingScope::class); + foreach ($reflection->getMethod('__construct')->getParameters() as $parameter) { + $property = $reflection->getProperty($parameter->getName()); + $property->setAccessible(true); + $args[] = $property->getValue($scope); + } + $scope = new ConditionalExpressionsRecordingScope(...$args); + + $container->getByType(ForeachHandler::class)->processStmt( + $container->getByType(NodeScopeResolver::class), + new Foreach_(new Variable('a'), new Variable('v'), ['keyVar' => new Variable('k')]), + $scope, + new ExpressionResultStorage(), + new NoopNodeCallback(), + StatementContext::createDeep(), + ); + + $this->assertSame([ + ['$v', $expectedHolders], + ['$a[$k]', $expectedHolders], + ], $scope->added); + } + +} diff --git a/turbo-ext/src/zv.h b/turbo-ext/src/zv.h index 9bf834d7cc5..72ec560d599 100644 --- a/turbo-ext/src/zv.h +++ b/turbo-ext/src/zv.h @@ -589,7 +589,15 @@ class Args static zend_always_inline void set(zval *slot, const zval *value) { ZVAL_COPY_VALUE(slot, value); } static zend_always_inline void set(zval *slot, zend_object *value) { ZVAL_OBJ(slot, value); } static zend_always_inline void set(zval *slot, zend_string *value) { ZVAL_STR(slot, value); } - static zend_always_inline void set(zval *slot, HashTable *value) { ZVAL_ARR(slot, value); } + /* immutable tables (a PHP [] literal) are wrapped non-refcounted, like + * ZVAL_EMPTY_ARRAY: the call's addref of its arguments must not touch them */ + static zend_always_inline void set(zval *slot, HashTable *value) + { + ZVAL_ARR(slot, value); + if (GC_FLAGS(value) & IS_ARRAY_IMMUTABLE) { + Z_TYPE_INFO_P(slot) = IS_ARRAY; + } + } static zend_always_inline void set(zval *slot, bool value) { ZVAL_BOOL(slot, value); } static zend_always_inline void set(zval *slot, zend_long value) { ZVAL_LONG(slot, value); } static zend_always_inline void set(zval *slot, double value) { ZVAL_DOUBLE(slot, value); } From b3ac0a4e2a3b8e5de06d21c0dbec7b770be412e6 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Wed, 23 Sep 2026 12:18:09 +0200 Subject: [PATCH 2/5] Do not addref interned parameter names in native PhpDocsResolver positionalNames() collected each parameter's Variable name with Z_ADDREF_P, which increments the refcount of an interned string too. Names built by PHP code (a `new Variable('foo')` literal) are interned, and with opcache those live in shared memory: the write faults under opcache.protect_memory=1 and trips the refcount assertion of debug PHP builds. Z_TRY_ADDREF_P leaves non-refcounted strings alone. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01EKX5yrrUnnbeRtc2oURC2u --- .../PHPStan/Analyser/PhpDocsResolverTest.php | 48 +++++++++++++++++++ .../php-docs-resolver-parameter-names.php | 16 +++++++ turbo-ext/src/PhpDocsResolver.cpp | 2 +- 3 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 tests/PHPStan/Analyser/PhpDocsResolverTest.php create mode 100644 tests/PHPStan/Analyser/data/php-docs-resolver-parameter-names.php diff --git a/tests/PHPStan/Analyser/PhpDocsResolverTest.php b/tests/PHPStan/Analyser/PhpDocsResolverTest.php new file mode 100644 index 00000000000..f18a439b494 --- /dev/null +++ b/tests/PHPStan/Analyser/PhpDocsResolverTest.php @@ -0,0 +1,48 @@ +getService('typeSpecifier')) + ->create(ScopeContext::create($file)) + ->enterClass($reflectionProvider->getClass('PhpDocsResolverParameterNames\Foo')); + + // the names are PHP literals (interned strings), unlike the ones + // the parser allocates + $node = new ClassMethod(new Identifier('doFoo'), [ + 'params' => [ + new Param(new Variable('a')), + new Param(new Variable('count')), + ], + ], [ + 'comments' => [ + new Doc("/**\n\t * @param non-empty-string \$a\n\t * @param positive-int \$count\n\t */"), + ], + ]); + + $phpDocParameterTypes = self::getContainer()->getByType(PhpDocsResolver::class)->getPhpDocs($scope, $node)[1]; + + $this->assertSame([ + 'a' => 'non-empty-string', + 'count' => 'int<1, max>', + ], array_map(static fn ($type) => $type->describe(VerbosityLevel::precise()), $phpDocParameterTypes)); + } + +} diff --git a/tests/PHPStan/Analyser/data/php-docs-resolver-parameter-names.php b/tests/PHPStan/Analyser/data/php-docs-resolver-parameter-names.php new file mode 100644 index 00000000000..3f6c2418fa1 --- /dev/null +++ b/tests/PHPStan/Analyser/data/php-docs-resolver-parameter-names.php @@ -0,0 +1,16 @@ + Date: Wed, 23 Sep 2026 12:19:16 +0200 Subject: [PATCH 3/5] Share the per-slot key table through copyOfTable in VariableLivenessResolver The offset-write branch wrapped a borrowed inner table with a bare ZVAL_ARR and addref'ed it, the pattern that faults when the table is immutable. The keysBySlot entries are always separated tables today, so nothing crashes, but zv::Arr::copyOfTable() handles the immutable case and says what the code means. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01EKX5yrrUnnbeRtc2oURC2u --- turbo-ext/src/VariableLivenessResolver.cpp | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/turbo-ext/src/VariableLivenessResolver.cpp b/turbo-ext/src/VariableLivenessResolver.cpp index dc193c7dd93..b93411d6e92 100644 --- a/turbo-ext/src/VariableLivenessResolver.cpp +++ b/turbo-ext/src/VariableLivenessResolver.cpp @@ -1365,13 +1365,7 @@ class VariableLivenessResolver zv::Str slotName = offsetKey(view.offset.raw()); selectedKeys = emptyArray(); HashTable *keys = innerTableByKey(keysBySlot, slotName.get()); - zval keysValue; - if (keys != NULL) { - ZVAL_ARR(&keysValue, keys); - Z_ADDREF(keysValue); - } else { - ZVAL_EMPTY_ARRAY(&keysValue); - } + zval keysValue = (keys != NULL ? zv::Arr::copyOfTable(keys) : zv::Arr::empty()).take(); SEPARATE_ARRAY(selectedKeys.raw()); zend_hash_update(Z_ARRVAL_P(selectedKeys.raw()), slotName.get(), &keysValue); } else { From 341ab113df1b3b4d74ff0f1599c003e27ec93ae7 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Wed, 23 Sep 2026 12:19:17 +0200 Subject: [PATCH 4/5] Separate optional keys after taking the snapshot in makeOffsetRequired() The snapshot was taken after separating, so it shared the table the loop deletes from: the deletes went into a table of refcount 2 that was also being iterated. Only the current entry is ever deleted, so the result was right, but the write violated copy-on-write and trips HT_ASSERT_RC1 on debug PHP builds. Taking the snapshot first makes separate() give the deletes their own table, at the same one duplication as before. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01EKX5yrrUnnbeRtc2oURC2u --- turbo-ext/src/ConstantArrayType.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/turbo-ext/src/ConstantArrayType.cpp b/turbo-ext/src/ConstantArrayType.cpp index 9505e7be3df..48026a0398e 100644 --- a/turbo-ext/src/ConstantArrayType.cpp +++ b/turbo-ext/src/ConstantArrayType.cpp @@ -5177,9 +5177,11 @@ class ConstantArrayType zv::Val keyValueZv = keyValue(keyType); if (UNEXPECTED(keyValueZv.isUndef())) return zv::Val(); - optionalKeysCopy.separate(); - /* iterated over a snapshot: the twin's foreach reads a copy */ + /* iterated over a snapshot: the twin's foreach reads a copy; + * taken before separating, so the deletes below go into a table + * of their own */ zv::Arr snapshot = zv::Arr::copyOfTable(optionalKeysCopy.table()); + optionalKeysCopy.separate(); for (zv::ArrayEntry optionalEntry : snapshot.arrRef()) { zval *key = optionalEntry.value().deref().raw(); bool keep = Z_TYPE_P(key) != IS_LONG || i != Z_LVAL_P(key); From 01c272f216d96ceed771e288ca176a051132e4c5 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Wed, 23 Sep 2026 12:19:17 +0200 Subject: [PATCH 5/5] Bump expected turbo version --- src/Turbo/TurboExtensionEnabler.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Turbo/TurboExtensionEnabler.php b/src/Turbo/TurboExtensionEnabler.php index a10fdc3d858..f3bea26a417 100644 --- a/src/Turbo/TurboExtensionEnabler.php +++ b/src/Turbo/TurboExtensionEnabler.php @@ -33,7 +33,7 @@ final class TurboExtensionEnabler { - public const EXPECTED_EXTENSION_VERSION = '56d1522'; + public const EXPECTED_EXTENSION_VERSION = '341ab11'; private static bool $active = false;