Fix incorrect sccp optimization of zend_switch with strict comparison

iluuu1994 · iluuu1994 · commit 3da5f996e647 · 2020-04-14T23:00:29.000+02:00
diff --git a/Zend/tests/match/018.phpt b/Zend/tests/match/018.phpt
@@ -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
diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
@@ -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));
 		}
diff --git a/Zend/zend_compile.h b/Zend/zend_compile.h
@@ -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()
 
diff --git a/ext/opcache/Optimizer/sccp.c b/ext/opcache/Optimizer/sccp.c
@@ -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;
@@ -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;
@@ -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]);

Original file line number	Diff line number	Diff line change
`@@ -5251,6 +5251,7 @@ void zend_compile_match(znode result, zend_ast ast) /* {{{ */`
`5251`	`5251`	`zend_op *opline = zend_emit_op(NULL,`
`5252`	`5252`	`jumptable_type == IS_LONG ? ZEND_SWITCH_LONG : ZEND_SWITCH_STRING,`
`5253`	`5253`	`&expr_node, &jumptable_op);`
	`5254`	`+ opline->extended_value = ZEND_SWITCH_STRICT_COMPARISON;`
`5254`	`5255`	`if (opline->op1_type == IS_CONST) {`
`5255`	`5256`	`Z_TRY_ADDREF_P(CT_CONSTANT(opline->op1));`
`5256`	`5257`	`}`