Merge pull request soot-oss#1819 from MarcMil/fix-def-if

Fix some cases where the wrong branch was eliminated

src/main/java/soot/PatchingChain.java

@@ -145,6 +145,19 @@ public void insertBeforeNoRedirect(E toInsert, E point) {
     innerChain.insertBefore(toInsert, point);
   }
 
+  /** Inserts <code>toInsert</code> in the Chain before <code>point</code> WITHOUT redirecting jumps. */
+  public void insertBeforeNoRedirect(List<E> toInsert, E point) {
+    if (!toInsert.isEmpty()) {
+      // Insert toInsert backwards into the list
+      E previousPoint = point;
+      for (ListIterator<E> it = toInsert.listIterator(toInsert.size()); it.hasPrevious();) {
+        E o = it.previous();
+        insertBeforeNoRedirect(o, previousPoint);
+        previousPoint = o;
+      }
+    }
+  }
+
   /** Returns true if object <code>a</code> follows object <code>b</code> in the Chain. */
   @Override
   public boolean follows(E a, E b) {

src/main/java/soot/dexpler/DexBody.java

@@ -40,6 +40,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+
 import org.jf.dexlib2.analysis.ClassPath;
 import org.jf.dexlib2.analysis.ClassPathResolver;
 import org.jf.dexlib2.analysis.ClassProvider;
@@ -60,6 +61,7 @@
 import org.jf.dexlib2.util.MethodUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+
 import soot.Body;
 import soot.DoubleType;
 import soot.Local;
@@ -112,6 +114,7 @@
 import soot.jimple.toolkits.scalar.IdentityOperationEliminator;
 import soot.jimple.toolkits.scalar.MethodStaticnessCorrector;
 import soot.jimple.toolkits.scalar.NopEliminator;
+import soot.jimple.toolkits.scalar.UnconditionalBranchFolder;
 import soot.jimple.toolkits.scalar.UnreachableCodeEliminator;
 import soot.jimple.toolkits.typing.TypeAssigner;
 import soot.options.JBOptions;
@@ -124,8 +127,7 @@
 import soot.toolkits.scalar.UnusedLocalEliminator;
 
 /**
- * A DexBody contains the code of a DexMethod and is used as a wrapper around JimpleBody in the
- * jimplification process.
+ * A DexBody contains the code of a DexMethod and is used as a wrapper around JimpleBody in the jimplification process.
  *
  * @author Michael Markert
  * @author Frank Hartmann
@@ -202,7 +204,8 @@ PseudoInstruction isAddressInData(int a) {
   /**
    * Allocate a fresh name for Jimple local
    *
-   * @param hint A name that the fresh name will look like
+   * @param hint
+   *          A name that the fresh name will look like
    * @author Zhixuan Yang ([email protected])
    */
   protected String freshLocalName(String hint) {
@@ -213,7 +216,7 @@ protected String freshLocalName(String hint) {
     if (!takenLocalNames.contains(hint)) {
       fresh = hint;
     } else {
-      for (int i = 1; ; i++) {
+      for (int i = 1;; i++) {
         fresh = hint + Integer.toString(i);
         if (!takenLocalNames.contains(fresh)) {
           break;
@@ -225,11 +228,12 @@ protected String freshLocalName(String hint) {
   }
 
   /**
-   * @param code the codeitem that is contained in this body
-   * @param method the method that is associated with this body
+   * @param code
+   *          the codeitem that is contained in this body
+   * @param method
+   *          the method that is associated with this body
    */
-  protected DexBody(
-      DexEntry<? extends DexFile> dexFile, Method method, RefType declaringClassType) {
+  protected DexBody(DexEntry<? extends DexFile> dexFile, Method method, RefType declaringClassType) {
     MethodImplementation code = method.getImplementation();
     if (code == null) {
       throw new RuntimeException("error: no code for method " + method.getName());
@@ -269,11 +273,7 @@ protected DexBody(
     // Check taken from Android's dalvik/libdex/DexSwapVerify.cpp
     if (numParameterRegisters > numRegisters) {
       throw new RuntimeException(
-          "Malformed dex file: insSize ("
-              + numParameterRegisters
-              + ") > registersSize ("
-              + numRegisters
-              + ")");
+          "Malformed dex file: insSize (" + numParameterRegisters + ") > registersSize (" + numRegisters + ")");
     }
 
     for (DebugItem di : code.getDebugItems()) {
@@ -307,8 +307,7 @@ protected DexBody(
           signature = sl.getSignature();
         }
         if (name != null && type != null) {
-          localDebugs.put(
-              reg, new RegDbgEntry(codeAddr, -1 /* endAddress */, reg, name, type, signature));
+          localDebugs.put(reg, new RegDbgEntry(codeAddr, -1 /* endAddress */, reg, name, type, signature));
         }
       } else if (di instanceof ImmutableEndLocal) {
         ImmutableEndLocal el = (ImmutableEndLocal) di;
@@ -327,10 +326,10 @@ protected DexBody(
   }
 
   /**
-   * Extracts the list of dalvik instructions from dexlib and converts them into our own instruction
-   * data model
+   * Extracts the list of dalvik instructions from dexlib and converts them into our own instruction data model
    *
-   * @param code The dexlib method implementation
+   * @param code
+   *          The dexlib method implementation
    */
   protected void extractDexInstructions(MethodImplementation code) {
     int address = 0;
@@ -369,7 +368,8 @@ public Set<Type> usedTypes() {
   /**
    * Add unit to this body.
    *
-   * @param u Unit to add.
+   * @param u
+   *          Unit to add.
    */
   public void add(Unit u) {
     getBody().getUnits().add(u);
@@ -378,7 +378,8 @@ public void add(Unit u) {
   /**
    * Add a deferred instruction to this body.
    *
-   * @param i the deferred instruction.
+   * @param i
+   *          the deferred instruction.
    */
   public void addDeferredJimplification(DeferableInstruction i) {
     deferredInstructions.add(i);
@@ -387,7 +388,8 @@ public void addDeferredJimplification(DeferableInstruction i) {
   /**
    * Add a retypeable instruction to this body.
    *
-   * @param i the retypeable instruction.
+   * @param i
+   *          the retypeable instruction.
    */
   public void addRetype(RetypeableInstruction i) {
     instructionsToRetype.add(i);
@@ -396,7 +398,8 @@ public void addRetype(RetypeableInstruction i) {
   /**
    * Return the associated JimpleBody.
    *
-   * @throws RuntimeException if no jimplification happened yet.
+   * @throws RuntimeException
+   *           if no jimplification happened yet.
    */
   public Body getBody() {
     if (jBody == null) {
@@ -413,20 +416,18 @@ public Local[] getRegisterLocals() {
   /**
    * Return the Local that are associated with the number in the current register state.
    *
-   * <p>Handles if the register number actually points to a method parameter.
+   * <p>
+   * Handles if the register number actually points to a method parameter.
    *
-   * @param num the register number
+   * @param num
+   *          the register number
    * @throws InvalidDalvikBytecodeException
    */
   public Local getRegisterLocal(int num) throws InvalidDalvikBytecodeException {
     int totalRegisters = registerLocals.length;
     if (num > totalRegisters) {
       throw new InvalidDalvikBytecodeException(
-          "Trying to access register "
-              + num
-              + " but only "
-              + totalRegisters
-              + " is/are available.");
+          "Trying to access register " + num + " but only " + totalRegisters + " is/are available.");
     }
     return registerLocals[num];
   }
@@ -438,8 +439,10 @@ public Local getStoreResultLocal() {
   /**
    * Return the instruction that is present at the byte code address.
    *
-   * @param address the byte code address.
-   * @throws RuntimeException if address is not part of this body.
+   * @param address
+   *          the byte code address.
+   * @throws RuntimeException
+   *           if address is not part of this body.
    */
   public DexlibAbstractInstruction instructionAtAddress(int address) {
     DexlibAbstractInstruction i = null;
@@ -468,7 +471,8 @@ public DexlibAbstractInstruction instructionAtAddress(int address) {
   /**
    * Return the jimple equivalent of this body.
    *
-   * @param m the SootMethod that contains this body
+   * @param m
+   *          the SootMethod that contains this body
    */
   public Body jimplify(Body b, SootMethod m) {
 
@@ -505,18 +509,15 @@ public Body jimplify(Body b, SootMethod m) {
     if (!isStatic) {
       int thisRegister = numRegisters - numParameterRegisters - 1;
 
-      Local thisLocal =
-          jimple.newLocal(freshLocalName("this"), unknownType); // generateLocal(UnknownType.v());
+      Local thisLocal = jimple.newLocal(freshLocalName("this"), unknownType); // generateLocal(UnknownType.v());
       jBody.getLocals().add(thisLocal);
 
       registerLocals[thisRegister] = thisLocal;
-      JIdentityStmt idStmt =
-          (JIdentityStmt) jimple.newIdentityStmt(thisLocal, jimple.newThisRef(declaringClassType));
+      JIdentityStmt idStmt = (JIdentityStmt) jimple.newIdentityStmt(thisLocal, jimple.newThisRef(declaringClassType));
       add(idStmt);
       paramLocals.add(thisLocal);
       if (IDalvikTyper.ENABLE_DVKTYPER) {
-        DalvikTyper.v()
-            .setType(idStmt.getLeftOpBox(), jBody.getMethod().getDeclaringClass().getType(), false);
+        DalvikTyper.v().setType(idStmt.getLeftOpBox(), jBody.getMethod().getDeclaringClass().getType(), false);
       }
     }
     {
@@ -551,8 +552,7 @@ public Body jimplify(Body b, SootMethod m) {
         jBody.getLocals().add(gen);
         registerLocals[parameterRegister] = gen;
 
-        JIdentityStmt idStmt =
-            (JIdentityStmt) jimple.newIdentityStmt(gen, jimple.newParameterRef(t, i++));
+        JIdentityStmt idStmt = (JIdentityStmt) jimple.newIdentityStmt(gen, jimple.newParameterRef(t, i++));
         add(idStmt);
         paramLocals.add(gen);
         if (IDalvikTyper.ENABLE_DVKTYPER) {
@@ -601,10 +601,8 @@ public Body jimplify(Body b, SootMethod m) {
 
     // process bytecode instructions
     final DexFile dexFile = dexEntry.getDexFile();
-    final boolean isOdex =
-        dexFile instanceof DexBackedDexFile
-            ? ((DexBackedDexFile) dexFile).supportsOptimizedOpcodes()
-            : false;
+    final boolean isOdex
+        = dexFile instanceof DexBackedDexFile ? ((DexBackedDexFile) dexFile).supportsOptimizedOpcodes() : false;
 
     ClassPath cp = null;
     if (isOdex) {
@@ -614,8 +612,7 @@ public Body jimplify(Body b, SootMethod m) {
         classpathList.add(str);
       }
       try {
-        ClassPathResolver resolver =
-            new ClassPathResolver(classpathList, classpathList, classpathList, dexEntry);
+        ClassPathResolver resolver = new ClassPathResolver(classpathList, classpathList, classpathList, dexEntry);
         cp = new ClassPath(resolver.getResolvedClassProviders().toArray(new ClassProvider[0]));
       } catch (IOException e) {
         throw new RuntimeException(e);
@@ -765,6 +762,15 @@ public Body jimplify(Body b, SootMethod m) {
 
     // Remove "instanceof" checks on the null constant
     DexNullInstanceofTransformer.v().transform(jBody);
+    DexNullIfTransformer ni = DexNullIfTransformer.v();
+    ni.transform(jBody);
+    if (ni.hasModifiedBody()) {
+      // Now we might have unreachable code
+      ConditionalBranchFolder.v().transform(jBody);
+      UnreachableCodeEliminator.v().transform(jBody);
+      DeadAssignmentEliminator.v().transform(jBody);
+      UnconditionalBranchFolder.v().transform(jBody);
+    }
 
     TypeAssigner.v().transform(jBody);
 
@@ -939,8 +945,7 @@ public Body jimplify(Body b, SootMethod m) {
           Type t = def.getLeftOp().getType();
           if (t instanceof RefType) {
             RefType rt = (RefType) t;
-            if (rt.getSootClass().isPhantom()
-                && !rt.getSootClass().hasSuperclass()
+            if (rt.getSootClass().isPhantom() && !rt.getSootClass().hasSuperclass()
                 && !rt.getSootClass().getName().equals("java.lang.Throwable")) {
               rt.getSootClass().setSuperclass(Scene.v().getSootClass("java.lang.Throwable"));
             }
@@ -974,8 +979,8 @@ public Body jimplify(Body b, SootMethod m) {
   }
 
   /**
-   * Fixes the line numbers. If there is a unit without a line number, it gets the line number of
-   * the last (transitive) predecessor that has a line number.
+   * Fixes the line numbers. If there is a unit without a line number, it gets the line number of the last (transitive)
+   * predecessor that has a line number.
    */
   protected void fixLineNumbers() {
     int prevLn = -1;
@@ -1028,7 +1033,8 @@ public void setDanglingInstruction(DanglingInstruction i) {
   /**
    * Return the instructions that appear (lexically) after the given instruction.
    *
-   * @param instruction the instruction which successors will be returned.
+   * @param instruction
+   *          the instruction which successors will be returned.
    */
   public List<DexlibAbstractInstruction> instructionsAfter(DexlibAbstractInstruction instruction) {
     int i = instructions.indexOf(instruction);
@@ -1042,9 +1048,11 @@ public List<DexlibAbstractInstruction> instructionsAfter(DexlibAbstractInstructi
   /**
    * Return the instructions that appear (lexically) before the given instruction.
    *
-   * <p>The instruction immediately before the given is the first instruction and so on.
+   * <p>
+   * The instruction immediately before the given is the first instruction and so on.
    *
-   * @param instruction the instruction which successors will be returned.
+   * @param instruction
+   *          the instruction which successors will be returned.
    */
   public List<DexlibAbstractInstruction> instructionsBefore(DexlibAbstractInstruction instruction) {
     int i = instructions.indexOf(instruction);
@@ -1061,7 +1069,8 @@ public List<DexlibAbstractInstruction> instructionsBefore(DexlibAbstractInstruct
   /**
    * Add the traps of this body.
    *
-   * <p>Should only be called at the end jimplify.
+   * <p>
+   * Should only be called at the end jimplify.
    */
   private void addTraps() {
     final Jimple jimple = Jimple.v();
@@ -1082,8 +1091,7 @@ private void addTraps() {
       // if the try block ends on the last instruction of the body, add a
       // nop instruction so Soot can include
       // the last instruction in the try block.
-      if (jBody.getUnits().getLast() == endStmt
-          && instructionAtAddress(endAddress - 1).getUnit() == endStmt) {
+      if (jBody.getUnits().getLast() == endStmt && instructionAtAddress(endAddress - 1).getUnit() == endStmt) {
         Unit nop = jimple.newNopStmt();
         jBody.getUnits().insertAfter(nop, endStmt);
         endStmt = nop;
@@ -1099,14 +1107,10 @@ && instructionAtAddress(endAddress - 1).getUnit() == endStmt) {
         // exceptions can only be of RefType
         if (t instanceof RefType) {
           SootClass exception = ((RefType) t).getSootClass();
-          DexlibAbstractInstruction instruction =
-              instructionAtAddress(handler.getHandlerCodeAddress());
+          DexlibAbstractInstruction instruction = instructionAtAddress(handler.getHandlerCodeAddress());
           if (!(instruction instanceof MoveExceptionInstruction)) {
-            logger.debug(
-                ""
-                    + String.format(
-                        "First instruction of trap handler unit not MoveException but %s",
-                        instruction.getClass().getName()));
+            logger.debug("" + String.format("First instruction of trap handler unit not MoveException but %s",
+                instruction.getClass().getName()));
           } else {
             ((MoveExceptionInstruction) instruction).setRealType(this, exception.getType());
           }