Skip to content

Commit a905731

Browse files
nielsdosnielsdos
committed
Fix phpGH-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.
1 parent 4294e8d commit a905731

File tree

3 files changed

+111
-35
lines changed

3 files changed

+111
-35
lines changed

‎Zend/Optimizer/block_pass.c

Lines changed: 43 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -257,46 +257,54 @@ static void zend_optimize_block(zend_basic_block *block, zend_op_array *op_array
257257
break;
258258

259259
case ZEND_FREE:
260+
src = VAR_SOURCE(opline->op1);
261+
if (!src) {
262+
break;
263+
}
264+
/* Note: Only remove the source if the source is local to this block.
265+
* If it's not local, then the other blocks successors must also eventually either FREE or consume the temporary,
266+
* hence removing the temporary is not safe in the general case unless specified otherwise.
267+
* The source's result can also not be removed in such a case because that would cause a memory leak. */
260268
if (opline->op1_type == IS_TMP_VAR) {
261-
src = VAR_SOURCE(opline->op1);
262-
if (src) {
263-
switch (src->opcode) {
264-
case ZEND_BOOL:
265-
case ZEND_BOOL_NOT:
266-
/* T = BOOL(X), FREE(T) => T = BOOL(X) */
267-
/* The remaining BOOL is removed by a separate optimization */
268-
VAR_SOURCE(opline->op1) = NULL;
269-
MAKE_NOP(opline);
270-
++(*opt_count);
271-
break;
272-
case ZEND_ASSIGN:
273-
case ZEND_ASSIGN_DIM:
274-
case ZEND_ASSIGN_OBJ:
275-
case ZEND_ASSIGN_STATIC_PROP:
276-
case ZEND_ASSIGN_OP:
277-
case ZEND_ASSIGN_DIM_OP:
278-
case ZEND_ASSIGN_OBJ_OP:
279-
case ZEND_ASSIGN_STATIC_PROP_OP:
280-
case ZEND_PRE_INC:
281-
case ZEND_PRE_DEC:
282-
case ZEND_PRE_INC_OBJ:
283-
case ZEND_PRE_DEC_OBJ:
284-
case ZEND_PRE_INC_STATIC_PROP:
285-
case ZEND_PRE_DEC_STATIC_PROP:
286-
src->result_type = IS_UNUSED;
287-
VAR_SOURCE(opline->op1) = NULL;
288-
MAKE_NOP(opline);
289-
++(*opt_count);
290-
break;
291-
default:
269+
switch (src->opcode) {
270+
case ZEND_BOOL:
271+
case ZEND_BOOL_NOT:
272+
/* T = BOOL(X), FREE(T) => T = BOOL(X) */
273+
/* The remaining BOOL is removed by a separate optimization */
274+
/* The source is a bool, no source removals take place, so can be done non-locally. */
275+
VAR_SOURCE(opline->op1) = NULL;
276+
MAKE_NOP(opline);
277+
++(*opt_count);
278+
break;
279+
case ZEND_ASSIGN:
280+
case ZEND_ASSIGN_DIM:
281+
case ZEND_ASSIGN_OBJ:
282+
case ZEND_ASSIGN_STATIC_PROP:
283+
case ZEND_ASSIGN_OP:
284+
case ZEND_ASSIGN_DIM_OP:
285+
case ZEND_ASSIGN_OBJ_OP:
286+
case ZEND_ASSIGN_STATIC_PROP_OP:
287+
case ZEND_PRE_INC:
288+
case ZEND_PRE_DEC:
289+
case ZEND_PRE_INC_OBJ:
290+
case ZEND_PRE_DEC_OBJ:
291+
case ZEND_PRE_INC_STATIC_PROP:
292+
case ZEND_PRE_DEC_STATIC_PROP:
293+
if (src < op_array->opcodes + block->start || src > op_array->opcodes + block->len) {
292294
break;
293-
}
295+
}
296+
src->result_type = IS_UNUSED;
297+
VAR_SOURCE(opline->op1) = NULL;
298+
MAKE_NOP(opline);
299+
++(*opt_count);
300+
break;
301+
default:
302+
break;
294303
}
295304
} else if (opline->op1_type == IS_VAR) {
296-
src = VAR_SOURCE(opline->op1);
297305
/* V = OP, FREE(V) => OP. NOP */
298-
if (src &&
299-
src->opcode != ZEND_FETCH_R &&
306+
if (src >= op_array->opcodes + block->start && src <= op_array->opcodes + block->len &&
307+
src->opcode != ZEND_FETCH_R &&
300308
src->opcode != ZEND_FETCH_STATIC_PROP_R &&
301309
src->opcode != ZEND_FETCH_DIM_R &&
302310
src->opcode != ZEND_FETCH_OBJ_R &&

‎ext/opcache/tests/opt/gh11245_1.phpt

Lines changed: 33 additions & 0 deletions
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

Lines changed: 35 additions & 0 deletions
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)