Skip to content

Commit d971b67

Browse files
committed
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.
1 parent 96b7251 commit d971b67

File tree

3 files changed

+35
-8
lines changed

3 files changed

+35
-8
lines changed

ext/opcache/Optimizer/ssa_integrity.c

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -287,8 +287,9 @@ int ssa_verify_integrity(zend_op_array *op_array, zend_ssa *ssa, const char *ext
287287

288288
/* Phis */
289289
FOREACH_PHI(phi) {
290-
int source;
291-
FOREACH_PHI_SOURCE(phi, source) {
290+
unsigned num_sources = NUM_PHI_SOURCES(phi);
291+
for (i = 0; i < num_sources; i++) {
292+
int source = phi->sources[i];
292293
if (source < 0) {
293294
FAIL(VARFMT " negative source\n", VAR(phi->ssa_var));
294295
}
@@ -298,7 +299,16 @@ int ssa_verify_integrity(zend_op_array *op_array, zend_ssa *ssa, const char *ext
298299
if (ssa->vars[source].var != ssa->vars[phi->ssa_var].var) {
299300
FAIL(VARFMT " source of phi for " VARFMT "\n", VAR(source), VAR(phi->ssa_var));
300301
}
301-
} FOREACH_PHI_SOURCE_END();
302+
if (phi->use_chains[i]) {
303+
int j;
304+
for (j = i + 1; j < num_sources; j++) {
305+
if (phi->sources[j] == source && phi->use_chains[j]) {
306+
FAIL("use chain for source " VARFMT " of phi " VARFMT
307+
" at %d despite earlier use\n", VAR(source), VAR(phi->ssa_var), j);
308+
}
309+
}
310+
}
311+
}
302312
if (ssa->vars[phi->ssa_var].definition_phi != phi) {
303313
FAIL(VARFMT " does not define this phi\n", VAR(phi->ssa_var));
304314
}

ext/opcache/Optimizer/zend_ssa.c

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1298,11 +1298,7 @@ static inline void zend_ssa_remove_phi_source(zend_ssa *ssa, zend_ssa_phi *phi,
12981298
for (j = 0; j < predecessors_count; j++) {
12991299
if (phi->sources[j] == var_num) {
13001300
if (j < pred_offset) {
1301-
if (next_phi == NULL) {
1302-
next_phi = phi->use_chains[pred_offset];
1303-
} else {
1304-
ZEND_ASSERT(phi->use_chains[pred_offset] == NULL);
1305-
}
1301+
ZEND_ASSERT(next_phi == NULL);
13061302
} else if (j >= pred_offset) {
13071303
phi->use_chains[j] = next_phi;
13081304
}
@@ -1591,6 +1587,8 @@ void zend_ssa_rename_var_uses(zend_ssa *ssa, int old, int new, zend_bool update_
15911587
new_var->phi_use_chain = phi;
15921588
}
15931589
after_first_new_source = 1;
1590+
} else {
1591+
phi->use_chains[j] = NULL;
15941592
}
15951593
}
15961594
}

ext/opcache/tests/phi_use_chain.phpt

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
--TEST--
2+
Check that phi use chains are correctly maintained when removing blocks
3+
--FILE--
4+
<?php
5+
6+
function test(array $adapters) {
7+
foreach ($adapters as $adapter) {
8+
if (\in_array('cli-server', ['cli', 'phpdbg'], true) && $adapter instanceof stdClass && !\filter_var('1', \FILTER_VALIDATE_BOOLEAN)) {
9+
continue;
10+
}
11+
12+
$adapters[] = $adapter;
13+
}
14+
}
15+
16+
?>
17+
===DONE===
18+
--EXPECT--
19+
===DONE===

0 commit comments

Comments
 (0)