Fixed incorect constant conditional jump elimination

diff --git a/NEWS b/NEWS
index f5c9cac6de566a5bf20c3a46b7cebccecf68a736..4fcd94ff2020c3c4e29e865b7abceead2e871a60 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -5,6 +5,9 @@ PHP                                                                        NEWS
 - GD:
   . Fixed bug #75139 (libgd/gd_interpolation.c:1786: suspicious if ?). (cmb)
 
+- Opcache
+  . Fixed incorect constant conditional jump elimination. (Dmitry)
+
 - OpenSSL
   . Automatically load OpenSSL configuration file. (Jakub Zelenka)
 

diff --git a/ext/opcache/Optimizer/block_pass.c b/ext/opcache/Optimizer/block_pass.c
index 41934f192b6012ed36e589d12c87be6bae97f537..738e2838068d28b6284c42649bf4dfa600b2fa79 100644 (file)
--- a/ext/opcache/Optimizer/block_pass.c
+++ b/ext/opcache/Optimizer/block_pass.c
@@ -195,6 +195,52 @@ static void zend_optimize_block(zend_basic_block *block, zend_op_array *op_array
                                                VAR_SOURCE(op1) = NULL;
                                                literal_dtor(&ZEND_OP1_LITERAL(src));
                                                MAKE_NOP(src);
+                                               switch (opline->opcode) {
+                                                       case ZEND_JMPZ:
+                                                               if (zend_is_true(&ZEND_OP1_LITERAL(opline))) {
+                                                                       MAKE_NOP(opline);
+                                                                       DEL_SOURCE(block, block->successors[0]);
+                                                                       block->successors_count = 1;
+                                                                       block->successors[0] = block->successors[1];
+                                                               } else {
+                                                                       opline->opcode = ZEND_JMP;
+                                                                       COPY_NODE(opline->op1, opline->op2);
+                                                                       DEL_SOURCE(block, block->successors[1]);
+                                                                       block->successors_count = 1;
+                                                               }
+                                                               break;
+                                                       case ZEND_JMPNZ:
+                                                               if (zend_is_true(&ZEND_OP1_LITERAL(opline))) {
+                                                                       opline->opcode = ZEND_JMP;
+                                                                       COPY_NODE(opline->op1, opline->op2);
+                                                                       DEL_SOURCE(block, block->successors[1]);
+                                                                       block->successors_count = 1;
+                                                               } else {
+                                                                       MAKE_NOP(opline);
+                                                                       DEL_SOURCE(block, block->successors[0]);
+                                                                       block->successors_count = 1;
+                                                                       block->successors[0] = block->successors[1];
+                                                               }
+                                                               break;
+                                                       case ZEND_JMPZNZ:
+                                                               if (zend_is_true(&ZEND_OP1_LITERAL(opline))) {
+                                                                       zend_op *target_opline = ZEND_OFFSET_TO_OPLINE(opline, opline->extended_value);
+                                                                       ZEND_SET_OP_JMP_ADDR(opline, opline->op1, target_opline);
+                                                                       DEL_SOURCE(block, block->successors[0]);
+                                                                       block->successors[0] = block->successors[1];
+                                                               } else {
+                                                                       zend_op *target_opline = ZEND_OP2_JMP_ADDR(opline);
+                                                                       ZEND_SET_OP_JMP_ADDR(opline, opline->op1, target_opline);
+                                                                       DEL_SOURCE(block, block->successors[0]);
+                                                               }
+                                                               block->successors_count = 1;
+                                                               opline->op1_type = IS_UNUSED;
+                                                               opline->extended_value = 0;
+                                                               opline->opcode = ZEND_JMP;
+                                                               break;
+                                                       default:
+                                                               break;
+                                               }
                                        } else {
                                                zval_ptr_dtor_nogc(&c);
                                        }

