From ac05bb758dfd84bd5c98dfad42b53d84af38f9e6 Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Wed, 1 Jan 2025 22:00:21 +0100
Subject: [PATCH] Fix GH-17307: Internal closure causes JIT failure

`bcadd(...)` is a closure for an internal function, and
`zend_jit_push_call_frame` takes into account both last_var and the
difference in argument numbers not only for user code but also for
internal code. However, this is inconsistent with
`zend_vm_calc_used_stack`, causing argument corruption.
Making this consistent fixes the issue.

I could only reproduce the assertion failure when using Valgrind.
---
 ext/opcache/jit/zend_jit_ir.c      | 22 ++++++++------------
 ext/opcache/tests/jit/gh17307.phpt | 32 ++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 14 deletions(-)
 create mode 100644 ext/opcache/tests/jit/gh17307.phpt

diff --git a/ext/opcache/jit/zend_jit_ir.c b/ext/opcache/jit/zend_jit_ir.c
index e14345215f6f4..0aea4eab2bff7 100644
--- a/ext/opcache/jit/zend_jit_ir.c
+++ b/ext/opcache/jit/zend_jit_ir.c
@@ -8503,18 +8503,16 @@ static int zend_jit_push_call_frame(zend_jit_ctx *jit, const zend_op *opline, co
 	} else {
 		ir_ref num_args_ref;
 		ir_ref if_internal_func = IR_UNUSED;
+		const size_t func_type_offset = is_closure ? offsetof(zend_closure, func.type) : offsetof(zend_function, type);
 
 		used_stack = (ZEND_CALL_FRAME_SLOT + opline->extended_value + ZEND_OBSERVER_ENABLED) * sizeof(zval);
 		used_stack_ref = ir_CONST_ADDR(used_stack);
+		used_stack_ref = ir_HARD_COPY_A(used_stack_ref); /* load constant once */
 
-		if (!is_closure) {
-			used_stack_ref = ir_HARD_COPY_A(used_stack_ref); /* load constant once */
-
-			// JIT: if (EXPECTED(ZEND_USER_CODE(func->type))) {
-			ir_ref tmp = ir_LOAD_U8(ir_ADD_OFFSET(func_ref, offsetof(zend_function, type)));
-			if_internal_func = ir_IF(ir_AND_U8(tmp, ir_CONST_U8(1)));
-			ir_IF_FALSE(if_internal_func);
-		}
+		// JIT: if (EXPECTED(ZEND_USER_CODE(func->type))) {
+		ir_ref tmp = ir_LOAD_U8(ir_ADD_OFFSET(func_ref, func_type_offset));
+		if_internal_func = ir_IF(ir_AND_U8(tmp, ir_CONST_U8(1)));
+		ir_IF_FALSE(if_internal_func);
 
 		// JIT: used_stack += (func->op_array.last_var + func->op_array.T - MIN(func->op_array.num_args, num_args)) * sizeof(zval);
 		num_args_ref = ir_CONST_U32(opline->extended_value);
@@ -8541,12 +8539,8 @@ static int zend_jit_push_call_frame(zend_jit_ctx *jit, const zend_op *opline, co
 		}
 		ref = ir_SUB_A(used_stack_ref, ref);
 
-		if (is_closure) {
-			used_stack_ref = ref;
-		} else {
-			ir_MERGE_WITH_EMPTY_TRUE(if_internal_func);
-			used_stack_ref = ir_PHI_2(IR_ADDR, ref, used_stack_ref);
-		}
+		ir_MERGE_WITH_EMPTY_TRUE(if_internal_func);
+		used_stack_ref = ir_PHI_2(IR_ADDR, ref, used_stack_ref);
 	}
 
 	zend_jit_start_reuse_ip(jit);
diff --git a/ext/opcache/tests/jit/gh17307.phpt b/ext/opcache/tests/jit/gh17307.phpt
new file mode 100644
index 0000000000000..292d695963c2e
--- /dev/null
+++ b/ext/opcache/tests/jit/gh17307.phpt
@@ -0,0 +1,32 @@
+--TEST--
+GH-17307 (Internal closure causes JIT failure)
+--EXTENSIONS--
+opcache
+simplexml
+bcmath
+--INI--
+opcache.jit=1254
+opcache.jit_hot_func=1
+opcache.jit_buffer_size=32M
+--FILE--
+<?php
+
+$simple = new SimpleXMLElement("<root><a/><b/></root>");
+
+function run_loop($firstTerms, $closure) {
+    foreach ($firstTerms as $firstTerm) {
+        \debug_zval_dump($firstTerm);
+        $closure($firstTerm, "10");
+    }
+}
+
+run_loop($simple, bcadd(...));
+echo "Done\n";
+
+?>
+--EXPECTF--
+object(SimpleXMLElement)#%d (0) refcount(3){
+}
+object(SimpleXMLElement)#%d (0) refcount(3){
+}
+Done