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 #8461

Author: dstogov

ext/opcache/jit/zend_jit_arm64.dasc

@@ -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) {

ext/opcache/jit/zend_jit_internal.h

@@ -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 */

ext/opcache/jit/zend_jit_trace.c

@@ -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 @@ static const void *zend_jit_trace(zend_jit_trace_rec *trace_buffer, uint32_t par
 						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 @@ static const void *zend_jit_trace(zend_jit_trace_rec *trace_buffer, uint32_t par
 						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 @@ static const void *zend_jit_trace(zend_jit_trace_rec *trace_buffer, uint32_t par
 						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 @@ static const void *zend_jit_trace(zend_jit_trace_rec *trace_buffer, uint32_t par
 							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);

ext/opcache/jit/zend_jit_x86.dasc

@@ -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

ext/opcache/tests/jit/gh8461-001.inc

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

ext/opcache/tests/jit/gh8461-001.phpt

@@ -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

ext/opcache/tests/jit/gh8461-002.inc

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

ext/opcache/tests/jit/gh8461-002.phpt

@@ -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

ext/opcache/tests/jit/gh8461-003.inc

@@ -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;
+    }
+}

ext/opcache/tests/jit/gh8461-003.phpt

@@ -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

ext/opcache/tests/jit/gh8461-004.inc

@@ -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;
+    }
+}