Don't push returning ifs by default, enable in recovery passes

leibnitz27 · Jul 18, 2014 · 26de9d5 · 26de9d5
1 parent 4732561
commit 26de9d5
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 13 deletions.
diff --git a/src/org/benf/cfr/reader/bytecode/CodeAnalyser.java b/src/org/benf/cfr/reader/bytecode/CodeAnalyser.java
@@ -94,7 +94,8 @@ private AnalysisResult(DecompilerComments comments, Op04StructuredStatement code
     );
 
     private static final RecoveryOptions recover0a = new RecoveryOptions(recover0,
-            new RecoveryOption.TrooleanRO(OptionsImpl.FORCE_COND_PROPAGATE, Troolean.TRUE, DecompilerComment.COND_PROPAGATE)
+            new RecoveryOption.TrooleanRO(OptionsImpl.FORCE_COND_PROPAGATE, Troolean.TRUE, DecompilerComment.COND_PROPAGATE),
+            new RecoveryOption.TrooleanRO(OptionsImpl.FORCE_RETURNING_IFS, Troolean.TRUE, DecompilerComment.RETURNING_IFS)
     );
 
     private static final RecoveryOptions recover1 = new RecoveryOptions(recover0,
@@ -106,10 +107,15 @@ private AnalysisResult(DecompilerComments comments, Op04StructuredStatement code
     );
 
     private static final RecoveryOptions recover2 = new RecoveryOptions(recover1,
+            new RecoveryOption.BooleanRO(OptionsImpl.COMMENT_MONITORS, Boolean.TRUE, BytecodeMeta.testFlag(BytecodeMeta.CodeInfoFlag.USES_MONITORS), DecompilerComment.COMMENT_MONITORS),
+            new RecoveryOption.TrooleanRO(OptionsImpl.FORCE_RETURNING_IFS, Troolean.TRUE, DecompilerComment.RETURNING_IFS)
+    );
+
+    private static final RecoveryOptions recover3 = new RecoveryOptions(recover2,
             new RecoveryOption.BooleanRO(OptionsImpl.COMMENT_MONITORS, Boolean.TRUE, BytecodeMeta.testFlag(BytecodeMeta.CodeInfoFlag.USES_MONITORS), DecompilerComment.COMMENT_MONITORS)
     );
 
-    private static final RecoveryOptions[] recoveryOptionsArr = new RecoveryOptions[]{recover0, recover0a, recover1, recover2};
+    private static final RecoveryOptions[] recoveryOptionsArr = new RecoveryOptions[]{recover0, recover0a, recover1, recover2, recover3};
 
     /*
      * This method should not throw.  If it does, something serious has gone wrong.
@@ -450,7 +456,9 @@ private AnalysisResult getAnalysisInner(List<Op01WithProcessedDataAndByteJumps>
         }
 
         if (options.getOption(OptionsImpl.FORCE_TOPSORT) == Troolean.TRUE) {
-            Op03SimpleStatement.replaceReturningIfs(op03SimpleParseNodes, true);
+            if (options.getOption(OptionsImpl.FORCE_RETURNING_IFS) == Troolean.TRUE) {
+                Op03SimpleStatement.replaceReturningIfs(op03SimpleParseNodes, true);
+            }
             op03SimpleParseNodes = Cleaner.removeUnreachableCode(op03SimpleParseNodes, false);
             op03SimpleParseNodes = Op03Blocks.topologicalSort(method, op03SimpleParseNodes, comments, options);
             Op03SimpleStatement.removePointlessJumps(op03SimpleParseNodes);
@@ -469,7 +477,9 @@ private AnalysisResult getAnalysisInner(List<Op01WithProcessedDataAndByteJumps>
             Op03SimpleStatement.combineTryCatchEnds(op03SimpleParseNodes);
             Op03SimpleStatement.rewriteTryBackJumps(op03SimpleParseNodes);
             Op03SimpleStatement.identifyFinally(options, method, op03SimpleParseNodes, blockIdentifierFactory);
-            Op03SimpleStatement.replaceReturningIfs(op03SimpleParseNodes, true);
+            if (options.getOption(OptionsImpl.FORCE_RETURNING_IFS) == Troolean.TRUE) {
+                Op03SimpleStatement.replaceReturningIfs(op03SimpleParseNodes, true);
+            }
         }
 
         /*
@@ -546,7 +556,9 @@ private AnalysisResult getAnalysisInner(List<Op01WithProcessedDataAndByteJumps>
 
         // Replacing returning ifs early (above, aggressively) interferes with some nice output.
         // Normally we'd do it AFTER loops.
-        Op03SimpleStatement.replaceReturningIfs(op03SimpleParseNodes, false);
+        if (options.getOption(OptionsImpl.FORCE_RETURNING_IFS) == Troolean.TRUE) {
+            Op03SimpleStatement.replaceReturningIfs(op03SimpleParseNodes, false);
+        }
 
         if (showOpsLevel == SHOW_L3_LOOPS1) {
             debugDumper.newln().newln();
@@ -737,7 +749,7 @@ private AnalysisResult getAnalysisInner(List<Op01WithProcessedDataAndByteJumps>
 
             Op04StructuredStatement.removePrimitiveDeconversion(options, method, block);
             // Tidy variable names
-            Op04StructuredStatement.tidyVariableNames(method, block);
+            Op04StructuredStatement.tidyVariableNames(method, block, comments);
 
             /*
              * Now finally run some extra checks to spot wierdness.

diff --git a/src/org/benf/cfr/reader/bytecode/analysis/opgraph/Op04StructuredStatement.java b/src/org/benf/cfr/reader/bytecode/analysis/opgraph/Op04StructuredStatement.java
@@ -603,8 +603,13 @@ public static void inlinePossibles(Op04StructuredStatement root) {
         root.transform(new Inliner(), new StructuredScope());
     }
 
-    public static void tidyVariableNames(Method method, Op04StructuredStatement root) {
-        new VariableNameTidier(method).transform(root);
+    public static void tidyVariableNames(Method method, Op04StructuredStatement root, DecompilerComments comments) {
+        VariableNameTidier variableNameTidier = new VariableNameTidier(method);
+        variableNameTidier.transform(root);
+
+        if (variableNameTidier.isClassRenamed()) {
+            comments.addComment(DecompilerComment.CLASS_RENAMED);
+        }
     }
 
     public static void removePointlessReturn(Op04StructuredStatement root) {

diff --git a/...nf/cfr/reader/bytecode/analysis/opgraph/op4rewriters/transformers/VariableNameTidier.java b/...nf/cfr/reader/bytecode/analysis/opgraph/op4rewriters/transformers/VariableNameTidier.java
@@ -20,10 +20,7 @@
 import org.benf.cfr.reader.bytecode.analysis.variables.Keywords;
 import org.benf.cfr.reader.bytecode.analysis.variables.NamedVariable;
 import org.benf.cfr.reader.entities.Method;
-import org.benf.cfr.reader.util.ListFactory;
-import org.benf.cfr.reader.util.MapFactory;
-import org.benf.cfr.reader.util.Predicate;
-import org.benf.cfr.reader.util.SetFactory;
+import org.benf.cfr.reader.util.*;
 import org.benf.cfr.reader.util.functors.UnaryFunction;
 
 import java.util.LinkedList;
@@ -34,6 +31,7 @@
 public class VariableNameTidier implements StructuredStatementTransformer {
 
     private final Method method;
+    private boolean classRenamed = false;
 
     public VariableNameTidier(Method method) {
         this.method = method;
@@ -49,6 +47,9 @@ public void transform(Op04StructuredStatement root) {
         root.transform(this, structuredScopeWithVars);
     }
 
+    public boolean isClassRenamed() {
+        return classRenamed;
+    }
 
     @Override
     public StructuredStatement transform(StructuredStatement in, StructuredScope scope) {
@@ -198,7 +199,9 @@ public void defineLocalClassHere(StructuredStatement statement, SentinelLocalCla
                 postfixedVarName = getNext(name);
             } while (alreadyDefined(mkLcMojo(postfixedVarName)));
             scope.getFirst().defineHere(mkLcMojo(postfixedVarName));
+            // TODO : This is bad - passes will interfere with each other!
             method.markUsedLocalClassType(type, postfixedVarName);
+            classRenamed = true;
         }
 
         public void defineHere(StructuredStatement statement, LocalVariable localVariable) {

diff --git a/src/org/benf/cfr/reader/util/DecompilerComment.java b/src/org/benf/cfr/reader/util/DecompilerComment.java
@@ -8,12 +8,14 @@ public class DecompilerComment implements Dumpable {
     public static DecompilerComment UNABLE_TO_STRUCTURE = new DecompilerComment("Unable to fully structure code", true);
     public static DecompilerComment AGGRESSIVE_TOPOLOGICAL_SORT = new DecompilerComment("Enabled aggressive block sorting");
     public static DecompilerComment COND_PROPAGATE = new DecompilerComment("Enabled force condition propagation");
+    public static DecompilerComment RETURNING_IFS = new DecompilerComment("Lifted jumps to return sites");
     public static DecompilerComment PRUNE_EXCEPTIONS = new DecompilerComment("Enabled unnecessary exception pruning");
     public static DecompilerComment COMMENT_MONITORS = new DecompilerComment("Converted monitor instructions to comments");
     public static DecompilerComment PARAMETER_CORRUPTION = new DecompilerComment("WARNING - Possible parameter corruption");
     public static DecompilerComment TRY_BACKEDGE_REMOVED = new DecompilerComment("WARNING - Removed back jump from a try to a catch block - possible behaviour change.");
     public static DecompilerComment TYPE_CLASHES = new DecompilerComment("Could not resolve type clashes", true);
     public static DecompilerComment LOOSE_CATCH_BLOCK = new DecompilerComment("Loose catch block", true);
+    public static DecompilerComment CLASS_RENAMED = new DecompilerComment("Inner class renamed, behaviour may change", true);
 
     private final String comment;
     private final String summaryMessage;

diff --git a/src/org/benf/cfr/reader/util/getopt/OptionsImpl.java b/src/org/benf/cfr/reader/util/getopt/OptionsImpl.java
@@ -214,6 +214,9 @@ public String getDefaultValue() {
     public static final PermittedOptionProvider.Argument<Troolean> FORCE_COND_PROPAGATE = new PermittedOptionProvider.Argument<Troolean>(
             "forcecondpropagate", defaultNeitherTrooleanDecoder,
             "Pull results of deterministic jumps back through some constant assignments");
+    public static final PermittedOptionProvider.Argument<Troolean> FORCE_RETURNING_IFS = new PermittedOptionProvider.Argument<Troolean>(
+            "forcereturningifs", defaultNeitherTrooleanDecoder,
+            "Move return up to jump site");
     public static final PermittedOptionProvider.Argument<Troolean> FORCE_PRUNE_EXCEPTIONS = new PermittedOptionProvider.Argument<Troolean>(
             "forceexceptionprune", defaultNeitherTrooleanDecoder,
             "Try to extend and merge exceptions more aggressively");
@@ -313,7 +316,8 @@ public List<String> getFlags() {
                     DECOMPILER_COMMENTS, FORCE_TOPSORT, FORCE_PRUNE_EXCEPTIONS, OUTPUT_DIR,
                     SUGAR_STRINGBUFFER, SUGAR_STRINGBUILDER, SILENT, RECOVER, ECLIPSE, OVERRIDES,
                     FORCE_AGGRESSIVE_EXCEPTION_AGG, FORCE_COND_PROPAGATE, HIDE_UTF8, HIDE_LONGSTRINGS, COMMENT_MONITORS,
-                    ALLOW_CORRECTING, LABELLED_BLOCKS, JAVA_4_CLASS_OBJECTS, HIDE_LANG_IMPORTS, FORCE_PASS, RECOVER_TYPECLASHES, HELP);
+                    ALLOW_CORRECTING, LABELLED_BLOCKS, JAVA_4_CLASS_OBJECTS, HIDE_LANG_IMPORTS, FORCE_PASS, RECOVER_TYPECLASHES,
+                    FORCE_RETURNING_IFS, HELP);
         }
 
         @Override