Core dumped in ext/reflection/php_reflection.c

[YuanchengJiang]

Description

The following code:

<?php
class C {
public stdClass $a = FOO;
}
$reflector = new ReflectionClass(C::class);
$c = $reflector->newLazyGhost(function () { });
function f() {
define('FOO', new stdClass);
}
f();
try {
var_dump($c->a);
} catch (\Error $e) {
}
$fusion = $reflector;
$s = 'C:11:"ArrayObject":' . strlen($p) . ':{' . $fusion . '}';

Resulted in this output:

/php-src/ext/reflection/php_reflection.c:686: format_default_value: Assertion `zval_get_type(&(*(value))) == 11' failed.
Aborted (core dumped)

PHP Version

PHP 8.4.0-dev

Operating System

ubuntu 22.04

[DanielEScherzer]

Minimal reproduction:

<?php

class C {
    public stdClass $a = FOO;
}

define('FOO', new stdClass);

new C;

$reflector = new ReflectionClass(C::class);
var_dump( (string)$reflector ); // var_dump just for debugging, only the string cast is needed for the error

[DanielEScherzer]

So it seems that depending on the reproduction case, the printed value for the default of $a changes (when not in debug mode, i.e. when the assertion is skipped). With the minimal example I posted above, I got Property [ public stdClass $a = callable ].

On the other hand, if the line $reflector = new ReflectionClass(C::class); is moved to go directly after the class declaration, rather than after the new C;, then I get Property [ public stdClass $a = __CLASS__ ].

With an example based on the original report, where the class declaration is followed by

$reflector = new ReflectionClass(C::class);
$c = $reflector->newLazyGhost(function () { });

define('FOO', new stdClass);

var_dump( $c->a );
var_dump( (string)$reflector );

the property is Property [ public stdClass $a = ... ].

And if the property never gets evaluated, i.e. the var_dump( $c->a ); is removed (and optionally also the $c = ... and the define() call removed), then the AST is maintained, and the result is Property [ public stdClass $a = FOO ]

The callable and __CLASS__ are definitely wrong, but I can see why the ... might be reasonable (it is an object). However, we probably always want the FOO output, i.e. the evaluation of the AST should not change the string output

[DanielEScherzer]

Also affects ReflectionProperty:

$reflector = new ReflectionProperty(C::class, 'a');
define('FOO', new stdClass);
var_dump( (string)$reflector );

has the default as FOO, adding new C; before the var_dump changes it to __CLASS__, and moving the declaration of $reflector to after the new C; then changes it to callable

[DanielEScherzer]

I believed I tracked down the cause to https://github.com/php/php-src/blob/c7397f51316b105fdca30929603874d66f7c1411/Zend/zend_API.c#L1468-L1485

the value that was of type IS_CONSTANT_AST is looked up in zval_update_constant_ex() and then the result is stored, overwriting the original with an IS_OBJECT. Patch coming momentarily

[DanielEScherzer]

Given that this is going to technically be a behavior change for something that wasn't crashing in non-debug builds, PR sent to target master