From: Dmitry Stogov Date: Mon, 5 May 2008 11:03:35 +0000 (+0000) Subject: - Use ZEND_FREE() opcode instead of ZEND_SWITCH_FREE(IS_TMP_VAR) X-Git-Tag: BEFORE_NEW_PARAMETER_PARSE~282 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=1b317f15264e72470291885e2da81d19873e1074;p=php - 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) --- diff --git a/NEWS b/NEWS index 25cacc7914..522a336229 100644 --- a/NEWS +++ b/NEWS @@ -109,6 +109,7 @@ - Added Windows support for asinh(), acosh(), atanh(), log1p() and expm1() (Kalle) - Improved PHP runtime speed and memory usage: + . Use ZEND_FREE() opcode instead of ZEND_SWITCH_FREE(IS_TMP_VAR). (Dmitry) . Lazy EG(active_symbol_table) initialization. (Dmitry) . Optimized ZEND_RETURN opcode to not allocate and copy return value if it is not used. (Dmitry) @@ -169,6 +170,8 @@ - Fixed an issue in date() where a : was printed for the O modifier after a P modifier was used. (Derick) +- Fixed bug #44913 (Segfault when using return in combination with nested loops + and continue 2). (Dmitry) - Fixed bug #44899 (__isset usage changes behavior of empty()) (Etienne) - Fixed bug #44805 (rename() function is not portable to Windows). (Pierre) - Fixed bug #44742 (timezone_offset_get() causes segmentation faults). (Derick) 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 09bf537156..8b3b15af4b 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -2024,7 +2024,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; @@ -2042,7 +2042,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; @@ -2050,7 +2050,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; @@ -2062,6 +2062,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)) { @@ -2071,6 +2072,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); @@ -2079,6 +2082,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; @@ -3075,7 +3084,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 8e06da218a..e6340d9dde 100644 --- a/Zend/zend_compile.h +++ b/Zend/zend_compile.h @@ -323,7 +323,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 2391fc08cc..bf8884907c 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -377,22 +377,18 @@ static inline zval *_get_obj_zval_ptr(znode *op, temp_variable *Ts, zend_free_op return get_zval_ptr(op, Ts, should_free, type); } -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); } } @@ -1241,10 +1237,14 @@ static inline zend_brk_cont_element* zend_brk_cont(zval *nest_levels_zval, int a 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 bbe1ee6b4c..df2d6e2665 100644 --- a/Zend/zend_vm_def.h +++ b/Zend/zend_vm_def.h @@ -2642,10 +2642,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); @@ -2683,11 +2687,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(); } @@ -4123,10 +4127,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 0d29a570d7..e23f8d344a 100644 --- a/Zend/zend_vm_execute.h +++ b/Zend/zend_vm_execute.h @@ -515,7 +515,7 @@ 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); + 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); @@ -724,7 +724,7 @@ 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); + 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); @@ -4735,14 +4735,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); @@ -7990,7 +7982,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(); } @@ -30747,11 +30739,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,