From 81b022eb2ff8d9578117b949cfc625eab8cc1ac4 Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Mon, 5 Aug 2024 18:28:35 +0200 Subject: [PATCH 1/3] Fix crash during GC of suspended generator delegate --- Zend/tests/ghxxx-001.phpt | 38 ++++++++++++++++++++++++++ Zend/tests/ghxxx-002.phpt | 57 +++++++++++++++++++++++++++++++++++++++ Zend/tests/ghxxx-003.phpt | 51 +++++++++++++++++++++++++++++++++++ Zend/tests/ghxxx-004.phpt | 44 ++++++++++++++++++++++++++++++ Zend/tests/ghxxx-005.phpt | 51 +++++++++++++++++++++++++++++++++++ Zend/zend_execute.c | 8 +++++- Zend/zend_generators.c | 19 +++++++++---- 7 files changed, 262 insertions(+), 6 deletions(-) create mode 100644 Zend/tests/ghxxx-001.phpt create mode 100644 Zend/tests/ghxxx-002.phpt create mode 100644 Zend/tests/ghxxx-003.phpt create mode 100644 Zend/tests/ghxxx-004.phpt create mode 100644 Zend/tests/ghxxx-005.phpt diff --git a/Zend/tests/ghxxx-001.phpt b/Zend/tests/ghxxx-001.phpt new file mode 100644 index 0000000000000..a0802043091cf --- /dev/null +++ b/Zend/tests/ghxxx-001.phpt @@ -0,0 +1,38 @@ +--TEST-- +GH-xxx 001: Crash during GC of suspended generator delegate +--FILE-- +current()); + $iterable->next(); + var_dump("not executed"); +}); + +$ref = $fiber; + +$fiber->start(); + +gc_collect_cycles(); + +?> +==DONE== +--EXPECT-- +string(3) "foo" +==DONE== diff --git a/Zend/tests/ghxxx-002.phpt b/Zend/tests/ghxxx-002.phpt new file mode 100644 index 0000000000000..e1bbfc8102971 --- /dev/null +++ b/Zend/tests/ghxxx-002.phpt @@ -0,0 +1,57 @@ +--TEST-- +GH-xxx 002: Crash during GC of suspended generator delegate +--FILE-- +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" diff --git a/Zend/tests/ghxxx-003.phpt b/Zend/tests/ghxxx-003.phpt new file mode 100644 index 0000000000000..245e6b65ebb05 --- /dev/null +++ b/Zend/tests/ghxxx-003.phpt @@ -0,0 +1,51 @@ +--TEST-- +GH-xxx 003: Crash during GC of suspended generator delegate +--FILE-- +current()); +$gen->next(); + +?> +==DONE== +--EXPECTF-- +string(3) "foo" +string(1) "f" +string(1) "g" + +Fatal error: Uncaught Exception in %s:%d +Stack trace: +#0 %s(%d): It->getIterator() +#1 %s(%d): f() +#2 [internal function]: g() +#3 %s(%d): Generator->next() +#4 {main} + thrown in %s on line %d diff --git a/Zend/tests/ghxxx-004.phpt b/Zend/tests/ghxxx-004.phpt new file mode 100644 index 0000000000000..27a2d92e9b6dc --- /dev/null +++ b/Zend/tests/ghxxx-004.phpt @@ -0,0 +1,44 @@ +--TEST-- +GH-xxx 004: Crash during GC of suspended generator delegate +--FILE-- +current()); +$gen->next(); + +gc_collect_cycles(); + +?> +==DONE== +--EXPECTF-- +string(3) "foo" +baz + +Fatal error: Uncaught Exception in %s:%d +Stack trace: +#0 %s(%d): It->getIterator() +#1 [internal function]: f() +#2 %s(%d): Generator->next() +#3 {main} + thrown in %s on line %d diff --git a/Zend/tests/ghxxx-005.phpt b/Zend/tests/ghxxx-005.phpt new file mode 100644 index 0000000000000..af42b0f906b91 --- /dev/null +++ b/Zend/tests/ghxxx-005.phpt @@ -0,0 +1,51 @@ +--TEST-- +GH-xxx 005: Crash during GC of suspended generator delegate +--FILE-- +current()); +$gen->next(); + +?> +==DONE== +--EXPECTF-- +string(3) "foo" +string(1) "f" +string(1) "g" + +Fatal error: Uncaught Exception in %s:%d +Stack trace: +#0 %s(%d): It->getIterator() +#1 %s(%d): f() +#2 [internal function]: g() +#3 %s(%d): Generator->next() +#4 {main} + thrown in %s on line %d diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index 3de48fb1358c5..716756ad93ba2 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -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 diff --git a/Zend/zend_generators.c b/Zend/zend_generators.c index f7475ad6fbb77..85127025679d6 100644 --- a/Zend/zend_generators.c +++ b/Zend/zend_generators.c @@ -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; @@ -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) { @@ -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; } @@ -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); @@ -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; } /* }}} */ @@ -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. */ } From 83a1cb17d4e8e1e35827a69246b5cbb2d4dca5dc Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Wed, 7 Aug 2024 16:05:44 +0200 Subject: [PATCH 2/3] Rename tests --- Zend/tests/{ghxxx-001.phpt => gh15275-001.phpt} | 2 +- Zend/tests/{ghxxx-002.phpt => gh15275-002.phpt} | 2 +- Zend/tests/{ghxxx-005.phpt => gh15275-003.phpt} | 2 +- Zend/tests/{ghxxx-004.phpt => gh15275-004.phpt} | 2 +- Zend/tests/{ghxxx-003.phpt => gh15275-005.phpt} | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) rename Zend/tests/{ghxxx-001.phpt => gh15275-001.phpt} (89%) rename Zend/tests/{ghxxx-002.phpt => gh15275-002.phpt} (93%) rename Zend/tests/{ghxxx-005.phpt => gh15275-003.phpt} (92%) rename Zend/tests/{ghxxx-004.phpt => gh15275-004.phpt} (91%) rename Zend/tests/{ghxxx-003.phpt => gh15275-005.phpt} (92%) diff --git a/Zend/tests/ghxxx-001.phpt b/Zend/tests/gh15275-001.phpt similarity index 89% rename from Zend/tests/ghxxx-001.phpt rename to Zend/tests/gh15275-001.phpt index a0802043091cf..bfea734c6ba9b 100644 --- a/Zend/tests/ghxxx-001.phpt +++ b/Zend/tests/gh15275-001.phpt @@ -1,5 +1,5 @@ --TEST-- -GH-xxx 001: Crash during GC of suspended generator delegate +GH-15275 001: Crash during GC of suspended generator delegate --FILE-- Date: Thu, 8 Aug 2024 15:13:29 +0200 Subject: [PATCH 3/3] Add line numbers, add test --- Zend/tests/gh15275-003.phpt | 10 ++++---- Zend/tests/gh15275-004.phpt | 8 +++--- Zend/tests/gh15275-005.phpt | 10 ++++---- Zend/tests/gh15275-006.phpt | 51 +++++++++++++++++++++++++++++++++++++ 4 files changed, 65 insertions(+), 14 deletions(-) create mode 100644 Zend/tests/gh15275-006.phpt diff --git a/Zend/tests/gh15275-003.phpt b/Zend/tests/gh15275-003.phpt index 1cdf2653befcc..d62187e5342b7 100644 --- a/Zend/tests/gh15275-003.phpt +++ b/Zend/tests/gh15275-003.phpt @@ -41,11 +41,11 @@ string(3) "foo" string(1) "f" string(1) "g" -Fatal error: Uncaught Exception in %s:%d +Fatal error: Uncaught Exception in %s:8 Stack trace: -#0 %s(%d): It->getIterator() -#1 %s(%d): f() +#0 %s(15): It->getIterator() +#1 %s(23): f() #2 [internal function]: g() -#3 %s(%d): Generator->next() +#3 %s(32): Generator->next() #4 {main} - thrown in %s on line %d + thrown in %s on line 8 diff --git a/Zend/tests/gh15275-004.phpt b/Zend/tests/gh15275-004.phpt index 064c44b3f7093..61939ad2a4557 100644 --- a/Zend/tests/gh15275-004.phpt +++ b/Zend/tests/gh15275-004.phpt @@ -35,10 +35,10 @@ gc_collect_cycles(); string(3) "foo" baz -Fatal error: Uncaught Exception in %s:%d +Fatal error: Uncaught Exception in %s:9 Stack trace: -#0 %s(%d): It->getIterator() +#0 %s(19): It->getIterator() #1 [internal function]: f() -#2 %s(%d): Generator->next() +#2 %s(25): Generator->next() #3 {main} - thrown in %s on line %d + thrown in %s on line 9 diff --git a/Zend/tests/gh15275-005.phpt b/Zend/tests/gh15275-005.phpt index d7f24b4618205..07a6f48e7a17e 100644 --- a/Zend/tests/gh15275-005.phpt +++ b/Zend/tests/gh15275-005.phpt @@ -41,11 +41,11 @@ string(3) "foo" string(1) "f" string(1) "g" -Fatal error: Uncaught Exception in %s:%d +Fatal error: Uncaught Exception in %s:8 Stack trace: -#0 %s(%d): It->getIterator() -#1 %s(%d): f() +#0 %s(15): It->getIterator() +#1 %s(23): f() #2 [internal function]: g() -#3 %s(%d): Generator->next() +#3 %s(32): Generator->next() #4 {main} - thrown in %s on line %d + thrown in %s on line 8 diff --git a/Zend/tests/gh15275-006.phpt b/Zend/tests/gh15275-006.phpt new file mode 100644 index 0000000000000..7f585acf0f40b --- /dev/null +++ b/Zend/tests/gh15275-006.phpt @@ -0,0 +1,51 @@ +--TEST-- +GH-15275 006: Crash during GC of suspended generator delegate +--FILE-- +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