From db26b663aa3b5bb721185b8798b6767710d3c243 Mon Sep 17 00:00:00 2001 From: "JesusFreke@JesusFreke.com" Date: Sat, 6 Mar 2010 06:12:30 +0000 Subject: [PATCH] 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 --- .../Adaptors/Format/ArrayDataMethodItem.java | 6 ++- .../Format/InstructionMethodItemFactory.java | 8 ++-- .../Format/PackedSwitchMethodItem.java | 8 +++- .../Format/SparseSwitchMethodItem.java | 8 +++- .../baksmali/Adaptors/MethodDefinition.java | 43 ++++++++++++++++--- .../templates/templates/baksmali.stg | 39 +++++++++++------ .../src/test/smali/deodex_test1/main.smali | 33 ++++++++++++++ .../dexlib/Code/Analysis/MethodAnalyzer.java | 37 ++++++++++++---- 8 files changed, 146 insertions(+), 36 deletions(-) diff --git a/baksmali/src/main/java/org/jf/baksmali/Adaptors/Format/ArrayDataMethodItem.java b/baksmali/src/main/java/org/jf/baksmali/Adaptors/Format/ArrayDataMethodItem.java index a8b6c8d9..6bf57f7f 100644 --- a/baksmali/src/main/java/org/jf/baksmali/Adaptors/Format/ArrayDataMethodItem.java +++ b/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 { - 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); } diff --git a/baksmali/src/main/java/org/jf/baksmali/Adaptors/Format/InstructionMethodItemFactory.java b/baksmali/src/main/java/org/jf/baksmali/Adaptors/Format/InstructionMethodItemFactory.java index 242a254b..d6dc6bd8 100644 --- a/baksmali/src/main/java/org/jf/baksmali/Adaptors/Format/InstructionMethodItemFactory.java +++ b/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, diff --git a/baksmali/src/main/java/org/jf/baksmali/Adaptors/Format/PackedSwitchMethodItem.java b/baksmali/src/main/java/org/jf/baksmali/Adaptors/Format/PackedSwitchMethodItem.java index 8e3ebca5..6404d67c 100644 --- a/baksmali/src/main/java/org/jf/baksmali/Adaptors/Format/PackedSwitchMethodItem.java +++ b/baksmali/src/main/java/org/jf/baksmali/Adaptors/Format/PackedSwitchMethodItem.java @@ -41,9 +41,10 @@ import java.util.List; public class PackedSwitchMethodItem extends InstructionMethodItem implements Iterable { - private List labels; + private final List 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 iterator() { diff --git a/baksmali/src/main/java/org/jf/baksmali/Adaptors/Format/SparseSwitchMethodItem.java b/baksmali/src/main/java/org/jf/baksmali/Adaptors/Format/SparseSwitchMethodItem.java index 5e37580b..3c4984bf 100644 --- a/baksmali/src/main/java/org/jf/baksmali/Adaptors/Format/SparseSwitchMethodItem.java +++ b/baksmali/src/main/java/org/jf/baksmali/Adaptors/Format/SparseSwitchMethodItem.java @@ -41,9 +41,10 @@ import java.util.List; public class SparseSwitchMethodItem extends InstructionMethodItem implements Iterable { - private List targets = null; + private final List 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 iterator() { diff --git a/baksmali/src/main/java/org/jf/baksmali/Adaptors/MethodDefinition.java b/baksmali/src/main/java/org/jf/baksmali/Adaptors/MethodDefinition.java index a4c431a5..b694a883 100644 --- a/baksmali/src/main/java/org/jf/baksmali/Adaptors/MethodDefinition.java +++ b/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 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 getMethodItems() { List methodItems = new ArrayList(); @@ -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 && - !instruction.getInstruction().getFormat().variableSizeFormat) { + addedInstruction = true; + } 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) { diff --git a/baksmali/src/main/resources/templates/templates/baksmali.stg b/baksmali/src/main/resources/templates/templates/baksmali.stg index 4053d739..8b5b42f7 100644 --- a/baksmali/src/main/resources/templates/templates/baksmali.stg +++ b/baksmali/src/main/resources/templates/templates/baksmali.stg @@ -282,11 +282,16 @@ throw >> -ArrayData(Opcode, ElementWidth, Values) ::= +ArrayData(Opcode, ElementWidth, Values, Dead) ::= << -.array-data - -.end array-data +#.array-data + +}; separator="\n"> + +}; separator="\n"> + + +#.end array-data >> ArrayElement(Bytes) ::= @@ -294,18 +299,28 @@ ArrayElement(Bytes) ::= >> -PackedSwitchData(Opcode, FirstKey, Targets) ::= +PackedSwitchData(Opcode, FirstKey, Targets, Dead) ::= << -.packed-switch - }; separator="\n"> -.end packed-switch +#.packed-switch + +}; separator="\n"> + +}; separator="\n"> + + +#.end packed-switch >> -SparseSwitchData(Opcode, Targets) ::= +SparseSwitchData(Opcode, Targets, Dead) ::= << -.sparse-switch - -> }; separator="\n"> -.end sparse-switch +#.sparse-switch + + -> }; separator="\n"> + + -> }; separator="\n"> + + +#.end sparse-switch >> diff --git a/baksmali/src/test/smali/deodex_test1/main.smali b/baksmali/src/test/smali/deodex_test1/main.smali index 8d05f305..71b8f090 100644 --- a/baksmali/src/test/smali/deodex_test1/main.smali +++ b/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; diff --git a/dexlib/src/main/java/org/jf/dexlib/Code/Analysis/MethodAnalyzer.java b/dexlib/src/main/java/org/jf/dexlib/Code/Analysis/MethodAnalyzer.java index 00647b90..54c0cfe3 100644 --- a/dexlib/src/main/java/org/jf/dexlib/Code/Analysis/MethodAnalyzer.java +++ b/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();