Skip to content

Commit 82f70db

Browse files
committed
Use original op_array when JIT compiling a Closure
zend_jit() assumes that Closure op_arrays have no scope, but this is not true when using the hot counters, first exec, or trace triggers as they use the executed op_array, which is in case of Closures is a copy, with a scope. In the tracing JIT this problem is avoided as we fetch the original op_array when compiling a Closure. Here I replicate this for the hot counters and first exec triggers. Fixes phpGH-16186 Closes phpGH-16200
1 parent 07e418a commit 82f70db

File tree

4 files changed

+95
-0
lines changed

4 files changed

+95
-0
lines changed

ext/opcache/jit/zend_jit.c

+17
Original file line numberDiff line numberDiff line change
@@ -1286,6 +1286,8 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op
12861286
bool ce_is_instanceof;
12871287
bool on_this;
12881288

1289+
ZEND_ASSERT(!(op_array->fn_flags & ZEND_ACC_CLOSURE) || !(op_array->scope));
1290+
12891291
if (JIT_G(bisect_limit)) {
12901292
jit_bisect_pos++;
12911293
if (jit_bisect_pos >= JIT_G(bisect_limit)) {
@@ -2818,6 +2820,18 @@ static int zend_real_jit_func(zend_op_array *op_array, zend_script *script, cons
28182820
/* Build SSA */
28192821
memset(&ssa, 0, sizeof(zend_ssa));
28202822

2823+
if (op_array->fn_flags & ZEND_ACC_CLOSURE) {
2824+
if (JIT_G(trigger) == ZEND_JIT_ON_FIRST_EXEC) {
2825+
zend_jit_op_array_extension *jit_extension = (zend_jit_op_array_extension*)ZEND_FUNC_INFO(op_array);
2826+
op_array = (zend_op_array*) jit_extension->op_array;
2827+
} else if (JIT_G(trigger) == ZEND_JIT_ON_HOT_COUNTERS) {
2828+
zend_jit_op_array_hot_extension *jit_extension = (zend_jit_op_array_hot_extension*)ZEND_FUNC_INFO(op_array);
2829+
op_array = (zend_op_array*) jit_extension->op_array;
2830+
} else {
2831+
ZEND_ASSERT(!op_array->scope);
2832+
}
2833+
}
2834+
28212835
if (zend_jit_op_array_analyze1(op_array, script, &ssa) != SUCCESS) {
28222836
goto jit_failure;
28232837
}
@@ -3035,6 +3049,7 @@ static int zend_jit_setup_hot_counters(zend_op_array *op_array)
30353049
}
30363050
memset(&jit_extension->func_info, 0, sizeof(zend_func_info));
30373051
jit_extension->func_info.flags = ZEND_FUNC_JIT_ON_HOT_COUNTERS;
3052+
jit_extension->op_array = op_array;
30383053
jit_extension->counter = &zend_jit_hot_counters[zend_jit_op_array_hash(op_array) & (ZEND_HOT_COUNTERS_COUNT - 1)];
30393054
for (i = 0; i < op_array->last; i++) {
30403055
jit_extension->orig_handlers[i] = op_array->opcodes[i].handler;
@@ -3079,6 +3094,7 @@ int zend_jit_op_array(zend_op_array *op_array, zend_script *script)
30793094
}
30803095
memset(&jit_extension->func_info, 0, sizeof(zend_func_info));
30813096
jit_extension->func_info.flags = ZEND_FUNC_JIT_ON_FIRST_EXEC;
3097+
jit_extension->op_array = op_array;
30823098
jit_extension->orig_handler = (void*)opline->handler;
30833099
ZEND_SET_FUNC_INFO(op_array, (void*)jit_extension);
30843100
opline->handler = (const void*)zend_jit_runtime_jit_handler;
@@ -3108,6 +3124,7 @@ int zend_jit_op_array(zend_op_array *op_array, zend_script *script)
31083124
}
31093125
memset(&jit_extension->func_info, 0, sizeof(zend_func_info));
31103126
jit_extension->func_info.flags = ZEND_FUNC_JIT_ON_PROF_REQUEST;
3127+
jit_extension->op_array = op_array;
31113128
jit_extension->orig_handler = (void*)opline->handler;
31123129
ZEND_SET_FUNC_INFO(op_array, (void*)jit_extension);
31133130
opline->handler = (const void*)zend_jit_profile_jit_handler;

ext/opcache/jit/zend_jit_internal.h

+2
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ static zend_always_inline bool zend_jit_same_addr(zend_jit_addr addr1, zend_jit_
123123

124124
typedef struct _zend_jit_op_array_extension {
125125
zend_func_info func_info;
126+
const zend_op_array *op_array;
126127
const void *orig_handler;
127128
} zend_jit_op_array_extension;
128129

@@ -160,6 +161,7 @@ void ZEND_FASTCALL zend_jit_hot_func(zend_execute_data *execute_data, const zend
160161

161162
typedef struct _zend_jit_op_array_hot_extension {
162163
zend_func_info func_info;
164+
const zend_op_array *op_array;
163165
int16_t *counter;
164166
const void *orig_handlers[1];
165167
} zend_jit_op_array_hot_extension;

ext/opcache/tests/gh16186_001.phpt

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
--TEST--
2+
GH-16186 001 (Non-tracing JIT uses Closure scope)
3+
--EXTENSIONS--
4+
opcache
5+
--INI--
6+
opcache.enable=1
7+
opcache.enable_cli=1
8+
opcache.jit_buffer_size=64M
9+
opcache.jit=0234
10+
opcache.file_update_protection=0
11+
opcache.revalidate_freq=0
12+
opcache.protect_memory=1
13+
--FILE--
14+
<?php
15+
16+
class A {
17+
private $x;
18+
public function getIncrementor() {
19+
return function() { $this->x++; };
20+
}
21+
}
22+
$a = new A();
23+
$f = $a->getIncrementor();
24+
$c = new stdClass();
25+
$f();
26+
$f2 = Closure::bind($f, $c);
27+
$f2();
28+
$f2();
29+
30+
var_dump($c);
31+
32+
?>
33+
--EXPECTF--
34+
Warning: Undefined property: stdClass::$x in %s on line %d
35+
object(stdClass)#%d (1) {
36+
["x"]=>
37+
int(2)
38+
}

ext/opcache/tests/gh16186_002.phpt

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
--TEST--
2+
GH-16186 002 (Non-tracing JIT uses Closure scope)
3+
--EXTENSIONS--
4+
opcache
5+
--INI--
6+
opcache.enable=1
7+
opcache.enable_cli=1
8+
opcache.jit_buffer_size=64M
9+
opcache.jit=0214
10+
opcache.file_update_protection=0
11+
opcache.revalidate_freq=0
12+
opcache.protect_memory=1
13+
--FILE--
14+
<?php
15+
16+
class A {
17+
private $x;
18+
public function getIncrementor() {
19+
return function() { $this->x++; };
20+
}
21+
}
22+
$a = new A();
23+
$f = $a->getIncrementor();
24+
$c = new stdClass();
25+
$f();
26+
$f2 = Closure::bind($f, $c);
27+
$f2();
28+
$f2();
29+
30+
var_dump($c);
31+
32+
?>
33+
--EXPECTF--
34+
Warning: Undefined property: stdClass::$x in %s on line %d
35+
object(stdClass)#%d (1) {
36+
["x"]=>
37+
int(2)
38+
}

0 commit comments

Comments
 (0)