Skip to content

Commit 28b448a

Browse files
committed
Fix GH-17307: Internal closure causes JIT failure
`bcadd(...)` is a closure for an internal function, and `zend_jit_push_call_frame` takes into account both last_var and the difference in argument numbers not only for user code but also for internal code. However, this is inconsistent with `zend_vm_calc_used_stack`, causing argument corruption. Making this consistent fixes the issue. I could only reproduce the assertion failure when using Valgrind. Closes GH-17319.
1 parent c790c5b commit 28b448a

File tree

3 files changed

+41
-14
lines changed

3 files changed

+41
-14
lines changed

NEWS

+1
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ PHP NEWS
3939
- Opcache:
4040
. Fixed bug GH-15981 (Segfault with frameless jumps and minimal JIT).
4141
(nielsdos)
42+
. Fixed bug GH-17307 (Internal closure causes JIT failure). (nielsdos)
4243

4344
- PHPDBG:
4445
. Fix crashes in function registration + test. (nielsdos, Girgias)

ext/opcache/jit/zend_jit_ir.c

+8-14
Original file line numberDiff line numberDiff line change
@@ -8503,18 +8503,16 @@ static int zend_jit_push_call_frame(zend_jit_ctx *jit, const zend_op *opline, co
85038503
} else {
85048504
ir_ref num_args_ref;
85058505
ir_ref if_internal_func = IR_UNUSED;
8506+
const size_t func_type_offset = is_closure ? offsetof(zend_closure, func.type) : offsetof(zend_function, type);
85068507

85078508
used_stack = (ZEND_CALL_FRAME_SLOT + opline->extended_value + ZEND_OBSERVER_ENABLED) * sizeof(zval);
85088509
used_stack_ref = ir_CONST_ADDR(used_stack);
8510+
used_stack_ref = ir_HARD_COPY_A(used_stack_ref); /* load constant once */
85098511

8510-
if (!is_closure) {
8511-
used_stack_ref = ir_HARD_COPY_A(used_stack_ref); /* load constant once */
8512-
8513-
// JIT: if (EXPECTED(ZEND_USER_CODE(func->type))) {
8514-
ir_ref tmp = ir_LOAD_U8(ir_ADD_OFFSET(func_ref, offsetof(zend_function, type)));
8515-
if_internal_func = ir_IF(ir_AND_U8(tmp, ir_CONST_U8(1)));
8516-
ir_IF_FALSE(if_internal_func);
8517-
}
8512+
// JIT: if (EXPECTED(ZEND_USER_CODE(func->type))) {
8513+
ir_ref tmp = ir_LOAD_U8(ir_ADD_OFFSET(func_ref, func_type_offset));
8514+
if_internal_func = ir_IF(ir_AND_U8(tmp, ir_CONST_U8(1)));
8515+
ir_IF_FALSE(if_internal_func);
85188516

85198517
// JIT: used_stack += (func->op_array.last_var + func->op_array.T - MIN(func->op_array.num_args, num_args)) * sizeof(zval);
85208518
num_args_ref = ir_CONST_U32(opline->extended_value);
@@ -8541,12 +8539,8 @@ static int zend_jit_push_call_frame(zend_jit_ctx *jit, const zend_op *opline, co
85418539
}
85428540
ref = ir_SUB_A(used_stack_ref, ref);
85438541

8544-
if (is_closure) {
8545-
used_stack_ref = ref;
8546-
} else {
8547-
ir_MERGE_WITH_EMPTY_TRUE(if_internal_func);
8548-
used_stack_ref = ir_PHI_2(IR_ADDR, ref, used_stack_ref);
8549-
}
8542+
ir_MERGE_WITH_EMPTY_TRUE(if_internal_func);
8543+
used_stack_ref = ir_PHI_2(IR_ADDR, ref, used_stack_ref);
85508544
}
85518545

85528546
zend_jit_start_reuse_ip(jit);

ext/opcache/tests/jit/gh17307.phpt

+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
--TEST--
2+
GH-17307 (Internal closure causes JIT failure)
3+
--EXTENSIONS--
4+
opcache
5+
simplexml
6+
bcmath
7+
--INI--
8+
opcache.jit=1254
9+
opcache.jit_hot_func=1
10+
opcache.jit_buffer_size=32M
11+
--FILE--
12+
<?php
13+
14+
$simple = new SimpleXMLElement("<root><a/><b/></root>");
15+
16+
function run_loop($firstTerms, $closure) {
17+
foreach ($firstTerms as $firstTerm) {
18+
\debug_zval_dump($firstTerm);
19+
$closure($firstTerm, "10");
20+
}
21+
}
22+
23+
run_loop($simple, bcadd(...));
24+
echo "Done\n";
25+
26+
?>
27+
--EXPECTF--
28+
object(SimpleXMLElement)#%d (0) refcount(3){
29+
}
30+
object(SimpleXMLElement)#%d (0) refcount(3){
31+
}
32+
Done

0 commit comments

Comments
 (0)