Change the way the "dead" instructions are handled, so that loops within the dead code are handled properly

git-svn-id: https://smali.googlecode.com/svn/trunk@674 55b6fa8a-2a1e-11de-a435-ffa8d773f76a

baksmali/src/main/java/org/jf/baksmali/Adaptors/Format/ArrayDataMethodItem.java

@@ -36,13 +36,17 @@ import org.jf.dexlib.CodeItem;
 import java.util.Iterator;
 
 public class ArrayDataMethodItem extends InstructionMethodItem<ArrayDataPseudoInstruction> {
-    public ArrayDataMethodItem(CodeItem codeItem, int codeAddress, StringTemplateGroup stg,
+    private final boolean dead;
+
+    public ArrayDataMethodItem(CodeItem codeItem, int codeAddress, boolean dead, StringTemplateGroup stg,
                                ArrayDataPseudoInstruction instruction) {
         super(codeItem, codeAddress, stg, instruction);
+        this.dead = dead;
     }
 
     protected void setAttributes(StringTemplate template) {
         template.setAttribute("ElementWidth", instruction.getElementWidth());
+        template.setAttribute("Dead", dead);
         setValuesAttribute(template);
     }
 

baksmali/src/main/java/org/jf/baksmali/Adaptors/Format/InstructionMethodItemFactory.java

@@ -42,7 +42,7 @@ public class InstructionMethodItemFactory {
 
     public static InstructionMethodItem makeInstructionFormatMethodItem(MethodDefinition methodDefinition,
                                                                               CodeItem codeItem,
-                                                                              int codeAddress,
+                                                                              int codeAddress, boolean dead,
                                                                               StringTemplateGroup stg,
                                                                               Instruction instruction,
                                                                               boolean isLastInstruction) {
@@ -53,13 +53,13 @@ public class InstructionMethodItemFactory {
 
         switch (instruction.getFormat()) {
             case ArrayData:
-                return new ArrayDataMethodItem(codeItem, codeAddress, stg,
+                return new ArrayDataMethodItem(codeItem, codeAddress, dead, stg,
                         (ArrayDataPseudoInstruction)instruction);
             case PackedSwitchData:
-                return new PackedSwitchMethodItem(methodDefinition, codeItem, codeAddress, stg,
+                return new PackedSwitchMethodItem(methodDefinition, codeItem, codeAddress, dead, stg,
                         (PackedSwitchDataPseudoInstruction)instruction);
             case SparseSwitchData:
-                return new SparseSwitchMethodItem(methodDefinition, codeItem, codeAddress, stg,
+                return new SparseSwitchMethodItem(methodDefinition, codeItem, codeAddress, dead, stg,
                         (SparseSwitchDataPseudoInstruction)instruction);
             case UnresolvedNullReference:
                 return new UnresolvedNullReferenceMethodItem(codeItem, codeAddress, stg,

baksmali/src/main/java/org/jf/baksmali/Adaptors/Format/PackedSwitchMethodItem.java

@@ -41,9 +41,10 @@ import java.util.List;
 
 public class PackedSwitchMethodItem extends InstructionMethodItem<PackedSwitchDataPseudoInstruction>
         implements Iterable<LabelMethodItem> {
-    private List<LabelMethodItem> labels;
+    private final List<LabelMethodItem> labels;
+    private final boolean dead;
 
-    public PackedSwitchMethodItem(MethodDefinition methodDefinition, CodeItem codeItem, int codeAddress,
+    public PackedSwitchMethodItem(MethodDefinition methodDefinition, CodeItem codeItem, int codeAddress, boolean dead,
                                   StringTemplateGroup stg, PackedSwitchDataPseudoInstruction instruction) {
         super(codeItem, codeAddress, stg, instruction);
 
@@ -58,11 +59,14 @@ public class PackedSwitchMethodItem extends InstructionMethodItem<PackedSwitchDa
             labels.add(label);
             label.setUncommented();
         }
+
+        this.dead = dead;
     }
 
     protected void setAttributes(StringTemplate template) {
         template.setAttribute("FirstKey", instruction.getFirstKey());
         template.setAttribute("Targets", labels);
+        template.setAttribute("Dead", dead);
     }
 
     public Iterator<LabelMethodItem> iterator() {

baksmali/src/main/java/org/jf/baksmali/Adaptors/Format/SparseSwitchMethodItem.java

@@ -41,9 +41,10 @@ import java.util.List;
 
 public class SparseSwitchMethodItem extends InstructionMethodItem<SparseSwitchDataPseudoInstruction>
         implements Iterable<LabelMethodItem> {
-    private List<SparseSwitchTarget> targets = null;
+    private final List<SparseSwitchTarget> targets;
+    private final boolean dead;
 
-    public SparseSwitchMethodItem(MethodDefinition methodDefinition, CodeItem codeItem, int codeAddress,
+    public SparseSwitchMethodItem(MethodDefinition methodDefinition, CodeItem codeItem, int codeAddress, boolean dead,
                                   StringTemplateGroup stg, SparseSwitchDataPseudoInstruction instruction) {
         super(codeItem, codeAddress, stg, instruction);
 
@@ -63,10 +64,13 @@ public class SparseSwitchMethodItem extends InstructionMethodItem<SparseSwitchDa
 
             targets.add(sparseSwitchTarget);
         }
+
+        this.dead = dead;
     }
 
     protected void setAttributes(StringTemplate template) {
         template.setAttribute("Targets", targets);
+        template.setAttribute("Dead", dead);
     }
 
     public Iterator<LabelMethodItem> iterator() {

baksmali/src/main/java/org/jf/baksmali/Adaptors/MethodDefinition.java

@@ -249,6 +249,30 @@ public class MethodDefinition {
         return annotationAdaptors;
     }
 
+    /**
+     * @param instructions The instructions array for this method
+     * @param instruction The instruction
+     * @return true if the specified instruction is a NOP, and the next instruction is one of the variable sized
+     * switch/array data structures
+     */
+    private boolean isInstructionPaddingNop(List<AnalyzedInstruction> instructions, AnalyzedInstruction instruction) {
+        if (instruction.getInstruction().opcode != Opcode.NOP ||
+            instruction.getInstruction().getFormat().variableSizeFormat) {
+
+            return false;
+        }
+
+        if (instruction.getInstructionIndex() == instructions.size()-1) {
+            return false;
+        }
+
+        AnalyzedInstruction nextInstruction = instructions.get(instruction.getInstructionIndex()+1);
+        if (nextInstruction.getInstruction().getFormat().variableSizeFormat) {
+            return true;
+        }
+        return false;
+    }
+
     private List<MethodItem> getMethodItems() {
         List<MethodItem> methodItems = new ArrayList<MethodItem>();
 
@@ -298,21 +322,25 @@ public class MethodDefinition {
             AnalyzedInstruction instruction = instructions.get(i);
 
             MethodItem methodItem = InstructionMethodItemFactory.makeInstructionFormatMethodItem(this,
-                    encodedMethod.codeItem, currentCodeAddress, stg, instruction.getInstruction(),
+                    encodedMethod.codeItem, currentCodeAddress, instruction.isDead(), stg, instruction.getInstruction(),
                     instruction == lastInstruction);
 
-            if (instruction.isDead()) {
+            boolean addedInstruction = false;
+            if (instruction.isDead() && !instruction.getInstruction().getFormat().variableSizeFormat) {
                 methodItems.add(new CommentedOutMethodItem(stg, methodItem));
                 lastIsUnreachable = false;
-            } else if (instruction.getPredecessorCount() == 0 &&
+                addedInstruction = true;
-                    !instruction.getInstruction().getFormat().variableSizeFormat) {
+            } else if ( instruction.getPredecessorCount() == 0 &&
+                        !instruction.getInstruction().getFormat().variableSizeFormat &&
+                        !isInstructionPaddingNop(instructions, instruction)) {
+
                 if (!lastIsUnreachable) {
                     methodItems.add(
-                            new CommentMethodItem(stg, "Unreachable code", currentCodeAddress, Double.MIN_VALUE));
+                        new CommentMethodItem(stg, "Unreachable code", currentCodeAddress, Double.MIN_VALUE));
                 }
 
                 methodItems.add(new CommentedOutMethodItem(stg, methodItem));
-                lastIsUnreachable = true;
+                    lastIsUnreachable = true;
             } else {
                 methodItems.add(methodItem);
                 lastIsUnreachable = false;
@@ -321,7 +349,8 @@ public class MethodDefinition {
             if (instruction.getInstruction().getFormat() == Format.UnresolvedNullReference) {
                 methodItems.add(new CommentedOutMethodItem(stg,
                         InstructionMethodItemFactory.makeInstructionFormatMethodItem(this, encodedMethod.codeItem,
-                                currentCodeAddress, stg, instruction.getOriginalInstruction(), false)));
+                                currentCodeAddress, instruction.isDead(), stg, instruction.getOriginalInstruction(),
+                                false)));
             }
 
             if (i != instructions.size() - 1) {

baksmali/src/main/resources/templates/templates/baksmali.stg

@@ -282,11 +282,16 @@ throw <Register>
 >>
 
 
-ArrayData(Opcode, ElementWidth, Values) ::=
+ArrayData(Opcode, ElementWidth, Values, Dead) ::=
 <<
-.array-data <ElementWidth>
+<if(Dead)>#<endif>.array-data <ElementWidth>
-    <Values; separator="\n">
+<if(Dead)>
-.end array-data
+<Values:  {#    <it>}; separator="\n">
+<else>
+<Values:  {     <it>}; separator="\n">
+<endif>
+
+<if(Dead)>#<endif>.end array-data
 >>
 
 ArrayElement(Bytes) ::=
@@ -294,18 +299,28 @@ ArrayElement(Bytes) ::=
 <Bytes; format="unsigned",separator=" ">
 >>
 
-PackedSwitchData(Opcode, FirstKey, Targets) ::=
+PackedSwitchData(Opcode, FirstKey, Targets, Dead) ::=
 <<
-.packed-switch <FirstKey>
+<if(Dead)>#<endif>.packed-switch <FirstKey>
-    <Targets: {<it>}; separator="\n">
+<if(Dead)>
-.end packed-switch
+<Targets:  {#   <it>}; separator="\n">
+<else>
+<Targets:  {    <it>}; separator="\n">
+<endif>
+
+<if(Dead)>#<endif>.end packed-switch
 >>
 
-SparseSwitchData(Opcode, Targets) ::=
+SparseSwitchData(Opcode, Targets, Dead) ::=
 <<
-.sparse-switch
+<if(Dead)>#<endif>.sparse-switch
-    <Targets:{<it.Key> -> <it.Target>}; separator="\n">
+<if(Dead)>
-.end sparse-switch
+<Targets:  {#   <it.Key> -> <it.Target>}; separator="\n">
+<else>
+<Targets:  {    <it.Key> -> <it.Target>}; separator="\n">
+<endif>
+
+<if(Dead)>#<endif>.end sparse-switch
 >>
 
 

baksmali/src/test/smali/deodex_test1/main.smali

@@ -58,6 +58,21 @@
     invoke-virtual {v2}, Lsuperclass;->somemethod()V
 
 
+    const v0, 1
+    new-array v0, v0, [I
+
+    #this should be marked as dead, and so should the corresponding .sparse-switch structure
+    fill-array-data v0, :array-data
+
+    const v0, 0
+
+    #this should be marked as dead, and so should the corresponding .sparse-switch structure
+    sparse-switch v0, :sparse-switch
+
+    #this should be marked as dead, and so should the corresponding .packed-switch structure
+    packed-switch v0, :packed-switch
+
+
     :here2
 
     #and we're back to the non-dead code
@@ -67,6 +82,24 @@
 
 
     return-void
+
+    #this should be marked as dead
+    :packed-switch
+    .packed-switch 0x0
+        :here
+    .end packed-switch
+
+    #this should be marked as dead
+    :sparse-switch
+    .sparse-switch
+        0x0 -> :here
+    .end sparse-switch
+
+    #this should be marked as dead
+    :array-data
+    .array-data 0x4
+        0x0t 0x0t 0x0t 0x0t
+    .end array-data
 .end method
 
 .method public static UnresolvedInstructionTest1()Lsuperclass;

dexlib/src/main/java/org/jf/dexlib/Code/Analysis/MethodAnalyzer.java

@@ -137,6 +137,10 @@ public class MethodAnalyzer {
 
         int nonParameterRegisters = totalRegisters - parameterRegisters;
 
+        for (AnalyzedInstruction instruction: instructions.getValues()) {
+            instruction.dead = true;
+        }
+
         //if this isn't a static method, determine which register is the "this" register and set the type to the
         //current class
         if ((encodedMethod.accessFlags & AccessFlags.STATIC.getValue()) == 0) {
@@ -189,6 +193,7 @@ public class MethodAnalyzer {
                         continue;
                     }
                     AnalyzedInstruction instructionToAnalyze = instructions.valueAt(i);
+                    instructionToAnalyze.dead = false;
                     try {
                         if (instructionToAnalyze.originalInstruction.opcode.odexOnly()) {
                             instructionToAnalyze.restoreOdexedInstruction();
@@ -233,7 +238,6 @@ public class MethodAnalyzer {
             }
         } while (true);
 
-
         for (int i=odexedInstructions.nextSetBit(0); i>=0; i=odexedInstructions.nextSetBit(i+1)) {
             AnalyzedInstruction instruction = instructions.valueAt(i);
 
@@ -254,11 +258,8 @@ public class MethodAnalyzer {
 
             instruction.setDeodexedInstruction(new UnresolvedNullReference(odexedInstruction,
                     objectRegisterNumber));
-
-            setAndPropagateDeadness(instruction);
         }
 
-
         analyzerState = ANALYZED;
     }
 
@@ -390,8 +391,6 @@ public class MethodAnalyzer {
     private void setAndPropagateDeadness(AnalyzedInstruction analyzedInstruction) {
         BitSet instructionsToProcess = new BitSet(instructions.size());
 
-        //temporarily set the undeodexeble instruction as dead, so that the "set dead if all predecessors are dead"
-        //operation works
         analyzedInstruction.dead = true;
 
         for (AnalyzedInstruction successor: analyzedInstruction.successors) {
@@ -427,8 +426,6 @@ public class MethodAnalyzer {
                 }
             }
         }
-
-        analyzedInstruction.dead = false;
     }
 
     private void setPostRegisterTypeAndPropagateChanges(AnalyzedInstruction analyzedInstruction, int registerNumber,
@@ -725,13 +722,17 @@ public class MethodAnalyzer {
                 return true;
             case FILLED_NEW_ARRAY:
             case FILLED_NEW_ARRAY_RANGE:
+                return true;
             case FILL_ARRAY_DATA:
+                analyzeArrayDataOrSwitch(analyzedInstruction);
             case THROW:
             case GOTO:
             case GOTO_16:
             case GOTO_32:
+                return true;
             case PACKED_SWITCH:
             case SPARSE_SWITCH:
+                analyzeArrayDataOrSwitch(analyzedInstruction);
                 return true;
             case CMPL_FLOAT:
             case CMPG_FLOAT:
@@ -2162,6 +2163,26 @@ public class MethodAnalyzer {
         }
     }
 
+    private void analyzeArrayDataOrSwitch(AnalyzedInstruction analyzedInstruction) {
+        int dataAddressOffset = ((OffsetInstruction)analyzedInstruction.instruction).getTargetAddressOffset();
+
+        int dataCodeAddress = this.getInstructionAddress(analyzedInstruction) + dataAddressOffset;
+        AnalyzedInstruction dataAnalyzedInstruction = instructions.get(dataCodeAddress);
+
+        if (dataAnalyzedInstruction != null) {
+            dataAnalyzedInstruction.dead = false;
+
+            //if there is a preceding nop, it's deadness should be the same
+            AnalyzedInstruction priorInstruction =
+                    instructions.valueAt(dataAnalyzedInstruction.getInstructionIndex()-1);
+            if (priorInstruction.getInstruction().opcode == Opcode.NOP &&
+                    !priorInstruction.getInstruction().getFormat().variableSizeFormat) {
+
+                priorInstruction.dead = false;
+            }
+        }
+    }
+
     private void verifySwitch(AnalyzedInstruction analyzedInstruction, Format expectedSwitchDataFormat) {
         int register = ((SingleRegisterInstruction)analyzedInstruction.instruction).getRegisterA();
         int switchCodeAddressOffset = ((OffsetInstruction)analyzedInstruction.instruction).getTargetAddressOffset();