From f6c2e40a11c9aba3ed9e9512c66de4e0b1f50343 Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Sun, 23 Feb 2025 22:36:17 +0100
Subject: [PATCH] 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.
---
 NEWS                               |  4 +++
 ext/opcache/jit/zend_jit.c         | 42 +++++++++++++++++++++++++-----
 ext/opcache/tests/jit/gh15834.phpt | 23 ++++++++++++++++
 3 files changed, 63 insertions(+), 6 deletions(-)
 create mode 100644 ext/opcache/tests/jit/gh15834.phpt

diff --git a/NEWS b/NEWS
index 1f2eea02530..fa4018bf8ed 100644
--- a/NEWS
+++ b/NEWS
@@ -12,6 +12,10 @@ PHP                                                                        NEWS
   . Fixed bug GH-17913 (ReflectionFunction::isDeprecated() returns incorrect
     results for closures created from magic __call()). (timwolla)
 
+- Opcache:
+  . Fixed bug GH-15834 (Segfault with hook "simple get" cache slot and minimal
+    JIT). (nielsdos)
+
 - Standard:
   . Fix memory leaks in array_any() / array_all(). (nielsdos)
 
diff --git a/ext/opcache/jit/zend_jit.c b/ext/opcache/jit/zend_jit.c
index 77694e07d13..2d4ef0bf4fc 100644
--- a/ext/opcache/jit/zend_jit.c
+++ b/ext/opcache/jit/zend_jit.c
@@ -1316,7 +1316,7 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op
 	uint32_t target_label, target_label2;
 	uint32_t op1_info, op1_def_info, op2_info, res_info, res_use_info, op1_mem_info;
 	zend_jit_addr op1_addr, op1_def_addr, op2_addr, op2_def_addr, res_addr;
-	zend_class_entry *ce;
+	zend_class_entry *ce = NULL;
 	bool ce_is_instanceof;
 	bool on_this;
 
@@ -2335,11 +2335,6 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op
 					case ZEND_FETCH_OBJ_R:
 					case ZEND_FETCH_OBJ_IS:
 					case ZEND_FETCH_OBJ_W:
-						if (opline->op2_type != IS_CONST
-						 || Z_TYPE_P(RT_CONSTANT(opline, opline->op2)) != IS_STRING
-						 || Z_STRVAL_P(RT_CONSTANT(opline, opline->op2))[0] == '\0') {
-							break;
-						}
 						ce = NULL;
 						ce_is_instanceof = 0;
 						on_this = 0;
@@ -2369,6 +2364,11 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op
 								}
 							}
 						}
+						if (opline->op2_type != IS_CONST
+						 || Z_TYPE_P(RT_CONSTANT(opline, opline->op2)) != IS_STRING
+						 || Z_STRVAL_P(RT_CONSTANT(opline, opline->op2))[0] == '\0') {
+							break;
+						}
 						if (!zend_jit_fetch_obj(&ctx, opline, op_array, ssa, ssa_op,
 								op1_info, op1_addr, 0, ce, ce_is_instanceof, on_this, 0, 0, NULL,
 								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
 						/* We skip over the DO_FCALL, so decrement call_level ourselves. */
 						call_level--;
 					}
+					break;
+				case ZEND_FETCH_OBJ_R:
+					if (!zend_jit_handler(&ctx, opline,
+						zend_may_throw(opline, ssa_op, op_array, ssa))) {
+						goto jit_failure;
+					}
+
+					/* Cache slot is only used for IS_CONST op2, so only that can result in hook fast path. */
+					if (opline->op2_type == IS_CONST) {
+						if (JIT_G(opt_level) < ZEND_JIT_LEVEL_INLINE) {
+							if (opline->op1_type == IS_UNUSED) {
+								ce = op_array->scope;
+							} else {
+								ce = NULL;
+							}
+						}
+
+						if (!ce || !(ce->ce_flags & ZEND_ACC_FINAL) || ce->num_hooked_props > 0) {
+							/* If a simple hook is called, exit to the VM. */
+							ir_ref if_hook_enter = ir_IF(jit_CMP_IP(jit, IR_EQ, opline + 1));
+							ir_IF_FALSE(if_hook_enter);
+							if (GCC_GLOBAL_REGS) {
+								ir_TAILCALL(IR_VOID, ir_LOAD_A(jit_IP(jit)));
+							} else {
+								ir_RETURN(ir_CONST_I32(1)); /* ZEND_VM_ENTER */
+							}
+							ir_IF_TRUE(if_hook_enter);
+						}
+					}
+
 					break;
 				default:
 					if (!zend_jit_handler(&ctx, opline,
diff --git a/ext/opcache/tests/jit/gh15834.phpt b/ext/opcache/tests/jit/gh15834.phpt
new file mode 100644
index 00000000000..a8cc6ce7a15
--- /dev/null
+++ b/ext/opcache/tests/jit/gh15834.phpt
@@ -0,0 +1,23 @@
+--TEST--
+GH-15834 (Segfault with hook "simple get" cache slot and minimal JIT)
+--EXTENSIONS--
+opcache
+--INI--
+opcache.jit=1111
+--FILE--
+<?php
+class A {
+    public $_prop = 1;
+    public $prop {
+        get => $this->_prop;
+    }
+}
+
+$a = new A;
+for ($i=0;$i<5;$i++) {
+    echo $a->prop;
+    $a->_prop++;
+}
+?>
+--EXPECT--
+12345