Use main VM stack for generators. Only single call frame for generator itself is allocated on heap, call frames for nested functions are allocated on main VM stack. In case "yield" used in context of another function call, call stack has to be frozen and then restored.

4 files changed, 95 insertions, 58 deletions

diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c
index bd5ccf6c37..bb27463c6c 100644
--- a/Zend/zend_execute.c
+++ b/Zend/zend_execute.c
@@ -2343,31 +2343,18 @@ ZEND_API zend_execute_data *zend_create_generator_execute_data(zend_execute_data
 	 * though this behavior would be suboptimal, because the (rather large)
 	 * structure would have to be copied back and forth every time execution is
 	 * suspended or resumed. That's why for generators the execution context
-	 * is allocated using a separate VM stack, thus allowing to save and
-	 * restore it simply by replacing a pointer.
+	 * is allocated using a separate VM stack frame.
 	 */
 	zend_execute_data *execute_data;
 	uint32_t num_args = ZEND_CALL_NUM_ARGS(call);
-	size_t stack_size = (ZEND_CALL_FRAME_SLOT + MAX(op_array->last_var + op_array->T, num_args)) * sizeof(zval);
-	uint32_t call_info;
-
-	EG(vm_stack) = zend_vm_stack_new_page(
-		EXPECTED(stack_size < ZEND_VM_STACK_FREE_PAGE_SIZE(1)) ?
-			ZEND_VM_STACK_PAGE_SIZE(1) :
-			ZEND_VM_STACK_PAGE_ALIGNED_SIZE(1, stack_size),
-		NULL);
-	EG(vm_stack_top) = EG(vm_stack)->top;
-	EG(vm_stack_end) = EG(vm_stack)->end;
+	uint32_t used_stack = (ZEND_CALL_FRAME_SLOT + num_args + op_array->last_var + op_array->T - MIN(op_array->num_args, num_args)) * sizeof(zval);
 
-	call_info = ZEND_CALL_TOP_FUNCTION | ZEND_CALL_ALLOCATED | (ZEND_CALL_INFO(call) & (ZEND_CALL_CLOSURE|ZEND_CALL_RELEASE_THIS));
-	execute_data = zend_vm_stack_push_call_frame(
-		call_info,
-		(zend_function*)op_array,
-		num_args,
-		Z_TYPE(call->This) != IS_OBJECT ? Z_CE(call->This) : NULL,
-		Z_TYPE(call->This) == IS_OBJECT ? Z_OBJ(call->This) : NULL);
+	execute_data = (zend_execute_data*)emalloc(used_stack);
+	ZEND_SET_CALL_INFO(execute_data, Z_TYPE(call->This) == IS_OBJECT, ZEND_CALL_TOP_FUNCTION | ZEND_CALL_ALLOCATED | (ZEND_CALL_INFO(call) & (ZEND_CALL_CLOSURE|ZEND_CALL_RELEASE_THIS)));
+	EX(func) = (zend_function*)op_array;
+	Z_OBJ(EX(This)) = Z_OBJ(call->This);
+	ZEND_CALL_NUM_ARGS(execute_data) = num_args;
 	EX(prev_execute_data) = NULL;
-	EX_NUM_ARGS() = num_args;
 
 	/* copy arguments */
 	if (num_args > 0) {
diff --git a/Zend/zend_execute.h b/Zend/zend_execute.h
index 24071719a2..9cca85c4cf 100644
--- a/Zend/zend_execute.h
+++ b/Zend/zend_execute.h
@@ -260,6 +260,11 @@ static zend_always_inline void zend_vm_stack_free_call_frame_ex(uint32_t call_in
 	ZEND_ASSERT_VM_STACK_GLOBAL;
 
 	if (UNEXPECTED(call_info & ZEND_CALL_ALLOCATED)) {
+		if (UNEXPECTED(call != (zend_execute_data*)ZEND_VM_STACK_ELEMENTS(EG(vm_stack)))) {
+			/* This is a generator's stack frame */
+			efree(call);
+			return;
+		}
 		zend_vm_stack p = EG(vm_stack);
 
 		zend_vm_stack prev = p->prev;
diff --git a/Zend/zend_generators.c b/Zend/zend_generators.c
index b4dfeba1da..5f8a6b2ce4 100644
--- a/Zend/zend_generators.c
+++ b/Zend/zend_generators.c
@@ -31,6 +31,71 @@ static zend_object_handlers zend_generator_handlers;
 
 static zend_object *zend_generator_create(zend_class_entry *class_type);
 
+static void zend_restore_call_stack(zend_generator *generator) /* {{{ */
+{
+	zend_execute_data *call, *new_call, *prev_call = NULL;
+
+	call = generator->frozen_call_stack;
+	do {
+		new_call = zend_vm_stack_push_call_frame(
+			(ZEND_CALL_INFO(call) & ~ZEND_CALL_ALLOCATED),
+			call->func,
+			ZEND_CALL_NUM_ARGS(call),
+			(Z_TYPE(call->This) == IS_UNDEF) ?
+				(zend_class_entry*)Z_OBJ(call->This) : NULL,
+			(Z_TYPE(call->This) != IS_UNDEF) ?
+				Z_OBJ(call->This) : NULL);
+		memcpy(((zval*)new_call) + ZEND_CALL_FRAME_SLOT, ((zval*)call) + ZEND_CALL_FRAME_SLOT, ZEND_CALL_NUM_ARGS(call) * sizeof(zval));
+		new_call->prev_execute_data = prev_call;
+		prev_call = new_call;
+
+		call = call->prev_execute_data;
+	} while (call);
+	generator->execute_data->call = prev_call;
+	efree(generator->frozen_call_stack);
+	generator->frozen_call_stack = NULL;
+}
+/* }}} */
+
+static zend_execute_data* zend_freeze_call_stack(zend_execute_data *execute_data) /* {{{ */
+{
+	size_t used_stack;
+	zend_execute_data *call, *new_call, *prev_call = NULL;
+	zval *stack;
+
+	/* calculate required stack size */
+	used_stack = 0;
+	call = EX(call);
+	do {
+		used_stack += ZEND_CALL_FRAME_SLOT + ZEND_CALL_NUM_ARGS(call);
+		call = call->prev_execute_data;
+	} while (call);
+
+	stack = emalloc(used_stack * sizeof(zval));
+
+	/* save stack, linking frames in reverse order */
+	call = EX(call);
+	do {
+		size_t frame_size = ZEND_CALL_FRAME_SLOT + ZEND_CALL_NUM_ARGS(call);
+
+		new_call = (zend_execute_data*)(stack + used_stack - frame_size);
+		memcpy(new_call, call, frame_size * sizeof(zval));
+		used_stack -= frame_size;
+		new_call->prev_execute_data = prev_call;
+		prev_call = new_call;
+
+		new_call = call->prev_execute_data;
+		zend_vm_stack_free_call_frame(call);
+		call = new_call;
+	} while (call);
+
+	execute_data->call = NULL;
+	ZEND_ASSERT(prev_call == (zend_execute_data*)stack);
+
+	return prev_call;
+}
+/* }}} */
+
 static void zend_generator_cleanup_unfinished_execution(
 		zend_generator *generator, uint32_t catch_op_num) /* {{{ */
 {
@@ -40,21 +105,10 @@ static void zend_generator_cleanup_unfinished_execution(
 		/* -1 required because we want the last run opcode, not the next to-be-run one. */
 		uint32_t op_num = execute_data->opline - execute_data->func->op_array.opcodes - 1;
 
-		/* There may be calls to zend_vm_stack_free_call_frame(), which modifies the VM stack
-		 * globals, so need to load/restore those. */
-		zend_vm_stack original_stack = EG(vm_stack);
-		original_stack->top = EG(vm_stack_top);
-		EG(vm_stack_top) = generator->stack->top;
-		EG(vm_stack_end) = generator->stack->end;
-		EG(vm_stack) = generator->stack;
-
-		zend_cleanup_unfinished_execution(execute_data, op_num, catch_op_num);
-
-		generator->stack = EG(vm_stack);
-		generator->stack->top = EG(vm_stack_top);
-		EG(vm_stack_top) = original_stack->top;
-		EG(vm_stack_end) = original_stack->end;
-		EG(vm_stack) = original_stack;
+		if (UNEXPECTED(generator->frozen_call_stack)) {
+			zend_restore_call_stack(generator);
+		}
+		zend_cleanup_unfinished_execution(execute_data, op_num, 0);
 	}
 }
 /* }}} */
@@ -100,7 +154,7 @@ ZEND_API void zend_generator_close(zend_generator *generator, zend_bool finished
 			generator->gc_buffer = NULL;
 		}
 
-		efree(generator->stack);
+		efree(generator->execute_data);
 		generator->execute_data = NULL;
 	}
 }
@@ -331,9 +385,6 @@ ZEND_API void zend_generator_create_zval(zend_execute_data *call, zend_op_array
 	zend_generator *generator;
 	zend_execute_data *current_execute_data;
 	zend_execute_data *execute_data;
-	zend_vm_stack current_stack = EG(vm_stack);
-
-	current_stack->top = EG(vm_stack_top);
 
 	/* Create new execution context. We have to back up and restore  EG(current_execute_data) here. */
 	current_execute_data = EG(current_execute_data);
@@ -350,11 +401,7 @@ ZEND_API void zend_generator_create_zval(zend_execute_data *call, zend_op_array
 	/* Save execution context in generator object. */
 	generator = (zend_generator *) Z_OBJ_P(return_value);
 	generator->execute_data = execute_data;
-	generator->stack = EG(vm_stack);
-	generator->stack->top = EG(vm_stack_top);
-	EG(vm_stack_top) = current_stack->top;
-	EG(vm_stack_end) = current_stack->end;
-	EG(vm_stack) = current_stack;
+	generator->frozen_call_stack = NULL;
 
 	/* EX(return_value) keeps pointer to zend_object (not a real zval) */
 	execute_data->return_value = (zval*)generator;
@@ -765,14 +812,9 @@ try_again:
 	{
 		/* Backup executor globals */
 		zend_execute_data *original_execute_data = EG(current_execute_data);
-		zend_vm_stack original_stack = EG(vm_stack);
-		original_stack->top = EG(vm_stack_top);
 
 		/* Set executor globals */
 		EG(current_execute_data) = generator->execute_data;
-		EG(vm_stack_top) = generator->stack->top;
-		EG(vm_stack_end) = generator->stack->end;
-		EG(vm_stack) = generator->stack;
 
 		/* We want the backtrace to look as if the generator function was
 		 * called from whatever method we are current running (e.g. next()).
@@ -786,22 +828,25 @@ try_again:
 			orig_generator->execute_fake.prev_execute_data = original_execute_data;
 		}
 
+		if (UNEXPECTED(generator->frozen_call_stack)) {
+			/* Restore frozen call-stack */
+			zend_restore_call_stack(generator);
+		}
+
 		/* Resume execution */
 		generator->flags |= ZEND_GENERATOR_CURRENTLY_RUNNING;
 		zend_execute_ex(generator->execute_data);
 		generator->flags &= ~ZEND_GENERATOR_CURRENTLY_RUNNING;
 
-		/* Unlink generator call_frame from the caller and backup vm_stack_top */
-		if (EXPECTED(generator->execute_data)) {
-			generator->stack = EG(vm_stack);
-			generator->stack->top = EG(vm_stack_top);
+		generator->frozen_call_stack = NULL;
+		if (EXPECTED(generator->execute_data) &&
+		    UNEXPECTED(generator->execute_data->call)) {
+			/* Frize call-stack */
+			generator->frozen_call_stack = zend_freeze_call_stack(generator->execute_data);
 		}
 
 		/* Restore executor globals */
 		EG(current_execute_data) = original_execute_data;
-		EG(vm_stack_top) = original_stack->top;
-		EG(vm_stack_end) = original_stack->end;
-		EG(vm_stack) = original_stack;
 
 		/* If an exception was thrown in the generator we have to internally
 		 * rethrow it in the parent scope.
diff --git a/Zend/zend_generators.h b/Zend/zend_generators.h
index 95c5147a93..4e6241fc02 100644
--- a/Zend/zend_generators.h
+++ b/Zend/zend_generators.h
@@ -62,8 +62,8 @@ struct _zend_generator {
 	/* The suspended execution context. */
 	zend_execute_data *execute_data;
 
-	/* The separate stack used by generator */
-	zend_vm_stack stack;
+	/* Frozen call stack for "yield" used in context of other calls */
+	zend_execute_data *frozen_call_stack;
 
 	/* Current value */
 	zval value;