Root arguments of internal frames

Author: arnaud-lb

Zend/tests/gc/gh13687-003.phpt

@@ -0,0 +1,34 @@
+--TEST--
+GH-13687 003: Result operand may leak if GC is triggered before consumption
+--FILE--
+<?php
+
+class A {
+    public $cycle;
+    public function __construct() { $this->cycle = $this; }
+}
+class B {
+    public function get() {
+        return new A();
+    }
+}
+class Debug {
+    public function __debugInfo() {
+        gc_collect_cycles();
+        return [];
+    }
+}
+
+$c = new B();
+var_dump($c->get(), new Debug);
+
+?>
+==DONE==
+--EXPECTF--
+object(A)#%d (1) {
+  ["cycle"]=>
+  *RECURSION*
+}
+object(Debug)#%d (0) {
+}
+==DONE==

Zend/tests/gc/gh13687-004.phpt

@@ -0,0 +1,30 @@
+--TEST--
+GH-13687 004: Result operand may leak if GC is triggered before consumption
+--ENV--
+func=call_user_func
+--FILE--
+<?php
+
+class A {
+    public $cycle;
+    public function __construct() { $this->cycle = $this; }
+}
+class B {
+    public function get() {
+        return new A();
+    }
+}
+class Debug {
+    public function __debugInfo() {
+        gc_collect_cycles();
+        return [];
+    }
+}
+
+$c = new B();
+getenv('func')(fn (...$args) => gc_collect_cycles(), a: $c->get());
+
+?>
+==DONE==
+--EXPECT--
+==DONE==

Zend/zend_gc.c

@@ -2221,7 +2221,7 @@ static void zend_gc_check_root_tmpvars(void) {
 		return;
 	}
 
-	if (ZEND_USER_CODE(ex->func->type)) {
+	if (ex->func && ZEND_USER_CODE(ex->func->type)) {
 		const zend_op *op = ex->opline;
 		if (op->result_type & (IS_VAR | IS_TMP_VAR)) {
 			switch (op->opcode) {
@@ -2249,7 +2249,29 @@ static void zend_gc_check_root_tmpvars(void) {
 
 	for (; ex; ex = ex->prev_execute_data) {
 		zend_function *func = ex->func;
-		if (!func || !ZEND_USER_CODE(func->type)) {
+		if (!func) {
+			continue;
+		}
+
+		if (!ZEND_USER_CODE(func->type)) {
+			/* For internal frames, arguments are not consumed yet, so we must
+			 * root them. */
+			zend_execute_data *caller = ex->prev_execute_data;
+			if (!caller->func || !ZEND_USER_CODE(caller->func->type)) {
+				continue;
+			}
+
+			uint32_t num_args = ZEND_CALL_NUM_ARGS(ex);
+			if (EXPECTED(num_args > 0)) {
+				zval *p = ZEND_CALL_ARG(ex, 1);
+				do {
+					if (Z_COLLECTABLE_P(p)) {
+						gc_check_possible_root(Z_COUNTED_P(p));
+					}
+					p++;
+				} while (--num_args);
+			}
+
 			continue;
 		}
 
@@ -2282,7 +2304,7 @@ static void zend_gc_remove_root_tmpvars(void) {
 		return;
 	}
 
-	if (ZEND_USER_CODE(ex->func->type)) {
+	if (ex->func && ZEND_USER_CODE(ex->func->type)) {
 		const zend_op *op = ex->opline;
 		if (op->result_type & (IS_VAR | IS_TMP_VAR)) {
 			switch (op->opcode) {
@@ -2310,7 +2332,29 @@ static void zend_gc_remove_root_tmpvars(void) {
 
 	for (; ex; ex = ex->prev_execute_data) {
 		zend_function *func = ex->func;
-		if (!func || !ZEND_USER_CODE(func->type)) {
+		if (!func) {
+			continue;
+		}
+
+		if (!ZEND_USER_CODE(func->type)) {
+			/* For internal frames, arguments are live, so we should unroot
+			 * them. */
+			zend_execute_data *caller = ex->prev_execute_data;
+			if (!caller->func || !ZEND_USER_CODE(caller->func->type)) {
+				continue;
+			}
+
+			uint32_t num_args = ZEND_CALL_NUM_ARGS(ex);
+			if (EXPECTED(num_args > 0)) {
+				zval *p = ZEND_CALL_ARG(ex, 1);
+				do {
+					if (Z_COLLECTABLE_P(p)) {
+						GC_REMOVE_FROM_BUFFER(Z_COUNTED_P(p));
+					}
+					p++;
+				} while (--num_args);
+			}
+
 			continue;
 		}