JIT dead code skipping does not update call_level

Description

The following code:

<?php
match($y=y){};
session_set_save_handler(new SessionHandler);
session_start();

Resulted in this output:

php: ext/opcache/jit/zend_jit_ir.c:8816: int zend_jit_init_fcall(zend_jit_ctx *, const zend_op *, uint32_t, const zend_op_array *, zend_ssa *, const zend_ssa_op *, int, zend_jit_trace_rec *, int): Assertion `call_level > 0' failed.
Aborted (core dumped)

To reproduce:

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

PHP Version

nightly

Operating System

ubuntu 22.04

Simplified:

<?php
match(y){};
var_dump(new stdClass);
var_dump(3);

This is the bytecode that is used for JITting:

BB0:
     ; start lines=[0-5]
     ; to=(BB1)
     ; level=0
     ; children=(BB1)
0000 T0 = FETCH_CONSTANT string("y")
0001 MATCH_ERROR T0
0002 FREE T0
0003 INIT_FCALL 1 96 string("var_dump")
0004 V0 = NEW 0 string("stdClass")
0005 DO_FCALL

BB1:
     ; follow exit entry lines=[6-11]
     ; from=(BB0)
     ; idom=BB0
     ; level=1
0006 SEND_VAR V0 1
0007 DO_ICALL
0008 INIT_FCALL 1 96 string("var_dump")
0009 SEND_VAL int(3) 1
0010 DO_ICALL
0011 RETURN int(1)

We intend to execute MATCH_ERROR in the VM and return to trace a hot function in BB1. We generate a tail handler and skip all remaining oplines of BB0. That means the INIT_FCALL in BB0 is missed and call_level is not increased to 1. This leads to the assertion failure.

@dstogov

Here is a patch that fixes the problem for me, but this may not be the best approach. It also duplicates code, although that can be solved by refactoring the call_level increment/decrement checks into separate functions. I may also be wrong, but intuitively I think at least the idea is right, I can update the patch with suggested improvements.

cc @dstogov

Patch:

diff --git a/ext/opcache/jit/zend_jit.c b/ext/opcache/jit/zend_jit.c
index 0c6ab6c5cbc..cc22e7375a8 100644
--- a/ext/opcache/jit/zend_jit.c
+++ b/ext/opcache/jit/zend_jit.c
@@ -2576,7 +2576,34 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op
 					}
 					/* THROW and EXIT may be used in the middle of BB */
 					/* don't generate code for the rest of BB */
-					i = end;
+
+					/* Skip current opline for call_level computation
+					 * Don't include last opline because end of loop already checks call level of last opline */
+					i++;
+					for (; i < end; i++) {
+						opline = op_array->opcodes + i;
+						switch (opline->opcode) {
+							case ZEND_INIT_FCALL:
+							case ZEND_INIT_FCALL_BY_NAME:
+							case ZEND_INIT_NS_FCALL_BY_NAME:
+							case ZEND_INIT_METHOD_CALL:
+							case ZEND_INIT_DYNAMIC_CALL:
+							case ZEND_INIT_STATIC_METHOD_CALL:
+							case ZEND_INIT_PARENT_PROPERTY_HOOK_CALL:
+							case ZEND_INIT_USER_CALL:
+							case ZEND_NEW:
+								call_level++;
+								break;
+							case ZEND_DO_FCALL:
+							case ZEND_DO_ICALL:
+							case ZEND_DO_UCALL:
+							case ZEND_DO_FCALL_BY_NAME:
+							case ZEND_CALLABLE_CONVERT:
+								call_level--;
+								break;
+						}
+					}
+					opline = op_array->opcodes + i;
 					break;
 				/* stackless execution */
 				case ZEND_INCLUDE_OR_EVAL:

@nielsdos thanks for the patch. It's right. Do we need this for PHP-8.4 or for PHP-8.2/3 as well?

@dstogov

@dstogov We only need this for PHP 8.4 and higher. This is because 8.2-8.3 JIT still loops over all the opcodes and generates dead code after ZEND_MATCH_ERROR opline is handled. Therefore call_level is updated too. PHP 8.4 instead skips the oplines, causing this issue by not updating call_level.

I will make a PR for PHP 8.4. I mentioned factoring out the check when call level should be increased or decreased to reduce repetition, I can make a separate PR for that later against the master branch.

Fix phpGH-16879: JIT dead code skipping does not update call_level

I will make a PR for PHP 8.4. I mentioned factoring out the check when call level should be increased or decreased to reduce repetition, I can make a separate PR for that later against the master branch.

Thanks. Please do. I don't think a separate PR for master is required.

Fix GH-16879: JIT dead code skipping does not update call_level

Merge branch 'PHP-8.4'

Navigation Menu

Repository navigation

JIT dead code skipping does not update call_level #16879

Description

PHP Version

Operating System

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

JIT dead code skipping does not update call_level #16879

Description

Description

PHP Version

Operating System

Activity

ndossche commented on Nov 21, 2024

ndossche commented on Nov 21, 2024

dstogov commented on Nov 25, 2024

ndossche commented on Nov 25, 2024

dstogov commented on Nov 26, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions