Skip to content

Fix crash during GC of suspended generator delegate #15275

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions Zend/tests/gh15275-001.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
--TEST--
GH-15275 001: Crash during GC of suspended generator delegate
--FILE--
<?php

class It implements \IteratorAggregate
{
public function getIterator(): \Generator
{
yield 'foo';
Fiber::suspend();
var_dump("not executed");
}
}

function f() {
var_dump(yield from new It());
}

$iterable = f();

$fiber = new Fiber(function () use ($iterable) {
var_dump($iterable->current());
$iterable->next();
var_dump("not executed");
});

$ref = $fiber;

$fiber->start();

gc_collect_cycles();

?>
==DONE==
--EXPECT--
string(3) "foo"
==DONE==
57 changes: 57 additions & 0 deletions Zend/tests/gh15275-002.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
--TEST--
GH-15275 002: Crash during GC of suspended generator delegate
--FILE--
<?php

class It implements \IteratorAggregate
{
public function getIterator(): \Generator
{
yield 'foo';
try {
Fiber::suspend();
} finally {
var_dump(__METHOD__);
}
var_dump("not executed");
}
}

function f() {
try {
var_dump(new stdClass, yield from new It());
} finally {
var_dump(__FUNCTION__);
}
}

function g() {
try {
var_dump(new stdClass, yield from f());
} finally {
var_dump(__FUNCTION__);
}
}

$gen = g();

$fiber = new Fiber(function () use ($gen) {
var_dump($gen->current());
$gen->next();
var_dump("not executed");
});

$ref = $fiber;

$fiber->start();

gc_collect_cycles();

?>
==DONE==
--EXPECT--
string(3) "foo"
==DONE==
string(15) "It::getIterator"
string(1) "f"
string(1) "g"
51 changes: 51 additions & 0 deletions Zend/tests/gh15275-003.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
--TEST--
GH-15275 003: Crash during GC of suspended generator delegate
--FILE--
<?php

class It implements \IteratorAggregate
{
public function getIterator(): \Generator
{
yield 'foo';
throw new \Exception();
var_dump("not executed");
}
}

function f() {
try {
var_dump(new stdClass, yield from new It());
} finally {
var_dump(__FUNCTION__);
}
}

function g() {
try {
var_dump(new stdClass, yield from f());
} finally {
var_dump(__FUNCTION__);
}
}

$gen = g();

var_dump($gen->current());
$gen->next();

?>
==DONE==
--EXPECTF--
string(3) "foo"
string(1) "f"
string(1) "g"

Fatal error: Uncaught Exception in %s:8
Stack trace:
#0 %s(15): It->getIterator()
#1 %s(23): f()
#2 [internal function]: g()
#3 %s(32): Generator->next()
#4 {main}
thrown in %s on line 8
44 changes: 44 additions & 0 deletions Zend/tests/gh15275-004.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
--TEST--
GH-15275 004: Crash during GC of suspended generator delegate
--FILE--
<?php

class It implements \IteratorAggregate
{
public function getIterator(): \Generator
{
yield 'foo';
echo "baz\n";
throw new \Exception();
}

public function __destruct()
{
gc_collect_cycles();
}
}

function f() {
var_dump(new stdClass, yield from new It());
}

$gen = f();

var_dump($gen->current());
$gen->next();

gc_collect_cycles();

?>
==DONE==
--EXPECTF--
string(3) "foo"
baz

Fatal error: Uncaught Exception in %s:9
Stack trace:
#0 %s(19): It->getIterator()
#1 [internal function]: f()
#2 %s(25): Generator->next()
#3 {main}
thrown in %s on line 9
51 changes: 51 additions & 0 deletions Zend/tests/gh15275-005.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
--TEST--
GH-15275 005: Crash during GC of suspended generator delegate
--FILE--
<?php

class It implements \IteratorAggregate
{
public function getIterator(): \Generator
{
yield 'foo';
throw new \Exception();
var_dump("not executed");
}
}

function f() {
try {
var_dump(new stdClass, yield from new It());
} finally {
var_dump(__FUNCTION__);
}
}

function g() {
try {
var_dump(new stdClass, yield from f());
} finally {
var_dump(__FUNCTION__);
}
}

$gen = g();

var_dump($gen->current());
$gen->next();

?>
==DONE==
--EXPECTF--
string(3) "foo"
string(1) "f"
string(1) "g"

Fatal error: Uncaught Exception in %s:8
Stack trace:
#0 %s(15): It->getIterator()
#1 %s(23): f()
#2 [internal function]: g()
#3 %s(32): Generator->next()
#4 {main}
thrown in %s on line 8
51 changes: 51 additions & 0 deletions Zend/tests/gh15275-006.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
--TEST--
GH-15275 006: Crash during GC of suspended generator delegate
--FILE--
<?php

class It implements \IteratorAggregate
{
public function getIterator(): \Generator
{
yield 'foo';
echo "baz\n";
throw new \Exception();
}

public function __destruct()
{
throw new \Exception();
}
}

function f() {
var_dump(new stdClass, yield from new It());
}

$gen = f();

var_dump($gen->current());
$gen->next();

gc_collect_cycles();

?>
==DONE==
--EXPECTF--
string(3) "foo"
baz

Fatal error: Uncaught Exception in %s:9
Stack trace:
#0 %s(19): It->getIterator()
#1 [internal function]: f()
#2 %s(25): Generator->next()
#3 {main}

Next Exception in %s:14
Stack trace:
#0 %s(19): It->__destruct()
#1 [internal function]: f()
#2 %s(25): Generator->next()
#3 {main}
thrown in %s on line 14
8 changes: 7 additions & 1 deletion Zend/zend_execute.c
Original file line number Diff line number Diff line change
Expand Up @@ -4471,7 +4471,13 @@ ZEND_API HashTable *zend_unfinished_execution_gc_ex(zend_execute_data *execute_d
}

if (call) {
uint32_t op_num = execute_data->opline - op_array->opcodes;
uint32_t op_num;
if (UNEXPECTED(execute_data->opline->opcode == ZEND_HANDLE_EXCEPTION)) {
op_num = EG(opline_before_exception) - op_array->opcodes;
} else {
op_num = execute_data->opline - op_array->opcodes;
}
ZEND_ASSERT(op_num < op_array->last);
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
Expand Down
19 changes: 14 additions & 5 deletions Zend/zend_generators.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "zend_closures.h"
#include "zend_generators_arginfo.h"
#include "zend_observer.h"
#include "zend_vm_opcodes.h"

ZEND_API zend_class_entry *zend_ce_generator;
ZEND_API zend_class_entry *zend_ce_ClosedGeneratorException;
Expand Down Expand Up @@ -512,6 +513,8 @@ static void zend_generator_throw_exception(zend_generator *generator, zval *exce
* to pretend the exception happened during the YIELD opcode. */
EG(current_execute_data) = generator->execute_data;
generator->execute_data->opline--;
ZEND_ASSERT(generator->execute_data->opline->opcode == ZEND_YIELD
|| generator->execute_data->opline->opcode == ZEND_YIELD_FROM);
generator->execute_data->prev_execute_data = original_execute_data;

if (exception) {
Expand All @@ -520,13 +523,14 @@ static void zend_generator_throw_exception(zend_generator *generator, zval *exce
zend_rethrow_exception(EG(current_execute_data));
}

generator->execute_data->opline++;

/* if we don't stop an array/iterator yield from, the exception will only reach the generator after the values were all iterated over */
if (UNEXPECTED(Z_TYPE(generator->values) != IS_UNDEF)) {
zval_ptr_dtor(&generator->values);
ZVAL_UNDEF(&generator->values);
}

generator->execute_data->opline++;
EG(current_execute_data) = original_execute_data;
}

Expand Down Expand Up @@ -656,8 +660,6 @@ ZEND_API zend_generator *zend_generator_update_current(zend_generator *generator

static zend_result zend_generator_get_next_delegated_value(zend_generator *generator) /* {{{ */
{
--generator->execute_data->opline;

zval *value;
if (Z_TYPE(generator->values) == IS_ARRAY) {
HashTable *ht = Z_ARR(generator->values);
Expand Down Expand Up @@ -739,14 +741,12 @@ static zend_result zend_generator_get_next_delegated_value(zend_generator *gener
}
}

++generator->execute_data->opline;
return SUCCESS;

failure:
zval_ptr_dtor(&generator->values);
ZVAL_UNDEF(&generator->values);

++generator->execute_data->opline;
return FAILURE;
}
/* }}} */
Expand Down Expand Up @@ -811,6 +811,15 @@ ZEND_API void zend_generator_resume(zend_generator *orig_generator) /* {{{ */
generator->flags &= ~ZEND_GENERATOR_IN_FIBER;
return;
}
if (UNEXPECTED(EG(exception))) {
/* Decrementing opline_before_exception to pretend the exception
* happened during the YIELD_FROM opcode. */
if (generator->execute_data) {
ZEND_ASSERT(generator->execute_data->opline == EG(exception_op));
ZEND_ASSERT((EG(opline_before_exception)-1)->opcode == ZEND_YIELD_FROM);
EG(opline_before_exception)--;
}
}
/* If there are no more delegated values, resume the generator
* after the "yield from" expression. */
}
Expand Down
Loading