From 9589cae8cb1b13710ca36491f30b569113f9e329 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Sat, 30 Nov 2013 13:05:40 +0100 Subject: [PATCH 1/2] Fixed bug #66041: list() fails to unpack yielded ArrayAccess object Yield return values now use IS_VAR rather than IS_TMP_VAR. This fixes the issue with list() and should also be faster as it avoids doing a zval copy. --- NEWS | 2 + Zend/tests/generators/bug66041.phpt | 17 ++ Zend/zend_compile.c | 2 +- Zend/zend_generators.c | 13 +- Zend/zend_generators.h | 2 +- Zend/zend_vm_def.h | 14 +- Zend/zend_vm_execute.h | 350 ++++++++++++++++++---------- 7 files changed, 266 insertions(+), 134 deletions(-) create mode 100644 Zend/tests/generators/bug66041.phpt diff --git a/NEWS b/NEWS index 55ee92e4e67..008b22dc180 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,8 @@ PHP NEWS - Core: . Added validation of class names in the autoload process. (Dmitry) + . Fixed buf #66041 (list() fails to unpack yielded ArrayAccess object). + (Nikita) - Date: . Fixed bug #66060 (Heap buffer over-read in DateInterval). (Remi) diff --git a/Zend/tests/generators/bug66041.phpt b/Zend/tests/generators/bug66041.phpt new file mode 100644 index 00000000000..d9442241349 --- /dev/null +++ b/Zend/tests/generators/bug66041.phpt @@ -0,0 +1,17 @@ +--TEST-- +Bug #66041: list() fails to unpack yielded ArrayAccess object +--FILE-- +send($fixedArray); +?> +--EXPECT-- +string(11) "the element" diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index 60b9e3e653c..c5f3769216f 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -2782,7 +2782,7 @@ void zend_do_yield(znode *result, znode *value, const znode *key, zend_bool is_v SET_UNUSED(opline->op2); } - opline->result_type = IS_TMP_VAR; + opline->result_type = IS_VAR; opline->result.var = get_temporary_variable(CG(active_op_array)); GET_NODE(result, opline->result); } diff --git a/Zend/zend_generators.c b/Zend/zend_generators.c index 0af20f4593f..c0116332e7b 100644 --- a/Zend/zend_generators.c +++ b/Zend/zend_generators.c @@ -55,6 +55,11 @@ ZEND_API void zend_generator_close(zend_generator *generator, zend_bool finished zval_ptr_dtor(&execute_data->current_this); } + if (!finished_execution && generator->send_target) { + Z_DELREF_PP(generator->send_target); + generator->send_target = NULL; + } + /* A fatal error / die occurred during the generator execution. Trying to clean * up the stack may not be safe in this case. */ if (CG(unclean_shutdown)) { @@ -519,8 +524,12 @@ ZEND_METHOD(Generator, send) return; } - /* Put sent value into the TMP_VAR slot */ - MAKE_COPY_ZVAL(&value, &generator->send_target->tmp_var); + /* Put sent value in the target VAR slot, if it is used */ + if (generator->send_target) { + Z_DELREF_PP(generator->send_target); + Z_ADDREF_P(value); + *generator->send_target = value; + } zend_generator_resume(generator TSRMLS_CC); diff --git a/Zend/zend_generators.h b/Zend/zend_generators.h index bc125658ee7..d5f61013155 100644 --- a/Zend/zend_generators.h +++ b/Zend/zend_generators.h @@ -49,7 +49,7 @@ typedef struct _zend_generator { /* Current key */ zval *key; /* Variable to put sent value into */ - temp_variable *send_target; + zval **send_target; /* Largest used integer key for auto-incrementing keys */ long largest_used_integer_key; diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h index 05557e98336..9db6ea19f9d 100644 --- a/Zend/zend_vm_def.h +++ b/Zend/zend_vm_def.h @@ -5353,11 +5353,15 @@ ZEND_VM_HANDLER(160, ZEND_YIELD, CONST|TMP|VAR|CV|UNUSED, CONST|TMP|VAR|CV|UNUSE ZVAL_LONG(generator->key, generator->largest_used_integer_key); } - /* If a value is sent it should go into the result var */ - generator->send_target = &EX_T(opline->result.var); - - /* Initialize the sent value to NULL */ - EX_T(opline->result.var).tmp_var = EG(uninitialized_zval); + if (RETURN_VALUE_USED(opline)) { + /* If the return value of yield is used set the send + * target and initialize it to NULL */ + generator->send_target = &EX_T(opline->result.var).var.ptr; + Z_ADDREF(EG(uninitialized_zval)); + EX_T(opline->result.var).var.ptr = &EG(uninitialized_zval); + } else { + generator->send_target = NULL; + } /* We increment to the next op, so we are at the correct position when the * generator is resumed. */ diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h index ec29f9e7d8d..3a0a70e9465 100644 --- a/Zend/zend_vm_execute.h +++ b/Zend/zend_vm_execute.h @@ -4198,11 +4198,15 @@ static int ZEND_FASTCALL ZEND_YIELD_SPEC_CONST_CONST_HANDLER(ZEND_OPCODE_HANDLE ZVAL_LONG(generator->key, generator->largest_used_integer_key); } - /* If a value is sent it should go into the result var */ - generator->send_target = &EX_T(opline->result.var); - - /* Initialize the sent value to NULL */ - EX_T(opline->result.var).tmp_var = EG(uninitialized_zval); + if (RETURN_VALUE_USED(opline)) { + /* If the return value of yield is used set the send + * target and initialize it to NULL */ + generator->send_target = &EX_T(opline->result.var).var.ptr; + Z_ADDREF(EG(uninitialized_zval)); + EX_T(opline->result.var).var.ptr = &EG(uninitialized_zval); + } else { + generator->send_target = NULL; + } /* We increment to the next op, so we are at the correct position when the * generator is resumed. */ @@ -4887,11 +4891,15 @@ static int ZEND_FASTCALL ZEND_YIELD_SPEC_CONST_TMP_HANDLER(ZEND_OPCODE_HANDLER_ ZVAL_LONG(generator->key, generator->largest_used_integer_key); } - /* If a value is sent it should go into the result var */ - generator->send_target = &EX_T(opline->result.var); - - /* Initialize the sent value to NULL */ - EX_T(opline->result.var).tmp_var = EG(uninitialized_zval); + if (RETURN_VALUE_USED(opline)) { + /* If the return value of yield is used set the send + * target and initialize it to NULL */ + generator->send_target = &EX_T(opline->result.var).var.ptr; + Z_ADDREF(EG(uninitialized_zval)); + EX_T(opline->result.var).var.ptr = &EG(uninitialized_zval); + } else { + generator->send_target = NULL; + } /* We increment to the next op, so we are at the correct position when the * generator is resumed. */ @@ -5904,11 +5912,15 @@ static int ZEND_FASTCALL ZEND_YIELD_SPEC_CONST_VAR_HANDLER(ZEND_OPCODE_HANDLER_ ZVAL_LONG(generator->key, generator->largest_used_integer_key); } - /* If a value is sent it should go into the result var */ - generator->send_target = &EX_T(opline->result.var); - - /* Initialize the sent value to NULL */ - EX_T(opline->result.var).tmp_var = EG(uninitialized_zval); + if (RETURN_VALUE_USED(opline)) { + /* If the return value of yield is used set the send + * target and initialize it to NULL */ + generator->send_target = &EX_T(opline->result.var).var.ptr; + Z_ADDREF(EG(uninitialized_zval)); + EX_T(opline->result.var).var.ptr = &EG(uninitialized_zval); + } else { + generator->send_target = NULL; + } /* We increment to the next op, so we are at the correct position when the * generator is resumed. */ @@ -6625,11 +6637,15 @@ static int ZEND_FASTCALL ZEND_YIELD_SPEC_CONST_UNUSED_HANDLER(ZEND_OPCODE_HANDL ZVAL_LONG(generator->key, generator->largest_used_integer_key); } - /* If a value is sent it should go into the result var */ - generator->send_target = &EX_T(opline->result.var); - - /* Initialize the sent value to NULL */ - EX_T(opline->result.var).tmp_var = EG(uninitialized_zval); + if (RETURN_VALUE_USED(opline)) { + /* If the return value of yield is used set the send + * target and initialize it to NULL */ + generator->send_target = &EX_T(opline->result.var).var.ptr; + Z_ADDREF(EG(uninitialized_zval)); + EX_T(opline->result.var).var.ptr = &EG(uninitialized_zval); + } else { + generator->send_target = NULL; + } /* We increment to the next op, so we are at the correct position when the * generator is resumed. */ @@ -7373,11 +7389,15 @@ static int ZEND_FASTCALL ZEND_YIELD_SPEC_CONST_CV_HANDLER(ZEND_OPCODE_HANDLER_A ZVAL_LONG(generator->key, generator->largest_used_integer_key); } - /* If a value is sent it should go into the result var */ - generator->send_target = &EX_T(opline->result.var); - - /* Initialize the sent value to NULL */ - EX_T(opline->result.var).tmp_var = EG(uninitialized_zval); + if (RETURN_VALUE_USED(opline)) { + /* If the return value of yield is used set the send + * target and initialize it to NULL */ + generator->send_target = &EX_T(opline->result.var).var.ptr; + Z_ADDREF(EG(uninitialized_zval)); + EX_T(opline->result.var).var.ptr = &EG(uninitialized_zval); + } else { + generator->send_target = NULL; + } /* We increment to the next op, so we are at the correct position when the * generator is resumed. */ @@ -9400,11 +9420,15 @@ static int ZEND_FASTCALL ZEND_YIELD_SPEC_TMP_CONST_HANDLER(ZEND_OPCODE_HANDLER_ ZVAL_LONG(generator->key, generator->largest_used_integer_key); } - /* If a value is sent it should go into the result var */ - generator->send_target = &EX_T(opline->result.var); - - /* Initialize the sent value to NULL */ - EX_T(opline->result.var).tmp_var = EG(uninitialized_zval); + if (RETURN_VALUE_USED(opline)) { + /* If the return value of yield is used set the send + * target and initialize it to NULL */ + generator->send_target = &EX_T(opline->result.var).var.ptr; + Z_ADDREF(EG(uninitialized_zval)); + EX_T(opline->result.var).var.ptr = &EG(uninitialized_zval); + } else { + generator->send_target = NULL; + } /* We increment to the next op, so we are at the correct position when the * generator is resumed. */ @@ -10091,11 +10115,15 @@ static int ZEND_FASTCALL ZEND_YIELD_SPEC_TMP_TMP_HANDLER(ZEND_OPCODE_HANDLER_AR ZVAL_LONG(generator->key, generator->largest_used_integer_key); } - /* If a value is sent it should go into the result var */ - generator->send_target = &EX_T(opline->result.var); - - /* Initialize the sent value to NULL */ - EX_T(opline->result.var).tmp_var = EG(uninitialized_zval); + if (RETURN_VALUE_USED(opline)) { + /* If the return value of yield is used set the send + * target and initialize it to NULL */ + generator->send_target = &EX_T(opline->result.var).var.ptr; + Z_ADDREF(EG(uninitialized_zval)); + EX_T(opline->result.var).var.ptr = &EG(uninitialized_zval); + } else { + generator->send_target = NULL; + } /* We increment to the next op, so we are at the correct position when the * generator is resumed. */ @@ -11110,11 +11138,15 @@ static int ZEND_FASTCALL ZEND_YIELD_SPEC_TMP_VAR_HANDLER(ZEND_OPCODE_HANDLER_AR ZVAL_LONG(generator->key, generator->largest_used_integer_key); } - /* If a value is sent it should go into the result var */ - generator->send_target = &EX_T(opline->result.var); - - /* Initialize the sent value to NULL */ - EX_T(opline->result.var).tmp_var = EG(uninitialized_zval); + if (RETURN_VALUE_USED(opline)) { + /* If the return value of yield is used set the send + * target and initialize it to NULL */ + generator->send_target = &EX_T(opline->result.var).var.ptr; + Z_ADDREF(EG(uninitialized_zval)); + EX_T(opline->result.var).var.ptr = &EG(uninitialized_zval); + } else { + generator->send_target = NULL; + } /* We increment to the next op, so we are at the correct position when the * generator is resumed. */ @@ -11690,11 +11722,15 @@ static int ZEND_FASTCALL ZEND_YIELD_SPEC_TMP_UNUSED_HANDLER(ZEND_OPCODE_HANDLER ZVAL_LONG(generator->key, generator->largest_used_integer_key); } - /* If a value is sent it should go into the result var */ - generator->send_target = &EX_T(opline->result.var); - - /* Initialize the sent value to NULL */ - EX_T(opline->result.var).tmp_var = EG(uninitialized_zval); + if (RETURN_VALUE_USED(opline)) { + /* If the return value of yield is used set the send + * target and initialize it to NULL */ + generator->send_target = &EX_T(opline->result.var).var.ptr; + Z_ADDREF(EG(uninitialized_zval)); + EX_T(opline->result.var).var.ptr = &EG(uninitialized_zval); + } else { + generator->send_target = NULL; + } /* We increment to the next op, so we are at the correct position when the * generator is resumed. */ @@ -12378,11 +12414,15 @@ static int ZEND_FASTCALL ZEND_YIELD_SPEC_TMP_CV_HANDLER(ZEND_OPCODE_HANDLER_ARG ZVAL_LONG(generator->key, generator->largest_used_integer_key); } - /* If a value is sent it should go into the result var */ - generator->send_target = &EX_T(opline->result.var); - - /* Initialize the sent value to NULL */ - EX_T(opline->result.var).tmp_var = EG(uninitialized_zval); + if (RETURN_VALUE_USED(opline)) { + /* If the return value of yield is used set the send + * target and initialize it to NULL */ + generator->send_target = &EX_T(opline->result.var).var.ptr; + Z_ADDREF(EG(uninitialized_zval)); + EX_T(opline->result.var).var.ptr = &EG(uninitialized_zval); + } else { + generator->send_target = NULL; + } /* We increment to the next op, so we are at the correct position when the * generator is resumed. */ @@ -16265,11 +16305,15 @@ static int ZEND_FASTCALL ZEND_YIELD_SPEC_VAR_CONST_HANDLER(ZEND_OPCODE_HANDLER_ ZVAL_LONG(generator->key, generator->largest_used_integer_key); } - /* If a value is sent it should go into the result var */ - generator->send_target = &EX_T(opline->result.var); - - /* Initialize the sent value to NULL */ - EX_T(opline->result.var).tmp_var = EG(uninitialized_zval); + if (RETURN_VALUE_USED(opline)) { + /* If the return value of yield is used set the send + * target and initialize it to NULL */ + generator->send_target = &EX_T(opline->result.var).var.ptr; + Z_ADDREF(EG(uninitialized_zval)); + EX_T(opline->result.var).var.ptr = &EG(uninitialized_zval); + } else { + generator->send_target = NULL; + } /* We increment to the next op, so we are at the correct position when the * generator is resumed. */ @@ -18350,11 +18394,15 @@ static int ZEND_FASTCALL ZEND_YIELD_SPEC_VAR_TMP_HANDLER(ZEND_OPCODE_HANDLER_AR ZVAL_LONG(generator->key, generator->largest_used_integer_key); } - /* If a value is sent it should go into the result var */ - generator->send_target = &EX_T(opline->result.var); - - /* Initialize the sent value to NULL */ - EX_T(opline->result.var).tmp_var = EG(uninitialized_zval); + if (RETURN_VALUE_USED(opline)) { + /* If the return value of yield is used set the send + * target and initialize it to NULL */ + generator->send_target = &EX_T(opline->result.var).var.ptr; + Z_ADDREF(EG(uninitialized_zval)); + EX_T(opline->result.var).var.ptr = &EG(uninitialized_zval); + } else { + generator->send_target = NULL; + } /* We increment to the next op, so we are at the correct position when the * generator is resumed. */ @@ -20818,11 +20866,15 @@ static int ZEND_FASTCALL ZEND_YIELD_SPEC_VAR_VAR_HANDLER(ZEND_OPCODE_HANDLER_AR ZVAL_LONG(generator->key, generator->largest_used_integer_key); } - /* If a value is sent it should go into the result var */ - generator->send_target = &EX_T(opline->result.var); - - /* Initialize the sent value to NULL */ - EX_T(opline->result.var).tmp_var = EG(uninitialized_zval); + if (RETURN_VALUE_USED(opline)) { + /* If the return value of yield is used set the send + * target and initialize it to NULL */ + generator->send_target = &EX_T(opline->result.var).var.ptr; + Z_ADDREF(EG(uninitialized_zval)); + EX_T(opline->result.var).var.ptr = &EG(uninitialized_zval); + } else { + generator->send_target = NULL; + } /* We increment to the next op, so we are at the correct position when the * generator is resumed. */ @@ -21970,11 +22022,15 @@ static int ZEND_FASTCALL ZEND_YIELD_SPEC_VAR_UNUSED_HANDLER(ZEND_OPCODE_HANDLER ZVAL_LONG(generator->key, generator->largest_used_integer_key); } - /* If a value is sent it should go into the result var */ - generator->send_target = &EX_T(opline->result.var); - - /* Initialize the sent value to NULL */ - EX_T(opline->result.var).tmp_var = EG(uninitialized_zval); + if (RETURN_VALUE_USED(opline)) { + /* If the return value of yield is used set the send + * target and initialize it to NULL */ + generator->send_target = &EX_T(opline->result.var).var.ptr; + Z_ADDREF(EG(uninitialized_zval)); + EX_T(opline->result.var).var.ptr = &EG(uninitialized_zval); + } else { + generator->send_target = NULL; + } /* We increment to the next op, so we are at the correct position when the * generator is resumed. */ @@ -24104,11 +24160,15 @@ static int ZEND_FASTCALL ZEND_YIELD_SPEC_VAR_CV_HANDLER(ZEND_OPCODE_HANDLER_ARG ZVAL_LONG(generator->key, generator->largest_used_integer_key); } - /* If a value is sent it should go into the result var */ - generator->send_target = &EX_T(opline->result.var); - - /* Initialize the sent value to NULL */ - EX_T(opline->result.var).tmp_var = EG(uninitialized_zval); + if (RETURN_VALUE_USED(opline)) { + /* If the return value of yield is used set the send + * target and initialize it to NULL */ + generator->send_target = &EX_T(opline->result.var).var.ptr; + Z_ADDREF(EG(uninitialized_zval)); + EX_T(opline->result.var).var.ptr = &EG(uninitialized_zval); + } else { + generator->send_target = NULL; + } /* We increment to the next op, so we are at the correct position when the * generator is resumed. */ @@ -25602,11 +25662,15 @@ static int ZEND_FASTCALL ZEND_YIELD_SPEC_UNUSED_CONST_HANDLER(ZEND_OPCODE_HANDL ZVAL_LONG(generator->key, generator->largest_used_integer_key); } - /* If a value is sent it should go into the result var */ - generator->send_target = &EX_T(opline->result.var); - - /* Initialize the sent value to NULL */ - EX_T(opline->result.var).tmp_var = EG(uninitialized_zval); + if (RETURN_VALUE_USED(opline)) { + /* If the return value of yield is used set the send + * target and initialize it to NULL */ + generator->send_target = &EX_T(opline->result.var).var.ptr; + Z_ADDREF(EG(uninitialized_zval)); + EX_T(opline->result.var).var.ptr = &EG(uninitialized_zval); + } else { + generator->send_target = NULL; + } /* We increment to the next op, so we are at the correct position when the * generator is resumed. */ @@ -26916,11 +26980,15 @@ static int ZEND_FASTCALL ZEND_YIELD_SPEC_UNUSED_TMP_HANDLER(ZEND_OPCODE_HANDLER ZVAL_LONG(generator->key, generator->largest_used_integer_key); } - /* If a value is sent it should go into the result var */ - generator->send_target = &EX_T(opline->result.var); - - /* Initialize the sent value to NULL */ - EX_T(opline->result.var).tmp_var = EG(uninitialized_zval); + if (RETURN_VALUE_USED(opline)) { + /* If the return value of yield is used set the send + * target and initialize it to NULL */ + generator->send_target = &EX_T(opline->result.var).var.ptr; + Z_ADDREF(EG(uninitialized_zval)); + EX_T(opline->result.var).var.ptr = &EG(uninitialized_zval); + } else { + generator->send_target = NULL; + } /* We increment to the next op, so we are at the correct position when the * generator is resumed. */ @@ -28231,11 +28299,15 @@ static int ZEND_FASTCALL ZEND_YIELD_SPEC_UNUSED_VAR_HANDLER(ZEND_OPCODE_HANDLER ZVAL_LONG(generator->key, generator->largest_used_integer_key); } - /* If a value is sent it should go into the result var */ - generator->send_target = &EX_T(opline->result.var); - - /* Initialize the sent value to NULL */ - EX_T(opline->result.var).tmp_var = EG(uninitialized_zval); + if (RETURN_VALUE_USED(opline)) { + /* If the return value of yield is used set the send + * target and initialize it to NULL */ + generator->send_target = &EX_T(opline->result.var).var.ptr; + Z_ADDREF(EG(uninitialized_zval)); + EX_T(opline->result.var).var.ptr = &EG(uninitialized_zval); + } else { + generator->send_target = NULL; + } /* We increment to the next op, so we are at the correct position when the * generator is resumed. */ @@ -28653,11 +28725,15 @@ static int ZEND_FASTCALL ZEND_YIELD_SPEC_UNUSED_UNUSED_HANDLER(ZEND_OPCODE_HAND ZVAL_LONG(generator->key, generator->largest_used_integer_key); } - /* If a value is sent it should go into the result var */ - generator->send_target = &EX_T(opline->result.var); - - /* Initialize the sent value to NULL */ - EX_T(opline->result.var).tmp_var = EG(uninitialized_zval); + if (RETURN_VALUE_USED(opline)) { + /* If the return value of yield is used set the send + * target and initialize it to NULL */ + generator->send_target = &EX_T(opline->result.var).var.ptr; + Z_ADDREF(EG(uninitialized_zval)); + EX_T(opline->result.var).var.ptr = &EG(uninitialized_zval); + } else { + generator->send_target = NULL; + } /* We increment to the next op, so we are at the correct position when the * generator is resumed. */ @@ -29964,11 +30040,15 @@ static int ZEND_FASTCALL ZEND_YIELD_SPEC_UNUSED_CV_HANDLER(ZEND_OPCODE_HANDLER_ ZVAL_LONG(generator->key, generator->largest_used_integer_key); } - /* If a value is sent it should go into the result var */ - generator->send_target = &EX_T(opline->result.var); - - /* Initialize the sent value to NULL */ - EX_T(opline->result.var).tmp_var = EG(uninitialized_zval); + if (RETURN_VALUE_USED(opline)) { + /* If the return value of yield is used set the send + * target and initialize it to NULL */ + generator->send_target = &EX_T(opline->result.var).var.ptr; + Z_ADDREF(EG(uninitialized_zval)); + EX_T(opline->result.var).var.ptr = &EG(uninitialized_zval); + } else { + generator->send_target = NULL; + } /* We increment to the next op, so we are at the correct position when the * generator is resumed. */ @@ -33449,11 +33529,15 @@ static int ZEND_FASTCALL ZEND_YIELD_SPEC_CV_CONST_HANDLER(ZEND_OPCODE_HANDLER_A ZVAL_LONG(generator->key, generator->largest_used_integer_key); } - /* If a value is sent it should go into the result var */ - generator->send_target = &EX_T(opline->result.var); - - /* Initialize the sent value to NULL */ - EX_T(opline->result.var).tmp_var = EG(uninitialized_zval); + if (RETURN_VALUE_USED(opline)) { + /* If the return value of yield is used set the send + * target and initialize it to NULL */ + generator->send_target = &EX_T(opline->result.var).var.ptr; + Z_ADDREF(EG(uninitialized_zval)); + EX_T(opline->result.var).var.ptr = &EG(uninitialized_zval); + } else { + generator->send_target = NULL; + } /* We increment to the next op, so we are at the correct position when the * generator is resumed. */ @@ -35396,11 +35480,15 @@ static int ZEND_FASTCALL ZEND_YIELD_SPEC_CV_TMP_HANDLER(ZEND_OPCODE_HANDLER_ARG ZVAL_LONG(generator->key, generator->largest_used_integer_key); } - /* If a value is sent it should go into the result var */ - generator->send_target = &EX_T(opline->result.var); - - /* Initialize the sent value to NULL */ - EX_T(opline->result.var).tmp_var = EG(uninitialized_zval); + if (RETURN_VALUE_USED(opline)) { + /* If the return value of yield is used set the send + * target and initialize it to NULL */ + generator->send_target = &EX_T(opline->result.var).var.ptr; + Z_ADDREF(EG(uninitialized_zval)); + EX_T(opline->result.var).var.ptr = &EG(uninitialized_zval); + } else { + generator->send_target = NULL; + } /* We increment to the next op, so we are at the correct position when the * generator is resumed. */ @@ -37725,11 +37813,15 @@ static int ZEND_FASTCALL ZEND_YIELD_SPEC_CV_VAR_HANDLER(ZEND_OPCODE_HANDLER_ARG ZVAL_LONG(generator->key, generator->largest_used_integer_key); } - /* If a value is sent it should go into the result var */ - generator->send_target = &EX_T(opline->result.var); - - /* Initialize the sent value to NULL */ - EX_T(opline->result.var).tmp_var = EG(uninitialized_zval); + if (RETURN_VALUE_USED(opline)) { + /* If the return value of yield is used set the send + * target and initialize it to NULL */ + generator->send_target = &EX_T(opline->result.var).var.ptr; + Z_ADDREF(EG(uninitialized_zval)); + EX_T(opline->result.var).var.ptr = &EG(uninitialized_zval); + } else { + generator->send_target = NULL; + } /* We increment to the next op, so we are at the correct position when the * generator is resumed. */ @@ -38729,11 +38821,15 @@ static int ZEND_FASTCALL ZEND_YIELD_SPEC_CV_UNUSED_HANDLER(ZEND_OPCODE_HANDLER_ ZVAL_LONG(generator->key, generator->largest_used_integer_key); } - /* If a value is sent it should go into the result var */ - generator->send_target = &EX_T(opline->result.var); - - /* Initialize the sent value to NULL */ - EX_T(opline->result.var).tmp_var = EG(uninitialized_zval); + if (RETURN_VALUE_USED(opline)) { + /* If the return value of yield is used set the send + * target and initialize it to NULL */ + generator->send_target = &EX_T(opline->result.var).var.ptr; + Z_ADDREF(EG(uninitialized_zval)); + EX_T(opline->result.var).var.ptr = &EG(uninitialized_zval); + } else { + generator->send_target = NULL; + } /* We increment to the next op, so we are at the correct position when the * generator is resumed. */ @@ -40724,11 +40820,15 @@ static int ZEND_FASTCALL ZEND_YIELD_SPEC_CV_CV_HANDLER(ZEND_OPCODE_HANDLER_ARGS ZVAL_LONG(generator->key, generator->largest_used_integer_key); } - /* If a value is sent it should go into the result var */ - generator->send_target = &EX_T(opline->result.var); - - /* Initialize the sent value to NULL */ - EX_T(opline->result.var).tmp_var = EG(uninitialized_zval); + if (RETURN_VALUE_USED(opline)) { + /* If the return value of yield is used set the send + * target and initialize it to NULL */ + generator->send_target = &EX_T(opline->result.var).var.ptr; + Z_ADDREF(EG(uninitialized_zval)); + EX_T(opline->result.var).var.ptr = &EG(uninitialized_zval); + } else { + generator->send_target = NULL; + } /* We increment to the next op, so we are at the correct position when the * generator is resumed. */ From b4f00be6c43c96ef214c728b76de1d077cc7d3e5 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Sat, 30 Nov 2013 13:35:33 +0100 Subject: [PATCH 2/2] Cleanup generator closing code a bit All code dealing with unfinished execution cleanup is now in a separate function (previously most of it was run even when execution was properly finished. Furthermore some code dealing with unclean shutdowns has been removed, which is no longer necessary, because we no longer try to clean up in this case. --- Zend/zend_generators.c | 141 +++++++++++++++++++++-------------------- 1 file changed, 73 insertions(+), 68 deletions(-) diff --git a/Zend/zend_generators.c b/Zend/zend_generators.c index c0116332e7b..bc3d6d9a186 100644 --- a/Zend/zend_generators.c +++ b/Zend/zend_generators.c @@ -29,6 +29,73 @@ static zend_object_handlers zend_generator_handlers; static zend_object_value zend_generator_create(zend_class_entry *class_type TSRMLS_DC); +static void zend_generator_cleanup_unfinished_execution(zend_generator *generator TSRMLS_DC) /* {{{ */ +{ + zend_execute_data *execute_data = generator->execute_data; + zend_op_array *op_array = execute_data->op_array; + + if (generator->send_target) { + Z_DELREF_PP(generator->send_target); + generator->send_target = NULL; + } + + /* Manually free loop variables, as execution couldn't reach their + * SWITCH_FREE / FREE opcodes. */ + { + /* -1 required because we want the last run opcode, not the + * next to-be-run one. */ + zend_uint op_num = execute_data->opline - op_array->opcodes - 1; + + int i; + for (i = 0; i < op_array->last_brk_cont; ++i) { + zend_brk_cont_element *brk_cont = op_array->brk_cont_array + i; + + if (brk_cont->start < 0) { + continue; + } else if (brk_cont->start > op_num) { + break; + } else if (brk_cont->brk > op_num) { + zend_op *brk_opline = op_array->opcodes + brk_cont->brk; + + switch (brk_opline->opcode) { + case ZEND_SWITCH_FREE: + { + temp_variable *var = EX_TMP_VAR(execute_data, brk_opline->op1.var); + zval_ptr_dtor(&var->var.ptr); + } + break; + case ZEND_FREE: + { + temp_variable *var = EX_TMP_VAR(execute_data, brk_opline->op1.var); + zval_dtor(&var->tmp_var); + } + break; + } + } + } + } + + /* Clear any backed up stack arguments */ + { + void **ptr = generator->stack->top - 1; + void **end = zend_vm_stack_frame_base(execute_data); + + for (; ptr >= end; --ptr) { + zval_ptr_dtor((zval **) ptr); + } + } + + /* If yield was used as a function argument there may be active + * method calls those objects need to be freed */ + while (execute_data->call >= execute_data->call_slots) { + if (execute_data->call->object) { + zval_ptr_dtor(&execute_data->call->object); + } + execute_data->call--; + } +} +/* }}} */ + ZEND_API void zend_generator_close(zend_generator *generator, zend_bool finished_execution TSRMLS_DC) /* {{{ */ { if (generator->value) { @@ -55,76 +122,12 @@ ZEND_API void zend_generator_close(zend_generator *generator, zend_bool finished zval_ptr_dtor(&execute_data->current_this); } - if (!finished_execution && generator->send_target) { - Z_DELREF_PP(generator->send_target); - generator->send_target = NULL; - } - /* A fatal error / die occurred during the generator execution. Trying to clean * up the stack may not be safe in this case. */ if (CG(unclean_shutdown)) { return; } - /* If the generator is closed before it can finish execution (reach - * a return statement) we have to free loop variables manually, as - * we don't know whether the SWITCH_FREE / FREE opcodes have run */ - if (!finished_execution) { - /* -1 required because we want the last run opcode, not the - * next to-be-run one. */ - zend_uint op_num = execute_data->opline - op_array->opcodes - 1; - - int i; - for (i = 0; i < op_array->last_brk_cont; ++i) { - zend_brk_cont_element *brk_cont = op_array->brk_cont_array + i; - - if (brk_cont->start < 0) { - continue; - } else if (brk_cont->start > op_num) { - break; - } else if (brk_cont->brk > op_num) { - zend_op *brk_opline = op_array->opcodes + brk_cont->brk; - - switch (brk_opline->opcode) { - case ZEND_SWITCH_FREE: - { - temp_variable *var = EX_TMP_VAR(execute_data, brk_opline->op1.var); - zval_ptr_dtor(&var->var.ptr); - } - break; - case ZEND_FREE: - { - temp_variable *var = EX_TMP_VAR(execute_data, brk_opline->op1.var); - zval_dtor(&var->tmp_var); - } - break; - } - } - } - } - - /* Clear any backed up stack arguments */ - if (generator->stack != EG(argument_stack)) { - void **ptr = generator->stack->top - 1; - void **end = zend_vm_stack_frame_base(execute_data); - - /* If the top stack element is the argument count, skip it */ - if (execute_data->function_state.arguments) { - ptr--; - } - - for (; ptr >= end; --ptr) { - zval_ptr_dtor((zval**) ptr); - } - } - - while (execute_data->call >= execute_data->call_slots) { - if (execute_data->call->object) { - zval_ptr_dtor(&execute_data->call->object); - } - execute_data->call--; - } - /* 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. */ @@ -143,6 +146,12 @@ ZEND_API void zend_generator_close(zend_generator *generator, zend_bool finished } } + /* Some cleanups are only necessary if the generator was closued + * before it could finish execution (reach a return statement). */ + if (!finished_execution) { + zend_generator_cleanup_unfinished_execution(generator TSRMLS_CC); + } + /* Free a clone of closure */ if (op_array->fn_flags & ZEND_ACC_CLOSURE) { destroy_op_array(op_array TSRMLS_CC); @@ -150,10 +159,6 @@ ZEND_API void zend_generator_close(zend_generator *generator, zend_bool finished } efree(generator->stack); - if (generator->stack == EG(argument_stack)) { - /* abnormal exit for running generator */ - EG(argument_stack) = NULL; - } generator->execute_data = NULL; } }