Skip to content

Commit 69d263e

Browse files
arnaud-lboleg-st
arnaud-lb
and
oleg-st
authored
Add JIT guards for INIT_METHOD_CALL when the method may be modified (#8600)
Non-polymorphic methods can be modified from one request to an other due to recompilation or conditional declaration. Fixes GH-8591 Co-authored-by: Oleg Stepanischev <Oleg.Stepanischev@tatar.ru>
1 parent 6d4ebe7 commit 69d263e

15 files changed

+345
-24
lines changed

‎ext/opcache/jit/zend_jit_arm64.dasc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8998,15 +8998,15 @@ static int zend_jit_init_method_call(dasm_State **Dst,
89988998
|2:
89998999
}
90009000

9001-
if (!func
9001+
if ((!func || zend_jit_may_be_modified(func, op_array))
90029002
&& trace
90039003
&& trace->op == ZEND_JIT_TRACE_INIT_CALL
90049004
&& trace->func
90059005
) {
90069006
int32_t exit_point;
90079007
const void *exit_addr;
90089008

9009-
exit_point = zend_jit_trace_get_exit_point(opline, ZEND_JIT_EXIT_METHOD_CALL);
9009+
exit_point = zend_jit_trace_get_exit_point(opline, func ? ZEND_JIT_EXIT_INVALIDATE : ZEND_JIT_EXIT_METHOD_CALL);
90109010
exit_addr = zend_jit_trace_get_exit_addr(exit_point);
90119011
if (!exit_addr) {
90129012
return 0;

‎ext/opcache/jit/zend_jit_internal.h

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -719,6 +719,26 @@ static zend_always_inline const zend_op* zend_jit_trace_get_exit_opline(zend_jit
719719
return NULL;
720720
}
721721

722+
static inline bool zend_jit_may_be_modified(const zend_function *func, const zend_op_array *called_from)
723+
{
724+
if (func->type == ZEND_INTERNAL_FUNCTION) {
725+
#ifdef _WIN32
726+
/* ASLR */
727+
return 1;
728+
#else
729+
return 0;
730+
#endif
731+
} else if (func->type == ZEND_USER_FUNCTION) {
732+
if (func->common.fn_flags & ZEND_ACC_PRELOADED) {
733+
return 0;
734+
}
735+
if (func->op_array.filename == called_from->filename && !func->op_array.scope) {
736+
return 0;
737+
}
738+
}
739+
return 1;
740+
}
741+
722742
static zend_always_inline bool zend_jit_may_be_polymorphic_call(const zend_op *opline)
723743
{
724744
if (opline->opcode == ZEND_INIT_FCALL

‎ext/opcache/jit/zend_jit_trace.c

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -366,26 +366,6 @@ static int zend_jit_trace_may_exit(const zend_op_array *op_array, const zend_op
366366
return 0;
367367
}
368368

369-
static bool zend_jit_may_be_modified(const zend_function *func, const zend_op_array *called_from)
370-
{
371-
if (func->type == ZEND_INTERNAL_FUNCTION) {
372-
#ifdef _WIN32
373-
/* ASLR */
374-
return 1;
375-
#else
376-
return 0;
377-
#endif
378-
} else if (func->type == ZEND_USER_FUNCTION) {
379-
if (func->common.fn_flags & ZEND_ACC_PRELOADED) {
380-
return 0;
381-
}
382-
if (func->op_array.filename == called_from->filename && !func->op_array.scope) {
383-
return 0;
384-
}
385-
}
386-
return 1;
387-
}
388-
389369
static zend_always_inline uint32_t zend_jit_trace_type_to_info_ex(zend_uchar type, uint32_t info)
390370
{
391371
if (type == IS_UNKNOWN) {

‎ext/opcache/jit/zend_jit_x86.dasc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9630,7 +9630,7 @@ static int zend_jit_init_method_call(dasm_State **Dst,
96309630
|2:
96319631
}
96329632

9633-
if (!func
9633+
if ((!func || zend_jit_may_be_modified(func, op_array))
96349634
&& trace
96359635
&& trace->op == ZEND_JIT_TRACE_INIT_CALL
96369636
&& trace->func
@@ -9641,7 +9641,7 @@ static int zend_jit_init_method_call(dasm_State **Dst,
96419641
int32_t exit_point;
96429642
const void *exit_addr;
96439643

9644-
exit_point = zend_jit_trace_get_exit_point(opline, ZEND_JIT_EXIT_METHOD_CALL);
9644+
exit_point = zend_jit_trace_get_exit_point(opline, func ? ZEND_JIT_EXIT_INVALIDATE : ZEND_JIT_EXIT_METHOD_CALL);
96459645
exit_addr = zend_jit_trace_get_exit_addr(exit_point);
96469646
if (!exit_addr) {
96479647
return 0;

‎ext/opcache/tests/jit/gh8591-001.inc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
interface ModelInterface
6+
{
7+
}

‎ext/opcache/tests/jit/gh8591-001.phpt

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
--TEST--
2+
Bug GH-8591 001 (JIT does not account for class re-compile)
3+
--EXTENSIONS--
4+
opcache
5+
--INI--
6+
opcache.enable=1
7+
opcache.enable_cli=1
8+
opcache.jit_buffer_size=1M
9+
opcache.jit=1255
10+
opcache.file_update_protection=0
11+
opcache.revalidate_freq=0
12+
opcache.protect_memory=1
13+
--FILE--
14+
<?php
15+
16+
// Checks that JITed code does not crash in --repeat 2 after the ModelInterface
17+
// interface is recompiled and Model is re-linked.
18+
19+
require __DIR__ . '/gh8591-001.inc';
20+
21+
class Model implements ModelInterface
22+
{
23+
protected static int $field = 1;
24+
25+
public function __construct()
26+
{
27+
for ($i = 0; $i < 10; $i++) {
28+
$this->cast();
29+
}
30+
}
31+
32+
private function cast()
33+
{
34+
global $x;
35+
$x = static::$field;
36+
}
37+
}
38+
39+
new Model();
40+
41+
// mark the file as changed (important)
42+
touch(__DIR__ . '/gh8591-001.inc');
43+
44+
var_dump($x);
45+
46+
print "OK";
47+
--EXPECT--
48+
int(1)
49+
OK

‎ext/opcache/tests/jit/gh8591-002.inc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
interface ModelInterface
6+
{
7+
}

‎ext/opcache/tests/jit/gh8591-002.phpt

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
--TEST--
2+
Bug GH-8591 002 (JIT does not account for class re-compile)
3+
--EXTENSIONS--
4+
opcache
5+
--INI--
6+
opcache.enable=1
7+
opcache.enable_cli=1
8+
opcache.jit_buffer_size=1M
9+
opcache.jit=1255
10+
opcache.file_update_protection=0
11+
opcache.revalidate_freq=0
12+
opcache.protect_memory=1
13+
--FILE--
14+
<?php
15+
16+
// Checks that JITed code does not crash in --repeat 2 after the ModelInterface
17+
// interface changes and Model is re-linked.
18+
19+
if (!isset(opcache_get_status()['scripts'][__DIR__ . '/gh8591-002.inc'])) {
20+
require __DIR__ . '/gh8591-001.inc';
21+
} else {
22+
interface ModelInterace
23+
{
24+
}
25+
}
26+
27+
class Model implements ModelInterface
28+
{
29+
protected static int $field = 1;
30+
31+
public function __construct()
32+
{
33+
for ($i = 0; $i < 10; $i++) {
34+
$this->cast();
35+
}
36+
}
37+
38+
private function cast()
39+
{
40+
global $x;
41+
$x = static::$field;
42+
}
43+
}
44+
45+
new Model();
46+
47+
var_dump($x);
48+
49+
print "OK";
50+
--EXPECT--
51+
int(1)
52+
OK

‎ext/opcache/tests/jit/gh8591-003.phpt

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
--TEST--
2+
Bug GH-8591 003 (JIT does not account for class re-compile)
3+
--EXTENSIONS--
4+
opcache
5+
--INI--
6+
opcache.enable=1
7+
opcache.enable_cli=1
8+
opcache.jit_buffer_size=1M
9+
opcache.jit=1255
10+
opcache.file_update_protection=0
11+
opcache.revalidate_freq=0
12+
opcache.protect_memory=1
13+
--FILE--
14+
<?php
15+
16+
interface ModelInterface
17+
{
18+
}
19+
20+
class Model implements ModelInterface
21+
{
22+
protected static int $field = 1;
23+
24+
public function __construct()
25+
{
26+
for ($i = 0; $i < 10; $i++) {
27+
$this->cast();
28+
}
29+
}
30+
31+
private function cast()
32+
{
33+
global $x;
34+
$x = static::$field;
35+
}
36+
}
37+
38+
new Model();
39+
40+
var_dump($x);
41+
42+
print "OK";
43+
--EXPECT--
44+
int(1)
45+
OK

‎ext/opcache/tests/jit/gh8591-004.inc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
trait ModelTrait
6+
{
7+
}

‎ext/opcache/tests/jit/gh8591-004.phpt

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
--TEST--
2+
Bug GH-8591 004 (JIT does not account for class re-compile)
3+
--EXTENSIONS--
4+
opcache
5+
--INI--
6+
opcache.enable=1
7+
opcache.enable_cli=1
8+
opcache.jit_buffer_size=1M
9+
opcache.jit=1255
10+
opcache.file_update_protection=0
11+
opcache.revalidate_freq=0
12+
opcache.protect_memory=1
13+
--FILE--
14+
<?php
15+
16+
// Checks that JITed code does not crash in --repeat 2 after the ModelTrait
17+
// trait is recompiled and Model is re-linked.
18+
19+
require __DIR__ . '/gh8591-004.inc';
20+
21+
class Model
22+
{
23+
use ModelTrait;
24+
25+
protected static int $field = 1;
26+
27+
public function __construct()
28+
{
29+
for ($i = 0; $i < 10; $i++) {
30+
$this->cast();
31+
}
32+
}
33+
34+
private function cast()
35+
{
36+
global $x;
37+
$x = static::$field;
38+
}
39+
}
40+
41+
new Model();
42+
43+
// mark the file as changed (important)
44+
touch(__DIR__ . '/gh8591-004.inc');
45+
46+
var_dump($x);
47+
48+
print "OK";
49+
--EXPECT--
50+
int(1)
51+
OK

‎ext/opcache/tests/jit/gh8591-005.inc

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?php
2+
3+
class AbstractModel
4+
{
5+
protected static int $field = 1;
6+
7+
final protected function cast()
8+
{
9+
global $x;
10+
$x = static::$field;
11+
}
12+
}

‎ext/opcache/tests/jit/gh8591-005.phpt

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
--TEST--
2+
Bug GH-8591 001 (JIT does not account for class re-compile)
3+
--EXTENSIONS--
4+
opcache
5+
--INI--
6+
opcache.enable=1
7+
opcache.enable_cli=1
8+
opcache.jit_buffer_size=1M
9+
opcache.jit=1255
10+
opcache.file_update_protection=0
11+
opcache.revalidate_freq=0
12+
opcache.protect_memory=1
13+
--FILE--
14+
<?php
15+
16+
// Checks that JITed code does not crash in --repeat 2 after the AbstractModel
17+
// class is recompiled and Model is re-linked.
18+
19+
require __DIR__ . '/gh8591-005.inc';
20+
21+
class Model extends AbstractModel
22+
{
23+
public function __construct()
24+
{
25+
for ($i = 0; $i < 10; $i++) {
26+
$this->cast();
27+
}
28+
}
29+
}
30+
31+
new Model();
32+
33+
// mark the file as changed (important)
34+
touch(__DIR__ . '/gh8591-005.inc');
35+
36+
var_dump($x);
37+
38+
print "OK";
39+
--EXPECT--
40+
int(1)
41+
OK

‎ext/opcache/tests/jit/gh8591-006.inc

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?php
2+
3+
class AbstractModel
4+
{
5+
protected static int $field = 1;
6+
7+
final protected function cast()
8+
{
9+
global $x;
10+
$x = static::$field;
11+
}
12+
}

0 commit comments

Comments
 (0)