Don't perform type narrowing after an instance-of on dalvik

2 files changed, 191 insertions, 83 deletions

diff --git a/dexlib2/src/main/java/org/jf/dexlib2/analysis/MethodAnalyzer.java b/dexlib2/src/main/java/org/jf/dexlib2/analysis/MethodAnalyzer.java
index 0c073b39..6b2c6528 100644
--- a/dexlib2/src/main/java/org/jf/dexlib2/analysis/MethodAnalyzer.java
+++ b/dexlib2/src/main/java/org/jf/dexlib2/analysis/MethodAnalyzer.java
@@ -1181,6 +1181,10 @@ public class MethodAnalyzer {
 
     static boolean canNarrowAfterInstanceOf(AnalyzedInstruction analyzedInstanceOfInstruction,
                                             AnalyzedInstruction analyzedIfInstruction, ClassPath classPath) {
+        if (!classPath.isArt()) {
+            return false;
+        }
+
         Instruction ifInstruction = analyzedIfInstruction.instruction;
         if (((Instruction21t)ifInstruction).getRegisterA() == analyzedInstanceOfInstruction.getDestinationRegister()) {
             Reference reference = ((Instruction22c)analyzedInstanceOfInstruction.getInstruction()).getReference();
@@ -1205,93 +1209,93 @@ public class MethodAnalyzer {
     /**
      * Art uses a peephole optimization for an if-eqz or if-nez that occur immediately after an instance-of. It will
      * narrow the type if possible, and then NOP out any corresponding check-cast instruction later on
-     *
-     * TODO: Is this still safe to do even for dalvik odexes? I think it should be..
      */
     private void analyzeIfEqzNez(@Nonnull AnalyzedInstruction analyzedInstruction) {
-        int instructionIndex = analyzedInstruction.getInstructionIndex();
-        if (instructionIndex > 0) {
-            if (analyzedInstruction.getPredecessorCount() != 1) {
-                return;
-            }
-            AnalyzedInstruction prevAnalyzedInstruction = analyzedInstruction.getPredecessors().first();
-            if (prevAnalyzedInstruction.instruction.getOpcode() == Opcode.INSTANCE_OF) {
-                Instruction22c instanceOfInstruction = (Instruction22c)prevAnalyzedInstruction.instruction;
-                if (canNarrowAfterInstanceOf(prevAnalyzedInstruction, analyzedInstruction, classPath)) {
-                    List<Integer> narrowingRegisters = Lists.newArrayList();
-
-                    RegisterType newType = RegisterType.getRegisterType(classPath,
-                            (TypeReference)instanceOfInstruction.getReference());
-
-                    if (instructionIndex > 1) {
-                        // If we have something like:
-                        //   move-object/from16 v0, p1
-                        //   instance-of v2, v0, Lblah;
-                        //   if-eqz v2, :target
-                        // Then we need to narrow both v0 AND p1, but only if all predecessors of instance-of are a
-                        // move-object for the same registers
-
-                        int additionalNarrowingRegister = -1;
-                        for (AnalyzedInstruction prevPrevAnalyzedInstruction: prevAnalyzedInstruction.predecessors) {
-                            Opcode opcode = prevPrevAnalyzedInstruction.instruction.getOpcode();
-                            if (opcode == Opcode.MOVE_OBJECT || opcode == Opcode.MOVE_OBJECT_16 ||
-                                    opcode == Opcode.MOVE_OBJECT_FROM16) {
-                                TwoRegisterInstruction moveInstruction =
-                                        ((TwoRegisterInstruction)prevPrevAnalyzedInstruction.instruction);
-                                RegisterType originalType =
-                                        prevPrevAnalyzedInstruction.getPostInstructionRegisterType(
-                                                moveInstruction.getRegisterB());
-                                if (moveInstruction.getRegisterA() != instanceOfInstruction.getRegisterB()) {
-                                    additionalNarrowingRegister = -1;
-                                    break;
-                                }
-                                if (originalType.type == null) {
-                                    additionalNarrowingRegister = -1;
-                                    break;
-                                }
-                                if (isNarrowingConversion(originalType, newType)) {
-                                    if (additionalNarrowingRegister != -1) {
-                                        if (additionalNarrowingRegister != moveInstruction.getRegisterB()) {
-                                            additionalNarrowingRegister = -1;
-                                            break;
+        if (classPath.isArt()) {
+            int instructionIndex = analyzedInstruction.getInstructionIndex();
+            if (instructionIndex > 0) {
+                if (analyzedInstruction.getPredecessorCount() != 1) {
+                    return;
+                }
+                AnalyzedInstruction prevAnalyzedInstruction = analyzedInstruction.getPredecessors().first();
+                if (prevAnalyzedInstruction.instruction.getOpcode() == Opcode.INSTANCE_OF) {
+                    Instruction22c instanceOfInstruction = (Instruction22c)prevAnalyzedInstruction.instruction;
+                    if (canNarrowAfterInstanceOf(prevAnalyzedInstruction, analyzedInstruction, classPath)) {
+                        List<Integer> narrowingRegisters = Lists.newArrayList();
+
+                        RegisterType newType = RegisterType.getRegisterType(classPath,
+                                (TypeReference)instanceOfInstruction.getReference());
+
+                        if (instructionIndex > 1) {
+                            // If we have something like:
+                            //   move-object/from16 v0, p1
+                            //   instance-of v2, v0, Lblah;
+                            //   if-eqz v2, :target
+                            // Then we need to narrow both v0 AND p1, but only if all predecessors of instance-of are a
+                            // move-object for the same registers
+
+                            int additionalNarrowingRegister = -1;
+                            for (AnalyzedInstruction prevPrevAnalyzedInstruction : prevAnalyzedInstruction.predecessors) {
+                                Opcode opcode = prevPrevAnalyzedInstruction.instruction.getOpcode();
+                                if (opcode == Opcode.MOVE_OBJECT || opcode == Opcode.MOVE_OBJECT_16 ||
+                                        opcode == Opcode.MOVE_OBJECT_FROM16) {
+                                    TwoRegisterInstruction moveInstruction =
+                                            ((TwoRegisterInstruction)prevPrevAnalyzedInstruction.instruction);
+                                    RegisterType originalType =
+                                            prevPrevAnalyzedInstruction.getPostInstructionRegisterType(
+                                                    moveInstruction.getRegisterB());
+                                    if (moveInstruction.getRegisterA() != instanceOfInstruction.getRegisterB()) {
+                                        additionalNarrowingRegister = -1;
+                                        break;
+                                    }
+                                    if (originalType.type == null) {
+                                        additionalNarrowingRegister = -1;
+                                        break;
+                                    }
+                                    if (isNarrowingConversion(originalType, newType)) {
+                                        if (additionalNarrowingRegister != -1) {
+                                            if (additionalNarrowingRegister != moveInstruction.getRegisterB()) {
+                                                additionalNarrowingRegister = -1;
+                                                break;
+                                            }
+                                        } else {
+                                            additionalNarrowingRegister = moveInstruction.getRegisterB();
                                         }
-                                    } else {
-                                        additionalNarrowingRegister = moveInstruction.getRegisterB();
                                     }
+                                } else {
+                                    additionalNarrowingRegister = -1;
+                                    break;
                                 }
-                            } else {
-                                additionalNarrowingRegister = -1;
-                                break;
+                            }
+                            if (additionalNarrowingRegister != -1) {
+                                narrowingRegisters.add(additionalNarrowingRegister);
                             }
                         }
-                        if (additionalNarrowingRegister != -1) {
-                            narrowingRegisters.add(additionalNarrowingRegister);
-                        }
-                    }
 
-                    // Propagate the original type to the failing branch, and the new type to the successful branch
-                    int narrowingRegister = ((Instruction22c)prevAnalyzedInstruction.instruction).getRegisterB();
-                    narrowingRegisters.add(narrowingRegister);
-                    RegisterType originalType = analyzedInstruction.getPreInstructionRegisterType(narrowingRegister);
+                        // Propagate the original type to the failing branch, and the new type to the successful branch
+                        int narrowingRegister = ((Instruction22c)prevAnalyzedInstruction.instruction).getRegisterB();
+                        narrowingRegisters.add(narrowingRegister);
+                        RegisterType originalType = analyzedInstruction.getPreInstructionRegisterType(narrowingRegister);
 
-                    AnalyzedInstruction fallthroughInstruction = analyzedInstructions.valueAt(
-                            analyzedInstruction.getInstructionIndex() + 1);
+                        AnalyzedInstruction fallthroughInstruction = analyzedInstructions.valueAt(
+                                analyzedInstruction.getInstructionIndex() + 1);
 
-                    int nextAddress = getInstructionAddress(analyzedInstruction) +
-                            ((Instruction21t)analyzedInstruction.instruction).getCodeOffset();
-                    AnalyzedInstruction branchInstruction = analyzedInstructions.get(nextAddress);
+                        int nextAddress = getInstructionAddress(analyzedInstruction) +
+                                ((Instruction21t)analyzedInstruction.instruction).getCodeOffset();
+                        AnalyzedInstruction branchInstruction = analyzedInstructions.get(nextAddress);
 
-                    for (int register: narrowingRegisters) {
-                        if (analyzedInstruction.instruction.getOpcode() == Opcode.IF_EQZ) {
-                            overridePredecessorRegisterTypeAndPropagateChanges(fallthroughInstruction, analyzedInstruction,
-                                    register, newType);
-                            overridePredecessorRegisterTypeAndPropagateChanges(branchInstruction, analyzedInstruction,
-                                    register, originalType);
-                        } else {
-                            overridePredecessorRegisterTypeAndPropagateChanges(fallthroughInstruction, analyzedInstruction,
-                                    register, originalType);
-                            overridePredecessorRegisterTypeAndPropagateChanges(branchInstruction, analyzedInstruction,
-                                    register, newType);
+                        for (int register : narrowingRegisters) {
+                            if (analyzedInstruction.instruction.getOpcode() == Opcode.IF_EQZ) {
+                                overridePredecessorRegisterTypeAndPropagateChanges(fallthroughInstruction, analyzedInstruction,
+                                        register, newType);
+                                overridePredecessorRegisterTypeAndPropagateChanges(branchInstruction, analyzedInstruction,
+                                        register, originalType);
+                            } else {
+                                overridePredecessorRegisterTypeAndPropagateChanges(fallthroughInstruction, analyzedInstruction,
+                                        register, originalType);
+                                overridePredecessorRegisterTypeAndPropagateChanges(branchInstruction, analyzedInstruction,
+                                        register, newType);
+                            }
                         }
                     }
                 }
diff --git a/dexlib2/src/test/java/org/jf/dexlib2/analysis/MethodAnalyzerTest.java b/dexlib2/src/test/java/org/jf/dexlib2/analysis/MethodAnalyzerTest.java
index 2588d58f..215ba179 100644
--- a/dexlib2/src/test/java/org/jf/dexlib2/analysis/MethodAnalyzerTest.java
+++ b/dexlib2/src/test/java/org/jf/dexlib2/analysis/MethodAnalyzerTest.java
@@ -31,6 +31,7 @@
 
 package org.jf.dexlib2.analysis;
 
+import com.google.common.collect.Lists;
 import org.jf.dexlib2.AccessFlags;
 import org.jf.dexlib2.Opcode;
 import org.jf.dexlib2.Opcodes;
@@ -55,10 +56,12 @@ import java.io.IOException;
 import java.util.Collections;
 import java.util.List;
 
+import static org.jf.dexlib2.Opcodes.forArtVersion;
+
 public class MethodAnalyzerTest {
 
     @Test
-    public void testInstanceOfNarrowingEqz() throws IOException {
+    public void testInstanceOfNarrowingEqz_art() throws IOException {
         MethodImplementationBuilder builder = new MethodImplementationBuilder(2);
 
         builder.addInstruction(new BuilderInstruction22c(Opcode.INSTANCE_OF, 0, 1,
@@ -76,9 +79,9 @@ public class MethodAnalyzerTest {
                 AccessFlags.PUBLIC.getValue(), null, methodImplementation);
         ClassDef classDef = new ImmutableClassDef("Lmain;", AccessFlags.PUBLIC.getValue(), "Ljava/lang/Object;", null,
                 null, null, null, Collections.singletonList(method));
-        DexFile dexFile = new ImmutableDexFile(Opcodes.getDefault(), Collections.singletonList(classDef));
+        DexFile dexFile = new ImmutableDexFile(forArtVersion(56), Collections.singletonList(classDef));
 
-        ClassPath classPath = new ClassPath(new DexClassProvider(dexFile));
+        ClassPath classPath = new ClassPath(Lists.newArrayList(new DexClassProvider(dexFile)), true, 56);
         MethodAnalyzer methodAnalyzer = new MethodAnalyzer(classPath, method, null, false);
 
         List<AnalyzedInstruction> analyzedInstructions = methodAnalyzer.getAnalyzedInstructions();
@@ -89,7 +92,70 @@ public class MethodAnalyzerTest {
     }
 
     @Test
-    public void testInstanceOfNarrowingNez() throws IOException {
+    public void testInstanceOfNarrowingEqz_dalvik() throws IOException {
+        MethodImplementationBuilder builder = new MethodImplementationBuilder(2);
+
+        builder.addInstruction(new BuilderInstruction22c(Opcode.INSTANCE_OF, 0, 1,
+                new ImmutableTypeReference("Lmain;")));
+        builder.addInstruction(new BuilderInstruction21t(Opcode.IF_EQZ, 0, builder.getLabel("not_instance_of")));
+        builder.addInstruction(new BuilderInstruction10x(Opcode.RETURN_VOID));
+
+        builder.addLabel("not_instance_of");
+        builder.addInstruction(new BuilderInstruction10x(Opcode.RETURN_VOID));
+
+        MethodImplementation methodImplementation = builder.getMethodImplementation();
+
+        Method method = new ImmutableMethod("Lmain;", "narrowing",
+                Collections.singletonList(new ImmutableMethodParameter("Ljava/lang/Object;", null, null)), "V",
+                AccessFlags.PUBLIC.getValue(), null, methodImplementation);
+        ClassDef classDef = new ImmutableClassDef("Lmain;", AccessFlags.PUBLIC.getValue(), "Ljava/lang/Object;", null,
+                null, null, null, Collections.singletonList(method));
+        DexFile dexFile = new ImmutableDexFile(Opcodes.forApi(19), Collections.singletonList(classDef));
+
+        ClassPath classPath = new ClassPath(new DexClassProvider(dexFile));
+        MethodAnalyzer methodAnalyzer = new MethodAnalyzer(classPath, method, null, false);
+
+        List<AnalyzedInstruction> analyzedInstructions = methodAnalyzer.getAnalyzedInstructions();
+        Assert.assertEquals("Ljava/lang/Object;",
+                analyzedInstructions.get(2).getPreInstructionRegisterType(1).type.getType());
+
+        Assert.assertEquals("Ljava/lang/Object;",
+                analyzedInstructions.get(3).getPreInstructionRegisterType(1).type.getType());
+    }
+
+    @Test
+    public void testInstanceOfNarrowingNez_art() throws IOException {
+        MethodImplementationBuilder builder = new MethodImplementationBuilder(2);
+
+        builder.addInstruction(new BuilderInstruction22c(Opcode.INSTANCE_OF, 0, 1,
+                new ImmutableTypeReference("Lmain;")));
+        builder.addInstruction(new BuilderInstruction21t(Opcode.IF_NEZ, 0, builder.getLabel("instance_of")));
+        builder.addInstruction(new BuilderInstruction10x(Opcode.RETURN_VOID));
+
+        builder.addLabel("instance_of");
+        builder.addInstruction(new BuilderInstruction10x(Opcode.RETURN_VOID));
+
+        MethodImplementation methodImplementation = builder.getMethodImplementation();
+
+        Method method = new ImmutableMethod("Lmain;", "narrowing",
+                Collections.singletonList(new ImmutableMethodParameter("Ljava/lang/Object;", null, null)), "V",
+                AccessFlags.PUBLIC.getValue(), null, methodImplementation);
+        ClassDef classDef = new ImmutableClassDef("Lmain;", AccessFlags.PUBLIC.getValue(), "Ljava/lang/Object;", null,
+                null, null, null, Collections.singletonList(method));
+        DexFile dexFile = new ImmutableDexFile(forArtVersion(56), Collections.singletonList(classDef));
+
+        ClassPath classPath = new ClassPath(Lists.newArrayList(new DexClassProvider(dexFile)), true, 56);
+        MethodAnalyzer methodAnalyzer = new MethodAnalyzer(classPath, method, null, false);
+
+        List<AnalyzedInstruction> analyzedInstructions = methodAnalyzer.getAnalyzedInstructions();
+        Assert.assertEquals("Ljava/lang/Object;",
+                analyzedInstructions.get(2).getPreInstructionRegisterType(1).type.getType());
+
+        Assert.assertEquals("Lmain;", analyzedInstructions.get(3).getPreInstructionRegisterType(1).type.getType());
+    }
+
+    @Test
+    public void testInstanceOfNarrowingNez_dalvik() throws IOException {
         MethodImplementationBuilder builder = new MethodImplementationBuilder(2);
 
         builder.addInstruction(new BuilderInstruction22c(Opcode.INSTANCE_OF, 0, 1,
@@ -116,11 +182,47 @@ public class MethodAnalyzerTest {
         Assert.assertEquals("Ljava/lang/Object;",
                 analyzedInstructions.get(2).getPreInstructionRegisterType(1).type.getType());
 
+        Assert.assertEquals("Ljava/lang/Object;",
+                analyzedInstructions.get(3).getPreInstructionRegisterType(1).type.getType());
+    }
+
+    @Test
+    public void testInstanceOfNarrowingAfterMove_art() throws IOException {
+        MethodImplementationBuilder builder = new MethodImplementationBuilder(3);
+
+        builder.addInstruction(new BuilderInstruction12x(Opcode.MOVE_OBJECT, 1, 2));
+        builder.addInstruction(new BuilderInstruction22c(Opcode.INSTANCE_OF, 0, 1,
+                new ImmutableTypeReference("Lmain;")));
+        builder.addInstruction(new BuilderInstruction21t(Opcode.IF_EQZ, 0, builder.getLabel("not_instance_of")));
+        builder.addInstruction(new BuilderInstruction10x(Opcode.RETURN_VOID));
+
+        builder.addLabel("not_instance_of");
+        builder.addInstruction(new BuilderInstruction10x(Opcode.RETURN_VOID));
+
+        MethodImplementation methodImplementation = builder.getMethodImplementation();
+
+        Method method = new ImmutableMethod("Lmain;", "narrowing",
+                Collections.singletonList(new ImmutableMethodParameter("Ljava/lang/Object;", null, null)), "V",
+                AccessFlags.PUBLIC.getValue(), null, methodImplementation);
+        ClassDef classDef = new ImmutableClassDef("Lmain;", AccessFlags.PUBLIC.getValue(), "Ljava/lang/Object;", null,
+                null, null, null, Collections.singletonList(method));
+        DexFile dexFile = new ImmutableDexFile(forArtVersion(56), Collections.singletonList(classDef));
+
+        ClassPath classPath = new ClassPath(Lists.newArrayList(new DexClassProvider(dexFile)), true, 56);
+        MethodAnalyzer methodAnalyzer = new MethodAnalyzer(classPath, method, null, false);
+
+        List<AnalyzedInstruction> analyzedInstructions = methodAnalyzer.getAnalyzedInstructions();
         Assert.assertEquals("Lmain;", analyzedInstructions.get(3).getPreInstructionRegisterType(1).type.getType());
+        Assert.assertEquals("Lmain;", analyzedInstructions.get(3).getPreInstructionRegisterType(2).type.getType());
+
+        Assert.assertEquals("Ljava/lang/Object;",
+                analyzedInstructions.get(4).getPreInstructionRegisterType(1).type.getType());
+        Assert.assertEquals("Ljava/lang/Object;",
+                analyzedInstructions.get(4).getPreInstructionRegisterType(2).type.getType());
     }
 
     @Test
-    public void testInstanceOfNarrowingAfterMove() throws IOException {
+    public void testInstanceOfNarrowingAfterMove_dalvik() throws IOException {
         MethodImplementationBuilder builder = new MethodImplementationBuilder(3);
 
         builder.addInstruction(new BuilderInstruction12x(Opcode.MOVE_OBJECT, 1, 2));
@@ -145,8 +247,10 @@ public class MethodAnalyzerTest {
         MethodAnalyzer methodAnalyzer = new MethodAnalyzer(classPath, method, null, false);
 
         List<AnalyzedInstruction> analyzedInstructions = methodAnalyzer.getAnalyzedInstructions();
-        Assert.assertEquals("Lmain;", analyzedInstructions.get(3).getPreInstructionRegisterType(1).type.getType());
-        Assert.assertEquals("Lmain;", analyzedInstructions.get(3).getPreInstructionRegisterType(2).type.getType());
+        Assert.assertEquals("Ljava/lang/Object;",
+                analyzedInstructions.get(3).getPreInstructionRegisterType(1).type.getType());
+        Assert.assertEquals("Ljava/lang/Object;",
+                analyzedInstructions.get(3).getPreInstructionRegisterType(2).type.getType());
 
         Assert.assertEquals("Ljava/lang/Object;",
                 analyzedInstructions.get(4).getPreInstructionRegisterType(1).type.getType());