From c1b01f7318c4a3a7f1a587deea2e239add64fe05 Mon Sep 17 00:00:00 2001 From: Dmitry Stogov Date: Mon, 5 May 2008 11:02:46 +0000 Subject: [PATCH] - Use ZEND_FREE() opcode instead of ZEND_SWITCH_FREE(IS_TMP_VAR) - Fixed bug #44913 (Segfault when using return in combination with nested loops and continue 2) --- Zend/tests/bug44913.phpt | 17 +++++++++++++++++ Zend/zend_compile.c | 17 +++++++++++++---- Zend/zend_compile.h | 3 ++- Zend/zend_execute.c | 32 ++++++++++++++++---------------- Zend/zend_vm_def.h | 20 ++++++++++++++------ Zend/zend_vm_execute.h | 36 ++++++++++++++++++------------------ 6 files changed, 80 insertions(+), 45 deletions(-) create mode 100644 Zend/tests/bug44913.phpt diff --git a/Zend/tests/bug44913.phpt b/Zend/tests/bug44913.phpt new file mode 100644 index 0000000000..0c2551fb04 --- /dev/null +++ b/Zend/tests/bug44913.phpt @@ -0,0 +1,17 @@ +--TEST-- +Bug #44913 (Segfault when using return in combination with nested loops and continue 2) +--FILE-- + +--EXPECT-- +ok diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index 1a0fc5e323..fe16738cd5 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -2129,7 +2129,7 @@ static int generate_free_switch_expr(zend_switch_entry *switch_entry TSRMLS_DC) opline = get_next_op(CG(active_op_array) TSRMLS_CC); - opline->opcode = ZEND_SWITCH_FREE; + opline->opcode = (switch_entry->cond.op_type == IS_TMP_VAR) ? ZEND_FREE : ZEND_SWITCH_FREE; opline->op1 = switch_entry->cond; SET_UNUSED(opline->op2); opline->extended_value = 0; @@ -2148,7 +2148,7 @@ static int generate_free_foreach_copy(zend_op *foreach_copy TSRMLS_DC) /* {{{ */ opline = get_next_op(CG(active_op_array) TSRMLS_CC); - opline->opcode = ZEND_SWITCH_FREE; + opline->opcode = (foreach_copy->result.op_type == IS_TMP_VAR) ? ZEND_FREE : ZEND_SWITCH_FREE; opline->op1 = foreach_copy->result; SET_UNUSED(opline->op2); opline->extended_value = 1; @@ -2156,7 +2156,7 @@ static int generate_free_foreach_copy(zend_op *foreach_copy TSRMLS_DC) /* {{{ */ if (foreach_copy->op1.op_type != IS_UNUSED) { opline = get_next_op(CG(active_op_array) TSRMLS_CC); - opline->opcode = ZEND_SWITCH_FREE; + opline->opcode = (foreach_copy->op1.op_type == IS_TMP_VAR) ? ZEND_FREE : ZEND_SWITCH_FREE; opline->op1 = foreach_copy->op1; SET_UNUSED(opline->op2); opline->extended_value = 0; @@ -2169,6 +2169,7 @@ static int generate_free_foreach_copy(zend_op *foreach_copy TSRMLS_DC) /* {{{ */ void zend_do_return(znode *expr, int do_end_vparse TSRMLS_DC) /* {{{ */ { zend_op *opline; + int start_op_number, end_op_number; if (do_end_vparse) { if (CG(active_op_array)->return_reference && !zend_is_function_or_method_call(expr)) { @@ -2178,6 +2179,8 @@ void zend_do_return(znode *expr, int do_end_vparse TSRMLS_DC) /* {{{ */ } } + start_op_number = get_next_op_number(CG(active_op_array)); + #ifdef ZTS zend_stack_apply_with_argument(&CG(switch_cond_stack), ZEND_STACK_APPLY_TOPDOWN, (int (*)(void *element, void *)) generate_free_switch_expr TSRMLS_CC); zend_stack_apply_with_argument(&CG(foreach_copy_stack), ZEND_STACK_APPLY_TOPDOWN, (int (*)(void *element, void *)) generate_free_foreach_copy TSRMLS_CC); @@ -2186,6 +2189,12 @@ void zend_do_return(znode *expr, int do_end_vparse TSRMLS_DC) /* {{{ */ zend_stack_apply(&CG(foreach_copy_stack), ZEND_STACK_APPLY_TOPDOWN, (int (*)(void *element)) generate_free_foreach_copy); #endif + end_op_number = get_next_op_number(CG(active_op_array)); + while (start_op_number < end_op_number) { + CG(active_op_array)->opcodes[start_op_number].op1.u.EA.type = EXT_TYPE_FREE_ON_RETURN; + start_op_number++; + } + opline = get_next_op(CG(active_op_array) TSRMLS_CC); opline->opcode = ZEND_RETURN; @@ -3225,7 +3234,7 @@ void zend_do_switch_end(znode *case_list TSRMLS_DC) /* {{{ */ if (switch_entry_ptr->cond.op_type==IS_VAR || switch_entry_ptr->cond.op_type==IS_TMP_VAR) { /* emit free for the switch condition*/ opline = get_next_op(CG(active_op_array) TSRMLS_CC); - opline->opcode = ZEND_SWITCH_FREE; + opline->opcode = (switch_entry_ptr->cond.op_type == IS_TMP_VAR) ? ZEND_FREE : ZEND_SWITCH_FREE; opline->op1 = switch_entry_ptr->cond; SET_UNUSED(opline->op2); } diff --git a/Zend/zend_compile.h b/Zend/zend_compile.h index fd6746caeb..9605002c2a 100644 --- a/Zend/zend_compile.h +++ b/Zend/zend_compile.h @@ -324,7 +324,8 @@ struct _zend_execute_data { #define IS_UNUSED (1<<3) /* Unused variable */ #define IS_CV (1<<4) /* Compiled variable */ -#define EXT_TYPE_UNUSED (1<<0) +#define EXT_TYPE_UNUSED (1<<0) +#define EXT_TYPE_FREE_ON_RETURN (2<<0) #include "zend_globals.h" diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index ce7a511b5c..29494387a6 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -412,22 +412,18 @@ static inline zval *_get_obj_zval_ptr(znode *op, temp_variable *Ts, zend_free_op } /* }}} */ -static inline void zend_switch_free(temp_variable *T, int type, int extended_value TSRMLS_DC) /* {{{ */ +static inline void zend_switch_free(temp_variable *T, int extended_value TSRMLS_DC) /* {{{ */ { - if (type == IS_VAR) { - if (T->var.ptr) { - if (extended_value & ZEND_FE_RESET_VARIABLE) { /* foreach() free */ - Z_DELREF_P(T->var.ptr); - } - zval_ptr_dtor(&T->var.ptr); - } else if (!T->var.ptr_ptr) { - /* perform the equivalent of equivalent of a - * quick & silent get_zval_ptr, and FREE_OP - */ - PZVAL_UNLOCK_FREE(T->str_offset.str); + if (T->var.ptr) { + if (extended_value & ZEND_FE_RESET_VARIABLE) { /* foreach() free */ + Z_DELREF_P(T->var.ptr); } - } else { /* IS_TMP_VAR */ - zendi_zval_dtor(T->tmp_var); + zval_ptr_dtor(&T->var.ptr); + } else if (!T->var.ptr_ptr) { + /* perform the equivalent of equivalent of a + * quick & silent get_zval_ptr, and FREE_OP + */ + PZVAL_UNLOCK_FREE(T->str_offset.str); } } /* }}} */ @@ -1347,10 +1343,14 @@ static inline zend_brk_cont_element* zend_brk_cont(int nest_levels, int array_of switch (brk_opline->opcode) { case ZEND_SWITCH_FREE: - zend_switch_free(&T(brk_opline->op1.u.var), brk_opline->op1.op_type, brk_opline->extended_value TSRMLS_CC); + if (brk_opline->op1.u.EA.type != EXT_TYPE_FREE_ON_RETURN) { + zend_switch_free(&T(brk_opline->op1.u.var), brk_opline->extended_value TSRMLS_CC); + } break; case ZEND_FREE: - zendi_zval_dtor(T(brk_opline->op1.u.var).tmp_var); + if (brk_opline->op1.u.EA.type != EXT_TYPE_FREE_ON_RETURN) { + zendi_zval_dtor(T(brk_opline->op1.u.var).tmp_var); + } break; } } diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h index 424570a167..fc04af9468 100644 --- a/Zend/zend_vm_def.h +++ b/Zend/zend_vm_def.h @@ -2702,10 +2702,14 @@ ZEND_VM_HANDLER(100, ZEND_GOTO, ANY, CONST) switch (brk_opline->opcode) { case ZEND_SWITCH_FREE: - zend_switch_free(&EX_T(brk_opline->op1.u.var), brk_opline->op1.op_type, brk_opline->extended_value TSRMLS_CC); + if (brk_opline->op1.u.EA.type != EXT_TYPE_FREE_ON_RETURN) { + zend_switch_free(&EX_T(brk_opline->op1.u.var), brk_opline->extended_value TSRMLS_CC); + } break; case ZEND_FREE: - zendi_zval_dtor(EX_T(brk_opline->op1.u.var).tmp_var); + if (brk_opline->op1.u.EA.type != EXT_TYPE_FREE_ON_RETURN) { + zendi_zval_dtor(EX_T(brk_opline->op1.u.var).tmp_var); + } break; } ZEND_VM_JMP(opline->op1.u.jmp_addr); @@ -2743,11 +2747,11 @@ ZEND_VM_HANDLER(48, ZEND_CASE, CONST|TMP|VAR|CV, CONST|TMP|VAR|CV) ZEND_VM_NEXT_OPCODE(); } -ZEND_VM_HANDLER(49, ZEND_SWITCH_FREE, TMP|VAR, ANY) +ZEND_VM_HANDLER(49, ZEND_SWITCH_FREE, VAR, ANY) { zend_op *opline = EX(opline); - zend_switch_free(&EX_T(opline->op1.u.var), OP1_TYPE, opline->extended_value TSRMLS_CC); + zend_switch_free(&EX_T(opline->op1.u.var), opline->extended_value TSRMLS_CC); ZEND_VM_NEXT_OPCODE(); } @@ -4287,10 +4291,14 @@ ZEND_VM_HANDLER(149, ZEND_HANDLE_EXCEPTION, ANY, ANY) switch (brk_opline->opcode) { case ZEND_SWITCH_FREE: - zend_switch_free(&EX_T(brk_opline->op1.u.var), brk_opline->op1.op_type, brk_opline->extended_value TSRMLS_CC); + if (brk_opline->op1.u.EA.type != EXT_TYPE_FREE_ON_RETURN) { + zend_switch_free(&EX_T(brk_opline->op1.u.var), brk_opline->extended_value TSRMLS_CC); + } break; case ZEND_FREE: - zendi_zval_dtor(EX_T(brk_opline->op1.u.var).tmp_var); + if (brk_opline->op1.u.EA.type != EXT_TYPE_FREE_ON_RETURN) { + zendi_zval_dtor(EX_T(brk_opline->op1.u.var).tmp_var); + } break; } } diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h index cc47d898ea..9280fc88ba 100644 --- a/Zend/zend_vm_execute.h +++ b/Zend/zend_vm_execute.h @@ -523,10 +523,14 @@ static int ZEND_HANDLE_EXCEPTION_SPEC_HANDLER(ZEND_OPCODE_HANDLER_ARGS) switch (brk_opline->opcode) { case ZEND_SWITCH_FREE: - zend_switch_free(&EX_T(brk_opline->op1.u.var), brk_opline->op1.op_type, brk_opline->extended_value TSRMLS_CC); + if (brk_opline->op1.u.EA.type != EXT_TYPE_FREE_ON_RETURN) { + zend_switch_free(&EX_T(brk_opline->op1.u.var), brk_opline->extended_value TSRMLS_CC); + } break; case ZEND_FREE: - zendi_zval_dtor(EX_T(brk_opline->op1.u.var).tmp_var); + if (brk_opline->op1.u.EA.type != EXT_TYPE_FREE_ON_RETURN) { + zendi_zval_dtor(EX_T(brk_opline->op1.u.var).tmp_var); + } break; } } @@ -733,10 +737,14 @@ static int ZEND_GOTO_SPEC_CONST_HANDLER(ZEND_OPCODE_HANDLER_ARGS) switch (brk_opline->opcode) { case ZEND_SWITCH_FREE: - zend_switch_free(&EX_T(brk_opline->op1.u.var), brk_opline->op1.op_type, brk_opline->extended_value TSRMLS_CC); + if (brk_opline->op1.u.EA.type != EXT_TYPE_FREE_ON_RETURN) { + zend_switch_free(&EX_T(brk_opline->op1.u.var), brk_opline->extended_value TSRMLS_CC); + } break; case ZEND_FREE: - zendi_zval_dtor(EX_T(brk_opline->op1.u.var).tmp_var); + if (brk_opline->op1.u.EA.type != EXT_TYPE_FREE_ON_RETURN) { + zendi_zval_dtor(EX_T(brk_opline->op1.u.var).tmp_var); + } break; } ZEND_VM_JMP(opline->op1.u.jmp_addr); @@ -4868,14 +4876,6 @@ static int ZEND_BOOL_SPEC_TMP_HANDLER(ZEND_OPCODE_HANDLER_ARGS) ZEND_VM_NEXT_OPCODE(); } -static int ZEND_SWITCH_FREE_SPEC_TMP_HANDLER(ZEND_OPCODE_HANDLER_ARGS) -{ - zend_op *opline = EX(opline); - - zend_switch_free(&EX_T(opline->op1.u.var), IS_TMP_VAR, opline->extended_value TSRMLS_CC); - ZEND_VM_NEXT_OPCODE(); -} - static int ZEND_CLONE_SPEC_TMP_HANDLER(ZEND_OPCODE_HANDLER_ARGS) { zend_op *opline = EX(opline); @@ -8266,7 +8266,7 @@ static int ZEND_SWITCH_FREE_SPEC_VAR_HANDLER(ZEND_OPCODE_HANDLER_ARGS) { zend_op *opline = EX(opline); - zend_switch_free(&EX_T(opline->op1.u.var), IS_VAR, opline->extended_value TSRMLS_CC); + zend_switch_free(&EX_T(opline->op1.u.var), opline->extended_value TSRMLS_CC); ZEND_VM_NEXT_OPCODE(); } @@ -31948,11 +31948,11 @@ void zend_init_opcodes_handlers(void) ZEND_NULL_HANDLER, ZEND_NULL_HANDLER, ZEND_NULL_HANDLER, - ZEND_SWITCH_FREE_SPEC_TMP_HANDLER, - ZEND_SWITCH_FREE_SPEC_TMP_HANDLER, - ZEND_SWITCH_FREE_SPEC_TMP_HANDLER, - ZEND_SWITCH_FREE_SPEC_TMP_HANDLER, - ZEND_SWITCH_FREE_SPEC_TMP_HANDLER, + ZEND_NULL_HANDLER, + ZEND_NULL_HANDLER, + ZEND_NULL_HANDLER, + ZEND_NULL_HANDLER, + ZEND_NULL_HANDLER, ZEND_SWITCH_FREE_SPEC_VAR_HANDLER, ZEND_SWITCH_FREE_SPEC_VAR_HANDLER, ZEND_SWITCH_FREE_SPEC_VAR_HANDLER, -- 2.40.0