From df610fd2f5ffc168df35cfec9cc78021f8fd6286 Mon Sep 17 00:00:00 2001 From: Alessio Giacobbe Date: Tue, 1 Sep 2026 13:55:00 +0200 Subject: [PATCH 1/4] fix(gen_stub): register enum-case class constants as persistent AST constants A class constant valued by an enum case was registered with the folded scalar (the backing value, or the case name for pure cases), so constant('K::CB'), $cls::CB and reflection observed an int/string where PHP has the case object, and K::CB === E::B was false on every dynamic path. Enum case objects have request lifetime and can never sit in the persistent class-entry tables, in any request-init rebinding scheme least of all: writing a request-owned object into the shared table is unsafe under concurrent ZTS requests. Reuse the engine's own mechanism for internal enums instead: the constant is declared as a persistent IS_CONSTANT_AST holding the Enum::Case fetch, so Zend separates the class constants table into request-local mutable storage on first access, evaluates the fetch there, and cleans it up at request shutdown. Identity is preserved for static access, constant(), dynamic class access and reflection, with no module-lifecycle hooks and no registration-order sensitivity. Case identity flows through compile-time constant evaluation as an EnumCaseRef value instead of a scalar, so it also survives constant expressions (true ? E::A : E::B), constant chains, typed class constants (declared type and AST value are registered together), and internal enum cases such as RoundingMode::HalfEven, which previously aborted stub generation. The runtime expression path (php::getEnumCase) is unchanged. The preprocessor also no longer reads the raw ->value property off arbitrary case expressions (`case A = 1 + 1;` warned and was recorded as a pure case): only literal backing values are recorded eagerly, and no compile-time consumer needs the evaluated scalar - gen_stub evaluates the registration value from the AST itself. --- phpunit/code/enum-case-class-constant.php | 24 ++++++++ phpunit/src/EnumCaseClassConstantTest.php | 61 +++++++++++++++++++ src/Entity/EnumCaseRef.php | 25 ++++++++ src/Preprocessor.php | 11 +++- src/Resolver/ClassConstantValueTrait.php | 19 ++++-- src/gen_stub.php | 49 +++++++++++++++ .../enum/enum-case-class-constant.phpt | 57 +++++++++++++++++ 7 files changed, 241 insertions(+), 5 deletions(-) create mode 100644 phpunit/code/enum-case-class-constant.php create mode 100644 phpunit/src/EnumCaseClassConstantTest.php create mode 100644 src/Entity/EnumCaseRef.php create mode 100644 tests/compiler/enum/enum-case-class-constant.phpt diff --git a/phpunit/code/enum-case-class-constant.php b/phpunit/code/enum-case-class-constant.php new file mode 100644 index 00000000..053e551d --- /dev/null +++ b/phpunit/code/enum-case-class-constant.php @@ -0,0 +1,24 @@ +addFiles([$source]); + $compiler->prepareFile($source); + $compiler->convertFile($source); + $this->arginfo = file_get_contents( + TYPEPHP_ROOT_PATH . '/' . 'build/include/' . basename($compiler->getArgInfoHeaderFile($source)) + ); + } + + public function testDirectCaseRegistersConstantAst(): void + { + self::assertStringContainsString('const_CB_value_fetch_ast->kind = ZEND_AST_CLASS_CONST;', $this->arginfo); + self::assertStringContainsString('zend_string_init_interned("CodegenEnum", sizeof("CodegenEnum") - 1, 1)', $this->arginfo); + self::assertStringNotContainsString('ZVAL_LONG(&const_CB_value', $this->arginfo); + } + + public function testConstantExpressionFoldsToCaseIdentity(): void + { + // true ? A : B folds to the A case identity, not to a scalar. + self::assertMatchesRegularExpression( + '/const_PICKED_value_case_name = zend_string_init_interned\("A"/', + $this->arginfo, + ); + } + + public function testTypedConstantKeepsDeclaredTypeAndAstValue(): void + { + self::assertStringContainsString('const_CASE_VALUE_value_fetch_ast->kind = ZEND_AST_CLASS_CONST;', $this->arginfo); + self::assertStringContainsString('zend_declare_typed_class_constant(class_entry, const_CASE_VALUE_name', $this->arginfo); + } + + public function testInternalEnumCaseRegistersConstantAst(): void + { + self::assertStringContainsString('zend_string_init_interned("RoundingMode", sizeof("RoundingMode") - 1, 1)', $this->arginfo); + } + + public function testExpressionValuedBackedCaseRegistersComputedValue(): void + { + self::assertStringContainsString('ZVAL_LONG(&enum_case_A_value, 2);', $this->arginfo); + } +} diff --git a/src/Entity/EnumCaseRef.php b/src/Entity/EnumCaseRef.php new file mode 100644 index 00000000..d92206cd --- /dev/null +++ b/src/Entity/EnumCaseRef.php @@ -0,0 +1,25 @@ +parseIdentifier($v->name); - $this->classDef->enumCases[$caseName] = $v->expr?->value; + // Only literal backing values are recorded here; an + // expression-valued case (`case A = 1 + 1;`) cannot be + // evaluated while declarations are still being collected, + // and no compile-time consumer needs the scalar: case + // identity flows as EnumCaseRef and gen_stub evaluates + // the registration value from the AST itself. + $this->classDef->enumCases[$caseName] = + $v->expr instanceof Node\Scalar\Int_ || $v->expr instanceof Node\Scalar\String_ + ? $v->expr->value + : null; break; case 'Stmt_ClassMethod': $this->prepareClassMethod($v, $class); diff --git a/src/Resolver/ClassConstantValueTrait.php b/src/Resolver/ClassConstantValueTrait.php index 90ec17e5..873e8e10 100644 --- a/src/Resolver/ClassConstantValueTrait.php +++ b/src/Resolver/ClassConstantValueTrait.php @@ -12,6 +12,7 @@ use PhpParser\Node; use PhpParser\NodeAbstract; use TypePhp\Entity\ConstantDef; +use TypePhp\Entity\EnumCaseRef; trait ClassConstantValueTrait { @@ -48,7 +49,12 @@ public function getClassConstValue(NodeAbstract $expr, string $_class, string $n if ($this->isInternalClass($class)) { $constName = $class . '::' . $name; if (defined($constName)) { - return constant($constName); + $value = constant($constName); + // Internal enum cases (and internal constants holding one) + // must keep their identity through constant evaluation. + return $value instanceof \UnitEnum + ? new EnumCaseRef(get_class($value), $value->name) + : $value; } } [$inheritedFound, $inherited] = $this->resolveInheritedClassConst($class, $name); @@ -58,8 +64,10 @@ public function getClassConstValue(NodeAbstract $expr, string $_class, string $n if ($this->hasClass($class)) { $classDef = $this->getClass($class); if ($classDef->enum && array_key_exists($name, $classDef->enumCases)) { - $caseValue = $classDef->enumCases[$name]; - return $caseValue ?? $name; + // The case IDENTITY is the constant's value; folding to the + // backing scalar (or the case name) would make + // `K::CONST === E::Case` false through every dynamic path. + return new EnumCaseRef($classDef->getNamespacedName(false), $name); } } $this->fatalError($expr, "Class constant `{$class}::{$name}` not found"); @@ -89,7 +97,10 @@ protected function resolveInheritedClassConst(string $class, string $name): arra } elseif (Reflection::isInternalClass($current)) { $constName = $current . '::' . $name; if (defined($constName)) { - return [true, constant($constName)]; + $value = constant($constName); + return [true, $value instanceof \UnitEnum + ? new EnumCaseRef(get_class($value), $value->name) + : $value]; } break; } else { diff --git a/src/gen_stub.php b/src/gen_stub.php index 97aeb91d..8b454266 100755 --- a/src/gen_stub.php +++ b/src/gen_stub.php @@ -2764,6 +2764,9 @@ private function __construct($value, SimpleType $type, Expr $expr, array $origin public function initializeZval(string $zvalName, bool $alreadyExists = false, string $forStringDef = '', string $varName = ''): string { + if ($this->value instanceof \TypePhp\Entity\EnumCaseRef) { + return $this->initializeEnumCaseZval($zvalName, $alreadyExists); + } $cExpr = $this->getCExpr(); $code = ''; @@ -2810,6 +2813,52 @@ public function initializeZval(string $zvalName, bool $alreadyExists = false, st return $code; } + /** + * Initialize the zval as a persistent IS_CONSTANT_AST holding + * `EnumClass::CaseName`. Enum case objects have request lifetime and can + * never sit in the persistent class-entry tables, so the engine's own + * mechanism for internal enums is reused: declaring an AST constant makes + * Zend separate the class constants table into request-local mutable + * storage, evaluate the fetch there on first access, and clean it up at + * request shutdown. This keeps case identity intact for static access, + * constant(), and reflection, and is safe under concurrent ZTS requests. + */ + private function initializeEnumCaseZval(string $zvalName, bool $alreadyExists): string + { + /** @var \TypePhp\Entity\EnumCaseRef $case */ + $case = $this->value; + $enumCName = '"' . getTranslator()->escapeString(ltrim($case->enumClass, '\\')) . '"'; + $caseCName = '"' . getTranslator()->escapeString($case->caseName) . '"'; + $id = preg_replace('/[^A-Za-z0-9_]/', '_', $zvalName); + + $code = $alreadyExists ? '' : "\tzval $zvalName;\n"; + $code .= "\t{\n"; + $code .= "\t\tzend_string *{$id}_enum_name = zend_string_init_interned($enumCName, sizeof($enumCName) - 1, 1);\n"; + $code .= "\t\tzend_string *{$id}_case_name = zend_string_init_interned($caseCName, sizeof($caseCName) - 1, 1);\n"; + $code .= "\t\tzend_ast_zval *{$id}_class_ast = (zend_ast_zval *) pemalloc(sizeof(zend_ast_zval), 1);\n"; + $code .= "\t\t{$id}_class_ast->kind = ZEND_AST_ZVAL;\n"; + $code .= "\t\t{$id}_class_ast->attr = ZEND_NAME_FQ;\n"; + $code .= "\t\tZVAL_INTERNED_STR(&{$id}_class_ast->val, {$id}_enum_name);\n"; + $code .= "\t\tZ_LINENO({$id}_class_ast->val) = 0;\n"; + $code .= "\t\tzend_ast_zval *{$id}_const_ast = (zend_ast_zval *) pemalloc(sizeof(zend_ast_zval), 1);\n"; + $code .= "\t\t{$id}_const_ast->kind = ZEND_AST_ZVAL;\n"; + $code .= "\t\t{$id}_const_ast->attr = 0;\n"; + $code .= "\t\tZVAL_INTERNED_STR(&{$id}_const_ast->val, {$id}_case_name);\n"; + $code .= "\t\tZ_LINENO({$id}_const_ast->val) = 0;\n"; + $code .= "\t\tzend_ast_ref *{$id}_ast_ref = (zend_ast_ref *) pemalloc(sizeof(zend_ast_ref) + ZEND_MM_ALIGNED_SIZE(zend_ast_size(2)), 1);\n"; + $code .= "\t\tGC_SET_REFCOUNT({$id}_ast_ref, 1);\n"; + $code .= "\t\tGC_TYPE_INFO({$id}_ast_ref) = GC_CONSTANT_AST | ((GC_PERSISTENT | GC_IMMUTABLE) << GC_FLAGS_SHIFT);\n"; + $code .= "\t\tzend_ast *{$id}_fetch_ast = GC_AST({$id}_ast_ref);\n"; + $code .= "\t\t{$id}_fetch_ast->kind = ZEND_AST_CLASS_CONST;\n"; + $code .= "\t\t{$id}_fetch_ast->attr = 0;\n"; + $code .= "\t\t{$id}_fetch_ast->lineno = 0;\n"; + $code .= "\t\t{$id}_fetch_ast->child[0] = (zend_ast *) {$id}_class_ast;\n"; + $code .= "\t\t{$id}_fetch_ast->child[1] = (zend_ast *) {$id}_const_ast;\n"; + $code .= "\t\tZVAL_AST(&$zvalName, {$id}_ast_ref);\n"; + $code .= "\t}\n"; + return $code; + } + public function getCExpr(): ?string { // $this->expr has all its PHP constants replaced by C constants diff --git a/tests/compiler/enum/enum-case-class-constant.phpt b/tests/compiler/enum/enum-case-class-constant.phpt new file mode 100644 index 00000000..0436d07c --- /dev/null +++ b/tests/compiler/enum/enum-case-class-constant.phpt @@ -0,0 +1,57 @@ +--TEST-- +Class constants valued by enum cases keep case identity everywhere +--FILE-- +getValue() === TypedCase::A); + var_dump((string) (new ReflectionClassConstant('K', 'CASE_VALUE'))->getType()); + // Expression-valued backed case keeps its computed backing value + var_dump(E::A->value); + var_dump(K::VALUE->value); +} +?> +--EXPECT-- +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) +string(9) "TypedCase" +int(2) +int(2) From ba07773b5a69b596ea76e28ea3ac51c955bb03c2 Mon Sep 17 00:00:00 2001 From: Alessio Giacobbe Date: Tue, 1 Sep 2026 15:47:13 +0200 Subject: [PATCH 2/4] fix(gen_stub): constrain enum-case AST registration to class constants The persistent IS_CONSTANT_AST representation leaked into property and parameter defaults, whose persistent tables reject refcounted zvals: startup died with "Internal zvals cannot be refcounted". EvaluatedValue now carries the case identity in a dedicated field while its value degrades to what those consumers read before case identity existed (the host case object for internal enums, the literal backing value or case name for compiled ones), and only class-constant registration opts into the AST. Property/parameter defaults keep flowing through their existing runtime-restore machinery unchanged. Also parenthesize a folded constant operand before appending a member access: the C++ ternary of `const VALUE = cond ? E::A : E::B;` bound `.attr("value")` to its else branch only, so `K::VALUE->value` evaluated to the case object instead of its backing value. --- src/Parser/PropertyAccessTrait.php | 37 +++++++++++++++++++++++- src/Resolver/ClassConstantValueTrait.php | 24 +++++++++++++++ src/gen_stub.php | 27 +++++++++++++---- 3 files changed, 81 insertions(+), 7 deletions(-) diff --git a/src/Parser/PropertyAccessTrait.php b/src/Parser/PropertyAccessTrait.php index 84f1db88..52881fb8 100644 --- a/src/Parser/PropertyAccessTrait.php +++ b/src/Parser/PropertyAccessTrait.php @@ -1170,7 +1170,7 @@ protected function parsePropertyFetch(Expr\PropertyFetch $expr): string return $this->getNativeObjectMemberReceiver($objectName) . $this->getNativeObjectPropertyCppName($resolution->propertyDef, $resolution->classDef); } - $objectVar = $objectName; + $objectVar = $this->parenthesizeOpenOperand($objectName); $directMagic = !$update && !$this->isNativePropertyAccess($expr) ? $this->resolveDirectMagicPropertyAccess($expr, $objectVar, '__get') : null; @@ -1335,4 +1335,39 @@ private function emitNativeInstancePropertyTypedFetch( return $result; } + + /** + * A folded constant value can be a full C++ expression (e.g. the ternary + * of `const VALUE = cond ? E::A : E::B;`). Appending `.attr(...)` to it + * unparenthesized would bind the member access to the last operand only, + * so any operand with top-level operators is wrapped first. Simple + * identifiers and closed call chains stay untouched. + */ + private function parenthesizeOpenOperand(string $code): string + { + $depth = 0; + $inString = false; + $length = strlen($code); + for ($i = 0; $i < $length; $i++) { + $char = $code[$i]; + if ($inString) { + if ($char === '\\') { + $i++; + } elseif ($char === '"') { + $inString = false; + } + continue; + } + if ($char === '"') { + $inString = true; + } elseif ($char === '(' || $char === '{' || $char === '[') { + $depth++; + } elseif ($char === ')' || $char === '}' || $char === ']') { + $depth--; + } elseif ($depth === 0 && ($char === ' ' || $char === '?')) { + return '(' . $code . ')'; + } + } + return $code; + } } diff --git a/src/Resolver/ClassConstantValueTrait.php b/src/Resolver/ClassConstantValueTrait.php index 873e8e10..649f7a5b 100644 --- a/src/Resolver/ClassConstantValueTrait.php +++ b/src/Resolver/ClassConstantValueTrait.php @@ -155,6 +155,30 @@ protected function evaluateClassConstValue(?NodeAbstract $origin, ConstantDef $c return $evaluator->evaluateDirectly($valueExpr); } + /** + * The pre-AST representation of an enum case for consumers that cannot + * register an IS_CONSTANT_AST (property and parameter defaults, attribute + * arguments): internal enums degrade to the host case object, compiled + * enums to the literal backing value or the case name — exactly the + * values those paths consumed before case identity existed. + */ + public function enumCaseLegacyValue(\TypePhp\Entity\EnumCaseRef $ref): mixed + { + if ($this->isInternalClass($ref->enumClass)) { + $constName = $ref->enumClass . '::' . $ref->caseName; + if (defined($constName)) { + return constant($constName); + } + } + if ($this->hasClass($ref->enumClass)) { + $classDef = $this->getClass($ref->enumClass); + if (array_key_exists($ref->caseName, $classDef->enumCases)) { + return $classDef->enumCases[$ref->caseName] ?? $ref->caseName; + } + } + return $ref->caseName; + } + public function getConstValue(string $name): mixed { if ($this->isInternalConstant($name)) { diff --git a/src/gen_stub.php b/src/gen_stub.php index 8b454266..4197368e 100755 --- a/src/gen_stub.php +++ b/src/gen_stub.php @@ -2568,6 +2568,10 @@ class EvaluatedValue public SimpleType $type; public Expr $expr; public bool $isUnknownConstValue; + /** Case identity when the expression evaluates to an enum case; only + * class-constant registration may use it (persistent AST) — every other + * consumer sees the legacy scalar/object in $value. */ + public ?\TypePhp\Entity\EnumCaseRef $enumCaseRef = null; /** @var ConstInfo[] */ public array $originatingConsts; @@ -2727,6 +2731,16 @@ static function (Expr $expr) use ( $result = $evaluator->evaluateDirectly($expr); + $enumCaseRef = null; + if ($result instanceof \TypePhp\Entity\EnumCaseRef) { + // Property/parameter defaults and attribute arguments must keep + // consuming the legacy value (persistent tables reject refcounted + // zvals, and those paths have their own runtime restore + // machinery); only class-constant registration uses the identity. + $enumCaseRef = $result; + $result = getTranslator()->enumCaseLegacyValue($result); + } + // The declared type is useful when an UNKNOWN placeholder must be // emitted through its @cvalue macro. For a concrete null expression, // however, the zval must be initialized as null even when the declared @@ -2735,13 +2749,15 @@ static function (Expr $expr) use ( ? SimpleType::null() : ($constType ?? SimpleType::fromValue($result)); - return new EvaluatedValue( + $evaluated = new EvaluatedValue( $result, // note: we are generally not interested in the actual value of $result, unless it's a bare value, without constants $valueType, $cConstName === null ? $expr : new Expr\ConstFetch(new Node\Name($cConstName)), $visitor->visitedConstants, $isUnknownConstValue ); + $evaluated->enumCaseRef = $enumCaseRef; + return $evaluated; } public static function null(): EvaluatedValue @@ -2762,9 +2778,9 @@ private function __construct($value, SimpleType $type, Expr $expr, array $origin $this->isUnknownConstValue = $isUnknownConstValue; } - public function initializeZval(string $zvalName, bool $alreadyExists = false, string $forStringDef = '', string $varName = ''): string + public function initializeZval(string $zvalName, bool $alreadyExists = false, string $forStringDef = '', string $varName = '', bool $allowConstantAst = false): string { - if ($this->value instanceof \TypePhp\Entity\EnumCaseRef) { + if ($this->enumCaseRef !== null && $allowConstantAst) { return $this->initializeEnumCaseZval($zvalName, $alreadyExists); } $cExpr = $this->getCExpr(); @@ -2825,8 +2841,7 @@ public function initializeZval(string $zvalName, bool $alreadyExists = false, st */ private function initializeEnumCaseZval(string $zvalName, bool $alreadyExists): string { - /** @var \TypePhp\Entity\EnumCaseRef $case */ - $case = $this->value; + $case = $this->enumCaseRef; $enumCName = '"' . getTranslator()->escapeString(ltrim($case->enumClass, '\\')) . '"'; $caseCName = '"' . getTranslator()->escapeString($case->caseName) . '"'; $id = preg_replace('/[^A-Za-z0-9_]/', '_', $zvalName); @@ -3296,7 +3311,7 @@ private function getClassConstDeclaration(EvaluatedValue $value): string { $constName = $this->name->getDeclarationName(); - $zvalCode = $value->initializeZval("const_{$constName}_value"); + $zvalCode = $value->initializeZval("const_{$constName}_value", allowConstantAst: true); $code = "\n" . $zvalCode; From bb4681ad2290bf748369163b2179bb011bf27f49 Mon Sep 17 00:00:00 2001 From: Alessio Giacobbe Date: Wed, 2 Sep 2026 09:49:47 +0200 Subject: [PATCH 3/4] fix(gen_stub): give persistent AST constants a complete teardown lifecycle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit destroy_zend_class() asserts (in debug builds) that every persistent AST constant remaining on an internal class is CONST_ENUM_INIT, and its teardown frees only the allocation referenced by Z_AST — the previous representation left a CLASS_CONST root behind (assertion failure at shutdown on 8.4/8.5 debug builds) and leaked the two separately allocated children. The AST is now built in one contiguous persistent allocation (ast_ref, root, both zval children — mirroring Zend's own persistent enum AST builder), and every generated file with AST constants emits a release function that runs from the module's MSHUTDOWN, before Zend's class teardown: it frees the single block and restores the constant slot to null, so destroy_zend_class() never sees a foreign AST. Request-local mutable copies are unaffected (no request is live at MSHUTDOWN). CONST_ENUM_INIT itself is not usable here: that node constructs a new case object rather than fetching the canonical registered one, which would break case identity again. --- src/Translator.php | 12 +++++++++++ src/gen_stub.php | 51 ++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 57 insertions(+), 6 deletions(-) diff --git a/src/Translator.php b/src/Translator.php index 5b5bd0a0..0fb1fcc1 100644 --- a/src/Translator.php +++ b/src/Translator.php @@ -83,6 +83,9 @@ class Translator extends Preprocessor protected array $argInfoHeaderFiles = []; protected array $registerSymbols = []; + /** Generated per-file teardown functions for persistent AST class constants. */ + protected array $releaseAstConstantFns = []; + // Windows resource file configuration (icon, version info, etc.) protected array $resourceConfig = []; @@ -1113,6 +1116,12 @@ private function doGenExtension(): string // minit end $code .= 'PHP_MSHUTDOWN_FUNCTION(' . $this->getModuleName() . ') {' . PHP_EOL; + // Persistent AST class constants must be restored before Zend's + // class teardown runs (its debug assertion only tolerates + // CONST_ENUM_INIT there), and their contiguous blocks freed. + foreach (array_unique($this->releaseAstConstantFns) as $releaseFn) { + $code .= $releaseFn . '();' . PHP_EOL; + } // The cache owns no Zend symbols, but its pointers must not survive a // complete module shutdown/startup cycle in an embedded process. $code .= 'for (auto &slot : ' . self::PREFIX . self::PERSISTENT_CLASS_MAP . ') {' . PHP_EOL; @@ -3104,6 +3113,9 @@ protected function genStubFile(string $file): void generateStubFile($file, $this->getIncludeDir() . '/' . $headerFile, true, $this->getPhpVersion()); $headerCode = file_get_contents($this->getBuildDir() . '/include/' . $headerFile); + if (preg_match('/\\bstatic\\s+void\\s+(typephp_release_ast_constants_[A-Za-z0-9_]+)\\s*\\(void\\)/', $headerCode, $releaseMatch)) { + $this->releaseAstConstantFns[] = $releaseMatch[1]; + } $needsAttributeSymbols = str_contains($headerCode, 'zend_add_function_attribute(') || str_contains($headerCode, 'zend_add_parameter_attribute(') || str_contains($headerCode, 'zend_add_global_constant_attribute('); diff --git a/src/gen_stub.php b/src/gen_stub.php index 4197368e..fd977b65 100755 --- a/src/gen_stub.php +++ b/src/gen_stub.php @@ -2839,6 +2839,9 @@ public function initializeZval(string $zvalName, bool $alreadyExists = false, st * request shutdown. This keeps case identity intact for static access, * constant(), and reflection, and is safe under concurrent ZTS requests. */ + /** @var array [declaring class, constant name] of every emitted AST constant, per generated file */ + public static array $emittedAstConstants = []; + private function initializeEnumCaseZval(string $zvalName, bool $alreadyExists): string { $case = $this->enumCaseRef; @@ -2846,24 +2849,30 @@ private function initializeEnumCaseZval(string $zvalName, bool $alreadyExists): $caseCName = '"' . getTranslator()->escapeString($case->caseName) . '"'; $id = preg_replace('/[^A-Za-z0-9_]/', '_', $zvalName); + // One contiguous persistent allocation holds the ast_ref, the root + // CLASS_CONST node and both zval children, mirroring Zend's own + // persistent enum AST builder: teardown frees exactly one block. $code = $alreadyExists ? '' : "\tzval $zvalName;\n"; $code .= "\t{\n"; $code .= "\t\tzend_string *{$id}_enum_name = zend_string_init_interned($enumCName, sizeof($enumCName) - 1, 1);\n"; $code .= "\t\tzend_string *{$id}_case_name = zend_string_init_interned($caseCName, sizeof($caseCName) - 1, 1);\n"; - $code .= "\t\tzend_ast_zval *{$id}_class_ast = (zend_ast_zval *) pemalloc(sizeof(zend_ast_zval), 1);\n"; + $code .= "\t\tsize_t {$id}_root_size = ZEND_MM_ALIGNED_SIZE(zend_ast_size(2));\n"; + $code .= "\t\tsize_t {$id}_child_size = ZEND_MM_ALIGNED_SIZE(sizeof(zend_ast_zval));\n"; + $code .= "\t\tchar *{$id}_block = (char *) pemalloc(sizeof(zend_ast_ref) + {$id}_root_size + 2 * {$id}_child_size, 1);\n"; + $code .= "\t\tzend_ast_ref *{$id}_ast_ref = (zend_ast_ref *) {$id}_block;\n"; + $code .= "\t\tGC_SET_REFCOUNT({$id}_ast_ref, 1);\n"; + $code .= "\t\tGC_TYPE_INFO({$id}_ast_ref) = GC_CONSTANT_AST | ((GC_PERSISTENT | GC_IMMUTABLE) << GC_FLAGS_SHIFT);\n"; + $code .= "\t\tzend_ast *{$id}_fetch_ast = GC_AST({$id}_ast_ref);\n"; + $code .= "\t\tzend_ast_zval *{$id}_class_ast = (zend_ast_zval *) ({$id}_block + sizeof(zend_ast_ref) + {$id}_root_size);\n"; + $code .= "\t\tzend_ast_zval *{$id}_const_ast = (zend_ast_zval *) ({$id}_block + sizeof(zend_ast_ref) + {$id}_root_size + {$id}_child_size);\n"; $code .= "\t\t{$id}_class_ast->kind = ZEND_AST_ZVAL;\n"; $code .= "\t\t{$id}_class_ast->attr = ZEND_NAME_FQ;\n"; $code .= "\t\tZVAL_INTERNED_STR(&{$id}_class_ast->val, {$id}_enum_name);\n"; $code .= "\t\tZ_LINENO({$id}_class_ast->val) = 0;\n"; - $code .= "\t\tzend_ast_zval *{$id}_const_ast = (zend_ast_zval *) pemalloc(sizeof(zend_ast_zval), 1);\n"; $code .= "\t\t{$id}_const_ast->kind = ZEND_AST_ZVAL;\n"; $code .= "\t\t{$id}_const_ast->attr = 0;\n"; $code .= "\t\tZVAL_INTERNED_STR(&{$id}_const_ast->val, {$id}_case_name);\n"; $code .= "\t\tZ_LINENO({$id}_const_ast->val) = 0;\n"; - $code .= "\t\tzend_ast_ref *{$id}_ast_ref = (zend_ast_ref *) pemalloc(sizeof(zend_ast_ref) + ZEND_MM_ALIGNED_SIZE(zend_ast_size(2)), 1);\n"; - $code .= "\t\tGC_SET_REFCOUNT({$id}_ast_ref, 1);\n"; - $code .= "\t\tGC_TYPE_INFO({$id}_ast_ref) = GC_CONSTANT_AST | ((GC_PERSISTENT | GC_IMMUTABLE) << GC_FLAGS_SHIFT);\n"; - $code .= "\t\tzend_ast *{$id}_fetch_ast = GC_AST({$id}_ast_ref);\n"; $code .= "\t\t{$id}_fetch_ast->kind = ZEND_AST_CLASS_CONST;\n"; $code .= "\t\t{$id}_fetch_ast->attr = 0;\n"; $code .= "\t\t{$id}_fetch_ast->lineno = 0;\n"; @@ -3312,6 +3321,9 @@ private function getClassConstDeclaration(EvaluatedValue $value): string $constName = $this->name->getDeclarationName(); $zvalCode = $value->initializeZval("const_{$constName}_value", allowConstantAst: true); + if ($value->enumCaseRef !== null) { + EvaluatedValue::$emittedAstConstants[] = [ClassInfo::$currentClass, $constName]; + } $code = "\n" . $zvalCode; @@ -5949,6 +5961,8 @@ function generateArgInfoCode( $code = "/* This is a generated file, edit the .stub.php file instead.\n" . " * Stub hash: $stubHash */\n"; + EvaluatedValue::$emittedAstConstants = []; + foreach ($fileInfo->classInfos as $classInfo) { $code .= $classInfo->getDnfConstantTypeFactoryCode(); $code .= $classInfo->getDnfPropertyTypeFactoryCode(); @@ -6028,6 +6042,31 @@ static function (FuncInfo $funcInfo) use ($fileInfo, &$generatedFunctionDeclarat $code .= $fileInfo->generateClassEntryCode($allConstInfos); } + if (EvaluatedValue::$emittedAstConstants !== []) { + // Zend's internal-class teardown asserts (in debug builds) that every + // remaining persistent AST constant is CONST_ENUM_INIT and frees only + // the ast_ref allocation. Restore the emitted CLASS_CONST ASTs before + // destroy_zend_class() runs — the module MSHUTDOWN calls this — and + // free their single contiguous block. + $fnName = 'typephp_release_ast_constants_' + . preg_replace('/[^A-Za-z0-9_]/', '_', $stubFilenameWithoutExtension); + $code .= "\nstatic void {$fnName}(void) {\n"; + foreach (EvaluatedValue::$emittedAstConstants as [$className, $constName]) { + $classLc = '"' . getTranslator()->escapeString(strtolower(ltrim($className, '\\'))) . '"'; + $constC = '"' . getTranslator()->escapeString($constName) . '"'; + $code .= "\t{\n"; + $code .= "\t\tzend_class_entry *ce = (zend_class_entry *) zend_hash_str_find_ptr(CG(class_table), $classLc, sizeof($classLc) - 1);\n"; + $code .= "\t\tzend_class_constant *c = ce ? (zend_class_constant *) zend_hash_str_find_ptr(&ce->constants_table, $constC, sizeof($constC) - 1) : NULL;\n"; + $code .= "\t\tif (c && Z_TYPE(c->value) == IS_CONSTANT_AST) {\n"; + $code .= "\t\t\tpefree(Z_AST(c->value), 1);\n"; + $code .= "\t\t\tZVAL_NULL(&c->value);\n"; + $code .= "\t\t}\n"; + $code .= "\t}\n"; + } + $code .= "}\n"; + EvaluatedValue::$emittedAstConstants = []; + } + return $code; } From 6b8ed64107e2569b1b212657c35044966811c1bf Mon Sep 17 00:00:00 2001 From: Alessio Giacobbe Date: Wed, 2 Sep 2026 14:34:07 +0200 Subject: [PATCH 4/4] fix(codegen): reject lifecycles that cannot release AST constants MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The typephp_release_ast_constants_*() teardown ran only from MSHUTDOWN, which is not a general pre-class-destruction hook: for a MODULE_TEMPORARY module loaded through dl(), module_destructor() runs clean_module_classes() before the shutdown callback, so the foreign ZEND_AST_CLASS_CONST reached destroy_zend_class() first and still tripped the debug assertion; and a MINIT that fails after registering such a constant never sets module_started, so MSHUTDOWN is not guaranteed to run at all. The generated module now enforces the lifecycle contract instead of assuming it. When the module declares any enum-case AST constant, MINIT opens with a guard that rejects MODULE_TEMPORARY (zend_error E_WARNING, return FAILURE) before a single class is registered — with nothing in the class table, teardown is trivially safe. MINIT is also restructured so that every step that can return FAILURE precedes the first register_class_*() call: the AST constants are installed by the infallible tail (class registration, then symbol registration), so a FAILURE return can never leave a foreign AST in the persistent tables. The generator itself throws if a future change introduces a FAILURE return after registration begins. The MSHUTDOWN release is unchanged and remains the supported, persistent-module path. EnumCaseAstConstantLifecycleTest asserts the guard exists exactly when AST constants exist, that it precedes every registration step, that no FAILURE return follows the first class registration, and that MSHUTDOWN releases the constants before any other teardown. The enum-case phpt gains a never-accessed constant so the full process shutdown it already performs also covers a pristine persistent AST; a dl()-path phpt is not feasible because the harness only builds standalone binaries whose module is registered persistently (documented in the test file). --- .../src/EnumCaseAstConstantLifecycleTest.php | 120 ++++++++++++++++++ src/Translator.php | 41 +++++- .../enum/enum-case-class-constant.phpt | 14 ++ 3 files changed, 171 insertions(+), 4 deletions(-) create mode 100644 phpunit/src/EnumCaseAstConstantLifecycleTest.php diff --git a/phpunit/src/EnumCaseAstConstantLifecycleTest.php b/phpunit/src/EnumCaseAstConstantLifecycleTest.php new file mode 100644 index 00000000..3e1b36c5 --- /dev/null +++ b/phpunit/src/EnumCaseAstConstantLifecycleTest.php @@ -0,0 +1,120 @@ +generateMinitBody('enum-case-class-constant.php', 'ast_lifecycle_guard'); + + $guardPos = strpos($minit, 'if (type == MODULE_TEMPORARY) {'); + self::assertIsInt($guardPos, 'MINIT must reject dl()-loaded temporary modules'); + self::assertStringContainsString( + 'registers enum-case class constants that must be released by MSHUTDOWN', + $minit, + ); + self::assertStringContainsString('Load the extension from php.ini instead.', $minit); + + $handlersPos = strpos($minit, 'typephp_install_reflection_attribute_handlers()'); + $firstRegisterPos = strpos($minit, 'register_class_'); + self::assertIsInt($handlersPos); + self::assertIsInt($firstRegisterPos); + self::assertLessThan($handlersPos, $guardPos, 'the lifecycle guard must be the first MINIT statement'); + self::assertLessThan($firstRegisterPos, $guardPos, 'the lifecycle guard must precede every class registration'); + } + + public function testAstConstantRegistrationIsOrderedAfterEveryFallibleMinitStep(): void + { + $minit = $this->generateMinitBody('enum-case-class-constant.php', 'ast_lifecycle_order'); + + $firstRegisterPos = strpos($minit, 'register_class_'); + $lastFailurePos = strrpos($minit, 'return FAILURE;'); + self::assertIsInt($firstRegisterPos); + self::assertIsInt($lastFailurePos); + self::assertLessThan( + $firstRegisterPos, + $lastFailurePos, + 'no MINIT step after the first class registration may return FAILURE: ' + . 'a failed MINIT never reaches MSHUTDOWN, so the persistent class ' + . 'table would keep an AST that destroy_zend_class() cannot handle', + ); + self::assertGreaterThan( + strrpos($minit, 'register_class_'), + strpos($minit, 'return SUCCESS;'), + ); + } + + public function testMshutdownReleasesAstConstantsBeforeAnyOtherTeardown(): void + { + $extension = $this->generateExtension('enum-case-class-constant.php', 'ast_lifecycle_shutdown'); + $mshutdown = $this->sliceFunction($extension, 'PHP_MSHUTDOWN_FUNCTION', 'THREAD_LOCAL zval globals_array'); + + $releasePos = strpos($mshutdown, 'typephp_release_ast_constants_enum_case_class_constant();'); + self::assertIsInt($releasePos, 'MSHUTDOWN must release the persistent AST constants'); + // The release must run before anything else so the class table is + // Zend-safe no matter what the rest of the teardown does. + $firstStatementPos = strpos($mshutdown, ';'); + self::assertSame($firstStatementPos, $releasePos + strlen('typephp_release_ast_constants_enum_case_class_constant();') - 1); + } + + public function testModulesWithoutAstConstantsCarryNeitherGuardNorRelease(): void + { + $extension = $this->generateExtension('class-constant-codegen.php', 'ast_lifecycle_none'); + + self::assertStringNotContainsString('MODULE_TEMPORARY', $extension); + self::assertStringNotContainsString('typephp_release_ast_constants_', $extension); + } + + private function generateMinitBody(string $fixture, string $target): string + { + return $this->sliceFunction( + $this->generateExtension($fixture, $target), + 'PHP_MINIT_FUNCTION', + 'PHP_MSHUTDOWN_FUNCTION', + ); + } + + private function generateExtension(string $fixture, string $target): string + { + global $translator; + + $compiler = CompilerTest::create(TYPEPHP_ROOT_PATH); + $translator = $compiler; + $compiler->setBuildMode(CompilerBase::BUILD_MODE_EXT); + $compiler->setTargetName($target); + $source = TYPEPHP_ROOT_PATH . '/phpunit/code/' . $fixture; + $compiler->addFiles([$source]); + $compiler->prepareFile($source); + $compiler->convertFile($source); + $extension = file_get_contents($compiler->genExtension()); + + self::assertIsString($extension); + return $extension; + } + + /** + * The generated function bodies keep statements at column zero, so the + * closing brace is not recognizable; slice up to the next known emission + * instead. + */ + private function sliceFunction(string $extension, string $startMarker, string $endMarker): string + { + $start = strpos($extension, $startMarker); + self::assertIsInt($start, "generated extension must contain {$startMarker}"); + $end = strpos($extension, $endMarker, $start + strlen($startMarker)); + self::assertIsInt($end, "generated extension must contain {$endMarker}"); + return substr($extension, $start, $end - $start); + } +} diff --git a/src/Translator.php b/src/Translator.php index 0fb1fcc1..5cc28030 100644 --- a/src/Translator.php +++ b/src/Translator.php @@ -1097,7 +1097,24 @@ private function doGenExtension(): string $code .= $this->getIndent() . "ZEND_FE_END\n};\n// clang-format on" . PHP_EOL . PHP_EOL; // minit begin + $releaseAstConstantFns = array_unique($this->releaseAstConstantFns); $code .= 'PHP_MINIT_FUNCTION(' . $this->getModuleName() . ') {' . PHP_EOL; + if ($releaseAstConstantFns !== []) { + // Lifecycle contract for persistent enum-case AST constants: Zend's + // internal-class teardown (destroy_zend_class) only tolerates them + // after this module's MSHUTDOWN has released them. A temporary + // module loaded through dl() cannot honor that ordering — + // module_destructor() runs clean_module_classes() before the + // shutdown callback — so reject the load here, before any class is + // registered: with nothing registered, teardown is trivially safe. + $code .= 'if (type == MODULE_TEMPORARY) {' . PHP_EOL; + $code .= $this->getIndent() . 'zend_error(E_WARNING, "' . $this->getModuleName() + . ' registers enum-case class constants that must be released by MSHUTDOWN' + . ' before class destruction; a temporary module loaded with dl() destroys' + . ' its classes first. Load the extension from php.ini instead.");' . PHP_EOL; + $code .= $this->getIndent() . 'return FAILURE;' . PHP_EOL; + $code .= '}' . PHP_EOL; + } $code .= '// class/interface class entries' . PHP_EOL; $code .= 'if (typephp_install_reflection_attribute_handlers() != SUCCESS) {' . PHP_EOL; $code .= $this->getIndent() . 'return FAILURE;' . PHP_EOL; @@ -1105,12 +1122,28 @@ private function doGenExtension(): string if (!$this->isWasiTarget()) { $code .= 'typephp_register_fiber_generator_class();' . PHP_EOL; } - $code .= $this->genClassPropertyInit() . PHP_EOL; - $code .= '// register symbols' . PHP_EOL; + // The register_class_*() calls below install the persistent AST + // constants. Their release runs in MSHUTDOWN, and Zend only calls + // MSHUTDOWN once module_started is set — i.e. once MINIT returned + // SUCCESS. So from the first class registration to the end of MINIT + // no step may return FAILURE; every fallible step stays above. This + // is enforced at generation time. + $registrationCode = $this->genClassPropertyInit() . PHP_EOL; + $registrationCode .= '// register symbols' . PHP_EOL; foreach ($this->registerSymbols as $registerSymbolFn) { - $code .= $registerSymbolFn . '(module_number);' . PHP_EOL; + $registrationCode .= $registerSymbolFn . '(module_number);' . PHP_EOL; + } + if ($releaseAstConstantFns !== [] && str_contains($registrationCode, 'return FAILURE')) { + throw new \LogicException( + 'MINIT must not fail after class registration begins: a FAILURE return would' + . ' leave persistent enum-case AST constants in the class table with no' + . ' MSHUTDOWN guaranteed to release them before destroy_zend_class().' + . ' Move the fallible step before the first register_class_*() call, or' + . ' release the AST constants on its failure path.' + ); } + $code .= $registrationCode; $code .= 'return SUCCESS;' . PHP_EOL; $code .= '}' . PHP_EOL . PHP_EOL; // minit end @@ -1119,7 +1152,7 @@ private function doGenExtension(): string // Persistent AST class constants must be restored before Zend's // class teardown runs (its debug assertion only tolerates // CONST_ENUM_INIT there), and their contiguous blocks freed. - foreach (array_unique($this->releaseAstConstantFns) as $releaseFn) { + foreach ($releaseAstConstantFns as $releaseFn) { $code .= $releaseFn . '();' . PHP_EOL; } // The cache owns no Zend symbols, but its pointers must not survive a diff --git a/tests/compiler/enum/enum-case-class-constant.phpt b/tests/compiler/enum/enum-case-class-constant.phpt index 0436d07c..5b201fe8 100644 --- a/tests/compiler/enum/enum-case-class-constant.phpt +++ b/tests/compiler/enum/enum-case-class-constant.phpt @@ -2,6 +2,17 @@ Class constants valued by enum cases keep case identity everywhere --FILE--