Skip to content

Commit c767fec

Browse files
arnaud-lbarnaud-lb
authored
Fix crash during GC of suspended generator delegate (php#15275)
1 parent 4d71580 commit c767fec

File tree

8 files changed

+313
-6
lines changed

8 files changed

+313
-6
lines changed

‎Zend/tests/gh15275-001.phpt‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
--TEST--
2+
GH-15275 001: Crash during GC of suspended generator delegate
3+
--FILE--
4+
<?php
5+
6+
class It implements \IteratorAggregate
7+
{
8+
public function getIterator(): \Generator
9+
{
10+
yield 'foo';
11+
Fiber::suspend();
12+
var_dump("not executed");
13+
}
14+
}
15+
16+
function f() {
17+
var_dump(yield from new It());
18+
}
19+
20+
$iterable = f();
21+
22+
$fiber = new Fiber(function () use ($iterable) {
23+
var_dump($iterable->current());
24+
$iterable->next();
25+
var_dump("not executed");
26+
});
27+
28+
$ref = $fiber;
29+
30+
$fiber->start();
31+
32+
gc_collect_cycles();
33+
34+
?>
35+
==DONE==
36+
--EXPECT--
37+
string(3) "foo"
38+
==DONE==

‎Zend/tests/gh15275-002.phpt‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
--TEST--
2+
GH-15275 002: Crash during GC of suspended generator delegate
3+
--FILE--
4+
<?php
5+
6+
class It implements \IteratorAggregate
7+
{
8+
public function getIterator(): \Generator
9+
{
10+
yield 'foo';
11+
try {
12+
Fiber::suspend();
13+
} finally {
14+
var_dump(__METHOD__);
15+
}
16+
var_dump("not executed");
17+
}
18+
}
19+
20+
function f() {
21+
try {
22+
var_dump(new stdClass, yield from new It());
23+
} finally {
24+
var_dump(__FUNCTION__);
25+
}
26+
}
27+
28+
function g() {
29+
try {
30+
var_dump(new stdClass, yield from f());
31+
} finally {
32+
var_dump(__FUNCTION__);
33+
}
34+
}
35+
36+
$gen = g();
37+
38+
$fiber = new Fiber(function () use ($gen) {
39+
var_dump($gen->current());
40+
$gen->next();
41+
var_dump("not executed");
42+
});
43+
44+
$ref = $fiber;
45+
46+
$fiber->start();
47+
48+
gc_collect_cycles();
49+
50+
?>
51+
==DONE==
52+
--EXPECT--
53+
string(3) "foo"
54+
==DONE==
55+
string(15) "It::getIterator"
56+
string(1) "f"
57+
string(1) "g"

‎Zend/tests/gh15275-003.phpt‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
--TEST--
2+
GH-15275 003: Crash during GC of suspended generator delegate
3+
--FILE--
4+
<?php
5+
6+
class It implements \IteratorAggregate
7+
{
8+
public function getIterator(): \Generator
9+
{
10+
yield 'foo';
11+
throw new \Exception();
12+
var_dump("not executed");
13+
}
14+
}
15+
16+
function f() {
17+
try {
18+
var_dump(new stdClass, yield from new It());
19+
} finally {
20+
var_dump(__FUNCTION__);
21+
}
22+
}
23+
24+
function g() {
25+
try {
26+
var_dump(new stdClass, yield from f());
27+
} finally {
28+
var_dump(__FUNCTION__);
29+
}
30+
}
31+
32+
$gen = g();
33+
34+
var_dump($gen->current());
35+
$gen->next();
36+
37+
?>
38+
==DONE==
39+
--EXPECTF--
40+
string(3) "foo"
41+
string(1) "f"
42+
string(1) "g"
43+
44+
Fatal error: Uncaught Exception in %s:8
45+
Stack trace:
46+
#0 %s(15): It->getIterator()
47+
#1 %s(23): f()
48+
#2 [internal function]: g()
49+
#3 %s(32): Generator->next()
50+
#4 {main}
51+
thrown in %s on line 8

‎Zend/tests/gh15275-004.phpt‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
--TEST--
2+
GH-15275 004: Crash during GC of suspended generator delegate
3+
--FILE--
4+
<?php
5+
6+
class It implements \IteratorAggregate
7+
{
8+
public function getIterator(): \Generator
9+
{
10+
yield 'foo';
11+
echo "baz\n";
12+
throw new \Exception();
13+
}
14+
15+
public function __destruct()
16+
{
17+
gc_collect_cycles();
18+
}
19+
}
20+
21+
function f() {
22+
var_dump(new stdClass, yield from new It());
23+
}
24+
25+
$gen = f();
26+
27+
var_dump($gen->current());
28+
$gen->next();
29+
30+
gc_collect_cycles();
31+
32+
?>
33+
==DONE==
34+
--EXPECTF--
35+
string(3) "foo"
36+
baz
37+
38+
Fatal error: Uncaught Exception in %s:9
39+
Stack trace:
40+
#0 %s(19): It->getIterator()
41+
#1 [internal function]: f()
42+
#2 %s(25): Generator->next()
43+
#3 {main}
44+
thrown in %s on line 9

‎Zend/tests/gh15275-005.phpt‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
--TEST--
2+
GH-15275 005: Crash during GC of suspended generator delegate
3+
--FILE--
4+
<?php
5+
6+
class It implements \IteratorAggregate
7+
{
8+
public function getIterator(): \Generator
9+
{
10+
yield 'foo';
11+
throw new \Exception();
12+
var_dump("not executed");
13+
}
14+
}
15+
16+
function f() {
17+
try {
18+
var_dump(new stdClass, yield from new It());
19+
} finally {
20+
var_dump(__FUNCTION__);
21+
}
22+
}
23+
24+
function g() {
25+
try {
26+
var_dump(new stdClass, yield from f());
27+
} finally {
28+
var_dump(__FUNCTION__);
29+
}
30+
}
31+
32+
$gen = g();
33+
34+
var_dump($gen->current());
35+
$gen->next();
36+
37+
?>
38+
==DONE==
39+
--EXPECTF--
40+
string(3) "foo"
41+
string(1) "f"
42+
string(1) "g"
43+
44+
Fatal error: Uncaught Exception in %s:8
45+
Stack trace:
46+
#0 %s(15): It->getIterator()
47+
#1 %s(23): f()
48+
#2 [internal function]: g()
49+
#3 %s(32): Generator->next()
50+
#4 {main}
51+
thrown in %s on line 8

‎Zend/tests/gh15275-006.phpt‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
--TEST--
2+
GH-15275 006: Crash during GC of suspended generator delegate
3+
--FILE--
4+
<?php
5+
6+
class It implements \IteratorAggregate
7+
{
8+
public function getIterator(): \Generator
9+
{
10+
yield 'foo';
11+
echo "baz\n";
12+
throw new \Exception();
13+
}
14+
15+
public function __destruct()
16+
{
17+
throw new \Exception();
18+
}
19+
}
20+
21+
function f() {
22+
var_dump(new stdClass, yield from new It());
23+
}
24+
25+
$gen = f();
26+
27+
var_dump($gen->current());
28+
$gen->next();
29+
30+
gc_collect_cycles();
31+
32+
?>
33+
==DONE==
34+
--EXPECTF--
35+
string(3) "foo"
36+
baz
37+
38+
Fatal error: Uncaught Exception in %s:9
39+
Stack trace:
40+
#0 %s(19): It->getIterator()
41+
#1 [internal function]: f()
42+
#2 %s(25): Generator->next()
43+
#3 {main}
44+
45+
Next Exception in %s:14
46+
Stack trace:
47+
#0 %s(19): It->__destruct()
48+
#1 [internal function]: f()
49+
#2 %s(25): Generator->next()
50+
#3 {main}
51+
thrown in %s on line 14

‎Zend/zend_execute.c‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4471,7 +4471,13 @@ ZEND_API HashTable *zend_unfinished_execution_gc_ex(zend_execute_data *execute_d
44714471
}
44724472

44734473
if (call) {
4474-
uint32_t op_num = execute_data->opline - op_array->opcodes;
4474+
uint32_t op_num;
4475+
if (UNEXPECTED(execute_data->opline->opcode == ZEND_HANDLE_EXCEPTION)) {
4476+
op_num = EG(opline_before_exception) - op_array->opcodes;
4477+
} else {
4478+
op_num = execute_data->opline - op_array->opcodes;
4479+
}
4480+
ZEND_ASSERT(op_num < op_array->last);
44754481
if (suspended_by_yield) {
44764482
/* When the execution was suspended by yield, EX(opline) points to
44774483
* next opline to execute. Otherwise, it points to the opline that

‎Zend/zend_generators.c‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include "zend_closures.h"
2727
#include "zend_generators_arginfo.h"
2828
#include "zend_observer.h"
29+
#include "zend_vm_opcodes.h"
2930

3031
ZEND_API zend_class_entry *zend_ce_generator;
3132
ZEND_API zend_class_entry *zend_ce_ClosedGeneratorException;
@@ -512,6 +513,8 @@ static void zend_generator_throw_exception(zend_generator *generator, zval *exce
512513
* to pretend the exception happened during the YIELD opcode. */
513514
EG(current_execute_data) = generator->execute_data;
514515
generator->execute_data->opline--;
516+
ZEND_ASSERT(generator->execute_data->opline->opcode == ZEND_YIELD
517+
|| generator->execute_data->opline->opcode == ZEND_YIELD_FROM);
515518
generator->execute_data->prev_execute_data = original_execute_data;
516519

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

526+
generator->execute_data->opline++;
527+
523528
/* if we don't stop an array/iterator yield from, the exception will only reach the generator after the values were all iterated over */
524529
if (UNEXPECTED(Z_TYPE(generator->values) != IS_UNDEF)) {
525530
zval_ptr_dtor(&generator->values);
526531
ZVAL_UNDEF(&generator->values);
527532
}
528533

529-
generator->execute_data->opline++;
530534
EG(current_execute_data) = original_execute_data;
531535
}
532536

@@ -656,8 +660,6 @@ ZEND_API zend_generator *zend_generator_update_current(zend_generator *generator
656660

657661
static zend_result zend_generator_get_next_delegated_value(zend_generator *generator) /* {{{ */
658662
{
659-
--generator->execute_data->opline;
660-
661663
zval *value;
662664
if (Z_TYPE(generator->values) == IS_ARRAY) {
663665
HashTable *ht = Z_ARR(generator->values);
@@ -739,14 +741,12 @@ static zend_result zend_generator_get_next_delegated_value(zend_generator *gener
739741
}
740742
}
741743

742-
++generator->execute_data->opline;
743744
return SUCCESS;
744745

745746
failure:
746747
zval_ptr_dtor(&generator->values);
747748
ZVAL_UNDEF(&generator->values);
748749

749-
++generator->execute_data->opline;
750750
return FAILURE;
751751
}
752752
/* }}} */
@@ -811,6 +811,15 @@ ZEND_API void zend_generator_resume(zend_generator *orig_generator) /* {{{ */
811811
generator->flags &= ~ZEND_GENERATOR_IN_FIBER;
812812
return;
813813
}
814+
if (UNEXPECTED(EG(exception))) {
815+
/* Decrementing opline_before_exception to pretend the exception
816+
* happened during the YIELD_FROM opcode. */
817+
if (generator->execute_data) {
818+
ZEND_ASSERT(generator->execute_data->opline == EG(exception_op));
819+
ZEND_ASSERT((EG(opline_before_exception)-1)->opcode == ZEND_YIELD_FROM);
820+
EG(opline_before_exception)--;
821+
}
822+
}
814823
/* If there are no more delegated values, resume the generator
815824
* after the "yield from" expression. */
816825
}

0 commit comments

Comments
 (0)