Skip to content

Tracing JIT triggers wrong Array to string conversion in Psalm #12255

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
danog opened this issue Sep 20, 2023 · 5 comments
Closed

Tracing JIT triggers wrong Array to string conversion in Psalm #12255

danog opened this issue Sep 20, 2023 · 5 comments

Comments

@danog
Copy link
Contributor

danog commented Sep 20, 2023

Description

Running the following command inside of https://github.com/vimeo/psalm/ @ 7428e49b115a2a837aa29cf0fafd0ca902fe2457, only when tracing JIT is enabled with the config specified in #12250, triggers an Array to string conversion error in a place that makes no sense:

PSALM_ALLOW_XDEBUG=1 ./psalm --no-cache --threads=1

Result:

░░░░░░░Uncaught RuntimeException: PHP Error: Array to string conversion in /home/daniil/repos/psalm/src/Psalm/Type/Atomic/TGenericObject.php:79 for command with CLI args "./psalm --no-cache --threads=1" in /home/daniil/repos/psalm/src/Psalm/Internal/ErrorHandler.php:75

The line that triggers the issue:

        return $this->value . '<' . substr($s, 0, -2) . '>' . $extra_types;

The code around it:

    public function getKey(bool $include_extra = true): string
    {
        $s = '';

        foreach ($this->type_params as $type_param) {
            $s .= $type_param->getKey() . ', ';
        }

        $extra_types = '';

        if ($include_extra && $this->extra_types) {
            $extra_types = '&' . implode('&', $this->extra_types);
        }

        return $this->value . '<' . substr($s, 0, -2) . '>' . $extra_types;
    }

PHP Version

PHP 8.2.10

Operating System

Arch linux

@dstogov
Copy link
Member

dstogov commented Oct 2, 2023

I can't reproduce this with PHP-8.2 and master.
https://github.com/danog/php-src/actions/runs/6262586495/job/17005155671 also doesn't show any psalm related problems.

Locally, I see the same memory leaks with and without JIT.

[Mon Oct  2 12:14:03 2023]  Script:  'psalm/psalm'
Zend/zend_closures.c(540) :  Freeing 0x00007fae0d85d480 (328 bytes), script=psalm/psalm
Last leak repeated 613 times
[Mon Oct  2 12:14:03 2023]  Script:  'psalm/psalm'
Zend/zend_objects.c(189) :  Freeing 0x00007fae0d893480 (152 bytes), script=psalm/psalm
Last leak repeated 2049 times
=== Total 2664 memory leaks detected ===

cc @iluuu1994 @nielsdos

@danog
Copy link
Contributor Author

danog commented Oct 2, 2023

Yeah this is a really strange case, unlike the other issues I could not reproduce this either when re-running later (and I could not reproduce at all when using master).

@nielsdos
Copy link
Member

nielsdos commented Oct 2, 2023

The memory leaks are because psalm disables GC in src/Psalm/Internal/Cli/Psalm.php in the run function, so that's a red herring.

I cannot reproduce the JIT failure either.

@iluuu1994
Copy link
Member

I couldn't reproduce this either. I'm closing this issue. @danog If you create the PR for Psalm in the COMMUNTIY build and if it fails again we can reopen this issue.

@iluuu1994 iluuu1994 closed this as not planned Won't fix, can't repro, duplicate, stale Oct 3, 2023
@danog
Copy link
Contributor Author

danog commented Oct 3, 2023

OK, no problem!
I have a suspicion that the issue is difficult to reproduce because of the order randomization that occurs during multithreaded analysis, removing it should simplify reproducing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants