From: Dmitry Stogov Date: Tue, 7 Jun 2016 23:20:45 +0000 (+0300) Subject: Better fix for bug #70228 (memleak if return in finally block) X-Git-Tag: php-7.1.0alpha3~42^2~50 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=10f056255efc741654393c21b234d29333489354;p=php Better fix for bug #70228 (memleak if return in finally block) --- diff --git a/Zend/tests/try/bug70228_5.phpt b/Zend/tests/try/bug70228_5.phpt new file mode 100644 index 0000000000..29cbf4910d --- /dev/null +++ b/Zend/tests/try/bug70228_5.phpt @@ -0,0 +1,20 @@ +--TEST-- +Bug #70228 (memleak if return hidden by throw in finally block) +--FILE-- +getMessage(), "\n"; +} +?> +--EXPECT-- +ops diff --git a/Zend/tests/try/bug70228_6.phpt b/Zend/tests/try/bug70228_6.phpt new file mode 100644 index 0000000000..fc68657f4c --- /dev/null +++ b/Zend/tests/try/bug70228_6.phpt @@ -0,0 +1,18 @@ +--TEST-- +Bug #70228 (memleak if return in finally block) +--FILE-- + +--EXPECT-- +int(42) diff --git a/Zend/tests/try/bug70228_7.phpt b/Zend/tests/try/bug70228_7.phpt new file mode 100644 index 0000000000..4b8a80351c --- /dev/null +++ b/Zend/tests/try/bug70228_7.phpt @@ -0,0 +1,29 @@ +--TEST-- +Bug #70228 (memleak if return in finally block) +--FILE-- + +===DONE=== +--EXPECT-- +int(1) +int(2) +int(3) +===DONE=== diff --git a/Zend/tests/try/bug70228_8.phpt b/Zend/tests/try/bug70228_8.phpt new file mode 100644 index 0000000000..ad13113c71 --- /dev/null +++ b/Zend/tests/try/bug70228_8.phpt @@ -0,0 +1,30 @@ +--TEST-- +Bug #70228 (memleak if return in finally block) +--FILE-- + +===DONE=== +--EXPECT-- +int(1) +int(2) +int(3) +===DONE=== diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index ce8224652d..30d28cd842 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -4000,7 +4000,7 @@ void zend_compile_unset(zend_ast *ast) /* {{{ */ } /* }}} */ -static int zend_handle_loops_and_finally_ex(zend_long depth) /* {{{ */ +static int zend_handle_loops_and_finally_ex(zend_long depth, znode *return_value) /* {{{ */ { zend_loop_var *base; zend_loop_var *loop_var = zend_stack_top(&CG(loop_var_stack)); @@ -4017,7 +4017,11 @@ static int zend_handle_loops_and_finally_ex(zend_long depth) /* {{{ */ opline->result_type = IS_TMP_VAR; opline->result.var = loop_var->var_num; SET_UNUSED(opline->op1); - SET_UNUSED(opline->op2); + if (return_value) { + SET_NODE(opline->op2, return_value); + } else { + SET_UNUSED(opline->op2); + } opline->op1.num = loop_var->u.try_catch_offset; } else if (loop_var->opcode == ZEND_DISCARD_EXCEPTION) { zend_op *opline = get_next_op(CG(active_op_array)); @@ -4051,9 +4055,9 @@ static int zend_handle_loops_and_finally_ex(zend_long depth) /* {{{ */ } /* }}} */ -static int zend_handle_loops_and_finally(void) /* {{{ */ +static int zend_handle_loops_and_finally(znode *return_value) /* {{{ */ { - return zend_handle_loops_and_finally_ex(zend_stack_count(&CG(loop_var_stack)) + 1); + return zend_handle_loops_and_finally_ex(zend_stack_count(&CG(loop_var_stack)) + 1, return_value); } /* }}} */ @@ -4080,7 +4084,7 @@ void zend_compile_return(zend_ast *ast) /* {{{ */ expr_ast ? &expr_node : NULL, CG(active_op_array)->arg_info - 1, 0); } - zend_handle_loops_and_finally(); + zend_handle_loops_and_finally((expr_node.op_type & (IS_TMP_VAR | IS_VAR)) ? &expr_node : NULL); opline = zend_emit_op(NULL, by_ref ? ZEND_RETURN_BY_REF : ZEND_RETURN, &expr_node, NULL); @@ -4150,7 +4154,7 @@ void zend_compile_break_continue(zend_ast *ast) /* {{{ */ zend_error_noreturn(E_COMPILE_ERROR, "'%s' not in the 'loop' or 'switch' context", ast->kind == ZEND_AST_BREAK ? "break" : "continue"); } else { - if (!zend_handle_loops_and_finally_ex(depth)) { + if (!zend_handle_loops_and_finally_ex(depth, NULL)) { zend_error_noreturn(E_COMPILE_ERROR, "Cannot '%s' %d level%s", ast->kind == ZEND_AST_BREAK ? "break" : "continue", depth, depth == 1 ? "" : "s"); @@ -4233,7 +4237,7 @@ void zend_compile_goto(zend_ast *ast) /* {{{ */ zend_compile_expr(&label_node, label_ast); /* Label resolution and unwinding adjustments happen in pass two. */ - zend_handle_loops_and_finally(); + zend_handle_loops_and_finally(NULL); opline = zend_emit_op(NULL, ZEND_GOTO, NULL, &label_node); opline->op1.num = get_next_op_number(CG(active_op_array)) - opnum_start - 1; opline->extended_value = CG(context).current_brk_cont; diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h index 57de22e518..cd542e5313 100644 --- a/Zend/zend_vm_def.h +++ b/Zend/zend_vm_def.h @@ -7173,8 +7173,17 @@ ZEND_VM_HELPER(zend_dispatch_try_catch_finally_helper, ANY, ANY, uint32_t try_ca ZEND_VM_CONTINUE(); } else if (op_num < try_catch->finally_end) { - /* Chain potential exception from wrapping finally block */ zval *fast_call = EX_VAR(EX(func)->op_array.opcodes[try_catch->finally_end].op1.var); + + /* cleanup incomplete RETURN statement */ + if (fast_call->u2.lineno != (uint32_t)-1 + && (EX(func)->op_array.opcodes[fast_call->u2.lineno].op2_type & (IS_TMP_VAR | IS_VAR))) { + zval *return_value = EX_VAR(EX(func)->op_array.opcodes[fast_call->u2.lineno].op2.var); + + zval_ptr_dtor(return_value); + } + + /* Chain potential exception from wrapping finally block */ if (Z_OBJ_P(fast_call)) { if (ex) { zend_exception_set_previous(ex, Z_OBJ_P(fast_call)); @@ -7591,15 +7600,15 @@ ZEND_VM_HANDLER(159, ZEND_DISCARD_EXCEPTION, ANY, ANY) zval *fast_call = EX_VAR(opline->op1.var); SAVE_OPLINE(); - /* check for incomplete RETURN statement */ + /* cleanup incomplete RETURN statement */ if (fast_call->u2.lineno != (uint32_t)-1 - && (EX(func)->op_array.opcodes[fast_call->u2.lineno + 1].opcode == ZEND_RETURN - || EX(func)->op_array.opcodes[fast_call->u2.lineno + 1].opcode == ZEND_RETURN_BY_REF) - && (EX(func)->op_array.opcodes[fast_call->u2.lineno + 1].op1_type & (IS_VAR|IS_TMP_VAR))) { - cleanup_live_vars(execute_data, fast_call->u2.lineno, fast_call->u2.lineno + 1); + && (EX(func)->op_array.opcodes[fast_call->u2.lineno].op2_type & (IS_TMP_VAR | IS_VAR))) { + zval *return_value = EX_VAR(EX(func)->op_array.opcodes[fast_call->u2.lineno].op2.var); + + zval_ptr_dtor(return_value); } - /* check for delayed exception */ + /* cleanup delayed exception */ if (Z_OBJ_P(fast_call) != NULL) { /* discard the previously thrown exception */ OBJ_RELEASE(Z_OBJ_P(fast_call)); diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h index 21f94fd1a2..09e7a70e68 100644 --- a/Zend/zend_vm_execute.h +++ b/Zend/zend_vm_execute.h @@ -1724,8 +1724,17 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_dispatch_try_catch_finally_hel ZEND_VM_CONTINUE(); } else if (op_num < try_catch->finally_end) { - /* Chain potential exception from wrapping finally block */ zval *fast_call = EX_VAR(EX(func)->op_array.opcodes[try_catch->finally_end].op1.var); + + /* cleanup incomplete RETURN statement */ + if (fast_call->u2.lineno != (uint32_t)-1 + && (EX(func)->op_array.opcodes[fast_call->u2.lineno].op2_type & (IS_TMP_VAR | IS_VAR))) { + zval *return_value = EX_VAR(EX(func)->op_array.opcodes[fast_call->u2.lineno].op2.var); + + zval_ptr_dtor(return_value); + } + + /* Chain potential exception from wrapping finally block */ if (Z_OBJ_P(fast_call)) { if (ex) { zend_exception_set_previous(ex, Z_OBJ_P(fast_call)); @@ -1830,15 +1839,15 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DISCARD_EXCEPTION_SPEC_HANDLER zval *fast_call = EX_VAR(opline->op1.var); SAVE_OPLINE(); - /* check for incomplete RETURN statement */ + /* cleanup incomplete RETURN statement */ if (fast_call->u2.lineno != (uint32_t)-1 - && (EX(func)->op_array.opcodes[fast_call->u2.lineno + 1].opcode == ZEND_RETURN - || EX(func)->op_array.opcodes[fast_call->u2.lineno + 1].opcode == ZEND_RETURN_BY_REF) - && (EX(func)->op_array.opcodes[fast_call->u2.lineno + 1].op1_type & (IS_VAR|IS_TMP_VAR))) { - cleanup_live_vars(execute_data, fast_call->u2.lineno, fast_call->u2.lineno + 1); + && (EX(func)->op_array.opcodes[fast_call->u2.lineno].op2_type & (IS_TMP_VAR | IS_VAR))) { + zval *return_value = EX_VAR(EX(func)->op_array.opcodes[fast_call->u2.lineno].op2.var); + + zval_ptr_dtor(return_value); } - /* check for delayed exception */ + /* cleanup delayed exception */ if (Z_OBJ_P(fast_call) != NULL) { /* discard the previously thrown exception */ OBJ_RELEASE(Z_OBJ_P(fast_call));