Better fix for bug #70228 (memleak if return in finally block)

7 files changed, 140 insertions, 21 deletions

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--
+<?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
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--
+<?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
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--
+<?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
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--
+<?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 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));