Re-enable Symfony test on PHP 8

[nicolas-grekas]

Following symfony/symfony#36872, Symfony is now green on PHP 8.

[nikic]

Fails with:

Fatal error: Return value of Exception::getMessage() must be of type string, object returned in Unknown on line 0

[nikic]

@kocsismate What do you think about always displaying the class name, even if the expected type is not a class? I think this kind of error would be easier to debug if it said must be of type string, SomeClassName returned.

[kocsismate]

@nikic Yeah, it's a good idea for sure. I'll implement this in the coming days.

[nicolas-grekas]

Thanks for updating the azure config to run community jobs.

Return value of Exception::getMessage() must be of type string, object returned

I don't have this failure locally, did anything related to return types change recently?

We're using a stringable object message. The reason is that computing this specific error message takes a lot of resources, while we might just skip the error entirely in most situations.

It's been working well since always.

[nikic]

@nicolas-grekas This is likely a debug-build only error. Most likely Exception::getMessage() now specifies a string return type, while maybe it shouldn't.

[kocsismate]

@nikic I've recently added that specific return type there. When should it return an object?

EDIT: I see it now.

[nikic]

Yes, looks like this is the case:

php-src/Zend/zend_exceptions.stub.php

Line 37 in ce668c0

final public function getMessage(): string {}

I'm wondering, what is the better behavior here: Return the Stringable object from getMessage(), or make getMessage() convert it into a string (and keep the string return type).

[nicolas-grekas]

make getMessage() convert it into a string (and keep the string return type).

this option would break our use case... (I'm fact-checking this claim, give me a few minutes)

[nicolas-grekas]

make getMessage() convert it into a string (and keep the string return type).

this option would break our use case

Hmm actually I'm not sure it would break our use case.
@nikic maybe you could push a commit that implements your option here to see how it goes?

For reference, here is the logic in Symfony:
https://github.com/symfony/symfony/blob/master/src/Symfony/Component/DependencyInjection/Exception/AutowiringFailedException.php

[nikic]

@kocsismate Generally, all the types on exception methods that return a protected property aren't right. We either need to make the necessary conversions in the getter, throw an exception, or drop the type.

[nikic]

Now we get a use-after-free :(

[nicolas-grekas]

The script now runs each component independently.

The use-after-free is in the DI component. It might be related to the very change about Exception::getMessage().

Most other failures are spammy Inaccurate func info for sodium_crypto_aead_xchacha20poly1305_ietf_decrypt() warnings.

The failures in the String component are due to a buggy version of the intl extension. I don't know if that's something php-internal can do anything about.

[kocsismate]

@nicolas-grekas The Inaccurate func info for sodium_crypto_aead_xchacha20poly1305_ietf_decrypt() issues mean that function return type info in zend_func_info.c and the related stub are inconsistent.

The issue in case of sodium_crypto_aead_xchacha20poly1305_ietf_decrypt() is that
F1("sodium_crypto_aead_xchacha20poly1305_ietf_decrypt", MAY_BE_NULL | MAY_BE_FALSE | MAY_BE_STRING),

should rather be

F1("sodium_crypto_aead_xchacha20poly1305_ietf_decrypt", MAY_BE_FALSE | MAY_BE_STRING),

[nikic]

The script now runs each component independently.

The use-after-free is in the DI component. It might be related to the very change about Exception::getMessage().

It's definitely related, but more in the sense that the change exposed an existing bug. I can reproduce locally with an asan build, but have some trouble debugging this, because gdb itself crashes :(

The failures in the String component are due to a buggy version of the intl extension. I don't know if that's something php-internal can do anything about.

This is most likely due to a different ICU version rather than something in the intl extension itself (I hope), so for now we can ignore it. For our purposes the main important part is that there are no crashes / assertion failures / fatal errors.

[nikic]

The exception use-after-free should be fixed with 55dd394. From a local run I'm still getting a

Fatal error: Error::getTrace(): Return value must be of type array, null returned in Unknown on line 0

though.

[nikic]

I've submitted #5636 to make the private Exception::$trace a property of type array. This makes it more obvious where the error originates:

TypeError: Cannot assign null to property Error::$trace of type array
/home/nikic/repos/symfony/src/Symfony/Component/ErrorHandler/Error/FatalError.php:77
/home/nikic/repos/symfony/src/Symfony/Component/ErrorHandler/Tests/ErrorEnhancer/ClassNotFoundErrorEnhancerTest.php:131

@nicolas-grekas Would you mind changing the FatalError code to use [] instead of null as the default value of trace?

[nikic]

Merged as ef9a790 and d579e18. Thanks for working on this so early!