Segmentation Fault (core dumped)

[internalsystemerror]

Bug report

When analysing a file with the following contents, I get a Segmentation Fault (using PHPStan 1.4.4, which didn't appear on 1.4.3).

Code snippet that reproduces the problem

The demo isn't able to give me a shareable link as it errors. I tried to reduce the amount of code from this class to isolate the exact issue but removing various methods appears to make the fault go away, so here is the code:

<?php

declare(strict_types=1);

namespace App\Hydrator;

use App\Entity\AttributeOption;
use App\Entity\ProductOption;
use App\Entity\EntityInterface;
use Doctrine\ORM\ORMException;

use function call_user_func;

/**
 * @template T of EntityInterface
 * @template R of AttributeOption|ProductOption
 */
abstract class AbstractWithOptionsHydrator
{
    /** @var string */
    protected const OPTION_OWNER_PROPERTY = '';

    /**
     * @param array<string, mixed> $data
     * @param T $entity
     *
     * @return R
     */
    protected function hydrateOption(array $data, EntityInterface $entity): EntityInterface
    {
        /** @var R $option */
        $option   = $this->optionHydrator->hydrate($data);
        $callable = [$option, 'set' . static::OPTION_OWNER_PROPERTY];
        call_user_func($callable, $entity);

        return $option;
    }
}

Expected output

Show analysis output.

Did PHPStan help you today? Did it make you happy in any way?

PHPStan making the world a brighter place one error at a time.

[canvural]

Looks like problem is with the hydrateOption method. You can remove everything else, and it still fails. Maybe you can debug more from there.

[internalsystemerror]

@canvural Ah you are quite correct thank you. I've updated the description accordingly and will continue my own debugging from there.

[internalsystemerror]

OK, I've narrowed it down to the code currently in the description. If you change the line:

* @template R of AttributeOption|ProductOption

to:

* @template R of OptionInterface

then it works as expected. So it appears to be caused by the union type.

[staabm]

segmentation faults usually are stemming from php-src or php-extension bugs.

which versions do you use? you should consider updating if its not up 2 date.

[internalsystemerror]

@staabm It occurs on the PHPStan playground, so same as that one (PHP 8.1.2).

[ondrejmirtes]

@staabm It's gonna be a bug in PHPStan coming from infinite recursion...

[ondrejmirtes]

The segfault is caused by this commit: phpstan/phpstan-src@f8af5dc (phpstan/phpstan-src#955).

[ondrejmirtes]

This is the loop:

of '256' frames in /Users/ondrej/Development/phpstan/src/Type/IntersectionType.php:97
#0 /Users/ondrej/Development/phpstan/src/Type/Generic/TemplateTypeTrait.php(125): PHPStan\Type\IntersectionType->isSubTypeOf(Object(PHPStan\Type\Generic\TemplateUnionType))
#1 /Users/ondrej/Development/phpstan/src/Type/IntersectionType.php(100): PHPStan\Type\Generic\TemplateUnionType->isSuperTypeOf(Object(PHPStan\Type\IntersectionType))
#2 /Users/ondrej/Development/phpstan/src/Type/Generic/TemplateTypeTrait.php(125): PHPStan\Type\IntersectionType->isSubTypeOf(Object(PHPStan\Type\Generic\TemplateUnionType))
#3 /Users/ondrej/Development/phpstan/src/Type/IntersectionType.php(100): PHPStan\Type\Generic\TemplateUnionType->isSuperTypeOf(Object(PHPStan\Type\IntersectionType))
#4 /Users/ondrej/Development/phpstan/src/Type/Generic/TemplateTypeTrait.php(125): PHPStan\Type\IntersectionType->isSubTypeOf(Object(PHPStan\Type\Generic\TemplateUnionType))
#5 /Users/ondrej/Development/phpstan/src/Type/IntersectionType.php(100): PHPStan\Type\Generic\TemplateUnionType->isSuperTypeOf(Object(PHPStan\Type\IntersectionType))
#6 /Users/ondrej/Development/phpstan/src/Type/Generic/TemplateTypeTrait.php(125): PHPStan\Type\IntersectionType->isSubTypeOf(Object(PHPStan\Type\Generic\TemplateUnionType))
#7 /Users/ondrej/Development/phpstan/src/Type/IntersectionType.php(100): PHPStan\Type\Generic\TemplateUnionType->isSuperTypeOf(Object(PHPStan\Type\IntersectionType))
#8 /Users/ondrej/Development/phpstan/src/Type/Generic/TemplateTypeTrait.php(125): PHPStan\Type\IntersectionType->isSubTypeOf(Object(PHPStan\Type\Generic\TemplateUnionType))
#9 /Users/ondrej/Development/phpstan/src/Type/IntersectionType.php(100): PHPStan\Type\Generic\TemplateUnionType->isSuperTypeOf(Object(PHPStan\Type\IntersectionType))
#10 /Users/ondrej/Development/phpstan/src/Type/Generic/TemplateTypeTrait.php(125): PHPStan\Type\IntersectionType->isSubTypeOf(Object(PHPStan\Type\Generic\TemplateUnionType))

[ondrejmirtes]

It happens when R of App\Entity\AttributeOption|App\Entity\ProductOption (class aaa\AbstractWithOptionsHydrator, parameter) is asked ->isSuperTypeOf('non-empty-string') (non-empty-string is an intersection of StringType and AccessoryNonEmptyStringType).

Do you have any advice for the right fix @arnaud-lb ? :) Thanks.

[arnaud-lb]

We could revert the commit and then do something like this:

diff --git a/src/Type/IntersectionType.php b/src/Type/IntersectionType.php
index 32331d84b..46ae22bb2 100644
--- a/src/Type/IntersectionType.php
+++ b/src/Type/IntersectionType.php
@@ -21,6 +21,7 @@ use PHPStan\Type\Accessory\AccessoryType;
 use PHPStan\Type\Accessory\NonEmptyArrayType;
 use PHPStan\Type\Generic\TemplateTypeMap;
 use PHPStan\Type\Generic\TemplateTypeVariance;
+use PHPStan\Type\Generic\TemplateUnionType;
 use function array_map;
 use function count;
 use function implode;
@@ -96,7 +97,10 @@ class IntersectionType implements CompoundType
 
        public function isSubTypeOf(Type $otherType): TrinaryLogic
        {
-               if ($otherType instanceof self || $otherType instanceof UnionType) {
+               if (
+                       ($otherType instanceof self || $otherType instanceof UnionType)
+                       && !$otherType instanceof TemplateUnionType
+               ) {
                        return $otherType->isSuperTypeOf($this);
                }
 
@@ -110,7 +114,10 @@ class IntersectionType implements CompoundType
 
        public function isAcceptedBy(Type $acceptingType, bool $strictTypes): TrinaryLogic
        {
-               if ($acceptingType instanceof self || $acceptingType instanceof UnionType) {
+               if (
+                       ($acceptingType instanceof self || $acceptingType instanceof UnionType)
+                       && !$acceptingType instanceof TemplateUnionType
+               ) {
                        return $acceptingType->isSuperTypeOf($this);
                }
 

Alternatively, if we don't want IntersectionType to have special cases about TemplateType, we could do something like this:

diff --git a/src/Type/Generic/TemplateTypeTrait.php b/src/Type/Generic/TemplateTypeTrait.php
index 387587c5a..1a7b9bf33 100644
--- a/src/Type/Generic/TemplateTypeTrait.php
+++ b/src/Type/Generic/TemplateTypeTrait.php
@@ -3,6 +3,7 @@
 namespace PHPStan\Type\Generic;
 
 use PHPStan\TrinaryLogic;
+use PHPStan\Type\Generic\TemplateUnionType;
 use PHPStan\Type\IntersectionType;
 use PHPStan\Type\MixedType;
 use PHPStan\Type\Type;
@@ -121,10 +122,19 @@ trait TemplateTypeTrait
 
        public function isSuperTypeOf(Type $type): TrinaryLogic
        {
-               if ($type instanceof self || $type instanceof IntersectionType) {
+               if ($type instanceof self) {
                        return $type->isSubTypeOf($this);
                }
 
+               if ($this instanceof TemplateUnionType && $type instanceof IntersectionType) {
+                       $results = [];
+                       foreach ($type->getTypes() as $innerType) {
+                               $results[] = $innerType->isSuperTypeOf($this);
+                       }
+
+                       return TrinaryLogic::createYes()->maxMin(...$results);
+               }
+
                return $this->getBound()->isSuperTypeOf($type)
                        ->and(TrinaryLogic::createMaybe());
        }

I would prefer this one, so that the logic related to template types remains encapsulated in TemplateTypeTrait.

(I haven't tested any of these thoroughly yet)

[ondrejmirtes]

Well I fixed it but I'm not happy about it: phpstan/phpstan-src@d101764

To me this seems like guessing "let's put an if somewhere and see if it works" rather than making a change that makes sense :) But I don't know how to structure the code to prevent problems like this...

[internalsystemerror]

@ondrejmirtes Thank you for the fix.

[arnaud-lb]

Well I fixed it but I'm not happy about it: phpstan/phpstan-src@d101764

To me this seems like guessing "let's put an if somewhere and see if it works" rather than making a change that makes sense :) But I don't know how to structure the code to prevent problems like this...

Agreed, that's not ideal. I think that it's the correct way to fix this in the current design, though.

Infinite recursions can happen when calling isSuperTypeOf from isSubTypeOf (which may call back isSuperTypeOf). Usually we don't call isSuperTypeOf from isSubTypeOf, which avoids infinite recursions. But some types may still need to do it (maybe especially CompoundTypes). From what I've seen, they may call isSuperTypeOf in two cases:

1. They call isSuperTypeOf on inner types, or with an inner type as $otherType. This is safe because this can not result in a self recursion
2. They call isSuperTypeOf on themselves or the $otherType when they know it will not call back isSubTypeOf

An example of the first case is UnionType doing this in isSubTypeOf:

         foreach ($this->getTypes() as $innerType) {                             
             $results[] = $otherType->isSuperTypeOf($innerType);                 
         }                                                                       

This is a isSuperTypeOf call with an inner type as $otherType, which is safe.

An example of the second case is CallableType doing this in isSubTypeOf:

     public function isSubTypeOf(Type $otherType): TrinaryLogic
     {
         if ($otherType instanceof IntersectionType || $otherType instanceof UnionType) {
             return $otherType->isSuperTypeOf($this);                            
         }                                                                       

CallableType relies on the fact that is knows that IntersectionType and UnionType not call back isSubTypeOf on the CallableType.

The second case is definitely more dangerous and brittle because it relies on assumptions about the behavior of other types.

IntersectionType::isSubTypeOf() allows itself to call $otherType->isSuperTypeOf($this) when $otherType is a UnionType of an IntersectionType, based on the assumption that UnionType::isSuperTypeOf() and IntersectionType::isSuperTypeOf() will never call back $otherType->isSubTypeOf() on a UnionType or IntersectionType.

It turns out that TemplateUnionType is a UnionType, and I broke this assumption by allowing TemplateUnionType::isSuperTypeOf($otherType) to call $otherType->isSubTypeOf() on IntersectionType.

Your commit fixes this by excluding TemplateUnionType from this assumption, which is a valid fix. An alternative fix would be to prevent TemplateType::isSuperTypeOf($otherType) from calling isSubTypeOf() on IntersectionType, like in the second part of my comment above.

I wonder if we could be able to make a Rule that detects infinite recursions in Type :)

I believe that there is a common issue between the original bug #6210, as well as the currnet one, and many other template-related issues, though. It is that template types inherit from their bound type. This causes template types to pass as their parent type, although they don't always behave like them (on purpose), and can not be substituted for their parent type. All bugs that are resolved by a !$x instanceof TemplateSomethingType are caused by this. I believe that #6210, #6418, #5860, #6230, #6438, #5591, and probably others are caused by this.

It appears to be easy to introduce generics bugs in unrelated code because of this.

I think that this could be solved by not inheriting from the bound type, and instead represent template types with a bound as TemplateType&BoundType (e.g. T of DateTime would be a TemplateType&ObjectType(DateTime). This way, template types would integrate nicely with everything. This is a lot of work, though.

[ondrejmirtes]

Thank you for your analysis! :)

I think that this could be solved by not inheriting from the bound type

What I'd like to do is to disallow any instanceof *Type. Mostly for the reasons you listed, and also because it's not often sufficient today, explained here: https://phpstan.org/developing-extensions/type-system#querying-a-specific-type

This means that instead of doing instanceof *Type, users should ask a method on PHPStan\Type\Type. When I refactor something to do this, it's really nice and usually solves a bunch of bugs. There's isArray(), and others already.

Once we do that, we can decouple things so that there doesn't need to be separate Template*Type for every bound type, there can be only one TemplateType (a class) that accepts a bound in its constructor :)

Of course the downside is that it's a lot of work, there's a lot of use-cases that need a separate method so I think the Type interface might grow by dozens or maybe even hundreds of methods. It might also be the only instance in the history of programming where it's the right solution 😂

It's one of the things I'd like to see in PHPStan 2.0, so we have a few years to incrementally achieve it :)

[arnaud-lb]

This sounds like a great plan !

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.