Fix phi use chain management when renaming variable

If there is a previous use of the new variable in the phi, we need
to NULL out the use chain of the new source we're adding.

Test case is reduced from an assertion failure in the Symfony Demo.

diff --git a/ext/opcache/Optimizer/ssa_integrity.c b/ext/opcache/Optimizer/ssa_integrity.c
index ede40be59a75fffa3c33d5a7cb6f35ba55049ad2..9d815b78883e6cec6246f6b38edb65a20e8b71c0 100644 (file)
--- a/ext/opcache/Optimizer/ssa_integrity.c
+++ b/ext/opcache/Optimizer/ssa_integrity.c
@@ -287,8 +287,9 @@ int ssa_verify_integrity(zend_op_array *op_array, zend_ssa *ssa, const char *ext
 
        /* Phis */
        FOREACH_PHI(phi) {
-               int source;
-               FOREACH_PHI_SOURCE(phi, source) {
+               unsigned num_sources = NUM_PHI_SOURCES(phi);
+               for (i = 0; i < num_sources; i++) {
+                       int source = phi->sources[i];
                        if (source < 0) {
                                FAIL(VARFMT " negative source\n", VAR(phi->ssa_var));
                        }
@@ -298,7 +299,16 @@ int ssa_verify_integrity(zend_op_array *op_array, zend_ssa *ssa, const char *ext
                        if (ssa->vars[source].var != ssa->vars[phi->ssa_var].var) {
                                FAIL(VARFMT " source of phi for " VARFMT "\n", VAR(source), VAR(phi->ssa_var));
                        }
-               } FOREACH_PHI_SOURCE_END();
+                       if (phi->use_chains[i]) {
+                               int j;
+                               for (j = i + 1; j < num_sources; j++) {
+                                       if (phi->sources[j] == source && phi->use_chains[j]) {
+                                               FAIL("use chain for source " VARFMT " of phi " VARFMT
+                                                       " at %d despite earlier use\n", VAR(source), VAR(phi->ssa_var), j);
+                                       }
+                               }
+                       }
+               }
                if (ssa->vars[phi->ssa_var].definition_phi != phi) {
                        FAIL(VARFMT " does not define this phi\n", VAR(phi->ssa_var));
                }

diff --git a/ext/opcache/Optimizer/zend_ssa.c b/ext/opcache/Optimizer/zend_ssa.c
index 89dfe57914a470aa846b6a719fcb088be158ba57..0b8c76d3ac29e425e53725e77eeb40130795632d 100644 (file)
--- a/ext/opcache/Optimizer/zend_ssa.c
+++ b/ext/opcache/Optimizer/zend_ssa.c
@@ -1298,11 +1298,7 @@ static inline void zend_ssa_remove_phi_source(zend_ssa *ssa, zend_ssa_phi *phi,
        for (j = 0; j < predecessors_count; j++) {
                if (phi->sources[j] == var_num) {
                        if (j < pred_offset) {
-                               if (next_phi == NULL) {
-                                       next_phi = phi->use_chains[pred_offset];
-                               } else {
-                                       ZEND_ASSERT(phi->use_chains[pred_offset] == NULL);
-                               }
+                               ZEND_ASSERT(next_phi == NULL);
                        } else if (j >= pred_offset) {
                                phi->use_chains[j] = next_phi;
                        }
@@ -1591,6 +1587,8 @@ void zend_ssa_rename_var_uses(zend_ssa *ssa, int old, int new, zend_bool update_
                                                new_var->phi_use_chain = phi;
                                        }
                                        after_first_new_source = 1;
+                               } else {
+                                       phi->use_chains[j] = NULL;
                                }
                        }
                }

diff --git a/ext/opcache/tests/phi_use_chain.phpt b/ext/opcache/tests/phi_use_chain.phpt
new file mode 100644 (file)
index 0000000..173d874
--- /dev/null
+++ b/ext/opcache/tests/phi_use_chain.phpt
@@ -0,0 +1,19 @@
+--TEST--
+Check that phi use chains are correctly maintained when removing blocks
+--FILE--
+<?php
+
+function test(array $adapters) {
+    foreach ($adapters as $adapter) {
+        if (\in_array('cli-server', ['cli', 'phpdbg'], true) && $adapter instanceof stdClass && !\filter_var('1', \FILTER_VALIDATE_BOOLEAN)) {
+            continue;
+        }
+
+        $adapters[] = $adapter;
+    }
+}
+
+?>
+===DONE===
+--EXPECT--
+===DONE===