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) {

0 commit comments

Comments
 (0)