Skip to content

Commit

Permalink
Fix GH-11245 (In some specific cases SWITCH with one default statemen…
Browse files Browse the repository at this point in the history
…t 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.
  • Loading branch information
nielsdos committed May 22, 2023
1 parent 93fa961 commit 5cad1a7
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 1 deletion.
2 changes: 2 additions & 0 deletions NEWS
Expand Up @@ -32,6 +32,8 @@ PHP NEWS
- Opcache:
. Fixed bug GH-11134 (Incorrect match default branch optimization). (ilutov)
. Fixed too wide OR and AND range inference. (nielsdos)
. Fixed bug GH-11245 (In some specific cases SWITCH with one default
statement will cause segfault). (nielsdos)

- PGSQL:
. Fixed parameter parsing of pg_lo_export(). (kocsismate)
Expand Down
10 changes: 9 additions & 1 deletion Zend/Optimizer/block_pass.c
Expand Up @@ -257,6 +257,10 @@ static void zend_optimize_block(zend_basic_block *block, zend_op_array *op_array
break;

case ZEND_FREE:
/* Note: Only remove the source if the source is local to this block.
* If it's not local, then the other blocks successors must also eventually either FREE or consume the temporary,
* hence removing the temporary is not safe in the general case, especially when other consumers are not FREE.
* A FREE may not be removed without also removing the source's result, because otherwise that would cause a memory leak. */
if (opline->op1_type == IS_TMP_VAR) {
src = VAR_SOURCE(opline->op1);
if (src) {
Expand All @@ -265,6 +269,7 @@ static void zend_optimize_block(zend_basic_block *block, zend_op_array *op_array
case ZEND_BOOL_NOT:
/* T = BOOL(X), FREE(T) => T = BOOL(X) */
/* The remaining BOOL is removed by a separate optimization */
/* The source is a bool, no source removals take place, so this may be done non-locally. */
VAR_SOURCE(opline->op1) = NULL;
MAKE_NOP(opline);
++(*opt_count);
Expand All @@ -283,6 +288,9 @@ static void zend_optimize_block(zend_basic_block *block, zend_op_array *op_array
case ZEND_PRE_DEC_OBJ:
case ZEND_PRE_INC_STATIC_PROP:
case ZEND_PRE_DEC_STATIC_PROP:
if (src < op_array->opcodes + block->start) {
break;
}
src->result_type = IS_UNUSED;
VAR_SOURCE(opline->op1) = NULL;
MAKE_NOP(opline);
Expand All @@ -295,7 +303,7 @@ static void zend_optimize_block(zend_basic_block *block, zend_op_array *op_array
} else if (opline->op1_type == IS_VAR) {
src = VAR_SOURCE(opline->op1);
/* V = OP, FREE(V) => OP. NOP */
if (src &&
if (src >= op_array->opcodes + block->start &&
src->opcode != ZEND_FETCH_R &&
src->opcode != ZEND_FETCH_STATIC_PROP_R &&
src->opcode != ZEND_FETCH_DIM_R &&
Expand Down
33 changes: 33 additions & 0 deletions ext/opcache/tests/opt/gh11245_1.phpt
@@ -0,0 +1,33 @@
--TEST--
GH-11245: In some specific cases SWITCH with one default statement will cause segfault (VAR variation)
--INI--
opcache.enable=1
opcache.enable_cli=1
opcache.optimization_level=0x7FFFBFFF
opcache.opt_debug_level=0x20000
opcache.preload=
--EXTENSIONS--
opcache
--FILE--
<?php
function xx() { return "somegarbage"; }
switch (xx()) {
default:
if (!empty($xx)) {return;}
}
?>
--EXPECTF--
$_main:
; (lines=4, args=0, vars=1, tmps=1)
; (after optimizer)
; %s
0000 T1 = ISSET_ISEMPTY_CV (empty) CV0($xx)
0001 JMPNZ T1 0003
0002 RETURN null
0003 RETURN int(1)

xx:
; (lines=1, args=0, vars=0, tmps=0)
; (after optimizer)
; %s
0000 RETURN string("somegarbage")
35 changes: 35 additions & 0 deletions ext/opcache/tests/opt/gh11245_2.phpt
@@ -0,0 +1,35 @@
--TEST--
GH-11245: In some specific cases SWITCH with one default statement will cause segfault (TMP variation)
--INI--
opcache.enable=1
opcache.enable_cli=1
opcache.optimization_level=0x7FFFBFFF
opcache.opt_debug_level=0x20000
opcache.preload=
--EXTENSIONS--
opcache
--FILE--
<?php
class X {
// Chosen to test for a memory leak.
static $prop = "aa";
}
switch (++X::$prop) {
default:
if (empty($xx)) {return;}
}
?>
--EXPECTF--
$_main:
; (lines=7, args=0, vars=1, tmps=2)
; (after optimizer)
; %s
0000 T1 = PRE_INC_STATIC_PROP string("prop") string("X")
0001 T2 = ISSET_ISEMPTY_CV (empty) CV0($xx)
0002 JMPZ T2 0005
0003 FREE T1
0004 RETURN null
0005 FREE T1
0006 RETURN int(1)
LIVE RANGES:
1: 0001 - 0005 (tmp/var)

0 comments on commit 5cad1a7

Please sign in to comment.