Assertion failure in ext/opcache/jit/zend_jit_vm_helpers.c

[YuanchengJiang]

Description

The following code:

<?php
function test(){
for($i = 0; $i < 2; $i++){
$a = @[3][$i];
}
}
test();

Resulted in this output:

/php-src/ext/opcache/jit/zend_jit_vm_helpers.c:208: zend_jit_undefined_long_key: Assertion `zval_get_type(&(*(dim))) == 4' failed.
Aborted (core dumped)

To reproduce:

-d "zend_extension=/php-src/modules/opcache.so" -d "opcache.enable_cli=1" -d "opcache.jit=function"

PHP Version

PHP 8.4.0-dev

Operating System

ubuntu 22.04

[cmb69]

This issue apparently does not affect PHP-8.3, so it's not unlikely that it is related to the new JIT implementation based on IR. Maybe @dstogov wants to have a look.

[nielsdos]

Generated assembly: https://gist.github.com/nielsdos/f3924612ef31df71d798af2cffb9a2d1

The generated assembly stores $i inside register %rbx. When the JIT$test .L1 label executes to handle the undefined offset, the opline is loaded into %r14 but the opline's op2 variable was never updated with the value in %rbx and also the type is not updated. I suppose a type+value store is missing somewhere.

I was also thinking that since we probably need to store some data anyway (except for CONSTs), we could as well pass the key to the JIT stub maybe? In that case the patch is very simple, but maybe this is not a proper fix and only works around the problem: https://gist.github.com/nielsdos/d220226b4df08f68daf0efe163467b8e

[dstogov]

@nielsdos your investigation is right. We should pass the key value (at least when it's allocated in a register).

Your implementation changes zend_jit_undefined_long_key() but doesn't change TAILCALL in zend_jit_undefined_offset_stub(). So it won't pass arguments properly. Actually, this stub is used to reduce the amount of code generated for slow paths. In case we need to pass arguments, it's better to use CALL zend_jit_undefined_long_key_ex directly.

I'll provide a fix soon.