Fix incorrect dominator filtering on finally blocks

src/org/jetbrains/java/decompiler/modules/decompiler/decompose/DomHelper.java

@@ -183,9 +183,12 @@ public static VBStyleCollection<List<Integer>, Integer> calcPostDominators(State
         lstPosts.add(stt.id);
       }
 
+      // The postdom list for this statement must be sorted based on the post reverse postorder of the general statement that it's contained in
+      // This should lead to proper iteration during general statement creation.
       lstPosts.sort(Comparator.comparing(mapSortOrder::get));
 
-      if (lstPosts.size() > 1 && (int) lstPosts.get(0) == st.id) {
+      // After sorting, ensure that the statement that owns this postdominance list comes last, if it comes first.
+      if (lstPosts.size() > 1 && lstPosts.get(0) == st.id) {
         lstPosts.add(lstPosts.remove(0));
       }
 
@@ -432,7 +435,7 @@ private static boolean processStatement(
 
             tracer.info(general, "Find simple statements");
 
-            // Find statements in this subgraph from the basicblocks that comprise it
+            // Find statements in this subgraph from the basic blocks that comprise it
             if (findSimpleStatements(general, mapExtPost, tracer)) {
               tracer.success(general, "Found some simple statements");
               reducibility = 0;
@@ -448,7 +451,7 @@ private static boolean processStatement(
             Statement stat = findGeneralStatement(general, forceall, mapExtPost/*, finallyInfos*/);
 
             if (stat != null) {
-              tracer.success(general, "Found general statement: " + stat, stat);
+              tracer.successCreated(general, "Found general statement: " + stat + " (" + stat.getStats() + ")", stat);
               // Recurse on the subgraph general statement that we found, and inherit the postdominator set if it's the first statement in the current general
               boolean complete = processStatement(stat, root, general.getFirst() == stat ? mapExtPost : new HashMap<>(), tracer /*, finallyInfos*/);
 
@@ -633,9 +636,12 @@ private static Statement findGeneralStatement(
         }
         setHandlers.removeAll(setNodes);
 
+        // Make sure that all the nodes that we're trying to create a new subgraph from are found in any relevant exception handlers
+        // This serves to avoid breaking the statement finder by having a subgraph only contain part of an exception handler
         boolean exceptionsOk = true;
         for (Statement handler : setHandlers) {
-          if (!handler.getNeighbours(StatEdge.TYPE_EXCEPTION, EdgeDirection.BACKWARD).containsAll(setNodes)) {
+          List<Statement> exceptionRange = handler.getNeighbours(StatEdge.TYPE_EXCEPTION, EdgeDirection.BACKWARD);
+          if (!exceptionRange.containsAll(setNodes)) {
             exceptionsOk = false;
             break;
           }
@@ -698,7 +704,7 @@ private static boolean findSimpleStatements(Statement stat, HashMap<Integer, Set
         Statement result = detectStatement(st);
 
         if (result != null) {
-          tracer.success(stat, "Transformed " + st + " to " + result, st);
+//          tracer.successCreated(stat, "Detected " + st + " to " + result + " (" + result.getStats() + ")", st);
 
           // If the statement we created contains the first statement of the general statement as it's first, we know that we've completed iteration to the point where every statment in the subgraph has been explored at least once, due to how the post order is created.
           // More iteration still happens to discover higher level structures (such as the case where basicblock -> if -> loop)
@@ -710,7 +716,7 @@ private static boolean findSimpleStatements(Statement stat, HashMap<Integer, Set
 
           stat.collapseNodesToStatement(result);
 
-          tracer.success(stat, "Transformed " + st + " to " + result, result);
+          tracer.successCreated(stat, "Transformed " + st + " to " + result + " (" + result.getStats() + ")", result);
 
           // update the postdominator map
           if (!mapExtPost.isEmpty()) {

src/org/jetbrains/java/decompiler/modules/decompiler/decompose/DomTracer.java

@@ -8,8 +8,8 @@
 import java.util.Map;
 
 class DomTracer {
-  private static final boolean COLLECT_STRINGS = false;
-  private static final boolean COLLECT_DOTS = false;
+  private static final boolean COLLECT_STRINGS = true;
+  private static final boolean COLLECT_DOTS = true;
 
   private final String filePrefix;
   private final StructMethod structMethod;
@@ -22,10 +22,6 @@ class DomTracer {
   }
 
   private void add(Statement gen, String s, Map<Statement, String> props) {
-    if (COLLECT_STRINGS) {
-      this.string += ("[" + gen + "] " + s + "\n");
-    }
-
     if (COLLECT_DOTS) {
       HashMap<Statement, String> map = new HashMap<>(props);
       if (map.containsKey(gen)) {
@@ -34,6 +30,14 @@ private void add(Statement gen, String s, Map<Statement, String> props) {
         map.put(gen, "xlabel=\"" + s + "\"");
       }
       DotExporter.toDotFile(gen, this.structMethod, this.filePrefix, "g" + this.counter, map);
+
+      if (COLLECT_STRINGS) {
+        string += "(g" + this.counter +") ";
+      }
+    }
+
+    if (COLLECT_STRINGS) {
+      this.string += ("[" + gen + "] " + s + "\n");
     }
 
     this.counter++;
@@ -52,11 +56,17 @@ void info(Statement stat, String s) {
   }
 
   void success(Statement stat, String s) {
-    this.add(stat, s, Map.of(stat, "fillcolor=lightgreen,style=filled"));
+    this.add(stat, s, Map.of(stat, "fillcolor=lawngreen,style=filled"));
   }
 
-  void success(Statement stat, String s, Statement stat2) {
-    this.add(stat, s, Map.of(stat, "fillcolor=lightgreen,style=filled", stat2, "fillcolor=lawngreen,style=filled"));
+  void successCreated(Statement stat, String s, Statement newStat) {
+    Map<Statement, String> props = new HashMap<>();
+    props.put(stat, "fillcolor=orange,style=filled");
+    props.put(newStat, "fillcolor=lawngreen,style=filled");
+    for (Statement st : newStat.getStats()) {
+      props.put(st, "fillcolor=pink,style=filled");
+    }
+    this.add(stat, s, props);
   }
 
   @Override

src/org/jetbrains/java/decompiler/modules/decompiler/decompose/DominatorTreeExceptionFilter.java

@@ -4,6 +4,7 @@
 import org.jetbrains.java.decompiler.modules.decompiler.StatEdge;
 import org.jetbrains.java.decompiler.modules.decompiler.stats.Statement;
 import org.jetbrains.java.decompiler.modules.decompiler.stats.Statement.EdgeDirection;
+import org.jetbrains.java.decompiler.util.DotExporter;
 import org.jetbrains.java.decompiler.util.collections.VBStyleCollection;
 
 import java.util.*;
@@ -76,13 +77,16 @@ private void buildDominatorTree() {
 
   private void buildExceptionRanges() {
     for (Statement stat : statement.getStats()) {
-      List<Statement> lstPreds = stat.getNeighbours(StatEdge.TYPE_EXCEPTION, EdgeDirection.BACKWARD);
-      if (!lstPreds.isEmpty()) {
+      // Internally, jumps into a finally handler are represented as exception edges with the exceptions field set to null
+      // Don't consider those for exception filtering, as it can lead to incorrect subgraph splitting within catch blocks
+      List<StatEdge> edges = stat.getPredecessorEdges(StatEdge.TYPE_EXCEPTION);
+      edges.removeIf(e -> e.getExceptions() == null);
 
+      if (!edges.isEmpty()) {
         Set<Integer> set = new LinkedHashSet<>();
 
-        for (Statement st : lstPreds) {
-          set.add(st.id);
+        for (StatEdge st : edges) {
+          set.add(st.getSource().id);
         }
 
         mapExceptionRanges.put(stat.id, set);
@@ -131,11 +135,9 @@ private void buildFilter(Integer id) {
             Integer exit;
             if (!range.contains(childid)) {
               exit = childid;
-            }
-            else if (map.containsKey(handler)) {
+            } else if (map.containsKey(handler)) {
               exit = -1;
-            }
-            else {
+            } else {
               exit = mapChild.get(handler);
             }
 

src/org/jetbrains/java/decompiler/modules/decompiler/decompose/FastExtendedPostdominanceHelper.java

@@ -185,10 +185,13 @@ private void filterOnExceptionRanges(DominatorTreeExceptionFilter filter) {
     for (int head : new HashSet<>(mapExtPostdominators.keySet())) {
       FastFixedSet<Integer> set = mapExtPostdominators.get(head);
       for (Iterator<Integer> it = set.iterator(); it.hasNext(); ) {
-        if (!filter.acceptStatementPair(head, it.next())) {
+        Integer next = it.next();
+
+        if (!filter.acceptStatementPair(head, next)) {
           it.remove();
         }
       }
+
       if (set.isEmpty()) {
         mapExtPostdominators.remove(head);
       }

test/org/jetbrains/java/decompiler/SingleClassesTest.java

@@ -678,6 +678,7 @@ private void registerDefault() {
     // TODO: Can't inline field initializer for a final field that depends on initialization in a static call
     register(JAVA_8, "TestStaticBlockFinalField");
     register(JAVA_8, "TestWhileLambda");
+    register(JAVA_8, "TestTrySplit");
   }
 
   private void registerEntireClassPath() {