JIT Assertion `info & (1 << type)' failed

[danog]

Description

https://github.com/danog/jit_bugs, reproducer 7.

Result:

php: ext/opcache/jit/zend_jit_trace.c:350: uint32_t zend_jit_trace_type_to_info_ex(uint8_t, uint32_t): Assertion `info & (1 << type)' failed.

Followed by an impossible RuntimeException PHP Error: Array to string conversion in /app/src/Psalm/Type/Atomic/TGenericObject.php, exactly from #12255.

ping @dstogov

PHP Version

3fb685b

Operating System

No response

[dstogov]

This scenario is not reproducible with PHP master branch.

[danog]

@dstogov Just ran this with b7c34b7 and can reproduce the issue, running on a 12-core x86_64 machine, will try to tweak the psalm settings a bit to simplify reproduction...

[danog]

@dstogov Cleaned up the reproducer a bit, could you try?

[dstogov]

@dstogov Cleaned up the reproducer a bit, could you try?

I'll try this tomorrow (actually today morining).
USE_ZEND_ALLOC=1 is already set in Dockerfile, so this shouldn't change anything.

Could you please also add the following patch to allow debugging in container.

diff --git a/Dockerfile b/Dockerfile
index f682628..a10d887 100755
--- a/Dockerfile
+++ b/Dockerfile
@@ -28,7 +28,8 @@ RUN true \
         pkg-config autoconf bison re2c \
         libxml2-dev libsqlite3-dev \
 		systemtap-sdt-dev libssl-dev \
-		libpcre2-dev libargon2-dev libedit-dev libsodium-dev llvm-16 libonig-dev
+		libpcre2-dev libargon2-dev libedit-dev libsodium-dev llvm-16 libonig-dev \
+		gdb libcapstone-dev
 
 RUN git clone https://github.com/php/php-src -b master --depth 1 && cd php-src \
     \
@@ -46,6 +47,7 @@ RUN git clone https://github.com/php/php-src -b master --depth 1 && cd php-src \
 		--with-password-argon2=/usr --with-external-pcre --with-mhash=/usr --with-libxml \
 		--enable-session --with-sodium --with-zlib=/usr --with-zlib-dir=/usr \
 		--enable-pcntl --with-libedit=shared,/usr \
+		--with-capstone \
     \
     && export CFLAGS='-g -fsanitize=address -shared-libasan -fno-sanitize-recover -DZEND_TRACK_ARENA_ALLOC' \
     && export CPPFLAGS='-g -fsanitize=address -shared-libasan -fno-sanitize-recover -DZEND_TRACK_ARENA_ALLOC' \
@@ -64,6 +66,6 @@ RUN git config --global --add safe.directory /app
 
 ENV USE_ZEND_ALLOC=0
 ENV PSALM_ALLOW_XDEBUG=1
-ENV ASAN_OPTIONS="detect_leaks=0:exitcode=139"
+ENV ASAN_OPTIONS="detect_leaks=0:exitcode=139:abort_on_error=true"
 
 WORKDIR /app

[danog]

Could you please also add the following patch to allow debugging in container.

Done!

USE_ZEND_ALLOC=1 is already set in Dockerfile, so this shouldn't change anything.

Actually it's set to 0 in the Dockerfile, the new setting should speed up stuff a bit :)

[dstogov]

USE_ZEND_ALLOC=1 is already set in Dockerfile, so this shouldn't change anything.

Actually it's set to 0 in the Dockerfile, the new setting should speed up stuff a bit :)

Right. My mistake.

Now I see the assertions.
May be I missed them before, because they don't interrupt the execution.
They occur in children processes that is even more complex to debug.
Is there a way to test everything in a single process?

[danog]

Is there a way to test everything in a single process?

@dstogov --threads=1 can help with that!

[dstogov]

The reduced test case

<?php
function bar(array &$a): ?bool {
    $ret = null;
    foreach ($a as $key => $val) {
        if ($val === 2) {
            unset($a[$key]);
        }
    }
    return $ret;
}

function foo($a, bool $b): bool {
    if ($b) return true;
    $n2 = count($a);
    do { 
        $n = $n2;
        $res = bar($a);
        $n2 = count($a);
    } while ($res === null && $n !== $n2);

    if ($res === null && $n === 0) {
        return false;
    }
    return true;
}

$a = [1,'a'=>5];
bar($a);
foo([1,'a'=>5], true);
foo([1,'a'=>5], false);
foo([2,'a'=>5], false);
?>

[dstogov]

This should be partially fixed by 93d5c0e
The commit fixes the reduced test case above.
Now running psalm with --threads=100, I see only a single assertion (previously I saw few ones).

There are no assertions when running psalm with --threads=1 and without wrap.php
I have no ideas how to catch this last assertion.

[danog]

Hmm, will see if I can reduce this this weekend...

[dstogov]

The assertion disappears if -f flag is removed in the command line (before /app/wrap.php)

[danog]

Thank you for all the work on the fixes @dstogov!
Strangely, I could not reproduce the last assertion you were talking about with pre-fix commits, did you use some special configs or changes to trigger it?
I'm asking because there may be some more issues that could by reproduced by running reproducer number 8 in https://github.com/danog/jit_bugs, have you tried that?

Also, since it seems like Psalm should now work fine, could you please take a look at and merge the following PRs when you have some free time?

