Fix bug in nullness analysis when variable graph has irreducible loops

core/src/main/java/org/teavm/model/analysis/NullnessInformationBuilder.java

@@ -169,7 +169,7 @@ class NullnessInformationBuilder {
                 }
             }
         }
-        assignmentGraph = GraphUtils.removeLoops(builder.build());
+        assignmentGraph = removeLoops(builder.build());
 
         notNullPredecessorsLeft = new int[assignmentGraph.size()];
         for (int i = 0; i < assignmentGraph.size(); ++i) {
@@ -177,6 +177,46 @@ class NullnessInformationBuilder {
         }
     }
 
+    private Graph removeLoops(Graph graph) {
+        int[][] sccs = GraphUtils.findStronglyConnectedComponents(graph);
+        if (sccs.length == 0) {
+            return graph;
+        }
+
+        int[] backMap = new int[graph.size()];
+        for (int i = 0; i < backMap.length; ++i) {
+            backMap[i] = i;
+        }
+        boolean hasNonTrivialSccs = false;
+        GraphBuilder builder = new GraphBuilder(graph.size());
+        for (int[] scc : sccs) {
+            if (scc.length == 1) {
+                continue;
+            }
+            hasNonTrivialSccs = true;
+
+            for (int i = 1; i < scc.length; ++i) {
+                backMap[scc[i]] = scc[0];
+                builder.addEdge(scc[0], scc[i]);
+            }
+        }
+        if (!hasNonTrivialSccs) {
+            return graph;
+        }
+
+        for (int i = 0; i < graph.size(); ++i) {
+            for (int j : graph.outgoingEdges(i)) {
+                if (backMap[j] == i) {
+                    continue;
+                }
+
+                builder.addEdge(i, backMap[j]);
+            }
+        }
+
+        return builder.build();
+    }
+
     private void initNullness(IntDeque queue) {
         NullnessInitVisitor visitor = new NullnessInitVisitor(queue);
         for (BasicBlock block : program.getBasicBlocks()) {

core/src/test/java/org/teavm/model/analysis/test/NullnessAnalysisTest.java

@@ -16,7 +16,9 @@
 package org.teavm.model.analysis.test;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
 import com.carrotsearch.hppc.ObjectByteHashMap;
 import com.carrotsearch.hppc.ObjectByteMap;
 import com.carrotsearch.hppc.cursors.ObjectCursor;
@@ -24,6 +26,7 @@ import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
+import java.nio.charset.StandardCharsets;
 import java.util.HashMap;
 import java.util.Map;
 import org.junit.Rule;
@@ -44,6 +47,7 @@ public class NullnessAnalysisTest {
 
     private static final String NOT_NULL_DIRECTIVE = "// NOT_NULL ";
     private static final String NULLABLE_DIRECTIVE = "// NULLABLE ";
+    private static final String NULL_DIRECTIVE = "// NULL ";
 
     @Test
     public void simple() {
@@ -90,6 +94,11 @@ public class NullnessAnalysisTest {
         test();
     }
 
+    @Test
+    public void irreduciblePhiLoop() {
+        test();
+    }
+
     private void test() {
         String baseName = "model/analysis/nullness/" + name.getMethodName();
         String originalResourceName = baseName + ".original.txt";
@@ -115,8 +124,20 @@ public class NullnessAnalysisTest {
             String varName = varNameCursor.value;
             Variable var = variablesByLabel.get(varName);
             assertNotNull("Variable " + varName + " is missing", var);
-            boolean notNull = expectedNullness.get(varName) != 0;
+            byte nullness = expectedNullness.get(varName);
-            assertEquals("Variable " + varName + " non-null", notNull, information.isNotNull(var));
+            switch (nullness) {
+                case 0:
+                    assertTrue("Variable " + varName + " must be null",  information.isNull(var));
+                    break;
+                case 1:
+                    assertTrue("Variable " + varName + " must be non-null",  information.isNotNull(var));
+                    break;
+                case 2:
+                    assertFalse("Variable " + varName + " must not be null",  information.isNull(var));
+                    assertFalse("Variable " + varName + " must not be non-null", information.isNotNull(var));
+                    break;
+            }
+
         }
 
         information.dispose();
@@ -127,7 +148,7 @@ public class NullnessAnalysisTest {
     private ObjectByteMap<String> extractExpectedNullness(String name) {
         ClassLoader classLoader = NullnessAnalysisTest.class.getClassLoader();
         try (InputStream input = classLoader.getResourceAsStream(name);
-                BufferedReader reader = new BufferedReader(new InputStreamReader(input, "UTF-8"))) {
+                BufferedReader reader = new BufferedReader(new InputStreamReader(input, StandardCharsets.UTF_8))) {
             ObjectByteMap<String> result = new ObjectByteHashMap<>();
 
             while (true) {
@@ -136,7 +157,13 @@ public class NullnessAnalysisTest {
                     break;
                 }
 
-                int index = line.indexOf(NOT_NULL_DIRECTIVE);
+                int index = line.indexOf(NULL_DIRECTIVE);
+                if (index >= 0) {
+                    String variable = line.substring(index + NULL_DIRECTIVE.length()).trim();
+                    result.put(variable, (byte) 0);
+                }
+
+                index = line.indexOf(NOT_NULL_DIRECTIVE);
                 if (index >= 0) {
                     String variable = line.substring(index + NOT_NULL_DIRECTIVE.length()).trim();
                     result.put(variable, (byte) 1);
@@ -145,7 +172,7 @@ public class NullnessAnalysisTest {
                 index = line.indexOf(NULLABLE_DIRECTIVE);
                 if (index >= 0) {
                     String variable = line.substring(index + NULLABLE_DIRECTIVE.length()).trim();
-                    result.put(variable, (byte) 0);
+                    result.put(variable, (byte) 2);
                 }
             }
 

core/src/test/resources/model/analysis/nullness/branch.extended.txt

@@ -15,5 +15,5 @@ $join
     return
 
 // NULLABLE v
-// NULLABLE v_1
+// NULL v_1
 // NOT_NULL v_3

core/src/test/resources/model/analysis/nullness/irreduciblePhiLoop.extended.txt

@@ -0,0 +1,29 @@
+var @this as this
+$start
+    @init := 'qwe'
+    @n := null
+    @xInit := 0
+    goto $head
+$head
+    @a := phi @init from $start, @b from $bodyEnd
+    @x := phi @xInit from $start, @xUpdate from $bodyEnd
+    @twenty := 20
+    @xCmp := @x compareTo @twenty as int
+    if @xCmp == 0 then goto $exit else goto $body
+$body
+    @ten := 10
+    @cmp := @x compareTo @ten as int
+    if @cmp == 0 then goto $update else goto $bodyEnd
+$update
+    goto $bodyEnd
+$bodyEnd
+    @b := phi @a from $body, @n from $update
+    @one := 1
+    @xUpdate := @x + @one as int
+    goto $head
+$exit
+    return
+
+
+// NULLABLE a
+// NULLABLE b

core/src/test/resources/model/analysis/nullness/irreduciblePhiLoop.original.txt

@@ -0,0 +1,25 @@
+var @this as this
+$start
+    @init := 'qwe'
+    @n := null
+    @xInit := 0
+    goto $head
+$head
+    @a := phi @init from $start, @b from $bodyEnd
+    @x := phi @xInit from $start, @xUpdate from $bodyEnd
+    @twenty := 20
+    @xCmp := @x compareTo @twenty as int
+    if @xCmp == 0 then goto $exit else goto $body
+$body
+    @ten := 10
+    @cmp := @x compareTo @ten as int
+    if @cmp == 0 then goto $update else goto $bodyEnd
+$update
+    goto $bodyEnd
+$bodyEnd
+    @b := phi @a from $body, @n from $update
+    @one := 1
+    @xUpdate := @x + @one as int
+    goto $head
+$exit
+    return

core/src/test/resources/model/analysis/nullness/nonDominatedBranch.extended.txt

@@ -13,5 +13,5 @@ $joint
     return @c
 
 // NULLABLE c
-// NULLABLE a_1
+// NULL a_1
 // NOT_NULL a_2

core/src/test/resources/model/analysis/nullness/nullAndNull.extended.txt

@@ -18,8 +18,8 @@ $join
     @v := @b_3
     return
 
-// NULLABLE tmp
+// NULL tmp
-// NULLABLE p
+// NULL p
-// NULLABLE q
+// NULL q
 // NULLABLE u
 // NULLABLE v