analysis: fix out-of-SSA interference checks

miasm/analysis/outofssa.py

@@ -248,54 +248,35 @@ def merge_nodes_interfere(self, node_a, node_b, parent):
             # Defined in the same AssignBlock -> interfere
             return True
 
+        # Virtual copies are defined in their predecessors, not at the Phi.
+        if node_a in self.new_var_to_srcs_parents:
+            return self.ssa_def_is_live_at(node_a, node_b, parent)
+        if node_b in self.new_var_to_srcs_parents:
+            return self.ssa_def_is_live_at(node_b, node_a, parent)
+
         if self.var_to_varinfo[node_a].live_index < self.var_to_varinfo[node_b].live_index:
             return self.ssa_def_is_live_at(node_a, node_b, parent)
         return self.ssa_def_is_live_at(node_b, node_a, parent)
 
     def merge_sets_interfere(self, merge_a, merge_b, parent):
         """
-        Return True if no variable in @merge_a and @merge_b interferes.
+        Return True if a variable in @merge_a interferes with one in @merge_b.
 
-        Implementation of "Algorithm 2: Check intersection in a set of variables"
+        Virtual phi copies can be defined in several predecessor blocks, so
+        checking only adjacent variables in dominance order can miss conflicts.
 
-        @merge_a: a dom ordered list of equivalent variables
-        @merge_b: a dom ordered list of equivalent variables
+        @merge_a: a set of equivalent variables
+        @merge_b: a set of equivalent variables
         @parent: Optional parent location of the phi source for liveness tests
         """
         if merge_a == merge_b:
             # No need to consider interference if equal
             return False
 
-        merge_a_list = self.merge_set_sort(merge_a)
-        merge_b_list = self.merge_set_sort(merge_b)
-        dom = []
-        while merge_a_list or merge_b_list:
-            if not merge_a_list:
-                _, current = merge_b_list.pop(0)
-            elif not merge_b_list:
-                _, current = merge_a_list.pop(0)
-            else:
-                # compare live_indexes (standing for dominance)
-                if merge_a_list[-1] < merge_b_list[-1]:
-                    _, current = merge_a_list.pop(0)
-                else:
-                    _, current = merge_b_list.pop(0)
-            while dom and not self.ssa_def_dominates(dom[-1], current):
-                dom.pop()
-
-            # Don't test node in same merge_set
-            if (
-                    # Is stack not empty?
-                    dom and
-                    # Trivial non-interference if dom.top() and current come
-                    # from the same merge set
-                    not (dom[-1] in merge_a and current in merge_a) and
-                    not (dom[-1] in merge_b and current in merge_b) and
-                    # Actually test for interference
-                    self.merge_nodes_interfere(current, dom[-1], parent)
-            ):
+        for node_a in merge_a:
+            for node_b in merge_b:
+                if self.merge_nodes_interfere(node_a, node_b, parent):
                     return True
-            dom.append(current)
         return False
 
     def aggressive_coalesce_parallel_copy(self, parallel_copies, parent):

test/analysis/unssa.py

@@ -1,12 +1,18 @@
 """ Test cases for dead code elimination"""
+from itertools import permutations, product
+
 from future.utils import viewvalues
 
 from miasm.expression.expression import ExprId, ExprInt, ExprAssign, ExprMem, \
-    ExprCond, ExprLoc
+    ExprCond, ExprLoc, ExprOp
 from miasm.core.locationdb import LocationDB
+from miasm.analysis.data_flow import DiGraphLivenessSSA
+from miasm.analysis.outofssa import UnSSADiGraph
 from miasm.analysis.simplifier import IRCFGSimplifierSSA
+from miasm.analysis.ssa import SSADiGraph
 from miasm.ir.analysis import LifterModelCall
 from miasm.ir.ir import IRCFG, IRBlock, AssignBlock
+from miasm.ir.symbexec import SymbolicExecutionEngine
 
 loc_db = LocationDB()
 
@@ -619,3 +625,46 @@ def get_forbidden_regs(self):
     open('final_%d.dot' % test_nb, 'w').write(ircfg.dot())
 
     # XXX TODO: add real regression test
+
+
+# Issue #1535: merging phi copies must preserve simultaneously live values.
+ssa_vars = [ExprId("v%d" % index, 32) for index in range(5)]
+blocks = [
+    (LBL0, [
+        [ExprAssign(ssa_vars[0], CST0)],
+        [ExprAssign(IRDst, ExprCond(x, ExprLoc(LBL1, 32), ExprLoc(LBL2, 32)))]
+    ]),
+    (LBL1, [
+        [ExprAssign(ssa_vars[1], CST1)],
+        [ExprAssign(ssa_vars[3], CST3)],
+        [ExprAssign(IRDst, ExprCond(y, ExprLoc(LBL2, 32), ExprLoc(LBL3, 32)))]
+    ]),
+    (LBL2, [
+        [ExprAssign(ssa_vars[2], ExprOp("Phi", ssa_vars[0], ssa_vars[1]))],
+        [ExprAssign(IRDst, ExprLoc(LBL3, 32))]
+    ]),
+    (LBL3, [
+        [ExprAssign(ssa_vars[4], ExprOp("Phi", ssa_vars[2], ssa_vars[3]))],
+        [ExprAssign(r, ssa_vars[4])],
+        [ExprAssign(IRDst, END)]
+    ])
+]
+for block_order in permutations(blocks):
+    ircfg = IRA.new_ircfg()
+    for label, assignments in block_order:
+        ircfg.add_irblock(gen_irblock(label, assignments))
+
+    ssa = SSADiGraph(ircfg)
+    ssa.immutable_ids.add(IRDst)
+    cfg_liveness = DiGraphLivenessSSA(ircfg)
+    cfg_liveness.init_var_info(IRA)
+    cfg_liveness.compute_liveness()
+    unssa = UnSSADiGraph(ssa, LBL0, cfg_liveness)
+    assert not unssa.merge_sets_interfere(set([ssa_vars[0]]), set([r]), None)
+
+    for x_value, y_value in product([0, 1], repeat=2):
+        state = {x: ExprInt(x_value, 32), y: ExprInt(y_value, 32)}
+        symbexec = SymbolicExecutionEngine(IRA, state)
+        symbexec.run_at(ircfg, LBL0)
+        expected = CST0 if not x_value else CST1 if y_value else CST3
+        assert symbexec.eval_expr(r) == expected