Fix colletion of unfinished function call in fibers

Fixes GH-10496.

Co-authored-by: Bob Weinand <[email protected]>

Author: arnaud-lb

NEWS

@@ -24,6 +24,8 @@ PHP                                                                        NEWS
 
 - Fiber:
   . Fixed assembly on alpine x86. (nielsdos)
+  . Fixed bug GH-10496 (segfault when garbage collector is invoked inside of
+    fiber). (Bob, Arnaud)
 
 - FPM:
   . Fixed bug GH-10315 (FPM unknown child alert not valid). (Jakub Zelenka)

Zend/tests/fibers/gh10496.phpt

@@ -0,0 +1,33 @@
+--TEST--
+Bug GH-10496 (Segfault when garbage collector is invoked inside of fiber)
+--FILE--
+<?php
+
+function x(&$ref) {
+	$ref = new class() {
+		function __destruct() {
+			print "Dtor x()\n";
+		}
+	};
+}
+function suspend($x) {
+	Fiber::suspend();
+}
+$f = new Fiber(function() use (&$f) {
+	try {
+		x($var);
+		\ord(suspend(1));
+	} finally {
+		print "Cleaned\n";
+	}
+});
+$f->start();
+unset($f);
+gc_collect_cycles();
+print "Collected\n";
+
+?>
+--EXPECT--
+Cleaned
+Dtor x()
+Collected

Zend/zend_execute.c

@@ -4459,7 +4459,24 @@ ZEND_API void zend_cleanup_unfinished_execution(zend_execute_data *execute_data,
 	cleanup_live_vars(execute_data, op_num, catch_op_num);
 }
 
-ZEND_API HashTable *zend_unfinished_execution_gc(zend_execute_data *execute_data, zend_execute_data *call, zend_get_gc_buffer *gc_buffer)
+ZEND_API ZEND_ATTRIBUTE_DEPRECATED HashTable *zend_unfinished_execution_gc(zend_execute_data *execute_data, zend_execute_data *call, zend_get_gc_buffer *gc_buffer)
+{
+	bool suspended_by_yield = false;
+
+	if (Z_TYPE_INFO(EX(This)) & ZEND_CALL_GENERATOR) {
+		ZEND_ASSERT(EX(return_value));
+
+		/* The generator object is stored in EX(return_value) */
+		zend_generator *generator = (zend_generator*) EX(return_value);
+		ZEND_ASSERT(execute_data == generator->execute_data);
+
+		suspended_by_yield = !(generator->flags & ZEND_GENERATOR_CURRENTLY_RUNNING);
+	}
+
+	return zend_unfinished_execution_gc_ex(execute_data, call, gc_buffer, suspended_by_yield);
+}
+
+ZEND_API HashTable *zend_unfinished_execution_gc_ex(zend_execute_data *execute_data, zend_execute_data *call, zend_get_gc_buffer *gc_buffer, bool suspended_by_yield)
 {
 	if (!EX(func) || !ZEND_USER_CODE(EX(func)->common.type)) {
 		return NULL;
@@ -4495,8 +4512,15 @@ ZEND_API HashTable *zend_unfinished_execution_gc(zend_execute_data *execute_data
 	}
 
 	if (call) {
-		/* -1 required because we want the last run opcode, not the next to-be-run one. */
-		uint32_t op_num = execute_data->opline - op_array->opcodes - 1;
+		uint32_t op_num = execute_data->opline - op_array->opcodes;
+		if (suspended_by_yield) {
+			/* When the execution was suspended by yield, EX(opline) points to
+			 * next opline to execute. Otherwise, it points to the opline that
+			 * suspended execution. */
+			op_num--;
+			ZEND_ASSERT(EX(func)->op_array.opcodes[op_num].opcode == ZEND_YIELD
+				|| EX(func)->op_array.opcodes[op_num].opcode == ZEND_YIELD_FROM);
+		}
 		zend_unfinished_calls_gc(execute_data, call, op_num, gc_buffer);
 	}
 

Zend/zend_execute.h

@@ -410,7 +410,8 @@ ZEND_API void zend_clean_and_cache_symbol_table(zend_array *symbol_table);
 ZEND_API void ZEND_FASTCALL zend_free_compiled_variables(zend_execute_data *execute_data);
 ZEND_API void zend_unfinished_calls_gc(zend_execute_data *execute_data, zend_execute_data *call, uint32_t op_num, zend_get_gc_buffer *buf);
 ZEND_API void zend_cleanup_unfinished_execution(zend_execute_data *execute_data, uint32_t op_num, uint32_t catch_op_num);
-ZEND_API HashTable *zend_unfinished_execution_gc(zend_execute_data *execute_data, zend_execute_data *call, zend_get_gc_buffer *gc_buffer);
+ZEND_API ZEND_ATTRIBUTE_DEPRECATED HashTable *zend_unfinished_execution_gc(zend_execute_data *execute_data, zend_execute_data *call, zend_get_gc_buffer *gc_buffer);
+ZEND_API HashTable *zend_unfinished_execution_gc_ex(zend_execute_data *execute_data, zend_execute_data *call, zend_get_gc_buffer *gc_buffer, bool suspended_by_yield);
 
 zval * ZEND_FASTCALL zend_handle_named_arg(
 		zend_execute_data **call_ptr, zend_string *arg_name,

Zend/zend_fibers.c

@@ -652,7 +652,7 @@ static HashTable *zend_fiber_object_gc(zend_object *object, zval **table, int *n
 	HashTable *lastSymTable = NULL;
 	zend_execute_data *ex = fiber->execute_data;
 	for (; ex; ex = ex->prev_execute_data) {
-		HashTable *symTable = zend_unfinished_execution_gc(ex, ex->call, buf);
+		HashTable *symTable = zend_unfinished_execution_gc_ex(ex, ex->call, buf, false);
 		if (symTable) {
 			if (lastSymTable) {
 				zval *val;

Zend/zend_generators.c

@@ -372,7 +372,7 @@ static HashTable *zend_generator_get_gc(zend_object *object, zval **table, int *
 		call = zend_generator_revert_call_stack(generator->frozen_call_stack);
 	}
 
-	zend_unfinished_execution_gc(execute_data, call, gc_buffer);
+	zend_unfinished_execution_gc_ex(execute_data, call, gc_buffer, true);
 
 	if (UNEXPECTED(generator->frozen_call_stack)) {
 		zend_generator_revert_call_stack(call);