8000 Fix GH-8289: Exceptions thrown within a yielded from iterator are not… · php/php-src@1364945 · GitHub
[go: up one dir, main page]
Skip to content
< 8000 header class="HeaderMktg header-logged-out js-details-container js-header Details f4 py-3" role="banner" data-is-top="true" data-color-mode=light data-light-theme=light data-dark-theme=dark>

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 1364945

Browse files
bwoebibwoebi
committed
Fix GH-8289: Exceptions thrown within a yielded from iterator are not rethrown into the generator
This also fixes the fact that exception traces were not including the generator frame when thrown in a yielded from iterator.
1 parent d82d62c commit 1364945

File tree

3 files changed

+110
-81
lines changed

3 files changed

+110
-81
lines changed

‎NEWS

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ PHP NEWS
77
. Fixed bug GH-8070 (memory leak of internal function attribute hash).
88
(Tim Düsterhus)
99
. Fixed bug GH-8160 (ZTS support on Alpine is broken). (Michael Voříšek)
10+
. Fixed bug GH-8289 (Exceptions thrown within a yielded from iterator are
11+
not rethrown into the generator). (Bob)
1012

1113
- Filter:
1214
. Fixed signedness confusion in php_filter_validate_domain(). (cmb)
@@ -43,8 +45,6 @@ PHP NEWS
4345

4446
- Core:
4547
. Fixed Haiku ZTS build. (David Carlier)
46-
. Fixed bug GH-8082 (op_arrays with temporary run_time_cache leak memory
47-
when observed). (Bob)
4848

4949
- GD:
5050
. Fixed libpng warning when loading interlaced images. (Brett)

‎Zend/tests/generators/gh8289.phpt

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
--TEST--
2+
GH-8289 (Exceptions thrown within a yielded from iterator are not rethrown into the generator)
3+
--FILE--
4+
<?php
5+
6+
function yieldFromIteratorGeneratorThrows() {
7+
try {
8+
yield from new class(new ArrayIterator([1, -2])) extends IteratorIterator {
9+
public function key() {
10+
if ($k = parent::key()) {
11+
throw new Exception;
12+
}
13+
return $k;
14+
}
15+
};
16+
} catch (Exception $e) {
17+
echo "$e\n";
18+
yield 2;
19+
}
20+
}
21+
22+
foreach (yieldFromIteratorGeneratorThrows() as $k => $v) {
23+
var_dump($v);
24+
}
25+
26+
?>
27+
--EXPECTF--
28+
int(1)
29+
Exception in %s:%d
30+
Stack trace:
31+
#0 %s(%d): IteratorIterator@anonymous->key()
32+
#1 %s(%d): yieldFromIteratorGeneratorThrows()
33+
#2 {main}
34+
int(2)

‎Zend/zend_generators.c

