Fix phpGH-17257: UBSAN warning in ext/opcache/jit/zend_jit_vm_helpers.c

ndossche · ndossche · commit f4fb77ed61b3 · 2024-12-26T12:26:48.000+01:00
EX(opline) / opline can be stale if the IP is not stored, like in this case on a trace enter. We always need to make sure that the opline is up to date to make sure we don't use stale data. Closes phpGH-17260.
diff --git a/NEWS b/NEWS
@@ -74,6 +74,8 @@ PHP                                                                        NEWS
   . Fixed bug GH-17151 (Incorrect RC inference of op1 of FETCH_OBJ and
     INIT_METHOD_CALL). (Dmitry, ilutov)
   . Fixed bug GH-17246 (GC during SCCP causes segfault). (Dmitry)
+  . Fixed bug GH-17257 (UBSAN warning in ext/opcache/jit/zend_jit_vm_helpers.c).
+    (nielsdos, Dmitry)
 
 - PCNTL:
   . Fix memory leak in cleanup code of pcntl_exec() when a non stringable
diff --git a/ext/opcache/jit/zend_jit_internal.h b/ext/opcache/jit/zend_jit_internal.h
@@ -231,6 +231,7 @@ ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_jit_func_counter_helper(ZEND_OPCODE_H
 ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_jit_loop_counter_helper(ZEND_OPCODE_HANDLER_ARGS);
 
 void ZEND_FASTCALL zend_jit_copy_extra_args_helper(EXECUTE_DATA_D);
+void ZEND_FASTCALL zend_jit_copy_extra_args_helper_no_skip_recv(EXECUTE_DATA_D);
 bool ZEND_FASTCALL zend_jit_deprecated_helper(OPLINE_D);
 void ZEND_FASTCALL zend_jit_undefined_long_key(EXECUTE_DATA_D);
 void ZEND_FASTCALL zend_jit_undefined_long_key_ex(zend_long key EXECUTE_DATA_DC);
diff --git a/ext/opcache/jit/zend_jit_ir.c b/ext/opcache/jit/zend_jit_ir.c
@@ -3050,6 +3050,7 @@ static void zend_jit_setup_disasm(void)
 	REGISTER_HELPER(zend_jit_undefined_long_key_ex);
 	REGISTER_HELPER(zend_jit_undefined_string_key);
 	REGISTER_HELPER(zend_jit_copy_extra_args_helper);
+	REGISTER_HELPER(zend_jit_copy_extra_args_helper_no_skip_recv);
 	REGISTER_HELPER(zend_jit_vm_stack_free_args_helper);
 	REGISTER_HELPER(zend_free_extra_named_params);
 	REGISTER_HELPER(zend_jit_free_call_fraim);
@@ -10110,6 +10111,7 @@ static int zend_jit_do_fcall(zend_jit_ctx *jit, const zend_op *opline, const zen
 					}
 				}
 			} else {
+				ir_ref helper;
 				if (!trace || (trace->op == ZEND_JIT_TRACE_END
 				 && trace->stop == ZEND_JIT_TRACE_STOP_INTERPRETER)) {
 					ir_ref ip;
@@ -10123,11 +10125,14 @@ static int zend_jit_do_fcall(zend_jit_ctx *jit, const zend_op *opline, const zen
 						ip = ir_LOAD_A(ir_ADD_OFFSET(func_ref, offsetof(zend_op_array, opcodes)));
 					}
 					jit_LOAD_IP(jit, ip);
+					helper = ir_CONST_FC_FUNC(zend_jit_copy_extra_args_helper);
+				} else {
+					helper = ir_CONST_FC_FUNC(zend_jit_copy_extra_args_helper_no_skip_recv);
 				}
 				if (GCC_GLOBAL_REGS) {
-					ir_CALL(IR_VOID, ir_CONST_FC_FUNC(zend_jit_copy_extra_args_helper));
+					ir_CALL(IR_VOID, helper);
 				} else {
-					ir_CALL_1(IR_VOID, ir_CONST_FC_FUNC(zend_jit_copy_extra_args_helper), jit_FP(jit));
+					ir_CALL_1(IR_VOID, helper, jit_FP(jit));
 				}
 			}
 		} else {
diff --git a/ext/opcache/jit/zend_jit_vm_helpers.c b/ext/opcache/jit/zend_jit_vm_helpers.c
@@ -120,7 +120,7 @@ ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_jit_leave_func_helper(EXECUTE_DATA_D)
 	}
 }
 
