From: Dmitry Stogov Date: Tue, 10 Mar 2015 20:04:41 +0000 (+0300) Subject: More accurate reference counting on closures X-Git-Tag: PRE_PHP7_NSAPI_REMOVAL~714 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=ed9c8a23ba573b3e81399a158f3dee4e9708b6ad;p=php More accurate reference counting on closures --- diff --git a/Zend/tests/closure_017.phpt b/Zend/tests/closure_017.phpt index 45a07f9441..85c7eca06a 100644 --- a/Zend/tests/closure_017.phpt +++ b/Zend/tests/closure_017.phpt @@ -8,5 +8,6 @@ $a = function(&$a) { $a = 1; }; $a($a); ?> ---EXPECTF-- -Fatal error: Cannot destroy active lambda function in %s on line %d +DONE +--EXPECT-- +DONE diff --git a/Zend/zend_closures.c b/Zend/zend_closures.c index 5904b8473e..84e35a85d5 100644 --- a/Zend/zend_closures.c +++ b/Zend/zend_closures.c @@ -484,7 +484,7 @@ ZEND_API void zend_create_closure(zval *res, zend_function *func, zend_class_ent closure = (zend_closure *)Z_OBJ_P(res); closure->func = *func; - closure->func.common.prototype = NULL; + closure->func.common.prototype = (zend_function*)closure; closure->func.common.fn_flags |= ZEND_ACC_CLOSURE; if ((scope == NULL) && this_ptr && (Z_TYPE_P(this_ptr) != IS_UNDEF)) { diff --git a/Zend/zend_execute_API.c b/Zend/zend_execute_API.c index 7cee3ba73d..c3bd69a5b1 100644 --- a/Zend/zend_execute_API.c +++ b/Zend/zend_execute_API.c @@ -857,6 +857,10 @@ int zend_call_function(zend_fcall_info *fci, zend_fcall_info_cache *fci_cache) / if (func->type == ZEND_USER_FUNCTION) { EG(scope) = func->common.scope; call->symbol_table = fci->symbol_table; + if (UNEXPECTED(func->op_array.fn_flags & ZEND_ACC_CLOSURE)) { + ZEND_ASSERT(GC_TYPE(func->op_array.prototype) == IS_OBJECT); + GC_REFCOUNT(func->op_array.prototype)++; + } if (EXPECTED((func->op_array.fn_flags & ZEND_ACC_GENERATOR) == 0)) { zend_init_execute_data(call, &func->op_array, fci->retval); zend_execute_ex(call); diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h index 56661276e5..e648938867 100644 --- a/Zend/zend_vm_def.h +++ b/Zend/zend_vm_def.h @@ -1995,7 +1995,7 @@ ZEND_VM_HELPER(zend_leave_helper, ANY, ANY) zend_vm_stack_free_extra_args(execute_data); old_execute_data = execute_data; execute_data = EG(current_execute_data) = EX(prev_execute_data); - if (UNEXPECTED((old_execute_data->func->op_array.fn_flags & ZEND_ACC_CLOSURE) != 0) && old_execute_data->func->op_array.prototype) { + if (UNEXPECTED(old_execute_data->func->op_array.fn_flags & ZEND_ACC_CLOSURE)) { OBJ_RELEASE((zend_object*)old_execute_data->func->op_array.prototype); } object = Z_OBJ(old_execute_data->This); @@ -2051,7 +2051,7 @@ ZEND_VM_HELPER(zend_leave_helper, ANY, ANY) } zend_vm_stack_free_extra_args(execute_data); EG(current_execute_data) = EX(prev_execute_data); - if ((EX(func)->op_array.fn_flags & ZEND_ACC_CLOSURE) && EX(func)->op_array.prototype) { + if (EX(func)->op_array.fn_flags & ZEND_ACC_CLOSURE) { OBJ_RELEASE((zend_object*)EX(func)->op_array.prototype); } } else /* if (call_kind == ZEND_CALL_TOP_CODE) */ { @@ -2699,12 +2699,12 @@ ZEND_VM_C_LABEL(try_function_name): if (object) { GC_REFCOUNT(object)++; } - if (OP2_TYPE == IS_VAR && (fbc->common.fn_flags & ZEND_ACC_CLOSURE)) { + if (fbc->common.fn_flags & ZEND_ACC_CLOSURE) { /* Delay closure destruction until its invocation */ - fbc->common.prototype = (zend_function*)Z_OBJ_P(free_op2); - } else if (OP2_TYPE == IS_CV) { - FREE_OP2(); + ZEND_ASSERT(GC_TYPE(fbc->common.prototype) == IS_OBJECT); + GC_REFCOUNT(fbc->common.prototype)++; } + FREE_OP2(); } else if (EXPECTED(Z_TYPE_P(function_name) == IS_ARRAY) && zend_hash_num_elements(Z_ARRVAL_P(function_name)) == 2) { zval *obj; @@ -2818,8 +2818,11 @@ ZEND_VM_HANDLER(118, ZEND_INIT_USER_CALL, CONST, CONST|TMPVAR|CV) func = fcc.function_handler; if (func->common.fn_flags & ZEND_ACC_CLOSURE) { /* Delay closure destruction until its invocation */ - func->common.prototype = (zend_function*)Z_OBJ_P(function_name); - Z_ADDREF_P(function_name); + if (OP2_TYPE & (IS_VAR|IS_CV)) { + ZVAL_DEREF(function_name); + } + ZEND_ASSERT(GC_TYPE(func->common.prototype) == IS_OBJECT); + GC_REFCOUNT(func->common.prototype)++; } called_scope = fcc.called_scope; object = fcc.object; @@ -4453,6 +4456,7 @@ ZEND_VM_HANDLER(72, ZEND_ADD_ARRAY_ELEMENT, CONST|TMP|VAR|CV, CONST|TMPVAR|UNUSE if (OP1_TYPE == IS_VAR && UNEXPECTED(expr_ptr == NULL)) { zend_error(E_EXCEPTION | E_ERROR, "Cannot create references to/from string offsets"); FREE_OP1_VAR_PTR(); + zend_array_destroy(Z_ARRVAL_P(EX_VAR(opline->result.var))); HANDLE_EXCEPTION(); } ZVAL_MAKE_REF(expr_ptr); diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h index 48665b81af..aa1cc1ae59 100644 --- a/Zend/zend_vm_execute.h +++ b/Zend/zend_vm_execute.h @@ -396,7 +396,7 @@ static int ZEND_FASTCALL zend_leave_helper_SPEC(ZEND_OPCODE_HANDLER_ARGS) zend_vm_stack_free_extra_args(execute_data); old_execute_data = execute_data; execute_data = EG(current_execute_data) = EX(prev_execute_data); - if (UNEXPECTED((old_execute_data->func->op_array.fn_flags & ZEND_ACC_CLOSURE) != 0) && old_execute_data->func->op_array.prototype) { + if (UNEXPECTED(old_execute_data->func->op_array.fn_flags & ZEND_ACC_CLOSURE)) { OBJ_RELEASE((zend_object*)old_execute_data->func->op_array.prototype); } object = Z_OBJ(old_execute_data->This); @@ -452,7 +452,7 @@ static int ZEND_FASTCALL zend_leave_helper_SPEC(ZEND_OPCODE_HANDLER_ARGS) } zend_vm_stack_free_extra_args(execute_data); EG(current_execute_data) = EX(prev_execute_data); - if ((EX(func)->op_array.fn_flags & ZEND_ACC_CLOSURE) && EX(func)->op_array.prototype) { + if (EX(func)->op_array.fn_flags & ZEND_ACC_CLOSURE) { OBJ_RELEASE((zend_object*)EX(func)->op_array.prototype); } } else /* if (call_kind == ZEND_CALL_TOP_CODE) */ { @@ -1792,7 +1792,7 @@ static int ZEND_FASTCALL ZEND_INIT_DYNAMIC_CALL_SPEC_CONST_HANDLER(ZEND_OPCODE_ zend_function *fbc; zval *function_name, *func; zend_string *lcname; - zend_free_op free_op2; + zend_class_entry *called_scope; zend_object *object; @@ -1824,12 +1824,12 @@ try_function_name: if (object) { GC_REFCOUNT(object)++; } - if (IS_CONST == IS_VAR && (fbc->common.fn_flags & ZEND_ACC_CLOSURE)) { + if (fbc->common.fn_flags & ZEND_ACC_CLOSURE) { /* Delay closure destruction until its invocation */ - fbc->common.prototype = (zend_function*)Z_OBJ_P(free_op2); - } else if (IS_CONST == IS_CV) { - + ZEND_ASSERT(GC_TYPE(fbc->common.prototype) == IS_OBJECT); + GC_REFCOUNT(fbc->common.prototype)++; } + } else if (EXPECTED(Z_TYPE_P(function_name) == IS_ARRAY) && zend_hash_num_elements(Z_ARRVAL_P(function_name)) == 2) { zval *obj; @@ -2179,7 +2179,7 @@ static int ZEND_FASTCALL ZEND_INIT_DYNAMIC_CALL_SPEC_CV_HANDLER(ZEND_OPCODE_HAN zend_function *fbc; zval *function_name, *func; zend_string *lcname; - zend_free_op free_op2; + zend_class_entry *called_scope; zend_object *object; @@ -2211,12 +2211,12 @@ try_function_name: if (object) { GC_REFCOUNT(object)++; } - if (IS_CV == IS_VAR && (fbc->common.fn_flags & ZEND_ACC_CLOSURE)) { + if (fbc->common.fn_flags & ZEND_ACC_CLOSURE) { /* Delay closure destruction until its invocation */ - fbc->common.prototype = (zend_function*)Z_OBJ_P(free_op2); - } else if (IS_CV == IS_CV) { - + ZEND_ASSERT(GC_TYPE(fbc->common.prototype) == IS_OBJECT); + GC_REFCOUNT(fbc->common.prototype)++; } + } else if (EXPECTED(Z_TYPE_P(function_name) == IS_ARRAY) && zend_hash_num_elements(Z_ARRVAL_P(function_name)) == 2) { zval *obj; @@ -2395,12 +2395,12 @@ try_function_name: if (object) { GC_REFCOUNT(object)++; } - if ((IS_TMP_VAR|IS_VAR) == IS_VAR && (fbc->common.fn_flags & ZEND_ACC_CLOSURE)) { + if (fbc->common.fn_flags & ZEND_ACC_CLOSURE) { /* Delay closure destruction until its invocation */ - fbc->common.prototype = (zend_function*)Z_OBJ_P(free_op2); - } else if ((IS_TMP_VAR|IS_VAR) == IS_CV) { - zval_ptr_dtor_nogc(free_op2); + ZEND_ASSERT(GC_TYPE(fbc->common.prototype) == IS_OBJECT); + GC_REFCOUNT(fbc->common.prototype)++; } + zval_ptr_dtor_nogc(free_op2); } else if (EXPECTED(Z_TYPE_P(function_name) == IS_ARRAY) && zend_hash_num_elements(Z_ARRVAL_P(function_name)) == 2) { zval *obj; @@ -4685,8 +4685,11 @@ static int ZEND_FASTCALL ZEND_INIT_USER_CALL_SPEC_CONST_CONST_HANDLER(ZEND_OPCO func = fcc.function_handler; if (func->common.fn_flags & ZEND_ACC_CLOSURE) { /* Delay closure destruction until its invocation */ - func->common.prototype = (zend_function*)Z_OBJ_P(function_name); - Z_ADDREF_P(function_name); + if (IS_CONST & (IS_VAR|IS_CV)) { + ZVAL_DEREF(function_name); + } + ZEND_ASSERT(GC_TYPE(func->common.prototype) == IS_OBJECT); + GC_REFCOUNT(func->common.prototype)++; } called_scope = fcc.called_scope; object = fcc.object; @@ -4851,6 +4854,7 @@ static int ZEND_FASTCALL ZEND_ADD_ARRAY_ELEMENT_SPEC_CONST_CONST_HANDLER(ZEND_O if (IS_CONST == IS_VAR && UNEXPECTED(expr_ptr == NULL)) { zend_error(E_EXCEPTION | E_ERROR, "Cannot create references to/from string offsets"); + zend_array_destroy(Z_ARRVAL_P(EX_VAR(opline->result.var))); HANDLE_EXCEPTION(); } ZVAL_MAKE_REF(expr_ptr); @@ -6544,6 +6548,7 @@ static int ZEND_FASTCALL ZEND_ADD_ARRAY_ELEMENT_SPEC_CONST_UNUSED_HANDLER(ZEND_ if (IS_CONST == IS_VAR && UNEXPECTED(expr_ptr == NULL)) { zend_error(E_EXCEPTION | E_ERROR, "Cannot create references to/from string offsets"); + zend_array_destroy(Z_ARRVAL_P(EX_VAR(opline->result.var))); HANDLE_EXCEPTION(); } ZVAL_MAKE_REF(expr_ptr); @@ -7673,8 +7678,11 @@ static int ZEND_FASTCALL ZEND_INIT_USER_CALL_SPEC_CONST_CV_HANDLER(ZEND_OPCODE_ func = fcc.function_handler; if (func->common.fn_flags & ZEND_ACC_CLOSURE) { /* Delay closure destruction until its invocation */ - func->common.prototype = (zend_function*)Z_OBJ_P(function_name); - Z_ADDREF_P(function_name); + if (IS_CV & (IS_VAR|IS_CV)) { + ZVAL_DEREF(function_name); + } + ZEND_ASSERT(GC_TYPE(func->common.prototype) == IS_OBJECT); + GC_REFCOUNT(func->common.prototype)++; } called_scope = fcc.called_scope; object = fcc.object; @@ -7789,6 +7797,7 @@ static int ZEND_FASTCALL ZEND_ADD_ARRAY_ELEMENT_SPEC_CONST_CV_HANDLER(ZEND_OPCO if (IS_CONST == IS_VAR && UNEXPECTED(expr_ptr == NULL)) { zend_error(E_EXCEPTION | E_ERROR, "Cannot create references to/from string offsets"); + zend_array_destroy(Z_ARRVAL_P(EX_VAR(opline->result.var))); HANDLE_EXCEPTION(); } ZVAL_MAKE_REF(expr_ptr); @@ -8860,8 +8869,11 @@ static int ZEND_FASTCALL ZEND_INIT_USER_CALL_SPEC_CONST_TMPVAR_HANDLER(ZEND_OPC func = fcc.function_handler; if (func->common.fn_flags & ZEND_ACC_CLOSURE) { /* Delay closure destruction until its invocation */ - func->common.prototype = (zend_function*)Z_OBJ_P(function_name); - Z_ADDREF_P(function_name); + if ((IS_TMP_VAR|IS_VAR) & (IS_VAR|IS_CV)) { + ZVAL_DEREF(function_name); + } + ZEND_ASSERT(GC_TYPE(func->common.prototype) == IS_OBJECT); + GC_REFCOUNT(func->common.prototype)++; } called_scope = fcc.called_scope; object = fcc.object; @@ -8927,6 +8939,7 @@ static int ZEND_FASTCALL ZEND_ADD_ARRAY_ELEMENT_SPEC_CONST_TMPVAR_HANDLER(ZEND_ if (IS_CONST == IS_VAR && UNEXPECTED(expr_ptr == NULL)) { zend_error(E_EXCEPTION | E_ERROR, "Cannot create references to/from string offsets"); + zend_array_destroy(Z_ARRVAL_P(EX_VAR(opline->result.var))); HANDLE_EXCEPTION(); } ZVAL_MAKE_REF(expr_ptr); @@ -10136,6 +10149,7 @@ static int ZEND_FASTCALL ZEND_ADD_ARRAY_ELEMENT_SPEC_TMP_CONST_HANDLER(ZEND_OPC if (IS_TMP_VAR == IS_VAR && UNEXPECTED(expr_ptr == NULL)) { zend_error(E_EXCEPTION | E_ERROR, "Cannot create references to/from string offsets"); + zend_array_destroy(Z_ARRVAL_P(EX_VAR(opline->result.var))); HANDLE_EXCEPTION(); } ZVAL_MAKE_REF(expr_ptr); @@ -10793,6 +10807,7 @@ static int ZEND_FASTCALL ZEND_ADD_ARRAY_ELEMENT_SPEC_TMP_UNUSED_HANDLER(ZEND_OP if (IS_TMP_VAR == IS_VAR && UNEXPECTED(expr_ptr == NULL)) { zend_error(E_EXCEPTION | E_ERROR, "Cannot create references to/from string offsets"); + zend_array_destroy(Z_ARRVAL_P(EX_VAR(opline->result.var))); HANDLE_EXCEPTION(); } ZVAL_MAKE_REF(expr_ptr); @@ -11270,6 +11285,7 @@ static int ZEND_FASTCALL ZEND_ADD_ARRAY_ELEMENT_SPEC_TMP_CV_HANDLER(ZEND_OPCODE if (IS_TMP_VAR == IS_VAR && UNEXPECTED(expr_ptr == NULL)) { zend_error(E_EXCEPTION | E_ERROR, "Cannot create references to/from string offsets"); + zend_array_destroy(Z_ARRVAL_P(EX_VAR(opline->result.var))); HANDLE_EXCEPTION(); } ZVAL_MAKE_REF(expr_ptr); @@ -11718,6 +11734,7 @@ static int ZEND_FASTCALL ZEND_ADD_ARRAY_ELEMENT_SPEC_TMP_TMPVAR_HANDLER(ZEND_OP if (IS_TMP_VAR == IS_VAR && UNEXPECTED(expr_ptr == NULL)) { zend_error(E_EXCEPTION | E_ERROR, "Cannot create references to/from string offsets"); + zend_array_destroy(Z_ARRVAL_P(EX_VAR(opline->result.var))); HANDLE_EXCEPTION(); } ZVAL_MAKE_REF(expr_ptr); @@ -14581,6 +14598,7 @@ static int ZEND_FASTCALL ZEND_ADD_ARRAY_ELEMENT_SPEC_VAR_CONST_HANDLER(ZEND_OPC if (IS_VAR == IS_VAR && UNEXPECTED(expr_ptr == NULL)) { zend_error(E_EXCEPTION | E_ERROR, "Cannot create references to/from string offsets"); if (free_op1) {zval_ptr_dtor_nogc(free_op1);}; + zend_array_destroy(Z_ARRVAL_P(EX_VAR(opline->result.var))); HANDLE_EXCEPTION(); } ZVAL_MAKE_REF(expr_ptr); @@ -16073,6 +16091,7 @@ static int ZEND_FASTCALL ZEND_ADD_ARRAY_ELEMENT_SPEC_VAR_UNUSED_HANDLER(ZEND_OP if (IS_VAR == IS_VAR && UNEXPECTED(expr_ptr == NULL)) { zend_error(E_EXCEPTION | E_ERROR, "Cannot create references to/from string offsets"); if (free_op1) {zval_ptr_dtor_nogc(free_op1);}; + zend_array_destroy(Z_ARRVAL_P(EX_VAR(opline->result.var))); HANDLE_EXCEPTION(); } ZVAL_MAKE_REF(expr_ptr); @@ -17636,6 +17655,7 @@ static int ZEND_FASTCALL ZEND_ADD_ARRAY_ELEMENT_SPEC_VAR_CV_HANDLER(ZEND_OPCODE if (IS_VAR == IS_VAR && UNEXPECTED(expr_ptr == NULL)) { zend_error(E_EXCEPTION | E_ERROR, "Cannot create references to/from string offsets"); if (free_op1) {zval_ptr_dtor_nogc(free_op1);}; + zend_array_destroy(Z_ARRVAL_P(EX_VAR(opline->result.var))); HANDLE_EXCEPTION(); } ZVAL_MAKE_REF(expr_ptr); @@ -19193,6 +19213,7 @@ static int ZEND_FASTCALL ZEND_ADD_ARRAY_ELEMENT_SPEC_VAR_TMPVAR_HANDLER(ZEND_OP if (IS_VAR == IS_VAR && UNEXPECTED(expr_ptr == NULL)) { zend_error(E_EXCEPTION | E_ERROR, "Cannot create references to/from string offsets"); if (free_op1) {zval_ptr_dtor_nogc(free_op1);}; + zend_array_destroy(Z_ARRVAL_P(EX_VAR(opline->result.var))); HANDLE_EXCEPTION(); } ZVAL_MAKE_REF(expr_ptr); @@ -27825,6 +27846,7 @@ static int ZEND_FASTCALL ZEND_ADD_ARRAY_ELEMENT_SPEC_CV_CONST_HANDLER(ZEND_OPCO if (IS_CV == IS_VAR && UNEXPECTED(expr_ptr == NULL)) { zend_error(E_EXCEPTION | E_ERROR, "Cannot create references to/from string offsets"); + zend_array_destroy(Z_ARRVAL_P(EX_VAR(opline->result.var))); HANDLE_EXCEPTION(); } ZVAL_MAKE_REF(expr_ptr); @@ -30194,6 +30216,7 @@ static int ZEND_FASTCALL ZEND_ADD_ARRAY_ELEMENT_SPEC_CV_UNUSED_HANDLER(ZEND_OPC if (IS_CV == IS_VAR && UNEXPECTED(expr_ptr == NULL)) { zend_error(E_EXCEPTION | E_ERROR, "Cannot create references to/from string offsets"); + zend_array_destroy(Z_ARRVAL_P(EX_VAR(opline->result.var))); HANDLE_EXCEPTION(); } ZVAL_MAKE_REF(expr_ptr); @@ -32244,6 +32267,7 @@ static int ZEND_FASTCALL ZEND_ADD_ARRAY_ELEMENT_SPEC_CV_CV_HANDLER(ZEND_OPCODE_ if (IS_CV == IS_VAR && UNEXPECTED(expr_ptr == NULL)) { zend_error(E_EXCEPTION | E_ERROR, "Cannot create references to/from string offsets"); + zend_array_destroy(Z_ARRVAL_P(EX_VAR(opline->result.var))); HANDLE_EXCEPTION(); } ZVAL_MAKE_REF(expr_ptr); @@ -34304,6 +34328,7 @@ static int ZEND_FASTCALL ZEND_ADD_ARRAY_ELEMENT_SPEC_CV_TMPVAR_HANDLER(ZEND_OPC if (IS_CV == IS_VAR && UNEXPECTED(expr_ptr == NULL)) { zend_error(E_EXCEPTION | E_ERROR, "Cannot create references to/from string offsets"); + zend_array_destroy(Z_ARRVAL_P(EX_VAR(opline->result.var))); HANDLE_EXCEPTION(); } ZVAL_MAKE_REF(expr_ptr); diff --git a/tests/output/bug65593.phpt b/tests/output/bug65593.phpt index 0a59554967..9ae62a1d72 100644 --- a/tests/output/bug65593.phpt +++ b/tests/output/bug65593.phpt @@ -10,4 +10,4 @@ ob_start(function(){ob_start();}); --EXPECTF-- Test -Fatal error: Cannot destroy active lambda function in %sbug65593.php on line %d +Catchable fatal error: ob_start(): Cannot use output buffering in output buffering display handlers in %sbug65593.php on line %d