Lines changed: 74 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -637,22 +637,17 @@ static zend_result zend_generator_get_next_delegated_value(zend_generator *gener
637637
if (iter->index++ > 0) {
638638
iter->funcs->move_forward(iter);
639639
if (UNEXPECTED(EG(exception) != NULL)) {
640-
goto exception;
640+
goto failure;
641641
}
642642
}
643643

644644
if (iter->funcs->valid(iter) == FAILURE) {
645-
if (UNEXPECTED(EG(exception) != NULL)) {
646-
goto exception;
647-
}
648645
/* reached end of iteration */
649646
goto failure;
650647
}
651648

652649
value = iter->funcs->get_current_data(iter);
653-
if (UNEXPECTED(EG(exception) != NULL)) {
654-
goto exception;
655-
} else if (UNEXPECTED(!value)) {
650+
if (UNEXPECTED(EG(exception) != NULL) || UNEXPECTED(!value)) {
656651
goto failure;
657652
}
658653

@@ -664,17 +659,14 @@ static zend_result zend_generator_get_next_delegated_value(zend_generator *gener
664659
iter->funcs->get_current_key(iter, &generator->key);
665660
if (UNEXPECTED(EG(exception) != NULL)) {
666661
ZVAL_UNDEF(&generator->key);
667-
goto exception;
662+
goto failure;
668663
}
669664
} else {
670665
ZVAL_LONG(&generator->key, iter->index);
671666
}
672667
}
673668
return SUCCESS;
674669

675-
exception:
676-
zend_generator_throw_exception(generator, NULL);
677-
678670
failure:
679671
zval_ptr_dtor(&generator->values);
680672
ZVAL_UNDEF(&generator->values);
@@ -706,94 +698,97 @@ ZEND_API void zend_generator_resume(zend_generator *orig_generator) /* {{{ */
706698
/* Drop the AT_FIRST_YIELD flag */
707699
orig_generator->flags &= ~ZEND_GENERATOR_AT_FIRST_YIELD;
708700

701+
/* Backup executor globals */
702+
zend_execute_data *original_execute_data = EG(current_execute_data);
703+
uint32_t original_jit_trace_num = EG(jit_trace_num);
704+
705+
/* Set executor globals */
706+
EG(current_execute_data) = generator->execute_data;
707+
EG(jit_trace_num) = 0;
708+
709+
/* We want the backtrace to look as if the generator function was
710+
* called from whatever method we are current running (e.g. next()).
711+
* So we have to link generator call frame with caller call frame. */
712+
if (generator == orig_generator) {
713+
generator->execute_data->prev_execute_data = original_execute_data;
714+
} else {
715+
/* We need some execute_data placeholder in stacktrace to be replaced
716+
* by the real stack trace when needed */
717+
generator->execute_data->prev_execute_data = &orig_generator->execute_fake;
718+
orig_generator->execute_fake.prev_execute_data = original_execute_data;
719+
}
720+
721+
/* Ensure this is run after executor_data swap to have a proper stack trace */
709722
if (UNEXPECTED(!Z_ISUNDEF(generator->values))) {
710723
if (EXPECTED(zend_generator_get_next_delegated_value(generator) == SUCCESS)) {
724+
/* Restore executor globals */
725+
EG(current_execute_data) = original_execute_data;
726+
EG(jit_trace_num) = original_jit_trace_num;
727+
711728
orig_generator->flags &= ~ZEND_GENERATOR_DO_INIT;
712729
return;
713730
}
714731
/* If there are no more deletegated values, resume the generator
715732
* after the "yield from" expression. */
716733
}
717734

718-
{
719-
/* Backup executor globals */
720-
zend_execute_data *original_execute_data = EG(current_execute_data);
721-
uint32_t original_jit_trace_num = EG(jit_trace_num);
722-
723-
/* Set executor globals */
724-
EG(current_execute_data) = generator->execute_data;
725-
EG(jit_trace_num) = 0;
726-
727-
/* We want the backtrace to look as if the generator function was
728-
* called from whatever method we are current running (e.g. next()).
729-
* So we have to link generator call frame with caller call frame. */
730-
if (generator == orig_generator) {
731-
generator->execute_data->prev_execute_data = original_execute_data;
732-
} else {
733-
/* We need some execute_data placeholder in stacktrace to be replaced
734-
* by the real stack trace when needed */
735-
generator->execute_data->prev_execute_data = &orig_generator->execute_fake;
736-
orig_generator->execute_fake.prev_execute_data = original_execute_data;
737-
}
735+
if (UNEXPECTED(generator->frozen_call_stack)) {
736+
/* Restore frozen call-stack */
737+
zend_generator_restore_call_stack(generator);
738+
}
738739

739-
if (UNEXPECTED(generator->frozen_call_stack)) {
740-
/* Restore frozen call-stack */
741-
zend_generator_restore_call_stack(generator);
740+
/* Resume execution */
741+
generator->flags |= ZEND_GENERATOR_CURRENTLY_RUNNING;
742+
if (!ZEND_OBSERVER_ENABLED) {
743+
zend_execute_ex(generator->execute_data);
744+
} else {
745+
zend_observer_generator_resume(generator->execute_data);
746+
zend_execute_ex(generator->execute_data);
747+
if (generator->execute_data) {
748+
/* On the final return, this will be called from ZEND_GENERATOR_RETURN */
749+
zend_observer_fcall_end(generator->execute_data, &generator->value);
742750
}
751+
}
752+
generator->flags &= ~ZEND_GENERATOR_CURRENTLY_RUNNING;
743753

744-
/* Resume execution */
745-
generator->flags |= ZEND_GENERATOR_CURRENTLY_RUNNING;
746-
if (!ZEND_OBSERVER_ENABLED) {
747-
zend_execute_ex(generator->execute_data);
748-
} else {
749-
zend_observer_generator_resume(generator->execute_data);
750-
zend_execute_ex(generator->execute_data);
751-
if (generator->execute_data) {
752-
/* On the final return, this will be called from ZEND_GENERATOR_RETURN */
753-
zend_observer_fcall_end(generator->execute_data, &generator->value);
754-
}
755-
}
756-
generator->flags &= ~ZEND_GENERATOR_CURRENTLY_RUNNING;
754+
generator->frozen_call_stack = NULL;
755+
if (EXPECTED(generator->execute_data) &&
756+
UNEXPECTED(generator->execute_data->call)) {
757+
/* Frize call-stack */
758+
generator->frozen_call_stack = zend_generator_freeze_call_stack(generator->execute_data);
759+
}
757760

758-
generator->frozen_call_stack = NULL;
759-
if (EXPECTED(generator->execute_data) &&
760-
UNEXPECTED(generator->execute_data->call)) {
761-
/* Frize call-stack */
762-
generator->frozen_call_stack = zend_generator_freeze_call_stack(generator->execute_data);
763-
}
761+
/* Restore executor globals */
762+
EG(current_execute_data) = original_execute_data;
763+
EG(jit_trace_num) = original_jit_trace_num;
764764

765-
/* Restore executor globals */
766-
EG(current_execute_data) = original_execute_data;
767-
EG(jit_trace_num) = original_jit_trace_num;
768-
769-
/* If an exception was thrown in the generator we have to internally
770-
* rethrow it in the parent scope.
771-
* In case we did yield from, the Exception must be rethrown into
772-
* its calling frame (see above in if (check_yield_from). */
773-
if (UNEXPECTED(EG(exception) != NULL)) {
774-
if (generator == orig_generator) {
775-
zend_generator_close(generator, 0);
776-
if (!EG(current_execute_data)) {
777-
zend_throw_exception_internal(NULL);
778-
} else if (EG(current_execute_data)->func &&
779-
ZEND_USER_CODE(EG(current_execute_data)->func->common.type)) {
780-
zend_rethrow_exception(EG(current_execute_data));
781-
}
782-
} else {
783-
generator = zend_generator_get_current(orig_generator);
784-
zend_generator_throw_exception(generator, NULL);
785-
orig_generator->flags &= ~ZEND_GENERATOR_DO_INIT;
786-
goto try_again;
765+
/* If an exception was thrown in the generator we have to internally
766+
* rethrow it in the parent scope.
767+
* In case we did yield from, the Exception must be rethrown into
768+
* its calling frame (see above in if (check_yield_from). */
769+
if (UNEXPECTED(EG(exception) != NULL)) {
770+
if (generator == orig_generator) {
771+
zend_generator_close(generator, 0);
772+
if (!EG(current_execute_data)) {
773+
zend_throw_exception_internal(NULL);
774+
} else if (EG(current_execute_data)->func &&
775+
ZEND_USER_CODE(EG(current_execute_data)->func->common.type)) {
776+
zend_rethrow_exception(EG(current_execute_data));
787777
}
788-
}
789-
790-
/* yield from was used, try another resume. */
791-
if (UNEXPECTED((generator != orig_generator && !Z_ISUNDEF(generator->retval)) || (generator->execute_data && (generator->execute_data->opline - 1)->opcode == ZEND_YIELD_FROM))) {
778+
} else {
792779
generator = zend_generator_get_current(orig_generator);
780+
zend_generator_throw_exception(generator, NULL);
781+
orig_generator->flags &= ~ZEND_GENERATOR_DO_INIT;
793782
goto try_again;
794783
}
795784
}
796785

786+
/* yield from was used, try another resume. */
787+
if (UNEXPECTED((generator != orig_generator && !Z_ISUNDEF(generator->retval)) || (generator->execute_data && (generator->execute_data->opline - 1)->opcode == ZEND_YIELD_FROM))) {
788+
generator = zend_generator_get_current(orig_generator);
789+
goto try_again;
790+
}
791+
797792
orig_generator->flags &= ~ZEND_GENERATOR_DO_INIT;
798793
}
799794
/* }}} */

0 commit comments

Comments
 (0)
0