Skip to content

Commit 5cad1a7

Browse files
committed
Fix GH-11245 (In some specific cases SWITCH with one default statement will cause segfault)
The block optimizer pass allows the use of sources of the preceding block if the block is a follower and not a target. This causes issues when trying to remove FREE instructions: if the source is not in the block of the FREE, then the FREE and source are still removed. Therefore the other successor blocks, which must consume or FREE the temporary, will still contain the FREE opline. This opline will now refer to a temporary that doesn't exist anymore, which most of the time results in a crash. For these kind of non-local scenarios, we'll let the SSA based optimizations handle those cases. Closes GH-11251.
1 parent 93fa961 commit 5cad1a7

File tree

4 files changed

+79
-1
lines changed

4 files changed

+79
-1
lines changed

NEWS

+2
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ PHP NEWS
3232
- Opcache:
3333
. Fixed bug GH-11134 (Incorrect match default branch optimization). (ilutov)
3434
. Fixed too wide OR and AND range inference. (nielsdos)
35+
. Fixed bug GH-11245 (In some specific cases SWITCH with one default
36+
statement will cause segfault). (nielsdos)
3537

3638
- PGSQL:
3739
. Fixed parameter parsing of pg_lo_export(). (kocsismate)

Zend/Optimizer/block_pass.c

+9-1
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,10 @@ static void zend_optimize_block(zend_basic_block *block, zend_op_array *op_array
257257
break;
258258

259259
case ZEND_FREE:
260+
/* Note: Only remove the source if the source is local to this block.
261+
* If it's not local, then the other blocks successors must also eventually either FREE or consume the temporary,
262+
* hence removing the temporary is not safe in the general case, especially when other consumers are not FREE.
263+
* A FREE may not be removed without also removing the source's result, because otherwise that would cause a memory leak. */
260264
if (opline->op1_type == IS_TMP_VAR) {
261265
src = VAR_SOURCE(opline->op1);
262266
if (src) {
@@ -265,6 +269,7 @@ static void zend_optimize_block(zend_basic_block *block, zend_op_array *op_array
265269
case ZEND_BOOL_NOT:
266270
/* T = BOOL(X), FREE(T) => T = BOOL(X) */
267271
/* The remaining BOOL is removed by a separate optimization */
272+
/* The source is a bool, no source removals take place, so this may be done non-locally. */
268273
VAR_SOURCE(opline->op1) = NULL;
269274
MAKE_NOP(opline);
270275
++(*opt_count);
@@ -283,6 +288,9 @@ static void zend_optimize_block(zend_basic_block *block, zend_op_array *op_array
283288
case ZEND_PRE_DEC_OBJ:
284289
case ZEND_PRE_INC_STATIC_PROP:
285290
case ZEND_PRE_DEC_STATIC_PROP:
291+
if (src < op_array->opcodes + block->start) {
292+
break;
293+
}
286294
src->result_type = IS_UNUSED;
287295
VAR_SOURCE(opline->op1) = NULL;
288296
MAKE_NOP(opline);
@@ -295,7 +303,7 @@ static void zend_optimize_block(zend_basic_block *block, zend_op_array *op_array
295303
} else if (opline->op1_type == IS_VAR) {
296304
src = VAR_SOURCE(opline->op1);
297305
/* V = OP, FREE(V) => OP. NOP */
298-
if (src &&
306+
if (src >= op_array->opcodes + block->start &&
299307
src->opcode != ZEND_FETCH_R &&
300308
src->opcode != ZEND_FETCH_STATIC_PROP_R &&
301309
src->opcode != ZEND_FETCH_DIM_R &&

ext/opcache/tests/opt/gh11245_1.phpt

+33
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
--TEST--
2+
GH-11245: In some specific cases SWITCH with one default statement will cause segfault (VAR variation)
3+
--INI--
4+
opcache.enable=1
5+
opcache.enable_cli=1
6+
opcache.optimization_level=0x7FFFBFFF
7+
opcache.opt_debug_level=0x20000
8+
opcache.preload=
9+
--EXTENSIONS--
10+
opcache
11+
--FILE--
12+
<?php
13+
function xx() { return "somegarbage"; }
14+
switch (xx()) {
15+
default:
16+
if (!empty($xx)) {return;}
17+
}
18+
?>
19+
--EXPECTF--
20+
$_main:
21+
; (lines=4, args=0, vars=1, tmps=1)
22+
; (after optimizer)
23+
; %s
24+
0000 T1 = ISSET_ISEMPTY_CV (empty) CV0($xx)
25+
0001 JMPNZ T1 0003
26+
0002 RETURN null
27+
0003 RETURN int(1)
28+
29+
xx:
30+
; (lines=1, args=0, vars=0, tmps=0)
31+
; (after optimizer)
32+
; %s
33+
0000 RETURN string("somegarbage")

ext/opcache/tests/opt/gh11245_2.phpt

+35
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
--TEST--
2+
GH-11245: In some specific cases SWITCH with one default statement will cause segfault (TMP variation)
3+
--INI--
4+
opcache.enable=1
5+
opcache.enable_cli=1
6+
opcache.optimization_level=0x7FFFBFFF
7+
opcache.opt_debug_level=0x20000
8+
opcache.preload=
9+
--EXTENSIONS--
10+
opcache
11+
--FILE--
12+
<?php
13+
class X {
14+
// Chosen to test for a memory leak.
15+
static $prop = "aa";
16+
}
17+
switch (++X::$prop) {
18+
default:
19+
if (empty($xx)) {return;}
20+
}
21+
?>
22+
--EXPECTF--
23+
$_main:
24+
; (lines=7, args=0, vars=1, tmps=2)
25+
; (after optimizer)
26+
; %s
27+
0000 T1 = PRE_INC_STATIC_PROP string("prop") string("X")
28+
0001 T2 = ISSET_ISEMPTY_CV (empty) CV0($xx)
29+
0002 JMPZ T2 0005
30+
0003 FREE T1
31+
0004 RETURN null
32+
0005 FREE T1
33+
0006 RETURN int(1)
34+
LIVE RANGES:
35+
1: 0001 - 0005 (tmp/var)

0 commit comments

Comments
 (0)