diff --git a/ext/opcache/Optimizer/sccp.c b/ext/opcache/Optimizer/sccp.c
index 24af78ee5db7c4749f522dd12e0146bc30c51c2b..e061e209c7752e89701d26ee65a3914715d39473 100644 (file)
--- a/ext/opcache/Optimizer/sccp.c
+++ b/ext/opcache/Optimizer/sccp.c
@@ -234,6 +234,93 @@ static zend_bool try_replace_op1(
                zval zv;
                ZVAL_COPY(&zv, value);
                if (zend_optimizer_update_op1_const(ctx->scdf.op_array, opline, &zv)) {
+                       /* Reconstruct SSA */
+                       int num;
+                       zend_basic_block *block;
+
+                       switch (opline->opcode) {
+                               case ZEND_JMPZ:
+                                       if (zend_is_true(&zv)) {
+                                               MAKE_NOP(opline);
+                                               num = ctx->scdf.ssa->cfg.map[opline - ctx->scdf.op_array->opcodes];
+                                               block = &ctx->scdf.ssa->cfg.blocks[num];
+                                               if (block->successors_count == 2) {
+                                                       if (block->successors[1] != block->successors[0]) {
+                                                               zend_ssa_remove_predecessor(ctx->scdf.ssa, num, block->successors[0]);
+                                                       }
+                                                       block->successors_count = 1;
+                                                       block->successors[0] = block->successors[1];
+                                               }
+                                       } else {
+                                               opline->opcode = ZEND_JMP;
+                                               COPY_NODE(opline->op1, opline->op2);
+                                               num = ctx->scdf.ssa->cfg.map[opline - ctx->scdf.op_array->opcodes];
+                                               block = &ctx->scdf.ssa->cfg.blocks[num];
+                                               if (block->successors_count == 2) {
+                                                       if (block->successors[1] != block->successors[0]) {
+                                                               zend_ssa_remove_predecessor(ctx->scdf.ssa, num, block->successors[1]);
+                                                       }
+                                                       block->successors_count = 1;
+                                               }
+                                       }
+                                       break;
+                               case ZEND_JMPNZ:
+                                       if (zend_is_true(&zv)) {
+                                               opline->opcode = ZEND_JMP;
+                                               COPY_NODE(opline->op1, opline->op2);
+                                               num = ctx->scdf.ssa->cfg.map[opline - ctx->scdf.op_array->opcodes];
+                                               block = &ctx->scdf.ssa->cfg.blocks[num];
+                                               if (block->successors_count == 2) {
+                                                       if (block->successors[1] != block->successors[0]) {
+                                                               zend_ssa_remove_predecessor(ctx->scdf.ssa, num, block->successors[1]);
+                                                       }
+                                                       block->successors_count = 1;
+                                               }
+                                       } else {
+                                               MAKE_NOP(opline);
+                                               num = ctx->scdf.ssa->cfg.map[opline - ctx->scdf.op_array->opcodes];
+                                               block = &ctx->scdf.ssa->cfg.blocks[num];
+                                               if (block->successors_count == 2) {
+                                                       if (block->successors[1] != block->successors[0]) {
+                                                               zend_ssa_remove_predecessor(ctx->scdf.ssa, num, block->successors[0]);
+                                                       }
+                                                       block->successors_count = 1;
+                                                       block->successors[0] = block->successors[1];
+                                               }
+                                       }
+                                       break;
+                               case ZEND_JMPZNZ:
+                                       if (zend_is_true(&zv)) {
+                                               zend_op *target_opline = ZEND_OFFSET_TO_OPLINE(opline, opline->extended_value);
+                                               ZEND_SET_OP_JMP_ADDR(opline, opline->op1, target_opline);
+                                               num = ctx->scdf.ssa->cfg.map[opline - ctx->scdf.op_array->opcodes];
+                                               block = &ctx->scdf.ssa->cfg.blocks[num];
+                                               if (block->successors_count == 2) {
+                                                       if (block->successors[1] != block->successors[0]) {
+                                                               zend_ssa_remove_predecessor(ctx->scdf.ssa, num, block->successors[0]);
+                                                       }
+                                                       block->successors_count = 1;
+                                                       block->successors[0] = block->successors[1];
+                                               }
+                                       } else {
+                                               zend_op *target_opline = ZEND_OP2_JMP_ADDR(opline);
+                                               ZEND_SET_OP_JMP_ADDR(opline, opline->op1, target_opline);
+                                               num = ctx->scdf.ssa->cfg.map[opline - ctx->scdf.op_array->opcodes];
+                                               block = &ctx->scdf.ssa->cfg.blocks[num];
+                                               if (block->successors_count == 2) {
+                                                       if (block->successors[1] != block->successors[0]) {
+                                                               zend_ssa_remove_predecessor(ctx->scdf.ssa, num, block->successors[1]);
+                                                       }
+                                                       block->successors_count = 1;
+                                               }
+                                       }
+                                       opline->op1_type = IS_UNUSED;
+                                       opline->extended_value = 0;
+                                       opline->opcode = ZEND_JMP;
+                                       break;
+                               default:
+                                       break;
+                       }
                        return 1;
                } else {
                        // TODO: check the following special cases ???

diff --git a/ext/opcache/Optimizer/zend_optimizer.c b/ext/opcache/Optimizer/zend_optimizer.c
index f8f42a127704b67715561a838e70be651437b917..cc197c0e5206d2f42316b46a50c9faac505900b8 100644 (file)
--- a/ext/opcache/Optimizer/zend_optimizer.c
+++ b/ext/opcache/Optimizer/zend_optimizer.c
@@ -258,41 +258,7 @@ int zend_optimizer_update_op1_const(zend_op_array *op_array,
                                     zend_op       *opline,
                                     zval          *val)
 {
-       zend_op *target_opline;
-
        switch (opline->opcode) {
-               case ZEND_JMPZ:
-                       if (zend_is_true(val)) {
-                               MAKE_NOP(opline);
-                       } else {
-                               opline->opcode = ZEND_JMP;
-                               COPY_NODE(opline->op1, opline->op2);
-                               opline->op2_type = IS_UNUSED;
-                       }
-                       zval_ptr_dtor_nogc(val);
-                       return 1;
-               case ZEND_JMPNZ:
-                       if (zend_is_true(val)) {
-                               opline->opcode = ZEND_JMP;
-                               COPY_NODE(opline->op1, opline->op2);
-                               opline->op2_type = IS_UNUSED;
-                       } else {
-                               MAKE_NOP(opline);
-                       }
-                       zval_ptr_dtor_nogc(val);
-                       return 1;
-               case ZEND_JMPZNZ:
-                       if (zend_is_true(val)) {
-                               target_opline = ZEND_OFFSET_TO_OPLINE(opline, opline->extended_value);
-                       } else {
-                               target_opline = ZEND_OP2_JMP_ADDR(opline);
-                       }
-                       ZEND_SET_OP_JMP_ADDR(opline, opline->op1, target_opline);
-                       opline->op1_type = IS_UNUSED;
-                       opline->extended_value = 0;
-                       opline->opcode = ZEND_JMP;
-                       zval_ptr_dtor_nogc(val);
-                       return 1;
                case ZEND_FREE:
                        MAKE_NOP(opline);
                        zval_ptr_dtor_nogc(val);

diff --git a/ext/opcache/Optimizer/zend_ssa.c b/ext/opcache/Optimizer/zend_ssa.c
index e100be426c32f03ff3f6c8e641dfca928949f219..38111659487856f6696fd39a6a6e0c10fcc3b50b 100644 (file)
--- a/ext/opcache/Optimizer/zend_ssa.c
+++ b/ext/opcache/Optimizer/zend_ssa.c
@@ -1339,6 +1339,52 @@ void zend_ssa_remove_uses_of_var(zend_ssa *ssa, int var_num) /* {{{ */
 }
 /* }}} */
 
+void zend_ssa_remove_predecessor(zend_ssa *ssa, int from, int to) /* {{{ */
+{
+       zend_basic_block *next_block = &ssa->cfg.blocks[to];
+       zend_ssa_block *next_ssa_block = &ssa->blocks[to];
+       zend_ssa_phi *phi;
+       int j;
+
+       /* Find at which predecessor offset this block is referenced */
+       int pred_offset = -1;
+       int *predecessors = &ssa->cfg.predecessors[next_block->predecessor_offset];
+
+       for (j = 0; j < next_block->predecessors_count; j++) {
+               if (predecessors[j] == from) {
+                       pred_offset = j;
+                       break;
+               }
+       }
+
+       /* If there are duplicate successors, the predecessors may have been removed in
+        * a previous iteration already. */
+       if (pred_offset == -1) {
+               return;
+       }
+
+       /* For phis in successor blocks, remove the operands associated with this block */
+       for (phi = next_ssa_block->phis; phi; phi = phi->next) {
+               if (phi->pi >= 0) {
+                       if (phi->pi == from) {
+                               zend_ssa_remove_uses_of_var(ssa, phi->ssa_var);
+                               zend_ssa_remove_phi(ssa, phi);
+                       }
+               } else {
+                       ZEND_ASSERT(phi->sources[pred_offset] >= 0);
+                       zend_ssa_remove_phi_source(ssa, phi, pred_offset, next_block->predecessors_count);
+               }
+       }
+
+       /* Remove this predecessor */
+       next_block->predecessors_count--;
+       if (pred_offset < next_block->predecessors_count) {
+               predecessors = &ssa->cfg.predecessors[next_block->predecessor_offset + pred_offset];
+               memmove(predecessors, predecessors + 1, (next_block->predecessors_count - pred_offset) * sizeof(uint32_t));
+       }
+}
+/* }}} */
+
 void zend_ssa_remove_block(zend_op_array *op_array, zend_ssa *ssa, int i) /* {{{ */
 {
        zend_basic_block *block = &ssa->cfg.blocks[i];
@@ -1369,45 +1415,7 @@ void zend_ssa_remove_block(zend_op_array *op_array, zend_ssa *ssa, int i) /* {{{
        }
 
        for (s = 0; s < block->successors_count; s++) {
-               zend_basic_block *next_block = &ssa->cfg.blocks[block->successors[s]];
-               zend_ssa_block *next_ssa_block = &ssa->blocks[block->successors[s]];
-               zend_ssa_phi *phi;
-
-               /* Find at which predecessor offset this block is referenced */
-               int pred_offset = -1;
-               predecessors = &ssa->cfg.predecessors[next_block->predecessor_offset];
-               for (j = 0; j < next_block->predecessors_count; j++) {
-                       if (predecessors[j] == i) {
-                               pred_offset = j;
-                               break;
-                       }
-               }
-
-               /* If there are duplicate successors, the predecessors may have been removed in
-                * a previous iteration already. */
-               if (pred_offset == -1) {
-                       continue;
-               }
-
-               /* For phis in successor blocks, remove the operands associated with this block */
-               for (phi = next_ssa_block->phis; phi; phi = phi->next) {
-                       if (phi->pi >= 0) {
-                               if (phi->pi == i) {
-                                       zend_ssa_remove_uses_of_var(ssa, phi->ssa_var);
-                                       zend_ssa_remove_phi(ssa, phi);
-                               }
-                       } else {
-                               ZEND_ASSERT(phi->sources[pred_offset] >= 0);
-                               zend_ssa_remove_phi_source(ssa, phi, pred_offset, next_block->predecessors_count);
-                       }
-               }
-
-               /* Remove this predecessor */
-               next_block->predecessors_count--;
-               if (pred_offset < next_block->predecessors_count) {
-                       predecessors = &ssa->cfg.predecessors[next_block->predecessor_offset + pred_offset];
-                       memmove(predecessors, predecessors + 1, (next_block->predecessors_count - pred_offset) * sizeof(uint32_t));
-               }
+               zend_ssa_remove_predecessor(ssa, i, block->successors[s]);
        }
 
        /* Remove successors of predecessors */

diff --git a/ext/opcache/Optimizer/zend_ssa.h b/ext/opcache/Optimizer/zend_ssa.h
index f5855f24530e3bea797563997e48e273355cc881..b281305de23953e23e39fd0eb0699550802e710f 100644 (file)
--- a/ext/opcache/Optimizer/zend_ssa.h
+++ b/ext/opcache/Optimizer/zend_ssa.h
@@ -139,6 +139,7 @@ int zend_build_ssa(zend_arena **arena, const zend_script *script, const zend_op_
 int zend_ssa_compute_use_def_chains(zend_arena **arena, const zend_op_array *op_array, zend_ssa *ssa);
 int zend_ssa_unlink_use_chain(zend_ssa *ssa, int op, int var);
 
+void zend_ssa_remove_predecessor(zend_ssa *ssa, int from, int to);
 void zend_ssa_remove_instr(zend_ssa *ssa, zend_op *opline, zend_ssa_op *ssa_op);
 void zend_ssa_remove_phi(zend_ssa *ssa, zend_ssa_phi *phi);
 void zend_ssa_remove_uses_of_var(zend_ssa *ssa, int var_num);

diff --git a/ext/opcache/tests/jmp_elim_001.phpt b/ext/opcache/tests/jmp_elim_001.phpt
new file mode 100644 (file)
index 0000000..349816e
--- /dev/null
+++ b/ext/opcache/tests/jmp_elim_001.phpt
@@ -0,0 +1,18 @@
+--TEST--
+Edge-cases in constant conditional jump elimination
+--SKIPIF--
+<?php if (PHP_INT_SIZE != 8) die("skip for machines with 64-bit longs"); ?>
+<?php require_once('skipif.inc'); ?>
+--FILE--
+<?php
+$webserver = "Apache";
+$cpuArc = "x86_64";
+$archName = (strstr($cpuArc, "64") || PHP_INT_SIZE === 8) ? "64" : "32";
+$info = array('os' => PHP_OS,
+              'phpversion' => phpversion(),
+              'arch' => $archName,
+              'webserver' =>$webserver);
+header('Content-Type: application/json');
+echo json_encode($info) . "\n";
+--EXPECT--
+{"os":"Linux","phpversion":"7.2.0-dev","arch":"64","webserver":"Apache"}