Skip to content

Commit f6c2e40

Browse files
committed
Fix GH-15834: Segfault with hook "simple get" cache slot and minimal JIT
The FETCH_OBJ_R VM handler has an optimization that directly enters into a hook if it is a simpler getter hook. This is not compatible with the minimal JIT because the minimal JIT will try to continue executing the opcodes after the FETCH_OBJ_R. To solve this, we check whether the opcode is still the expected one after the execution of the VM handler. If it is not, we know that we are going to execute a simple hook. In that case, exit to the VM. Closes GH-17909.
1 parent 4d5a88c commit f6c2e40

File tree

3 files changed

+63
-6
lines changed

3 files changed

+63
-6
lines changed

NEWS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ PHP NEWS
1212
. Fixed bug GH-17913 (ReflectionFunction::isDeprecated() returns incorrect
1313
results for closures created from magic __call()). (timwolla)
1414

15+
- Opcache:
16+
. Fixed bug GH-15834 (Segfault with hook "simple get" cache slot and minimal
17+
JIT). (nielsdos)
18+
1519
- Standard:
1620
. Fix memory leaks in array_any() / array_all(). (nielsdos)
1721

ext/opcache/jit/zend_jit.c

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1316,7 +1316,7 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op
13161316
uint32_t target_label, target_label2;
13171317
uint32_t op1_info, op1_def_info, op2_info, res_info, res_use_info, op1_mem_info;
13181318
zend_jit_addr op1_addr, op1_def_addr, op2_addr, op2_def_addr, res_addr;
1319-
zend_class_entry *ce;
1319+
zend_class_entry *ce = NULL;
13201320
bool ce_is_instanceof;
13211321
bool on_this;
13221322

@@ -2335,11 +2335,6 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op
23352335
case ZEND_FETCH_OBJ_R:
23362336
case ZEND_FETCH_OBJ_IS:
23372337
case ZEND_FETCH_OBJ_W:
2338-
if (opline->op2_type != IS_CONST
2339-
|| Z_TYPE_P(RT_CONSTANT(opline, opline->op2)) != IS_STRING
2340-
|| Z_STRVAL_P(RT_CONSTANT(opline, opline->op2))[0] == '\0') {
2341-
break;
2342-
}
23432338
ce = NULL;
23442339
ce_is_instanceof = 0;
23452340
on_this = 0;
@@ -2369,6 +2364,11 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op
23692364
}
23702365
}
23712366
}
2367+
if (opline->op2_type != IS_CONST
2368+
|| Z_TYPE_P(RT_CONSTANT(opline, opline->op2)) != IS_STRING
2369+
|| Z_STRVAL_P(RT_CONSTANT(opline, opline->op2))[0] == '\0') {
2370+
break;
2371+
}
23722372
if (!zend_jit_fetch_obj(&ctx, opline, op_array, ssa, ssa_op,
23732373
op1_info, op1_addr, 0, ce, ce_is_instanceof, on_this, 0, 0, NULL,
23742374
RES_REG_ADDR(), IS_UNKNOWN,
@@ -2709,6 +2709,36 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op
27092709
/* We skip over the DO_FCALL, so decrement call_level ourselves. */
27102710
call_level--;
27112711
}
2712+
break;
2713+
case ZEND_FETCH_OBJ_R:
2714+
if (!zend_jit_handler(&ctx, opline,
2715+
zend_may_throw(opline, ssa_op, op_array, ssa))) {
2716+
goto jit_failure;
2717+
}
2718+
2719+
/* Cache slot is only used for IS_CONST op2, so only that can result in hook fast path. */
2720+
if (opline->op2_type == IS_CONST) {
2721+
if (JIT_G(opt_level) < ZEND_JIT_LEVEL_INLINE) {
2722+
if (opline->op1_type == IS_UNUSED) {
2723+
ce = op_array->scope;
2724+
} else {
2725+
ce = NULL;
2726+
}
2727+
}
2728+
2729+
if (!ce || !(ce->ce_flags & ZEND_ACC_FINAL) || ce->num_hooked_props > 0) {
2730+
/* If a simple hook is called, exit to the VM. */
2731+
ir_ref if_hook_enter = ir_IF(jit_CMP_IP(jit, IR_EQ, opline + 1));
2732+
ir_IF_FALSE(if_hook_enter);
2733+
if (GCC_GLOBAL_REGS) {
2734+
ir_TAILCALL(IR_VOID, ir_LOAD_A(jit_IP(jit)));
2735+
} else {
2736+
ir_RETURN(ir_CONST_I32(1)); /* ZEND_VM_ENTER */
2737+
}
2738+
ir_IF_TRUE(if_hook_enter);
2739+
}
2740+
}
2741+
27122742
break;
27132743
default:
27142744
if (!zend_jit_handler(&ctx, opline,

ext/opcache/tests/jit/gh15834.phpt

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
--TEST--
2+
GH-15834 (Segfault with hook "simple get" cache slot and minimal JIT)
3+
--EXTENSIONS--
4+
opcache
5+
--INI--
6+
opcache.jit=1111
7+
--FILE--
8+
<?php
9+
class A {
10+
public $_prop = 1;
11+
public $prop {
12+
get => $this->_prop;
13+
}
14+
}
15+
16+
$a = new A;
17+
for ($i=0;$i<5;$i++) {
18+
echo $a->prop;
19+
$a->_prop++;
20+
}
21+
?>
22+
--EXPECT--
23+
12345

0 commit comments

Comments
 (0)