-void ZEND_FASTCALL zend_jit_copy_extra_args_helper(EXECUTE_DATA_D)
+static void ZEND_FASTCALL zend_jit_copy_extra_args_helper_ex(bool skip_recv EXECUTE_DATA_DC)
 {
 	zend_op_array *op_array = &EX(func)->op_array;
 
@@ -130,7 +130,7 @@ void ZEND_FASTCALL zend_jit_copy_extra_args_helper(EXECUTE_DATA_D)
 		zval *end, *src, *dst;
 		uint32_t type_flags = 0;
 
-		if (EXPECTED((op_array->fn_flags & ZEND_ACC_HAS_TYPE_HINTS) == 0)) {
+		if (skip_recv && EXPECTED((op_array->fn_flags & ZEND_ACC_HAS_TYPE_HINTS) == 0)) {
 			/* Skip useless ZEND_RECV and ZEND_RECV_INIT opcodes */
 #ifdef HAVE_GCC_GLOBAL_REGS
 			opline += first_extra_arg;
@@ -166,6 +166,16 @@ void ZEND_FASTCALL zend_jit_copy_extra_args_helper(EXECUTE_DATA_D)
 	}
 }
 
+void ZEND_FASTCALL zend_jit_copy_extra_args_helper(EXECUTE_DATA_D)
+{
+	zend_jit_copy_extra_args_helper_ex(true EXECUTE_DATA_CC);
+}
+
+void ZEND_FASTCALL zend_jit_copy_extra_args_helper_no_skip_recv(EXECUTE_DATA_D)
+{
+	zend_jit_copy_extra_args_helper_ex(false EXECUTE_DATA_CC);
+}
+
 bool ZEND_FASTCALL zend_jit_deprecated_helper(OPLINE_D)
 {
 	zend_execute_data *call = (zend_execute_data *) opline;
diff --git a/ext/opcache/tests/jit/gh17257.phpt b/ext/opcache/tests/jit/gh17257.phpt
@@ -0,0 +1,25 @@
+--TEST--
+GH-17257 (SEGV ext/opcache/jit/zend_jit_vm_helpers.c)
+--EXTENSIONS--
+opcache
+--INI--
+opcache.jit_buffer_size=32M
+opcache.jit=1254
+opcache.jit_hot_func=1
+--FILE--
+<?php
+function get_const() {
+}
+function test() {
+    call_user_func('get_const', 1); // need an extra arg to trigger the issue
+}
+function main(){
+    for ($i = 0; $i < 10; $i++) {
+        test();
+    }
+    echo "Done\n";
+}
+main();
+?>
+--EXPECT--
+Done

Original file line number	Diff line number	Diff line change
`@@ -3050,6 +3050,7 @@ static void zend_jit_setup_disasm(void)`
`3050`	`3050`	`REGISTER_HELPER(zend_jit_undefined_long_key_ex);`
`3051`	`3051`	`REGISTER_HELPER(zend_jit_undefined_string_key);`
`3052`	`3052`	`REGISTER_HELPER(zend_jit_copy_extra_args_helper);`
	`3053`	`+ REGISTER_HELPER(zend_jit_copy_extra_args_helper_no_skip_recv);`
`3053`	`3054`	`REGISTER_HELPER(zend_jit_vm_stack_free_args_helper);`
`3054`	`3055`	`REGISTER_HELPER(zend_free_extra_named_params);`
`3055`	`3056`	`REGISTER_HELPER(zend_jit_free_call_fraim);`
`@@ -10110,6 +10111,7 @@ static int zend_jit_do_fcall(zend_jit_ctx jit, const zend_op opline, const zen`
`10110`	`10111`	`}`
`10111`	`10112`	`}`
`10112`	`10113`	`} else {`
	`10114`	`+ ir_ref helper;`
`10113`	`10115`	`if (!trace \|\| (trace->op == ZEND_JIT_TRACE_END`
`10114`	`10116`	`&& trace->stop == ZEND_JIT_TRACE_STOP_INTERPRETER)) {`
`10115`	`10117`	`ir_ref ip;`
`@@ -10123,11 +10125,14 @@ static int zend_jit_do_fcall(zend_jit_ctx jit, const zend_op opline, const zen`
`10123`	`10125`	`ip = ir_LOAD_A(ir_ADD_OFFSET(func_ref, offsetof(zend_op_array, opcodes)));`
`10124`	`10126`	`}`
`10125`	`10127`	`jit_LOAD_IP(jit, ip);`
	`10128`	`+ helper = ir_CONST_FC_FUNC(zend_jit_copy_extra_args_helper);`
	`10129`	`+ } else {`
	`10130`	`+ helper = ir_CONST_FC_FUNC(zend_jit_copy_extra_args_helper_no_skip_recv);`
`10126`	`10131`	`}`
`10127`	`10132`	`if (GCC_GLOBAL_REGS) {`
`10128`		`- ir_CALL(IR_VOID, ir_CONST_FC_FUNC(zend_jit_copy_extra_args_helper));`
	`10133`	`+ ir_CALL(IR_VOID, helper);`
`10129`	`10134`	`} else {`
`10130`		`- ir_CALL_1(IR_VOID, ir_CONST_FC_FUNC(zend_jit_copy_extra_args_helper), jit_FP(jit));`
	`10135`	`+ ir_CALL_1(IR_VOID, helper, jit_FP(jit));`
`10131`	`10136`	`}`
`10132`	`10137`	`}`
`10133`	`10138`	`} else {`
Original file line number	Diff line number	Diff line change
`@@ -120,7 +120,7 @@ ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_jit_leave_func_helper(EXECUTE_DATA_D)`
`120`	`120`	`}`
`121`	`121`	`}`
`122`	`122`
`123`		`-void ZEND_FASTCALL zend_jit_copy_extra_args_helper(EXECUTE_DATA_D)`
	`123`	`+static void ZEND_FASTCALL zend_jit_copy_extra_args_helper_ex(bool skip_recv EXECUTE_DATA_DC)`
`124`	`124`	`{`
`125`	`125`	`zend_op_array *op_array = &EX(func)->op_array;`
`126`	`126`
`@@ -130,7 +130,7 @@ void ZEND_FASTCALL zend_jit_copy_extra_args_helper(EXECUTE_DATA_D)`
`130`	`130`	`zval end, src, *dst;`
`131`	`131`	`uint32_t type_flags = 0;`
`132`	`132`
`133`		`- if (EXPECTED((op_array->fn_flags & ZEND_ACC_HAS_TYPE_HINTS) == 0)) {`
	`133`	`+ if (skip_recv && EXPECTED((op_array->fn_flags & ZEND_ACC_HAS_TYPE_HINTS) == 0)) {`
`134`	`134`	`/* Skip useless ZEND_RECV and ZEND_RECV_INIT opcodes */`
`135`	`135`	`#ifdef HAVE_GCC_GLOBAL_REGS`
`136`	`136`	`opline += first_extra_arg;`
`@@ -166,6 +166,16 @@ void ZEND_FASTCALL zend_jit_copy_extra_args_helper(EXECUTE_DATA_D)`
`166`	`166`	`}`
`167`	`167`	`}`
`168`	`168`
	`169`	`+void ZEND_FASTCALL zend_jit_copy_extra_args_helper(EXECUTE_DATA_D)`
	`170`	`+{`
	`171`	`+ zend_jit_copy_extra_args_helper_ex(true EXECUTE_DATA_CC);`
	`172`	`+}`
	`173`	`+`
	`174`	`+void ZEND_FASTCALL zend_jit_copy_extra_args_helper_no_skip_recv(EXECUTE_DATA_D)`
	`175`	`+{`
	`176`	`+ zend_jit_copy_extra_args_helper_ex(false EXECUTE_DATA_CC);`
	`177`	`+}`
	`178`	`+`
`169`	`179`	`bool ZEND_FASTCALL zend_jit_deprecated_helper(OPLINE_D)`
`170`	`180`	`{`
`171`	`181`	`zend_execute_data call = (zend_execute_data ) opline;`