Skip to content

Commit 34ea00f

Browse files
devnexendevnexen
committed
Merge branch 'PHP-8.6'
* PHP-8.6: Fix GH-24480: finally block running twice when a VM interrupt throws in FAST_RET
2 parents 14bb592 + 198bce1 commit 34ea00f

8 files changed

Lines changed: 181 additions & 10 deletions

‎Zend/tests/try/finally_interrupt_after_return.phpt‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
--TEST--
2+
VM interrupt throwing at the end of a finally block after "return" in the try block
3+
--DESCRIPTION--
4+
"return" inside a try block runs the finally block with FAST_CALL, and FAST_RET
5+
then jumps back to the RETURN, which is inside the try block. An interrupt that
6+
throws as the finally block ends belongs to the finally block: it must not run
7+
the finally block again, and it discards the pending return value.
8+
--EXTENSIONS--
9+
zend_test
10+
--INI--
11+
zend_test.observer.enabled=1
12+
zend_test.observer.show_output=0
13+
zend_test.observer.observe_function_names=release
14+
zend_test.observer.set_vm_interrupt_on_end=1
15+
--FILE--
16+
<?php
17+
$runs = 0;
18+
19+
function release() {
20+
global $runs;
21+
$runs++;
22+
}
23+
24+
function test() {
25+
try {
26+
return str_repeat('a', 3);
27+
} finally {
28+
release();
29+
}
30+
}
31+
32+
try {
33+
$result = test();
34+
echo "test() returned $result\n";
35+
} catch (Exception $e) {
36+
echo get_class($e), ': ', $e->getMessage(), "\n";
37+
}
38+
echo "finally ran $runs time(s)\n";
39+
?>
40+
--EXPECT--
41+
Exception: VM interrupt
42+
finally ran 1 time(s)

‎Zend/tests/try/finally_interrupt_after_return_generator.phpt‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
--TEST--
2+
VM interrupt throwing at the end of a generator's finally block after "return" in the try block
3+
--EXTENSIONS--
4+
zend_test
5+
--INI--
6+
zend_test.observer.enabled=1
7+
zend_test.observer.show_output=0
8+
zend_test.observer.observe_function_names=release
9+
zend_test.observer.set_vm_interrupt_on_end=1
10+
--FILE--
11+
<?php
12+
$runs = 0;
13+
14+
function release() {
15+
global $runs;
16+
$runs++;
17+
}
18+
19+
function gen() {
20+
try {
21+
yield 1;
22+
return str_repeat('a', 3);
23+
} finally {
24+
release();
25+
}
26+
}
27+
28+
$gen = gen();
29+
try {
30+
echo "yielded ", $gen->current(), "\n";
31+
$gen->next();
32+
echo "gen() returned ", $gen->getReturn(), "\n";
33+
} catch (Exception $e) {
34+
echo get_class($e), ': ', $e->getMessage(), "\n";
35+
}
36+
var_dump($gen->valid());
37+
echo "finally ran $runs time(s)\n";
38+
?>
39+
--EXPECT--
40+
yielded 1
41+
Exception: VM interrupt
42+
bool(false)
43+
finally ran 1 time(s)

‎Zend/tests/try/finally_interrupt_after_return_nested.phpt‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
--TEST--
2+
VM interrupt throwing at the end of nested finally blocks after "return" in the try block
3+
--DESCRIPTION--
4+
The inner finally block's FAST_RET jumps to the outer FAST_CALL, which is inside
5+
the inner try block. An interrupt that throws there must not run the inner
6+
finally block again, but the outer finally block still runs.
7+
--EXTENSIONS--
8+
zend_test
9+
--INI--
10+
zend_test.observer.enabled=1
11+
zend_test.observer.show_output=0
12+
zend_test.observer.observe_function_names=release
13+
zend_test.observer.set_vm_interrupt_on_end=1
14+
--FILE--
15+
<?php
16+
$runs = [];
17+
18+
function release($name) {
19+
global $runs;
20+
$runs[] = $name;
21+
}
22+
23+
function test() {
24+
try {
25+
try {
26+
return str_repeat('a', 3);
27+
} finally {
28+
release('inner');
29+
}
30+
} finally {
31+
release('outer');
32+
}
33+
}
34+
35+
try {
36+
$result = test();
37+
echo "test() returned $result\n";
38+
} catch (Exception $e) {
39+
echo get_class($e), ': ', $e->getMessage(), "\n";
40+
echo 'previous: ', get_class($e->getPrevious()), ': ', $e->getPrevious()->getMessage(), "\n";
41+
}
42+
echo implode(', ', $runs), "\n";
43+
?>
44+
--EXPECT--
45+
Exception: VM interrupt
46+
previous: Exception: VM interrupt
47+
inner, outer

