Skip to content

Commit c0cd669

Browse files
iluuu1994iluuu1994
committed
Fix incorrect sccp optimization of zend_switch with strict comparison
1 parent feb874a commit c0cd669

20 files changed

+171
-16
lines changed

‎Zend/tests/match/013.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ test:
5252
; (after optimizer)
5353
; %s
5454
0000 CV0($char) = RECV 1
55-
0001 SWITCH_STRING CV0($char) "a": 0020, "b": 0021, "c": 0021, "d": 0022, "e": 0023, "f": 0023, "g": 0024, "h": 0025, "i": 0025, default: 0026
55+
0001 MATCH_STRING CV0($char) "a": 0020, "b": 0021, "c": 0021, "d": 0022, "e": 0023, "f": 0023, "g": 0024, "h": 0025, "i": 0025, default: 0026
5656
0002 T1 = IS_IDENTICAL CV0($char) string("a")
5757
0003 JMPNZ T1 0020
5858
0004 T1 = IS_IDENTICAL CV0($char) string("b")

‎Zend/tests/match/018.phpt

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
--TEST--
2+
Test match jump table optimizer
3+
--INI--
4+
opcache.enable=1
5+
opcache.enable_cli=1
6+
opcache.opt_debug_level=0x20000
7+
--SKIPIF--
8+
<?php if (!extension_loaded('Zend OPcache')) die('skip Zend OPcache extension not available'); ?>
9+
--FILE--
10+
<?php
11+
12+
function test() {
13+
$x = '2';
14+
match($x) {
15+
1, 2, 3, 4, 5 => { throw new RuntimeException(); },
16+
default => { echo "No match\n"; },
17+
}
18+
}
19+
test();
20+
21+
function test2() {
22+
$x = 2;
23+
match($x) {
24+
'1', '2', '3', '4', '5' => { throw new RuntimeException(); },
25+
default => { echo "No match\n"; },
26+
}
27+
}
28+
test2();
29+
30+
--EXPECTF--
31+
$_main:
32+
; (lines=5, args=0, vars=0, tmps=0)
33+
; (after optimizer)
34+
; %s
35+
0000 INIT_FCALL 0 %d string("test")
36+
0001 DO_UCALL
37+
0002 INIT_FCALL 0 %d string("test2")
38+
0003 DO_UCALL
39+
0004 RETURN int(1)
40+
41+
test:
42+
; (lines=2, args=0, vars=0, tmps=0)
43+
; (after optimizer)
44+
; %s
45+
0000 ECHO string("No match
46+
")
47+
0001 RETURN null
48+
49+
test2:
50+
; (lines=2, args=0, vars=0, tmps=0)
51+
; (after optimizer)
52+
; %s
53+
0000 ECHO string("No match
54+
")
55+
0001 RETURN null
56+
No match
57+
No match

‎Zend/zend_compile.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5249,7 +5249,7 @@ void zend_compile_match(znode *result, zend_ast *ast) /* {{{ */
52495249
ZVAL_ARR(&jumptable_op.u.constant, jumptable);
52505250

52515251
zend_op *opline = zend_emit_op(NULL,
5252-
jumptable_type == IS_LONG ? ZEND_SWITCH_LONG : ZEND_SWITCH_STRING,
5252+
jumptable_type == IS_LONG ? ZEND_MATCH_LONG : ZEND_MATCH_STRING,
52535253
&expr_node, &jumptable_op);
52545254
if (opline->op1_type == IS_CONST) {
52555255
Z_TRY_ADDREF_P(CT_CONSTANT(opline->op1));

‎Zend/zend_opcode.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -745,11 +745,13 @@ static zend_bool keeps_op1_alive(zend_op *opline) {
745745
* it is later freed by something else. */
746746
if (opline->opcode == ZEND_CASE
747747
|| opline->opcode == ZEND_SWITCH_LONG
748+
|| opline->opcode == ZEND_MATCH_LONG
748749
|| opline->opcode == ZEND_FETCH_LIST_R
749750
|| opline->opcode == ZEND_COPY_TMP) {
750751
return 1;
751752
}
752753
ZEND_ASSERT(opline->opcode != ZEND_SWITCH_STRING
754+
&& opline->opcode != ZEND_MATCH_STRING
753755
&& opline->opcode != ZEND_FE_FETCH_R
754756
&& opline->opcode != ZEND_FE_FETCH_RW
755757
&& opline->opcode != ZEND_FETCH_LIST_W
@@ -1020,6 +1022,8 @@ ZEND_API int pass_two(zend_op_array *op_array)
10201022
break;
10211023
case ZEND_SWITCH_LONG:
10221024
case ZEND_SWITCH_STRING:
1025+
case ZEND_MATCH_LONG:
1026+
case ZEND_MATCH_STRING:
10231027
{
10241028
/* absolute indexes to relative offsets */
10251029
HashTable *jumptable = Z_ARRVAL_P(CT_CONSTANT(opline->op2));

‎Zend/zend_vm_def.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8402,6 +8402,16 @@ ZEND_VM_COLD_CONSTCONST_HANDLER(188, ZEND_SWITCH_STRING, CONST|TMPVARCV, CONST,
84028402
}
84038403
}
84048404

8405+
ZEND_VM_COLD_CONSTCONST_HANDLER(195, ZEND_MATCH_LONG, CONST|TMPVARCV, CONST, JMP_ADDR)
8406+
{
8407+
ZEND_VM_DISPATCH_TO_HANDLER(ZEND_SWITCH_LONG);
8408+
}
8409+
8410+
ZEND_VM_COLD_CONSTCONST_HANDLER(196, ZEND_MATCH_STRING, CONST|TMPVARCV, CONST, JMP_ADDR)
8411+
{
8412+
ZEND_VM_DISPATCH_TO_HANDLER(ZEND_SWITCH_STRING);
8413+
}
8414+
84058415
ZEND_VM_COLD_CONSTCONST_HANDLER(189, ZEND_IN_ARRAY, CONST|TMP|VAR|CV, CONST, NUM)
84068416
{
84078417
USE_OPLINE

‎Zend/zend_vm_execute.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59568,6 +59568,8 @@ void zend_vm_init(void)
5956859568
2284,
5956959569
2285 | SPEC_RULE_OP1,
5957059570
2290 | SPEC_RULE_OP1 | SPEC_RULE_OP2,
59571+
2259 | SPEC_RULE_OP1,
59572+
2264 | SPEC_RULE_OP1,
5957159573
3218
5957259574
};
5957359575
#if (ZEND_VM_KIND == ZEND_VM_KIND_HYBRID)

‎Zend/zend_vm_opcodes.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
#include <zend.h>
2323
#include <zend_vm_opcodes.h>
2424

25-
static const char *zend_vm_opcodes_names[195] = {
25+
static const char *zend_vm_opcodes_names[197] = {
2626
"ZEND_NOP",
2727
"ZEND_ADD",
2828
"ZEND_SUB",
@@ -218,9 +218,11 @@ static const char *zend_vm_opcodes_names[195] = {
218218
"ZEND_GET_CALLED_CLASS",
219219
"ZEND_GET_TYPE",
220220
"ZEND_ARRAY_KEY_EXISTS",
221+
"ZEND_MATCH_LONG",
222+
"ZEND_MATCH_STRING",
221223
};
222224

223-
static uint32_t zend_vm_opcodes_flags[195] = {
225+
static uint32_t zend_vm_opcodes_flags[197] = {
224226
0x00000000,
225227
0x00000b0b,
226228
0x00000b0b,
@@ -416,6 +418,8 @@ static uint32_t zend_vm_opcodes_flags[195] = {
416418
0x00000101,
417419
0x00000103,
418420
0x00000707,
421+
0x0300030b,
422+
0x0300030b,
419423
};
420424

421425
ZEND_API const char* ZEND_FASTCALL zend_get_opcode_name(zend_uchar opcode) {

‎Zend/zend_vm_opcodes.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,9 @@ END_EXTERN_C()
271271
#define ZEND_GET_CALLED_CLASS 192
272272
#define ZEND_GET_TYPE 193
273273
#define ZEND_ARRAY_KEY_EXISTS 194
274+
#define ZEND_MATCH_LONG 195
275+
#define ZEND_MATCH_STRING 196
274276

275-
#define ZEND_VM_LAST_OPCODE 194
277+
#define ZEND_VM_LAST_OPCODE 196
276278

277279
#endif

‎ext/opcache/Optimizer/block_pass.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,9 @@ static int get_const_switch_target(zend_cfg *cfg, zend_op_array *op_array, zend_
111111
HashTable *jumptable = Z_ARRVAL(ZEND_OP2_LITERAL(opline));
112112
zval *zv;
113113
if ((opline->opcode == ZEND_SWITCH_LONG && Z_TYPE_P(val) != IS_LONG)
114-
|| (opline->opcode == ZEND_SWITCH_STRING && Z_TYPE_P(val) != IS_STRING)) {
114+
|| (opline->opcode == ZEND_SWITCH_STRING && Z_TYPE_P(val) != IS_STRING)
115+
|| (opline->opcode == ZEND_MATCH_LONG && Z_TYPE_P(val) != IS_LONG)
116+
|| (opline->opcode == ZEND_MATCH_STRING && Z_TYPE_P(val) != IS_STRING)) {
115117
/* fallback to next block */
116118
return block->successors[block->successors_count - 1];
117119
}
@@ -369,6 +371,8 @@ static void zend_optimize_block(zend_basic_block *block, zend_op_array *op_array
369371

370372
case ZEND_SWITCH_LONG:
371373
case ZEND_SWITCH_STRING:
374+
case ZEND_MATCH_LONG:
375+
case ZEND_MATCH_STRING:
372376
if (opline->op1_type & (IS_TMP_VAR|IS_VAR)) {
373377
/* SWITCH variable will be deleted later by FREE, so we can't optimize it */
374378
Tsource[VAR_NUM(opline->op1.var)] = NULL;
@@ -1046,6 +1050,8 @@ static void assemble_code_blocks(zend_cfg *cfg, zend_op_array *op_array, zend_op
10461050
break;
10471051
case ZEND_SWITCH_LONG:
10481052
case ZEND_SWITCH_STRING:
1053+
case ZEND_MATCH_LONG:
1054+
case ZEND_MATCH_STRING:
10491055
{
10501056
HashTable *jumptable = Z_ARRVAL(ZEND_OP2_LITERAL(opline));
10511057
zval *zv;

‎ext/opcache/Optimizer/dfa_pass.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -642,6 +642,8 @@ static void zend_ssa_replace_control_link(zend_op_array *op_array, zend_ssa *ssa
642642
break;
643643
case ZEND_SWITCH_LONG:
644644
case ZEND_SWITCH_STRING:
645+
case ZEND_MATCH_LONG:
646+
case ZEND_MATCH_STRING:
645647
{
646648
HashTable *jumptable = Z_ARRVAL(ZEND_OP2_LITERAL(opline));
647649
zval *zv;
@@ -896,6 +898,7 @@ static int zend_dfa_optimize_jmps(zend_op_array *op_array, zend_ssa *ssa)
896898
break;
897899
}
898900
case ZEND_SWITCH_LONG:
901+
case ZEND_MATCH_LONG:
899902
if (opline->op1_type == IS_CONST) {
900903
zval *zv = CT_CONSTANT_EX(op_array, opline->op1.constant);
901904
if (Z_TYPE_P(zv) != IS_LONG) {
@@ -925,6 +928,7 @@ static int zend_dfa_optimize_jmps(zend_op_array *op_array, zend_ssa *ssa)
925928
}
926929
break;
927930
case ZEND_SWITCH_STRING:
931+
case ZEND_MATCH_STRING:
928932
if (opline->op1_type == IS_CONST) {
929933
zval *zv = CT_CONSTANT_EX(op_array, opline->op1.constant);
930934
if (Z_TYPE_P(zv) != IS_STRING) {

‎ext/opcache/Optimizer/sccp.c

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,8 @@ static zend_bool try_replace_op1(
304304
case ZEND_FETCH_LIST_R:
305305
case ZEND_SWITCH_STRING:
306306
case ZEND_SWITCH_LONG:
307+
case ZEND_MATCH_STRING:
308+
case ZEND_MATCH_LONG:
307309
if (Z_TYPE(zv) == IS_STRING) {
308310
zend_string_hash_val(Z_STR(zv));
309311
}
@@ -1975,6 +1977,9 @@ static void sccp_mark_feasible_successors(
19751977
s = zend_hash_num_elements(Z_ARR_P(op1)) != 0;
19761978
break;
19771979
case ZEND_SWITCH_LONG:
1980+
case ZEND_MATCH_LONG:
1981+
{
1982+
zend_bool strict_comparison = opline->opcode == ZEND_MATCH_LONG;
19781983
if (Z_TYPE_P(op1) == IS_LONG) {
19791984
zend_op_array *op_array = scdf->op_array;
19801985
zend_ssa *ssa = scdf->ssa;
@@ -1989,10 +1994,20 @@ static void sccp_mark_feasible_successors(
19891994
}
19901995
scdf_mark_edge_feasible(scdf, block_num, target);
19911996
return;
1997+
} else if (strict_comparison) {
1998+
zend_op_array *op_array = scdf->op_array;
1999+
zend_ssa *ssa = scdf->ssa;
2000+
int target = ssa->cfg.map[ZEND_OFFSET_TO_OPLINE_NUM(op_array, opline, opline->extended_value)];
2001+
scdf_mark_edge_feasible(scdf, block_num, target);
2002+
return;
19922003
}
19932004
s = 0;
19942005
break;
2006+
}
19952007
case ZEND_SWITCH_STRING:
2008+
case ZEND_MATCH_STRING:
2009+
{
2010+
zend_bool strict_comparison = opline->opcode == ZEND_MATCH_STRING;
19962011
if (Z_TYPE_P(op1) == IS_STRING) {
19972012
zend_op_array *op_array = scdf->op_array;
19982013
zend_ssa *ssa = scdf->ssa;
@@ -2007,9 +2022,16 @@ static void sccp_mark_feasible_successors(
20072022
}
20082023
scdf_mark_edge_feasible(scdf, block_num, target);
20092024
return;
2025+
} else if (strict_comparison) {
2026+
zend_op_array *op_array = scdf->op_array;
2027+
zend_ssa *ssa = scdf->ssa;
2028+
int target = ssa->cfg.map[ZEND_OFFSET_TO_OPLINE_NUM(op_array, opline, opline->extended_value)];
2029+
scdf_mark_edge_feasible(scdf, block_num, target);
2030+
return;
20102031
}
20112032
s = 0;
20122033
break;
2034+
}
20132035
default:
20142036
for (s = 0; s < block->successors_count; s++) {
20152037
scdf_mark_edge_feasible(scdf, block_num, block->successors[s]);

‎ext/opcache/Optimizer/zend_cfg.c

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,12 @@ static void zend_mark_reachable(zend_op *opcodes, zend_cfg *cfg, zend_basic_bloc
7373
succ->flags |= ZEND_BB_FOLLOW;
7474
}
7575
} else {
76-
ZEND_ASSERT(opcode == ZEND_SWITCH_LONG || opcode == ZEND_SWITCH_STRING);
76+
ZEND_ASSERT(
77+
opcode == ZEND_SWITCH_LONG
78+
|| opcode == ZEND_SWITCH_STRING
79+
|| opcode == ZEND_MATCH_LONG
80+
|| opcode == ZEND_MATCH_STRING
81+
);
7782
if (i == b->successors_count - 1) {
7883
succ->flags |= ZEND_BB_FOLLOW | ZEND_BB_TARGET;
7984
} else {
@@ -385,6 +390,8 @@ int zend_build_cfg(zend_arena **arena, const zend_op_array *op_array, uint32_t b
385390
break;
386391
case ZEND_SWITCH_LONG:
387392
case ZEND_SWITCH_STRING:
393+
case ZEND_MATCH_LONG:
394+
case ZEND_MATCH_STRING:
388395
{
389396
HashTable *jumptable = Z_ARRVAL_P(CRT_CONSTANT(opline->op2));
390397
zval *zv;
@@ -551,6 +558,8 @@ int zend_build_cfg(zend_arena **arena, const zend_op_array *op_array, uint32_t b
551558
break;
552559
case ZEND_SWITCH_LONG:
553560
case ZEND_SWITCH_STRING:
561+
case ZEND_MATCH_LONG:
562+
case ZEND_MATCH_STRING:
554563
{
555564
HashTable *jumptable = Z_ARRVAL_P(CRT_CONSTANT(opline->op2));
556565
zval *zv;

‎ext/opcache/Optimizer/zend_dump.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -615,7 +615,12 @@ void zend_dump_op(const zend_op_array *op_array, const zend_basic_block *b, cons
615615

616616
if (opline->op2_type == IS_CONST) {
617617
zval *op = CRT_CONSTANT(opline->op2);
618-
if (opline->opcode == ZEND_SWITCH_LONG || opline->opcode == ZEND_SWITCH_STRING) {
618+
if (
619+
opline->opcode == ZEND_SWITCH_LONG
620+
|| opline->opcode == ZEND_SWITCH_STRING
621+
|| opline->opcode == ZEND_MATCH_LONG
622+
|| opline->opcode == ZEND_MATCH_STRING
623+
) {
619624
HashTable *jumptable = Z_ARRVAL_P(op);
620625
zend_string *key;
621626
zend_ulong num_key;

‎ext/opcache/Optimizer/zend_inference.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4337,6 +4337,8 @@ int zend_may_throw(const zend_op *opline, const zend_ssa_op *ssa_op, const zend_
43374337
case ZEND_COALESCE:
43384338
case ZEND_SWITCH_LONG:
43394339
case ZEND_SWITCH_STRING:
4340+
case ZEND_MATCH_LONG:
4341+
case ZEND_MATCH_STRING:
43404342
case ZEND_ISSET_ISEMPTY_VAR:
43414343
case ZEND_ISSET_ISEMPTY_CV:
43424344
case ZEND_FUNC_NUM_ARGS:

‎ext/opcache/Optimizer/zend_optimizer.c

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -593,13 +593,19 @@ int zend_optimizer_replace_by_const(zend_op_array *op_array,
593593
}
594594
case ZEND_SWITCH_LONG:
595595
case ZEND_SWITCH_STRING:
596+
case ZEND_MATCH_LONG:
597+
case ZEND_MATCH_STRING:
596598
case ZEND_CASE: {
597599
zend_op *end = op_array->opcodes + op_array->last;
598600
while (opline < end) {
599601
if (opline->op1_type == type && opline->op1.var == var) {
600-
if (opline->opcode == ZEND_CASE
601-
|| opline->opcode == ZEND_SWITCH_LONG
602-
|| opline->opcode == ZEND_SWITCH_STRING) {
602+
if (
603+
opline->opcode == ZEND_CASE
604+
|| opline->opcode == ZEND_SWITCH_LONG
605+
|| opline->opcode == ZEND_SWITCH_STRING
606+
|| opline->opcode == ZEND_MATCH_LONG
607+
|| opline->opcode == ZEND_MATCH_STRING
608+
) {
603609
zval v;
604610

605611
if (opline->opcode == ZEND_CASE) {
@@ -693,6 +699,8 @@ void zend_optimizer_migrate_jump(zend_op_array *op_array, zend_op *new_opline, z
693699
break;
694700
case ZEND_SWITCH_LONG:
695701
case ZEND_SWITCH_STRING:
702+
case ZEND_MATCH_LONG:
703+
case ZEND_MATCH_STRING:
696704
{
697705
HashTable *jumptable = Z_ARRVAL(ZEND_OP2_LITERAL(opline));
698706
zval *zv;
@@ -737,6 +745,8 @@ void zend_optimizer_shift_jump(zend_op_array *op_array, zend_op *opline, uint32_
737745
break;
738746
case ZEND_SWITCH_LONG:
739747
case ZEND_SWITCH_STRING:
748+
case ZEND_MATCH_LONG:
749+
case ZEND_MATCH_STRING:
740750
{
741751
HashTable *jumptable = Z_ARRVAL(ZEND_OP2_LITERAL(opline));
742752
zval *zv;
@@ -1111,6 +1121,8 @@ static void zend_redo_pass_two(zend_op_array *op_array)
11111121
case ZEND_FE_FETCH_RW:
11121122
case ZEND_SWITCH_LONG:
11131123
case ZEND_SWITCH_STRING:
1124+
case ZEND_MATCH_LONG:
1125+
case ZEND_MATCH_STRING:
11141126
/* relative extended_value don't have to be changed */
11151127
break;
11161128
#endif
@@ -1231,6 +1243,8 @@ static void zend_redo_pass_two_ex(zend_op_array *op_array, zend_ssa *ssa)
12311243
case ZEND_FE_FETCH_RW:
12321244
case ZEND_SWITCH_LONG:
12331245
case ZEND_SWITCH_STRING:
1246+
case ZEND_MATCH_LONG:
1247+
case ZEND_MATCH_STRING:
12341248
/* relative extended_value don't have to be changed */
12351249
break;
12361250
#endif

0 commit comments

Comments
 (0)