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 (file)
index 0000000..29cbf49
--- /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--
+<?php
+function test() {
+    try {
+        return str_repeat("a", 2);
+    } finally {
+        throw new Exception("ops");
+    }
+}
+
+try {
+    test();
+} catch (Exception $e) {
+    echo $e->getMessage(), "\n";
+}
+?>
+--EXPECT--
+ops

diff --git a/Zend/tests/try/bug70228_6.phpt b/Zend/tests/try/bug70228_6.phpt
new file mode 100644 (file)
index 0000000..fc68657
--- /dev/null
+++ b/Zend/tests/try/bug70228_6.phpt
@@ -0,0 +1,18 @@
+--TEST--
+Bug #70228 (memleak if return in finally block)
+--FILE--
+<?php
+function test($x) {
+    foreach ($x as $v) {
+        try {
+            return str_repeat("a", 2);
+        } finally {
+            return 42;
+        }
+    }
+}
+
+var_dump(test([1]));
+?>
+--EXPECT--
+int(42)

diff --git a/Zend/tests/try/bug70228_7.phpt b/Zend/tests/try/bug70228_7.phpt
new file mode 100644 (file)
index 0000000..4b8a803
--- /dev/null
+++ b/Zend/tests/try/bug70228_7.phpt
@@ -0,0 +1,29 @@
+--TEST--
+Bug #70228 (memleak if return in finally block)
+--FILE--
+<?php
+
+function foo() {
+    $array = [1, 2, $n = 3];
+    foreach ($array as $value) {
+        var_dump($value);
+        try {
+            try {
+                foreach ($array as $_) {
+                    return str_repeat("a", 2);
+                }
+            } finally {
+                throw new Exception;
+            }
+        } catch (Exception $e) { }
+    }
+}
+
+foo();
+?>
+===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 (file)
index 0000000..ad13113
--- /dev/null
+++ b/Zend/tests/try/bug70228_8.phpt
@@ -0,0 +1,30 @@
+--TEST--
+Bug #70228 (memleak if return in finally block)
+--FILE--
+<?php
+
+function foo() {
+    $array = [1, 2, $n = 3];
+    foreach ($array as $value) {
+        var_dump($value);
+        try {
+            try {
+               switch (str_repeat("b", 2)) {
+                       case "bb":
+                           return str_repeat("a", 2);
+                }
+            } finally {
+                throw new Exception;
+            }
+        } catch (Exception $e) { }
+    }
+}
+
+foo();
+?>
+===DONE===
+--EXPECT--
+int(1)
+int(2)
+int(3)
+===DONE===

diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
index ce8224652d028ca3a3ab81554b95f652962fac0b..30d28cd842b1e152bdc888d14d56af4499a2926c 100644 (file)
--- 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 57de22e518202698facbedb10e898d7afd37bcb3..cd542e5313cc18ca218a3aa3bb92cacc1c15c3da 100644 (file)
--- 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 21f94fd1a24600f591ef4437bf3bedb94ca09454..09e7a70e68ae0348f069545ded60bd52653d5940 100644 (file)
--- 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));