Repository navigation
Hooked properties combined with lazy ghosts cause an assertion failure #16615
Description
Description
This issue started over in docker-library/php#1545 because I initially thought this was an issue with the Debian based Docker images for PHP 8.4.0RC, but since then @yosifkit and @tianon helped with realizing that it's not related to Docker at all.
The problem
This is a minimal example of a class with a property that has a set-hook, but specifically no get-hook. When you create a lazy ghost object from this class and try to access its property, PHP should call its initialization function. And it usually does.
Test code
<?php
class Foo {
public string $bar {
set => $value;
}
}
$reflector = new ReflectionClass(Foo::class);
$foo = $reflector->newLazyGhost(function ($ghost) {
$ghost->bar = 'bar';
});
echo $foo->bar;Expected output
bar
Actual output
However, when PHP was compiled with GCC 12.2.0 (I did not have time to test any other versions) and CFLAGS="-O2", the code throws the following fatal error instead:
Fatal error: Uncaught Error: Typed property Foo::$bar must not be accessed before initialization in /workspace/test.php:15
Stack trace:
#0 {main}
thrown in /workspace/test.php on line 15
Some more information
GCC 12.2.0 is the current version shipped with Debian "Bookworm", hence why this bug appears in the official Docker images. However, I could reproduce it outside of Docker, on a different Linux distribution, but with the same compiler version. So GCC was the common denominator, as far as I could tell.
@yosifkit could also reproduce the issue with Debian's PHP 8.4 package from the experimental repository.
In the other thread, we suspected that this could be a compiler bug in GCC. However, I'm not well-versed enough in compilers - or the internals of PHP for that matter - to really tell. So I'm leaving this here with all the information I could gather for someone who can.
PHP Version
PHP 8.4.0RC3
Operating System
No response
Activity
ndossche commentedon Oct 26, 2024
Not a compiler bug, but an assertion failure, which confuses the compiler because assertions are turned into assumes in release builds:
This is likely the same bug as this oss-fuzz bug: https://issues.oss-fuzz.com/issues/373025987
[-]Fatal Error in PHP 8.4 when compiled with certain versions of GCC and -O2[/-][+]Hooked properties combined with lazy ghosts cause an assertion failure[/+]ndossche commentedon Oct 26, 2024
I think the assertion/assumption is too conservative. This fixes it and makes sense to me although I haven't deep-dived into the internals of lazy objects yet:
cc @arnaud-lb
arnaud-lb commentedon Oct 28, 2024
Agreed. An alternative fix would be to assert that
Z_PROP_FLAG_P(retval)containsIS_PROP_UNINIT:Z_PROP_FLAG_P(retval)in this case isIS_PROP_UNINIT | IS_PROP_LAZY.ndossche commentedon Oct 28, 2024
Looks fine too. Idk what is better, I would still prefer though if your patch had the comment updated too so that we know why it's a bitwise and.
Fix assertion failure in zend_std_read_property