Skip to content

Commit

Permalink
Fix incorrect sccp optimization of zend_switch with strict comparison
Browse files Browse the repository at this point in the history
  • Loading branch information
iluuu1994 committed Apr 14, 2020
1 parent 296cc4b commit 3da5f99
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 0 deletions.
57 changes: 57 additions & 0 deletions Zend/tests/match/018.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
--TEST--
Test match jump table optimizer
--INI--
opcache.enable=1
opcache.enable_cli=1
opcache.opt_debug_level=0x20000
--SKIPIF--
<?php if (!extension_loaded('Zend OPcache')) die('skip Zend OPcache extension not available'); ?>
--FILE--
<?php

function test() {
$x = 20;
match($x) {
1, 2, 3, 4, 5 => { throw new RuntimeException(); },
default => { echo "No match\n"; },
}
}
test();

function test2() {
$x = 'foo';
match($x) {
'1', '2', '3', '4', '5' => { throw new RuntimeException(); },
default => { echo "No match\n"; },
}
}
test2();

--EXPECTF--
$_main:
; (lines=5, args=0, vars=0, tmps=0)
; (after optimizer)
; %s
0000 INIT_FCALL 0 %d string("test")
0001 DO_UCALL
0002 INIT_FCALL 0 %d string("test2")
0003 DO_UCALL
0004 RETURN int(1)

test:
; (lines=2, args=0, vars=0, tmps=0)
; (after optimizer)
; %s
0000 ECHO string("No match
")
0001 RETURN null

test2:
; (lines=2, args=0, vars=0, tmps=0)
; (after optimizer)
; %s
0000 ECHO string("No match
")
0001 RETURN null
No match
No match
1 change: 1 addition & 0 deletions Zend/zend_compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -5251,6 +5251,7 @@ void zend_compile_match(znode *result, zend_ast *ast) /* {{{ */
zend_op *opline = zend_emit_op(NULL,
jumptable_type == IS_LONG ? ZEND_SWITCH_LONG : ZEND_SWITCH_STRING,
&expr_node, &jumptable_op);
opline->extended_value = ZEND_SWITCH_STRICT_COMPARISON;
if (opline->op1_type == IS_CONST) {
Z_TRY_ADDREF_P(CT_CONSTANT(opline->op1));
}
Expand Down
3 changes: 3 additions & 0 deletions Zend/zend_compile.h
Original file line number Diff line number Diff line change
Expand Up @@ -1027,6 +1027,9 @@ static zend_always_inline int zend_check_arg_send_type(const zend_function *zf,
#define ZEND_BRK 254
#define ZEND_CONT 255

/* Attribute whether ZEND_SWITCH_STRING or ZEND_SWITCH_LONG instructions use strict comparison */
#define ZEND_SWITCH_STRICT_COMPARISON 1


END_EXTERN_C()

Expand Down
18 changes: 18 additions & 0 deletions ext/opcache/Optimizer/sccp.c
Original file line number Diff line number Diff line change
Expand Up @@ -1975,6 +1975,8 @@ static void sccp_mark_feasible_successors(
s = zend_hash_num_elements(Z_ARR_P(op1)) != 0;
break;
case ZEND_SWITCH_LONG:
{
zend_bool strict_comparison = (opline->extended_value & ZEND_SWITCH_STRICT_COMPARISON) != 0;
if (Z_TYPE_P(op1) == IS_LONG) {
zend_op_array *op_array = scdf->op_array;
zend_ssa *ssa = scdf->ssa;
Expand All @@ -1989,10 +1991,19 @@ static void sccp_mark_feasible_successors(
}
scdf_mark_edge_feasible(scdf, block_num, target);
return;
} else if (strict_comparison) {
zend_op_array *op_array = scdf->op_array;
zend_ssa *ssa = scdf->ssa;
int target = ssa->cfg.map[ZEND_OFFSET_TO_OPLINE_NUM(op_array, opline, opline->extended_value)];
scdf_mark_edge_feasible(scdf, block_num, target);
return;
}
s = 0;
break;
}
case ZEND_SWITCH_STRING:
{
zend_bool strict_comparison = (opline->extended_value & ZEND_SWITCH_STRICT_COMPARISON) != 0;
if (Z_TYPE_P(op1) == IS_STRING) {
zend_op_array *op_array = scdf->op_array;
zend_ssa *ssa = scdf->ssa;
Expand All @@ -2007,9 +2018,16 @@ static void sccp_mark_feasible_successors(
}
scdf_mark_edge_feasible(scdf, block_num, target);
return;
} else if (strict_comparison) {
zend_op_array *op_array = scdf->op_array;
zend_ssa *ssa = scdf->ssa;
int target = ssa->cfg.map[ZEND_OFFSET_TO_OPLINE_NUM(op_array, opline, opline->extended_value)];
scdf_mark_edge_feasible(scdf, block_num, target);
return;
}
s = 0;
break;
}
default:
for (s = 0; s < block->successors_count; s++) {
scdf_mark_edge_feasible(scdf, block_num, block->successors[s]);
Expand Down

0 comments on commit 3da5f99

Please sign in to comment.