Fix GH-17257: SEGV ext/opcache/jit/zend_jit_vm_helpers.c

[nielsdos]

EX(opline) / opline can be stale if the IP is not stored, like in this case on a trace enter. We always need to make sure that the opline is up to date to make sure we don't use stale data.
I also simplified the original test case a bit.
Needs to be fixed on 8.3, but the backport is simple.

[dstogov]

I'm not completely sure, but this looks suspicious to me.

The original code avoided some unnecessary opline and EX(opline) modifications on purpose. In JIT-ed code they don't have to be always in consistent state. but they should be restored when exit to VM (during de-optimization) or before calling sensitive functions.

May be instead of opline modification, we should invalidate last_known_opline.

Unfortunately, I can't reproduce the failure with the provided test case and I can't spend significant time now. Can you please improve the test case (may be add ASSERT to make it reproducible) or explain the problem in more details.

[nielsdos]

Sorry, the bug title was misleading.
EX(opline) is NULL, and it's adding a non-zero offset to the opline for skipping the argument receives. This triggers an undefined behaviour report.
I reproduced this with CC=clang ./configure --enable-debug --enable-undefined-sanitizer.
Alternatively, adding ZEND_ASSERT(EX(opline) != NULL) would trigger the assertion.

[dstogov]

Now I see. I would treat this as not a SEGV but just an undefined behavior warning. Right?
The result if that EX(opline) modification shouldn't be used.

To fix this we may create a copy of zend_jit_copy_extra_args_helper() without opline/EX(opline) modification.

[nielsdos]

Now I see. I would treat this as not a SEGV but just an undefined behavior warning. Right?

Yes

To fix this we may create a copy of zend_jit_copy_extra_args_helper() without opline/EX(opline) modification.

I may try this a bit later

[dstogov]

I may try this a bit later

Please do.

[nielsdos]

Ok I pushed that.
Moving the skipping to zend_jit_ir.c with a ir_ADD_OFFSET instruction conditional on the right fn_flags also works, but I suppose the reason it's not implemented that way is to reduce the amount of code generated?