Add JIT guards for INIT_FCALL instructions and functions that may be modified

For methods we reuse mechanism of polymorphic calls.
For regular function we invalidate the whole root trace.

This fixes https://github.com/php/php-src/issues/8461
This commit is contained in:
Dmitry Stogov 2022-05-12 18:44:12 +03:00
parent 84c1e99ecf
commit 6c25413183
19 changed files with 543 additions and 12 deletions

View file

@ -8811,7 +8811,8 @@ static int zend_jit_init_fcall(dasm_State **Dst, const zend_op *opline, uint32_t
| // Get the return value of function zend_jit_find_func_helper/zend_jit_find_ns_func_helper
| mov REG0, RETVALx
if (JIT_G(trigger) == ZEND_JIT_ON_HOT_TRACE) {
int32_t exit_point = zend_jit_trace_get_exit_point(opline, 0);
int32_t exit_point = zend_jit_trace_get_exit_point(opline,
func && (func->common.fn_flags & ZEND_ACC_IMMUTABLE) ? ZEND_JIT_EXIT_INVALIDATE : 0);
const void *exit_addr = zend_jit_trace_get_exit_addr(exit_point);
if (!exit_addr) {

View file

@ -412,6 +412,7 @@ typedef enum _zend_jit_trace_stop {
#define ZEND_JIT_EXIT_PACKED_GUARD (1<<7)
#define ZEND_JIT_EXIT_CLOSURE_CALL (1<<8) /* exit because of polymorphic INIT_DYNAMIC_CALL call */
#define ZEND_JIT_EXIT_METHOD_CALL (1<<9) /* exit because of polymorphic INIT_METHOD_CALL call */
#define ZEND_JIT_EXIT_INVALIDATE (1<<10) /* invalidate current trace */
typedef union _zend_op_trace_info {
zend_op dummy; /* the size of this structure must be the same as zend_op */
@ -584,6 +585,7 @@ typedef struct _zend_jit_trace_info {
uint32_t flags; /* See ZEND_JIT_TRACE_... defines above */
uint32_t polymorphism; /* Counter of polymorphic calls */
uint32_t jmp_table_size;/* number of jmp_table slots */
const zend_op_array *op_array; /* function */
const zend_op *opline; /* first opline */
const void *code_start; /* address of native code */
zend_jit_trace_exit_info *exit_info; /* info about side exits */

View file

@ -366,6 +366,26 @@ static int zend_jit_trace_may_exit(const zend_op_array *op_array, const zend_op
return 0;
}
static bool zend_jit_may_be_modified(const zend_function *func, const zend_op_array *called_from)
{
if (func->type == ZEND_INTERNAL_FUNCTION) {
#ifdef _WIN32
/* ASLR */
return 1;
#else
return 0;
#endif
} else if (func->type == ZEND_USER_FUNCTION) {
if (func->common.fn_flags & ZEND_ACC_PRELOADED) {
return 0;
}
if (func->op_array.filename == called_from->filename && !func->op_array.scope) {
return 0;
}
}
return 1;
}
static zend_always_inline uint32_t zend_jit_trace_type_to_info_ex(zend_uchar type, uint32_t info)
{
if (type == IS_UNKNOWN) {
@ -6081,10 +6101,11 @@ generic_dynamic_call:
if (!zend_jit_trace_handler(&dasm_state, op_array, opline, zend_may_throw(opline, ssa_op, op_array, ssa), p + 1)) {
goto jit_failure;
}
if ((opline->opcode != ZEND_INIT_STATIC_METHOD_CALL
if ((p+1)->op == ZEND_JIT_TRACE_INIT_CALL && (p+1)->func
&& (opline->opcode != ZEND_INIT_STATIC_METHOD_CALL
|| opline->op1_type != IS_CONST
|| opline->op2_type != IS_CONST)
&& (p+1)->op == ZEND_JIT_TRACE_INIT_CALL && (p+1)->func) {
|| opline->op2_type != IS_CONST
|| zend_jit_may_be_modified((p+1)->func, op_array))) {
if (!zend_jit_init_fcall_guard(&dasm_state, 0, (p+1)->func, opline+1)) {
goto jit_failure;
}
@ -6094,8 +6115,9 @@ generic_dynamic_call:
if (!zend_jit_trace_handler(&dasm_state, op_array, opline, zend_may_throw(opline, ssa_op, op_array, ssa), p + 1)) {
goto jit_failure;
}
if (opline->op2_type != IS_CONST
&& (p+1)->op == ZEND_JIT_TRACE_INIT_CALL && (p+1)->func) {
if ((p+1)->op == ZEND_JIT_TRACE_INIT_CALL && (p+1)->func
&& (opline->op2_type != IS_CONST
|| zend_jit_may_be_modified((p+1)->func, op_array))) {
if (!zend_jit_init_fcall_guard(&dasm_state, 0, (p+1)->func, opline+1)) {
goto jit_failure;
}
@ -6105,8 +6127,9 @@ generic_dynamic_call:
if (!zend_jit_trace_handler(&dasm_state, op_array, opline, zend_may_throw(opline, ssa_op, op_array, ssa), p + 1)) {
goto jit_failure;
}
if (opline->op1_type != IS_CONST
&& (p+1)->op == ZEND_JIT_TRACE_INIT_CALL && (p+1)->func) {
if ((p+1)->op == ZEND_JIT_TRACE_INIT_CALL && (p+1)->func
&& (opline->op1_type != IS_CONST
|| zend_jit_may_be_modified((p+1)->func, op_array))) {
SET_STACK_TYPE(stack, EX_VAR_TO_NUM(opline->result.var), IS_OBJECT, 1);
if (!zend_jit_init_fcall_guard(&dasm_state, 0, (p+1)->func, opline+1)) {
goto jit_failure;
@ -6672,8 +6695,8 @@ done:
call_info = call_info->next_callee;
}
if (!skip_guard
&& !zend_jit_may_be_polymorphic_call(init_opline)) {
// TODO: recompilation may change target ???
&& !zend_jit_may_be_polymorphic_call(init_opline)
&& !zend_jit_may_be_modified(p->func, op_array)) {
skip_guard = 1;
}
}
@ -7006,6 +7029,7 @@ static zend_jit_trace_stop zend_jit_compile_root_trace(zend_jit_trace_rec *trace
t->flags = 0;
t->polymorphism = 0;
t->jmp_table_size = 0;
t->op_array = trace_buffer[0].op_array;
t->opline = trace_buffer[1].opline;
t->exit_info = exit_info;
t->stack_map = NULL;
@ -8034,6 +8058,35 @@ int ZEND_FASTCALL zend_jit_trace_exit(uint32_t exit_num, zend_jit_registers_buf
EX(opline)->lineno);
}
if (t->exit_info[exit_num].flags & ZEND_JIT_EXIT_INVALIDATE) {
zend_jit_op_array_trace_extension *jit_extension;
uint32_t num = trace_num;
while (t->root != num) {
num = t->root;
t = &zend_jit_traces[num];
}
SHM_UNPROTECT();
zend_jit_unprotect();
jit_extension = (zend_jit_op_array_trace_extension*)ZEND_FUNC_INFO(t->op_array);
if (ZEND_OP_TRACE_INFO(t->opline, jit_extension->offset)->trace_flags & ZEND_JIT_TRACE_START_LOOP) {
((zend_op*)(t->opline))->handler = (const void*)zend_jit_loop_trace_counter_handler;
} else if (ZEND_OP_TRACE_INFO(t->opline, jit_extension->offset)->trace_flags & ZEND_JIT_TRACE_START_ENTER) {
((zend_op*)(t->opline))->handler = (const void*)zend_jit_func_trace_counter_handler;
} else if (ZEND_OP_TRACE_INFO(t->opline, jit_extension->offset)->trace_flags & ZEND_JIT_TRACE_START_RETURN) {
((zend_op*)(t->opline))->handler = (const void*)zend_jit_ret_trace_counter_handler;
}
ZEND_OP_TRACE_INFO(t->opline, jit_extension->offset)->trace_flags &=
ZEND_JIT_TRACE_START_LOOP|ZEND_JIT_TRACE_START_ENTER|ZEND_JIT_TRACE_START_RETURN;
zend_jit_protect();
SHM_PROTECT();
return 0;
}
if (t->exit_info[exit_num].flags & ZEND_JIT_EXIT_TO_VM) {
if (zend_jit_trace_exit_is_bad(trace_num, exit_num)) {
zend_jit_blacklist_trace_exit(trace_num, exit_num);

View file

@ -9422,13 +9422,13 @@ static int zend_jit_init_fcall(dasm_State **Dst, const zend_op *opline, uint32_t
ZEND_UNREACHABLE();
}
if (JIT_G(trigger) == ZEND_JIT_ON_HOT_TRACE) {
int32_t exit_point = zend_jit_trace_get_exit_point(opline, 0);
int32_t exit_point = zend_jit_trace_get_exit_point(opline,
func && (func->common.fn_flags & ZEND_ACC_IMMUTABLE) ? ZEND_JIT_EXIT_INVALIDATE : 0);
const void *exit_addr = zend_jit_trace_get_exit_addr(exit_point);
if (!exit_addr) {
return 0;
}
if (!func || opline->opcode == ZEND_INIT_FCALL) {
| test r0, r0
| jnz >3

View file

@ -0,0 +1,14 @@
<?php
$x = 0;
class UniqueList
{
const A = 1;
public static function foo()
{
global $x;
$x += self::A;
}
}

View file

@ -0,0 +1,37 @@
--TEST--
Bug GH-8461 001 (JIT does not account for class re-compile)
--EXTENSIONS--
opcache
--INI--
opcache.enable=1
opcache.enable_cli=1
opcache.jit_buffer_size=1M
opcache.jit=1255
opcache.file_update_protection=0
opcache.revalidate_freq=0
opcache.protect_memory=1
--FILE--
<?php
// Checks that JITed code does not crash in --repeat 2 after the UniqueList
// class is recompiled.
require __DIR__ . '/gh8461-001.inc';
class UniqueListLast extends UniqueList
{
public static function bar() {
parent::foo();
}
}
for ($i = 0; $i < 10; $i++) {
UniqueListLast::bar();
}
// mark the file as changed (important)
touch(__DIR__ . '/gh8461-001.inc');
print "OK";
--EXPECT--
OK

View file

@ -0,0 +1,14 @@
<?php
$x = 0;
class UniqueList
{
const A = 1;
public static function foo()
{
global $x;
$x += self::A;
}
}

View file

@ -0,0 +1,30 @@
--TEST--
Bug GH-8461 002 (JIT does not account for class re-compile)
--EXTENSIONS--
opcache
--INI--
opcache.enable=1
opcache.enable_cli=1
opcache.jit_buffer_size=1M
opcache.jit=1255
opcache.file_update_protection=0
opcache.revalidate_freq=0
opcache.protect_memory=1
--FILE--
<?php
// Checks that JITed code does not crash in --repeat 2 after the UniqueList
// class is recompiled.
require __DIR__ . '/gh8461-002.inc';
for ($i = 0; $i < 10; $i++) {
UniqueList::foo();
}
// mark the file as changed (important)
touch(__DIR__ . '/gh8461-002.inc');
print "OK";
--EXPECT--
OK

View file

@ -0,0 +1,19 @@
<?php
$x = 0;
class UniqueList
{
public const A = 1;
public const B = 1;
private $foo;
public function __construct($b)
{
global $x;
$x++;
$this->foo = self::A + $b;
}
}

View file

@ -0,0 +1,38 @@
--TEST--
Bug GH-8461 003 (JIT does not account for class re-compile)
--EXTENSIONS--
opcache
--INI--
opcache.enable=1
opcache.enable_cli=1
opcache.jit_buffer_size=1M
opcache.jit=1255
opcache.file_update_protection=0
opcache.revalidate_freq=0
opcache.protect_memory=1
--FILE--
<?php
// Checks that JITed code does not crash in --repeat 2 after the UniqueList
// class is recompiled.
require __DIR__ . '/gh8461-003.inc';
class UniqueListLast extends UniqueList
{
public function __construct()
{
parent::__construct(self::B);
}
}
for ($i = 0; $i < 10; $i++) {
new UniqueListLast();
}
// mark the file as changed (important)
touch(__DIR__ . '/gh8461-003.inc');
print "OK";
--EXPECT--
OK

View file

@ -0,0 +1,19 @@
<?php
$x = 0;
class UniqueList
{
public const A = 1;
public const B = 1;
private $foo;
public function __construct($b)
{
global $x;
$x++;
$this->foo = self::A + $b;
}
}

View file

@ -0,0 +1,60 @@
--TEST--
Bug GH-8461 004 (JIT does not account for class re-compile)
--EXTENSIONS--
opcache
--INI--
opcache.enable=1
opcache.enable_cli=1
opcache.jit_buffer_size=1M
opcache.jit=1255
opcache.file_update_protection=0
opcache.revalidate_freq=0
opcache.protect_memory=1
--FILE--
<?php
// Checks that JITed code does not crash in --repeat 2 after the UniqueList
// class changes.
if (!isset(opcache_get_status()['scripts'][__DIR__ . '/gh8461-004.inc'])) {
$initialRequest = true;
require __DIR__ . '/gh8461-004.inc';
} else {
$initialRequest = false;
$y = 0;
class UniqueList
{
public const A = 1;
public const B = 1;
private $foo;
public function __construct($b)
{
global $y;
$y++;
$this->foo = self::A + $b;
}
}
}
class UniqueListLast extends UniqueList
{
public function __construct()
{
parent::__construct(self::B);
}
}
for ($i = 0; $i < 10; $i++) {
new UniqueListLast();
}
var_dump($initialRequest ? $x : $y);
print "OK";
--EXPECT--
int(10)
OK

View file

@ -0,0 +1,8 @@
<?php
$x = 0;
function test() {
global $x;
$x += 1;
}

View file

@ -0,0 +1,37 @@
--TEST--
Bug GH-8461 005 (JIT does not account for function re-compile)
--EXTENSIONS--
opcache
--INI--
opcache.enable=1
opcache.enable_cli=1
opcache.jit_buffer_size=1M
opcache.jit=1255
opcache.file_update_protection=0
opcache.revalidate_freq=0
opcache.protect_memory=1
--FILE--
<?php
if (!isset(opcache_get_status()['scripts'][__DIR__ . '/gh8461-005.inc'])) {
$initialRequest = true;
require __DIR__ . '/gh8461-005.inc';
} else {
$initialRequest = false;
$y = 0;
function test() {
global $y;
$y += 1;
}
}
for ($i = 0; $i < 10; $i++) {
test();
}
var_dump($initialRequest ? $x : $y);
print "OK";
--EXPECT--
int(10)
OK

View file

@ -0,0 +1,2 @@
<?php
// empty

View file

@ -0,0 +1,49 @@
--TEST--
Bug GH-8461 006 (JIT does not account for function re-compile)
--EXTENSIONS--
opcache
--INI--
opcache.enable=1
opcache.enable_cli=1
opcache.jit_buffer_size=1M
opcache.jit=1255
opcache.file_update_protection=0
opcache.revalidate_freq=0
opcache.protect_memory=1
--FILE--
<?php
namespace {
$x = 0;
function test() {
global $x;
$x += 1;
}
}
namespace test {
if (!isset(opcache_get_status()['scripts'][__DIR__ . '/gh8461-006.inc'])) {
$initialRequest = true;
require __DIR__ . '/gh8461-006.inc';
} else {
$initialRequest = false;
$y = 0;
function test() {
global $y;
$y += 1;
}
}
for ($i = 0; $i < 10; $i++) {
test();
}
var_dump($initialRequest ? $x : $y);
print "OK";
}
--EXPECT--
int(10)
OK

View file

@ -0,0 +1,42 @@
<?php
$x = 0;
class UniqueList
{
const A = 1;
const B = 1;
private $foo;
public function __construct($b)
{
global $x;
$x++;
$this->foo = self::A + $b;
}
public static function foo()
{
global $x;
$x += self::A;
}
}
class UniqueListLast extends UniqueList
{
public function __construct()
{
parent::__construct(self::B);
}
public static function bar() {
parent::foo();
}
}
function test() {
global $x;
$x += 1;
}

View file

@ -0,0 +1,35 @@
--TEST--
Bug GH-8461 007 (JIT does not account for function re-compile)
--EXTENSIONS--
opcache
--SKIPIF--
<?php
if (PHP_OS_FAMILY === "Windows") die("skip Windows does not support preloading");
?>
--INI--
opcache.enable=1
opcache.enable_cli=1
opcache.jit_buffer_size=1M
opcache.jit=1255
opcache.file_update_protection=0
opcache.revalidate_freq=0
opcache.protect_memory=1
opcache.preload={PWD}/gh8461-007.inc
--FILE--
<?php
for ($i = 0; $i < 100; $i++) {
UniqueListLast::bar();
}
for ($i = 0; $i < 100; $i++) {
new UniqueListLast();
}
for ($i = 0; $i < 10; $i++) {
test();
}
print "OK";
--EXPECT--
OK

View file

@ -0,0 +1,71 @@
--TEST--
Bug GH-8461 008 (JIT does not account for function re-compile)
--EXTENSIONS--
opcache
--INI--
opcache.enable=1
opcache.enable_cli=1
opcache.jit_buffer_size=1M
opcache.jit=1255
opcache.file_update_protection=0
opcache.revalidate_freq=0
opcache.protect_memory=1
--FILE--
<?php
$x = 0;
class UniqueList
{
const A = 1;
const B = 1;
private $foo;
public function __construct($b)
{
global $x;
$x++;
$this->foo = self::A + $b;
}
public static function foo()
{
global $x;
$x += self::A;
}
}
class UniqueListLast extends UniqueList
{
public function __construct()
{
parent::__construct(self::B);
}
public static function bar() {
parent::foo();
}
}
function test() {
global $x;
$x += 1;
}
for ($i = 0; $i < 100; $i++) {
UniqueListLast::bar();
}
for ($i = 0; $i < 100; $i++) {
new UniqueListLast();
}
for ($i = 0; $i < 10; $i++) {
test();
}
print "OK";
--EXPECT--
OK