Root current opline result after GC

Author: arnaud-lb

Zend/tests/gc/gh13687-001.phpt

@@ -0,0 +1,31 @@
+--TEST--
+GH-13687 001: 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();
+    }
+}
+
+$c = new B();
+$objs = [];
+
+while (gc_status()['roots']+2 < gc_status()['threshold']) {
+    $obj = new stdClass;
+    $objs[] = $obj;
+}
+
+var_dump($c->get());
+
+?>
+--EXPECTF--
+object(A)#%d (1) {
+  ["cycle"]=>
+  *RECURSION*
+}

Zend/tests/gc/gh13687-002.phpt

@@ -0,0 +1,31 @@
+--TEST--
+GH-13687 002: Result operand may leak if GC is triggered before consumption
+--FILE--
+<?php
+
+class A {
+    public $cycle;
+    public function __construct() { $this->cycle = $this; }
+    public function __toString() { return __CLASS__; }
+}
+class B {
+    public function get() {
+        return new A();
+    }
+}
+
+$root = new stdClass;
+gc_collect_cycles();
+
+$objs = [];
+while (gc_status()['roots']+2 < gc_status()['threshold']) {
+    $obj = new stdClass;
+    $objs[] = $obj;
+}
+
+$a = [new A, $root][0]::class;
+
+?>
+==DONE==
+--EXPECT--
+==DONE==

Zend/zend_gc.c

@@ -2045,14 +2045,45 @@ static void zend_get_gc_buffer_release(void) {
  * of the value, we would end up leaking it. To avoid this, root all live TMPVAR values here. */
 static void zend_gc_check_root_tmpvars(void) {
 	zend_execute_data *ex = EG(current_execute_data);
+
+	if (!ex) {
+		return;
+	}
+
+	if (ZEND_USER_CODE(ex->func->type)) {
+		const zend_op *op = ex->opline;
+		if (op->result_type & (IS_VAR | IS_TMP_VAR)) {
+			switch (op->opcode) {
+				case ZEND_ADD_ARRAY_ELEMENT:
+				case ZEND_ADD_ARRAY_UNPACK:
+				case ZEND_ROPE_INIT:
+				case ZEND_ROPE_ADD:
+					break; /* live range handling will check those */
+
+				case ZEND_FETCH_CLASS:
+				case ZEND_DECLARE_ANON_CLASS:
+					break; /* return value is zend_class_entry pointer */
+
+				default:
+					/* smart branch opcodes may not initialize result */
+					if (!zend_is_smart_branch(op)) {
+						zval *var = ZEND_CALL_VAR(ex, op->result.var);
+						if (Z_COLLECTABLE_P(var)) {
+							gc_check_possible_root(Z_COUNTED_P(var));
+						}
+					}
+			}
+		}
+	}
+
 	for (; ex; ex = ex->prev_execute_data) {
 		zend_function *func = ex->func;
 		if (!func || !ZEND_USER_CODE(func->type)) {
 			continue;
 		}
 
 		uint32_t op_num = ex->opline - ex->func->op_array.opcodes;
-		for (uint32_t i = 0; i < func->op_array.last_live_range; i++) {
+		for (int i = 0; i < func->op_array.last_live_range; i++) {
 			const zend_live_range *range = &func->op_array.live_range[i];
 			if (range->start > op_num) {
 				break;
@@ -2075,14 +2106,45 @@ static void zend_gc_check_root_tmpvars(void) {
 
 static void zend_gc_remove_root_tmpvars(void) {
 	zend_execute_data *ex = EG(current_execute_data);
+
+	if (!ex) {
+		return;
+	}
+
+	if (ZEND_USER_CODE(ex->func->type)) {
+		const zend_op *op = ex->opline;
+		if (op->result_type & (IS_VAR | IS_TMP_VAR)) {
+			switch (op->opcode) {
+				case ZEND_ADD_ARRAY_ELEMENT:
+				case ZEND_ADD_ARRAY_UNPACK:
+				case ZEND_ROPE_INIT:
+				case ZEND_ROPE_ADD:
+					break; /* live range handling will remove those */
+
+				case ZEND_FETCH_CLASS:
+				case ZEND_DECLARE_ANON_CLASS:
+					break; /* return value is zend_class_entry pointer */
+
+				default:
+					/* smart branch opcodes may not initialize result */
+					if (!zend_is_smart_branch(op)) {
+						zval *var = ZEND_CALL_VAR(ex, op->result.var);
+						if (Z_COLLECTABLE_P(var)) {
+							GC_REMOVE_FROM_BUFFER(Z_COUNTED_P(var));
+						}
+					}
+			}
+		}
+	}
+
 	for (; ex; ex = ex->prev_execute_data) {
 		zend_function *func = ex->func;
 		if (!func || !ZEND_USER_CODE(func->type)) {
 			continue;
 		}
 
 		uint32_t op_num = ex->opline - ex->func->op_array.opcodes;
-		for (uint32_t i = 0; i < func->op_array.last_live_range; i++) {
+		for (int i = 0; i < func->op_array.last_live_range; i++) {
 			const zend_live_range *range = &func->op_array.live_range[i];
 			if (range->start > op_num) {
 				break;