Skip to content

Commit

Permalink
Fixed bug #63132
Browse files Browse the repository at this point in the history
EG(arg_types_stack) is now also backed up when generators are used. This
allows the use of yield in nested method calls.

This commit adds two new functions to the zend_ptr_stack API:

    zend_ptr_stack_push_from_memory
    zend_ptr_stack_pop_into_memory

both taking the following arguments:

    zend_ptr_stack *stack, int count, void **pointers
  • Loading branch information
nikic committed Sep 22, 2012
1 parent 6c135df commit a31fa55
Show file tree
Hide file tree
Showing 5 changed files with 119 additions and 0 deletions.
39 changes: 39 additions & 0 deletions Zend/tests/generators/nested_method_calls.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
--TEST--
Yield can be used in nested method calls
--FILE--
<?php

class A {
function foo() {
echo "Called A::foo\n";
}
}

class B {
function foo() {
echo "Called B::foo\n";
}
}

function gen($obj) {
$obj->foo($obj->foo(yield));
}

$g1 = gen(new A);
$g1->current();

$g2 = gen(new B);
$g2->current();

$g1->next();

$g3 = clone $g2;
unset($g2);
$g3->next();

?>
--EXPECT--
Called A::foo
Called A::foo
Called B::foo
Called B::foo
57 changes: 57 additions & 0 deletions Zend/zend_generators.c
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,21 @@ void zend_generator_close(zend_generator *generator, zend_bool finished_executio
efree(generator->backed_up_stack);
}

if (generator->backed_up_arg_types_stack) {
/* The arg types stack contains three elements per call: fbc, object
* and called_scope. Here we traverse the stack from top to bottom
* and dtor the object. */
int i = generator->backed_up_arg_types_stack_count / 3;
while (i--) {
zval *object = (zval *) generator->backed_up_arg_types_stack[3*i + 1];
if (object) {
zval_ptr_dtor(&object);
}
}

efree(generator->backed_up_arg_types_stack);
}

/* We have added an additional stack frame in prev_execute_data, so we
* have to free it. It also contains the arguments passed to the
* generator (for func_get_args) so those have to be freed too. */
Expand Down Expand Up @@ -288,6 +303,25 @@ static void zend_generator_clone_storage(zend_generator *orig, zend_generator **
}
}

if (orig->backed_up_arg_types_stack) {
size_t stack_size = orig->backed_up_arg_types_stack_count * sizeof(void *);

clone->backed_up_arg_types_stack = emalloc(stack_size);
memcpy(clone->backed_up_arg_types_stack, orig->backed_up_arg_types_stack, stack_size);

/* We have to add refs to the objects in the arg types stack (the
* object is always the second element of a three-pack. */
{
int i, stack_frames = clone->backed_up_arg_types_stack_count / 3;
for (i = 0; i < stack_frames; i++) {
zval *object = (zval *) clone->backed_up_arg_types_stack[3*i + 1];
if (object) {
Z_ADDREF_P(object);
}
}
}
}

/* Update the send_target to use the temporary variable with the same
* offset as the original generator, but in our temporary variable
* memory segment. */
Expand Down Expand Up @@ -449,6 +483,7 @@ void zend_generator_resume(zend_generator *generator TSRMLS_DC) /* {{{ */
zval *original_This = EG(This);
zend_class_entry *original_scope = EG(scope);
zend_class_entry *original_called_scope = EG(called_scope);
int original_arg_types_stack_count = EG(arg_types_stack).top;

/* Remember the current stack position so we can back up pushed args */
generator->original_stack_top = zend_vm_stack_top(TSRMLS_C);
Expand All @@ -461,6 +496,16 @@ void zend_generator_resume(zend_generator *generator TSRMLS_DC) /* {{{ */
generator->backed_up_stack = NULL;
}