‎Zend/zend_vm_def.h‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8199,7 +8199,9 @@ ZEND_VM_HELPER(zend_dispatch_try_catch_finally_helper, ANY, ANY, uint32_t try_ca
81998199
Z_OPLINE_NUM_P(fast_call) = (uint32_t)-1;
82008200
ZEND_VM_JMP_EX(&EX(func)->op_array.opcodes[try_catch->finally_op], 0);
82018201

8202-
} else if (op_num < try_catch->finally_end) {
8202+
} else if (op_num <= try_catch->finally_end) {
8203+
/* Exception thrown in the finally block, or at its FAST_RET
8204+
* (by an interrupt) */
82038205
zval *fast_call = EX_VAR(EX(func)->op_array.opcodes[try_catch->finally_end].op1.var);
82048206

82058207
/* cleanup incomplete RETURN statement */
@@ -8279,7 +8281,7 @@ ZEND_VM_HANDLER(149, ZEND_HANDLE_EXCEPTION, ANY, ANY)
82798281
/* further blocks will not be relevant... */
82808282
break;
82818283
}
8282-
if (throw_op_num < try_catch->catch_op || throw_op_num < try_catch->finally_end) {
8284+
if (throw_op_num < try_catch->catch_op || throw_op_num <= try_catch->finally_end) {
82838285
current_try_catch_offset = i;
82848286
}
82858287
}
@@ -8758,7 +8760,13 @@ ZEND_VM_HANDLER(163, ZEND_FAST_RET, ANY, TRY_CATCH)
87588760
if (Z_OPLINE_NUM_P(fast_call) != (uint32_t)-1) {
87598761
const zend_op *fast_ret = EX(func)->op_array.opcodes + Z_OPLINE_NUM_P(fast_call);
87608762

8761-
ZEND_VM_JMP_EX(fast_ret + 1, 0);
8763+
/* Handle interrupts before jumping: fast_ret + 1 is inside the try
8764+
* block, so an exception thrown there would run the finally block
8765+
* again. An exception thrown at FAST_RET belongs to the finally
8766+
* block. */
8767+
ZEND_VM_INTERRUPT_CHECK();
8768+
ZEND_VM_SET_OPCODE_NO_INTERRUPT(fast_ret + 1);
8769+
ZEND_VM_CONTINUE();
87628770
}
87638771

87648772
/* special case for unhandled exceptions */

‎Zend/zend_vm_execute.h‎

Lines changed: 22 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎ext/zend_test/observer.c‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "zend_smart_str.h"
1919
#include "ext/standard/php_var.h"
2020
#include "zend_generators.h"
21+
#include "zend_exceptions.h"
2122

2223
static zend_observer_fcall_handlers observer_fcall_init(zend_execute_data *execute_data);
2324

@@ -125,6 +126,11 @@ static void observer_end(zend_execute_data *execute_data, zval *retval)
125126
{
126127
assert_observer_opline(execute_data);
127128

129+
if (ZT_G(observer_set_vm_interrupt_on_end) && !EG(exception)) {
130+
ZT_G(observer_throw_on_vm_interrupt) = true;
131+
atomic_store(&EG(vm_interrupt), true);
132+
}
133+
128134
if (!ZT_G(observer_show_output)) {
129135
return;
130136
}
@@ -154,6 +160,11 @@ static void zend_test_interrupt_function(zend_execute_data *execute_data)
154160
if (zend_test_prev_interrupt_function) {
155161
zend_test_prev_interrupt_function(execute_data);
156162
}
163+
164+
if (ZT_G(observer_throw_on_vm_interrupt)) {
165+
ZT_G(observer_throw_on_vm_interrupt) = false;
166+
zend_throw_exception(NULL, "VM interrupt", 0);
167+
}
157168
}
158169