• Multiple JIT test improvements #12406
• Improve jit tests #12425
• Improve JIT config in fuzzer SAPI #12519
• Fix memory leak in syslog extension #12501
• Repeat JIT tests  #12495 (just as a review of the general concept of the PR, it's blocked by a memory leak fixed in Fix memory leak in syslog extension #12501 and by other issues surfaced by the repeated execution of JIT tests)

Thanks :)

[dstogov]

Thank you for all the work on the fixes @dstogov!

Thanks for the test cases that helped to fix the bugs! :)

Strangely, I could not reproduce the last assertion you were talking about with pre-fix commits, did you use some special configs or changes to trigger it? I'm asking because there may be some more issues that could by reproduced by running reproducer number 8 in https://github.com/danog/jit_bugs, have you tried that?

No. The assertion occurred in some worker on 7 test inside a container (it didn't stop the test).

Also, since it seems like Psalm should now work fine, could you please take a look at and merge the following PRs when you have some free time?

All your https://github.com/danog/jit_bugs are fixed now?
Could you please also test them with PHP-8.3 and with default JIT tracing setting (I mean ``opcache.jit_hot_loop, jit_hot_func, jit_hot_return and jit_hot_side_exit").

* [Add phpseclib, Psalm, PHPStan nightly tests #12406](https://github.com/php/php-src/pull/12406)

* [Improve jit tests #12425](https://github.com/php/php-src/pull/12425)

* [Improve JIT config in fuzzer SAPI #12519](https://github.com/php/php-src/pull/12519)

* [Fix memory leak in syslog extension #12501](https://github.com/php/php-src/pull/12501)

* [ Add infection to nightly testsuite, repeat JIT tests  #12495](https://github.com/php/php-src/pull/12495) (just as a review of the general concept of the PR, it's blocked by a memory leak fixed in [Fix memory leak in syslog extension #12501](https://github.com/php/php-src/pull/12501) and by other issues surfaced by the repeated execution of JIT tests)

I'm not sure if all these PRs should be merged. It's a question about load, execution time, etc
Psalm failed in workers and continued execution. This won't allow to catch the failures. Also this kind of failures are extremely difficult to debug.

cc @iluuu1994

[iluuu1994]

@danog Sorry for taking a long time to get back. The PRs are a bit all over the place. 🙂 Some things are in multiple PRs or unrelated. I'll take some time today to review them. I think adding more things to the COMMUNITY tests is fine, as long as it's usefulness is proportional to the additional execution time. Of course, at some points we'll have diminishing returns. For the COMMUNITY tests, it would be helpful if you temporarily copied them to push.yml to prove they actually pass. If they don't, we should try to resolve the issues first. This will also show whether execution time is significantly impacted.

[danog]

Hi @dstogov and @iluuu1994,

All your https://github.com/danog/jit_bugs are fixed now?

It seems like it, I could not reproduce any of the issues.

Could you please also test them with PHP-8.3 and with default JIT tracing setting (I mean ``opcache.jit_hot_loop, jit_hot_func, jit_hot_return and jit_hot_side_exit").

Tested with PHP 8.3, not quite sure about which changes are needed, shouldn't the ones I use in the repo already increase the likelyhood of catching JIT bugs?

No. The assertion occurred in some worker on 7 test inside a container (it didn't stop the test).
Psalm failed in workers and continued execution. This won't allow to catch the failures

Even with an old pre-3844593 commit, I could not for the life of me reproduce the silent assertions.
This is super strange, I even tried using a slightly tweaked version of https://github.com/php/php-src/blob/886dd7c06fd410ede6f332031f052787c3340368/ext/opcache/tests/jit/gh12512_2.phpt that only runs the buggy code in a child (not in the parent) after a fork on an old commit; the assertion message is printed like it should, and given the code in https://github.com/vimeo/psalm/blob/master/src/Psalm/Internal/Fork/Pool.php, Psalm always exits with exit code 1 in cause of failures in child processes which prevent their result from being gathered by the parent, and in case of failures after the result is sent, an error is at the very least printed if the process exited due to a signal (https://github.com/vimeo/psalm/blob/5.x/src/Psalm/Internal/Fork/Pool.php#L454, codepath not triggered if an exception was thrown, which should already cause termination of the parent either way); this problem which causes assertions to not surface seems like a problem in PHP itself....

For the COMMUNITY tests, it would be helpful if you temporarily copied them to push.yml to prove they actually pass.

I've made the simpler thing, I've started a CI task for #12406 on my repo: https://github.com/danog/php-src/actions/runs/6837624271/job/18593863162

[dstogov]

All your https://github.com/danog/jit_bugs are fixed now?
It seems like it, I could not reproduce any of the issues.

Great! And thank you for helping fixing all these bugs!

Could you please also test them with PHP-8.3 and with default JIT tracing setting (I mean ``opcache.jit_hot_loop, jit_hot_func, jit_hot_return and jit_hot_side_exit").
Tested with PHP 8.3, not quite sure about which changes are needed, shouldn't the ones I use in the repo already increase the likelyhood of catching JIT bugs?

I meant just changing the branch in Dockerfile. JIT implementation in master and early PHP versions are different and may cause different problems. I plan to backport the fix for https://github.com/php/php-src/blob/886dd7c06fd410ede6f332031f052787c3340368/ext/opcache/tests/jit/gh12512_2.phpt today. It seems like PHP-8.3 wasn't affected by other problems, but I might miss something.