if (generator->backed_up_arg_types_stack) {
zend_ptr_stack_push_from_memory(
&EG(arg_types_stack),
generator->backed_up_arg_types_stack_count,
generator->backed_up_arg_types_stack
);
efree(generator->backed_up_arg_types_stack);
generator->backed_up_arg_types_stack = NULL;
}

/* We (mis)use the return_value_ptr_ptr to provide the generator object
* to the executor, so YIELD will be able to set the yielded value */
EG(return_value_ptr_ptr) = (zval **) generator;
Expand Down Expand Up @@ -506,6 +551,18 @@ void zend_generator_resume(zend_generator *generator TSRMLS_DC) /* {{{ */
zend_vm_stack_free(generator->original_stack_top TSRMLS_CC);
}

if (original_arg_types_stack_count != EG(arg_types_stack).top) {
generator->backed_up_arg_types_stack_count =
EG(arg_types_stack).top - original_arg_types_stack_count;

generator->backed_up_arg_types_stack = emalloc(generator->backed_up_arg_types_stack_count * sizeof(void *));
zend_ptr_stack_pop_into_memory(
&EG(arg_types_stack),
generator->backed_up_arg_types_stack_count,
generator->backed_up_arg_types_stack
);
}

/* If an exception was thrown in the generator we have to internally
* rethrow it in the parent scope. */
if (UNEXPECTED(EG(exception) != NULL)) {
Expand Down
5 changes: 5 additions & 0 deletions Zend/zend_generators.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ typedef struct _zend_generator {
void *backed_up_stack;
size_t backed_up_stack_size;

/* For method calls PHP also pushes various type information on a second
* stack, which also needs to be backed up. */
void **backed_up_arg_types_stack;
int backed_up_arg_types_stack_count;

/* The original stack top before resuming the generator. This is required
* for proper cleanup during exception handling. */
void **original_stack_top;
Expand Down
16 changes: 16 additions & 0 deletions Zend/zend_ptr_stack.c
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,22 @@ ZEND_API int zend_ptr_stack_num_elements(zend_ptr_stack *stack)
return stack->top;
}

ZEND_API void zend_ptr_stack_push_from_memory(zend_ptr_stack *stack, int count, void **pointers)
{
ZEND_PTR_STACK_RESIZE_IF_NEEDED(stack, count);

memcpy(stack->top_element, pointers, count * sizeof(void *));
stack->top_element += count;
stack->top += count;
}

ZEND_API void zend_ptr_stack_pop_into_memory(zend_ptr_stack *stack, int count, void **pointers)
{
memcpy(pointers, stack->top_element - count, count * sizeof(void *));
stack->top_element -= count;
stack->top -= count;
}

/*
* Local variables:
* tab-width: 4
Expand Down
2 changes: 2 additions & 0 deletions Zend/zend_ptr_stack.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ ZEND_API void zend_ptr_stack_destroy(zend_ptr_stack *stack);
ZEND_API void zend_ptr_stack_apply(zend_ptr_stack *stack, void (*func)(void *));
ZEND_API void zend_ptr_stack_clean(zend_ptr_stack *stack, void (*func)(void *), zend_bool free_elements);
ZEND_API int zend_ptr_stack_num_elements(zend_ptr_stack *stack);
ZEND_API void zend_ptr_stack_push_from_memory(zend_ptr_stack *stack, int count, void **pointers);
ZEND_API void zend_ptr_stack_pop_into_memory(zend_ptr_stack *stack, int count, void **pointers);

This comment has been minimized.

Copy link
@laruence

laruence Sep 22, 2012

Member

Why a new function? Cannot use the n_push APIs?

This comment has been minimized.

Copy link
@nikic

nikic Sep 22, 2012

Author Member

The n_push APIs are varargs functions, they can't be used to push a variable number of pointers programatically (or can they?)

END_EXTERN_C()

#define ZEND_PTR_STACK_RESIZE_IF_NEEDED(stack, count) \
Expand Down

0 comments on commit a31fa55

Please sign in to comment.