159170
static void observer_show_init(zend_function *fbc)
@@ -397,6 +408,7 @@ PHP_INI_BEGIN()
397408
STD_PHP_INI_ENTRY("zend_test.observer.show_opcode_in_user_handler", "", PHP_INI_SYSTEM, OnUpdateString, observer_show_opcode_in_user_handler, zend_zend_test_globals, zend_test_globals)
398409
STD_PHP_INI_BOOLEAN("zend_test.observer.set_vm_interrupt_on_begin", "0", PHP_INI_SYSTEM, OnUpdateBool, observer_set_vm_interrupt_on_begin, zend_zend_test_globals, zend_test_globals)
399410
STD_PHP_INI_BOOLEAN("zend_test.observer.set_vm_interrupt_on_begin_without_callback", "0", PHP_INI_SYSTEM, OnUpdateBool, observer_set_vm_interrupt_on_begin_without_callback, zend_zend_test_globals, zend_test_globals)
411+
STD_PHP_INI_BOOLEAN("zend_test.observer.set_vm_interrupt_on_end", "0", PHP_INI_SYSTEM, OnUpdateBool, observer_set_vm_interrupt_on_end, zend_zend_test_globals, zend_test_globals)
400412
STD_PHP_INI_BOOLEAN("zend_test.observer.fiber_init", "0", PHP_INI_SYSTEM, OnUpdateBool, observer_fiber_init, zend_zend_test_globals, zend_test_globals)
401413
STD_PHP_INI_BOOLEAN("zend_test.observer.fiber_switch", "0", PHP_INI_SYSTEM, OnUpdateBool, observer_fiber_switch, zend_zend_test_globals, zend_test_globals)
402414
STD_PHP_INI_BOOLEAN("zend_test.observer.fiber_destroy", "0", PHP_INI_SYSTEM, OnUpdateBool, observer_fiber_destroy, zend_zend_test_globals, zend_test_globals)
@@ -439,7 +451,7 @@ void zend_test_observer_init(INIT_FUNC_ARGS)
439451
zend_execute_internal = zend_test_execute_internal;
440452
}
441453

442-
if (ZT_G(observer_set_vm_interrupt_on_begin)) {
454+
if (ZT_G(observer_set_vm_interrupt_on_begin) || ZT_G(observer_set_vm_interrupt_on_end)) {
443455
zend_test_prev_interrupt_function = zend_interrupt_function;
444456
zend_interrupt_function = zend_test_interrupt_function;
445457
}

‎ext/zend_test/php_test.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ ZEND_BEGIN_MODULE_GLOBALS(zend_test)
4343
char *observer_show_opcode_in_user_handler;
4444
int observer_set_vm_interrupt_on_begin;
4545
int observer_set_vm_interrupt_on_begin_without_callback;
46+
int observer_set_vm_interrupt_on_end;
47+
bool observer_throw_on_vm_interrupt;
4648
int observer_nesting_depth;
4749
int observer_fiber_init;
4850
int observer_fiber_switch;

‎ext/zend_test/test.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2538,6 +2538,7 @@ PHP_RINIT_FUNCTION(zend_test)
25382538
ALLOC_HASHTABLE(ZT_G(global_weakmap));
25392539
zend_hash_init(ZT_G(global_weakmap), 8, NULL, ZVAL_PTR_DTOR, 0);
25402540
ZT_G(observer_nesting_depth) = 0;
2541+
ZT_G(observer_throw_on_vm_interrupt) = false;
25412542
zend_test_mm_custom_handlers_rinit();
25422543

25432544
if (ZT_G(rearm_vm_interrupt)) {

0 commit comments

Comments
 (0)