Add better handling for various types of invalid instructions

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

@@ -32,6 +32,7 @@ import com.google.common.collect.Lists;
 import org.jf.baksmali.baksmaliOptions;
 import org.jf.dexlib2.AccessFlags;
 import org.jf.dexlib2.dexbacked.DexBackedClassDef;
+import org.jf.dexlib2.dexbacked.DexBackedDexFile.InvalidItemIndex;
 import org.jf.dexlib2.iface.*;
 import org.jf.dexlib2.iface.instruction.Instruction;
 import org.jf.dexlib2.iface.instruction.formats.Instruction21c;
@@ -79,8 +80,15 @@ public class ClassDefinition {
                             case SPUT_SHORT:
                             case SPUT_WIDE: {
                                 Instruction21c ins = (Instruction21c)instruction;
-                                FieldReference fieldRef = (FieldReference)ins.getReference();
+                                FieldReference fieldRef = null;
-                                if (fieldRef.getDefiningClass().equals((classDef.getType()))) {
+                                try {
+                                    fieldRef = (FieldReference)ins.getReference();
+                                } catch (InvalidItemIndex ex) {
+                                    // just ignore it for now. We'll deal with it later, when processing the instructions
+                                    // themselves
+                                }
+                                if (fieldRef != null &&
+                                        fieldRef.getDefiningClass().equals((classDef.getType()))) {
                                     fieldsSetInStaticConstructor.add(ReferenceUtil.getShortFieldDescriptor(fieldRef));
                                 }
                                 break;

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

@@ -29,16 +29,21 @@
 package org.jf.baksmali.Adaptors.Format;
 
 import org.jf.baksmali.Adaptors.MethodDefinition;
+import org.jf.baksmali.Adaptors.MethodDefinition.InvalidSwitchPayload;
 import org.jf.baksmali.Adaptors.MethodItem;
-import org.jf.baksmali.Adaptors.ReferenceFormatter;
 import org.jf.baksmali.Renderers.LongRenderer;
+import org.jf.baksmali.baksmaliOptions;
+import org.jf.dexlib2.Opcode;
 import org.jf.dexlib2.ReferenceType;
 import org.jf.dexlib2.VerificationError;
 import org.jf.dexlib2.dexbacked.DexBackedDexFile.InvalidItemIndex;
 import org.jf.dexlib2.iface.instruction.*;
 import org.jf.dexlib2.iface.instruction.formats.Instruction20bc;
+import org.jf.dexlib2.iface.instruction.formats.Instruction31t;
 import org.jf.dexlib2.iface.instruction.formats.UnknownInstruction;
 import org.jf.dexlib2.iface.reference.Reference;
+import org.jf.dexlib2.util.ReferenceUtil;
+import org.jf.util.ExceptionWithContext;
 import org.jf.util.IndentingWriter;
 
 import javax.annotation.Nonnull;
@@ -60,29 +65,105 @@ public class InstructionMethodItem<T extends Instruction> extends MethodItem {
         return 100;
     }
 
+    private boolean isAllowedOdex(@Nonnull Opcode opcode) {
+        baksmaliOptions options = methodDef.classDef.options;
+        if (options.allowOdex) {
+            return true;
+        }
+
+        if (methodDef.classDef.options.apiLevel >= 14) {
+            return false;
+        }
+
+        return opcode.isOdexedInstanceVolatile() || opcode.isOdexedStaticVolatile() ||
+            opcode == Opcode.THROW_VERIFICATION_ERROR;
+    }
+
     @Override
     public boolean writeTo(IndentingWriter writer) throws IOException {
-        boolean invalidReference = false;
+        Opcode opcode = instruction.getOpcode();
-        if (instruction instanceof ReferenceInstruction) {
+        String verificationErrorName = null;
-            try {
+        String referenceString = null;
-                Reference reference = ((ReferenceInstruction)instruction).getReference();
-            } catch (InvalidItemIndex ex) {
-                invalidReference = true;
 
-                writer.write("#invalid ");
+        boolean commentOutInstruction = false;
-                writer.write(ReferenceType.toString(instruction.getOpcode().referenceType));
+
-                writer.write(" index: ");
+        if (instruction instanceof Instruction20bc) {
-                writer.printSignedIntAsDec(ex.getInvalidIndex());
+            int verificationError = ((Instruction20bc)instruction).getVerificationError();
-                writer.write("\n#");
+            verificationErrorName = VerificationError.getVerificationErrorName(verificationError);
+            if (verificationErrorName == null) {
+                writer.write("#was invalid verification error type: ");
+                writer.printSignedIntAsDec(verificationError);
+                writer.write("\n");
+                verificationErrorName = "generic-error";
             }
         }
 
+        if (instruction instanceof ReferenceInstruction) {
+            ReferenceInstruction referenceInstruction = (ReferenceInstruction)instruction;
+            try {
+                Reference reference = referenceInstruction.getReference();
+                referenceString = ReferenceUtil.getReferenceString(reference);
+                assert referenceString != null;
+            } catch (InvalidItemIndex ex) {
+                writer.write("#");
+                writer.write(ex.getMessage());
+                writer.write("\n");
+                commentOutInstruction = true;
+
+                referenceString = String.format("%s@%d",
+                    ReferenceType.toString(referenceInstruction.getReferenceType()),
+                    ex.getInvalidIndex());
+            } catch (ReferenceType.InvalidReferenceTypeException ex) {
+                writer.write("#invalid reference type: ");
+                writer.printSignedIntAsDec(ex.getReferenceType());
+                commentOutInstruction = true;
+
+                referenceString = "invalid_reference";
+            }
+        }
+
+        if (instruction instanceof Instruction31t) {
+            Opcode payloadOpcode;
+            switch (instruction.getOpcode()) {
+                case PACKED_SWITCH:
+                    payloadOpcode = Opcode.PACKED_SWITCH_PAYLOAD;
+                    break;
+                case SPARSE_SWITCH:
+                    payloadOpcode = Opcode.SPARSE_SWITCH_PAYLOAD;
+                    break;
+                case FILL_ARRAY_DATA:
+                    payloadOpcode = Opcode.ARRAY_PAYLOAD;
+                    break;
+                default:
+                    throw new ExceptionWithContext("Invalid 31t opcode: %s", instruction.getOpcode());
+            }
+
+            try {
+                methodDef.findSwitchPayload(this.codeAddress + ((Instruction31t)instruction).getCodeOffset(),
+                        payloadOpcode);
+            } catch (InvalidSwitchPayload ex) {
+                writer.write("#invalid payload reference");
+                commentOutInstruction = true;
+            }
+        }
+
+        if (opcode.odexOnly()) {
+            if (!isAllowedOdex(opcode)) {
+                writer.write("#disallowed odex opcode\n");
+                commentOutInstruction = true;
+            }
+        }
+
+        if (commentOutInstruction) {
+            writer.write("#");
+        }
+
         switch (instruction.getOpcode().format) {
             case Format10t:
                 writeOpcode(writer);
                 writer.write(' ');
                 writeTargetLabel(writer);
-                return true;
+                break;
             case Format10x:
                 if (instruction instanceof UnknownInstruction) {
                     writer.write("#unknown opcode: 0x");
@@ -90,47 +171,47 @@ public class InstructionMethodItem<T extends Instruction> extends MethodItem {
                     writer.write('\n');
                 }
                 writeOpcode(writer);
-                return true;
+                break;
             case Format11n:
                 writeOpcode(writer);
                 writer.write(' ');
                 writeFirstRegister(writer);
                 writer.write(", ");
                 writeLiteral(writer);
-                return true;
+                break;
             case Format11x:
                 writeOpcode(writer);
                 writer.write(' ');
                 writeFirstRegister(writer);
-                return true;
+                break;
             case Format12x:
                 writeOpcode(writer);
                 writer.write(' ');
                 writeFirstRegister(writer);
                 writer.write(", ");
                 writeSecondRegister(writer);
-                return true;
+                break;
             case Format20bc:
                 writeOpcode(writer);
                 writer.write(' ');
-                writeVerificationErrorType(writer);
+                writer.write(verificationErrorName);
                 writer.write(", ");
-                writeReference(writer);
+                writer.write(referenceString);
-                return true;
+                break;
             case Format20t:
             case Format30t:
                 writeOpcode(writer);
                 writer.write(' ');
                 writeTargetLabel(writer);
-                return true;
+                break;
             case Format21c:
             case Format31c:
                 writeOpcode(writer);
                 writer.write(' ');
                 writeFirstRegister(writer);
                 writer.write(", ");
-                writeReference(writer);
+                writer.write(referenceString);
-                return true;
+                break;
             case Format21ih:
             case Format21lh:
             case Format21s:
@@ -143,7 +224,7 @@ public class InstructionMethodItem<T extends Instruction> extends MethodItem {
                 writeLiteral(writer);
                 if (instruction.getOpcode().setsWideRegister() == false)
                     writeResourceId(writer);
-                return true;
+                break;
             case Format21t:
             case Format31t:
                 writeOpcode(writer);
@@ -151,7 +232,7 @@ public class InstructionMethodItem<T extends Instruction> extends MethodItem {
                 writeFirstRegister(writer);
                 writer.write(", ");
                 writeTargetLabel(writer);
-                return true;
+                break;
             case Format22b:
             case Format22s:
                 writeOpcode(writer);
@@ -161,7 +242,7 @@ public class InstructionMethodItem<T extends Instruction> extends MethodItem {
                 writeSecondRegister(writer);
                 writer.write(", ");
                 writeLiteral(writer);
-                return true;
+                break;
             case Format22c:
                 writeOpcode(writer);
                 writer.write(' ');
@@ -169,8 +250,8 @@ public class InstructionMethodItem<T extends Instruction> extends MethodItem {
                 writer.write(", ");
                 writeSecondRegister(writer);
                 writer.write(", ");
-                writeReference(writer);
+                writer.write(referenceString);
-                return true;
+                break;
             case Format22cs:
                 writeOpcode(writer);
                 writer.write(' ');
@@ -179,7 +260,7 @@ public class InstructionMethodItem<T extends Instruction> extends MethodItem {
                 writeSecondRegister(writer);
                 writer.write(", ");
                 writeFieldOffset(writer);
-                return true;
+                break;
             case Format22t:
                 writeOpcode(writer);
                 writer.write(' ');
@@ -188,7 +269,7 @@ public class InstructionMethodItem<T extends Instruction> extends MethodItem {
                 writeSecondRegister(writer);
                 writer.write(", ");
                 writeTargetLabel(writer);
-                return true;
+                break;
             case Format22x:
             case Format32x:
                 writeOpcode(writer);
@@ -196,7 +277,7 @@ public class InstructionMethodItem<T extends Instruction> extends MethodItem {
                 writeFirstRegister(writer);
                 writer.write(", ");
                 writeSecondRegister(writer);
-                return true;
+                break;
             case Format23x:
                 writeOpcode(writer);
                 writer.write(' ');
@@ -205,54 +286,61 @@ public class InstructionMethodItem<T extends Instruction> extends MethodItem {
                 writeSecondRegister(writer);
                 writer.write(", ");
                 writeThirdRegister(writer);
-                return true;
+                break;
             case Format35c:
                 writeOpcode(writer);
                 writer.write(' ');
                 writeInvokeRegisters(writer);
                 writer.write(", ");
-                writeReference(writer);
+                writer.write(referenceString);
-                return true;
+                break;
             case Format35mi:
                 writeOpcode(writer);
                 writer.write(' ');
                 writeInvokeRegisters(writer);
                 writer.write(", ");
                 writeInlineIndex(writer);
-                return true;
+                break;
             case Format35ms:
                 writeOpcode(writer);
                 writer.write(' ');
                 writeInvokeRegisters(writer);
                 writer.write(", ");
                 writeVtableIndex(writer);
-                return true;
+                break;
             case Format3rc:
                 writeOpcode(writer);
                 writer.write(' ');
                 writeInvokeRangeRegisters(writer);
                 writer.write(", ");
-                writeReference(writer);
+                writer.write(referenceString);
-                return true;
+                break;
             case Format3rmi:
                 writeOpcode(writer);
                 writer.write(' ');
                 writeInvokeRangeRegisters(writer);
                 writer.write(", ");
                 writeInlineIndex(writer);
-                return true;
+                break;
             case Format3rms:
                 writeOpcode(writer);
                 writer.write(' ');
                 writeInvokeRangeRegisters(writer);
                 writer.write(", ");
                 writeVtableIndex(writer);
-                return true;
+                break;
-        }
+            default:
                 assert false;
                 return false;
         }
 
+        if (commentOutInstruction) {
+            writer.write("\nnop");
+        }
+
+        return true;
+    }
+
     protected void writeOpcode(IndentingWriter writer) throws IOException {
         writer.write(instruction.getOpcode().name);
     }
@@ -367,20 +455,4 @@ public class InstructionMethodItem<T extends Instruction> extends MethodItem {
         writer.write("vtable@");
         writer.printSignedIntAsDec(((VtableIndexInstruction)instruction).getVtableIndex());
     }
-
-    protected void writeReference(IndentingWriter writer) throws IOException {
-        try {
-            ReferenceFormatter.writeReference(writer, instruction.getOpcode().referenceType,
-                    ((ReferenceInstruction)instruction).getReference());
-        } catch (InvalidItemIndex ex) {
-            writer.write(ReferenceType.toString(instruction.getOpcode().referenceType));
-            writer.write("@");
-            writer.printSignedIntAsDec(ex.getInvalidIndex());
-        }
-    }
-
-    protected void writeVerificationErrorType(IndentingWriter writer) throws IOException {
-        int verificationError = ((Instruction20bc)instruction).getVerificationError();
-        writer.write(VerificationError.getVerificationErrorName(verificationError));
-    }
 }

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

@@ -39,6 +39,7 @@ import org.jf.dexlib2.ReferenceType;
 import org.jf.dexlib2.analysis.AnalysisException;
 import org.jf.dexlib2.analysis.AnalyzedInstruction;
 import org.jf.dexlib2.analysis.MethodAnalyzer;
+import org.jf.dexlib2.dexbacked.DexBackedDexFile.InvalidItemIndex;
 import org.jf.dexlib2.iface.*;
 import org.jf.dexlib2.iface.debug.DebugItem;
 import org.jf.dexlib2.iface.instruction.Instruction;
@@ -46,6 +47,7 @@ import org.jf.dexlib2.iface.instruction.OffsetInstruction;
 import org.jf.dexlib2.iface.instruction.ReferenceInstruction;
 import org.jf.dexlib2.iface.reference.MethodReference;
 import org.jf.dexlib2.util.InstructionOffsetMap;
+import org.jf.dexlib2.util.InstructionOffsetMap.InvalidInstructionOffset;
 import org.jf.dexlib2.util.ReferenceUtil;
 import org.jf.dexlib2.util.SyntheticAccessorResolver;
 import org.jf.dexlib2.util.TypeUtils;
@@ -92,17 +94,33 @@ public class MethodDefinition {
 
                 Opcode opcode = instruction.getOpcode();
                 if (opcode == Opcode.PACKED_SWITCH) {
+                    boolean valid = true;
                     int codeOffset = instructionOffsetMap.getInstructionCodeOffset(i);
                     int targetOffset = codeOffset + ((OffsetInstruction)instruction).getCodeOffset();
+                    try {
                         targetOffset = findSwitchPayload(targetOffset, Opcode.PACKED_SWITCH_PAYLOAD);
+                    } catch (InvalidSwitchPayload ex) {
+                        valid = false;
+                    }
+                    if (valid) {
                         packedSwitchMap.append(targetOffset, codeOffset);
+                    }
                 } else if (opcode == Opcode.SPARSE_SWITCH) {
+                    boolean valid = true;
                     int codeOffset = instructionOffsetMap.getInstructionCodeOffset(i);
                     int targetOffset = codeOffset + ((OffsetInstruction)instruction).getCodeOffset();
+                    try {
                         targetOffset = findSwitchPayload(targetOffset, Opcode.SPARSE_SWITCH_PAYLOAD);
+                    } catch (InvalidSwitchPayload ex) {
+                        valid = false;
+                        // The offset to the payload instruction was invalid. Nothing to do, except that we won't
+                        // add this instruction to the map.
+                    }
+                    if (valid) {
                         sparseSwitchMap.append(targetOffset, codeOffset);
                     }
                 }
+            }
         }catch (Exception ex) {
             String methodString;
             try {
@@ -187,8 +205,13 @@ public class MethodDefinition {
         writer.write(".end method\n");
     }
 
-    private int findSwitchPayload(int targetOffset, Opcode type) {
+    public int findSwitchPayload(int targetOffset, Opcode type) {
-        int targetIndex = instructionOffsetMap.getInstructionIndexAtCodeOffset(targetOffset);
+        int targetIndex;
+        try {
+            targetIndex = instructionOffsetMap.getInstructionIndexAtCodeOffset(targetOffset);
+        } catch (InvalidInstructionOffset ex) {
+            throw new InvalidSwitchPayload(targetOffset);
+        }
 
         //TODO: does dalvik let you pad with multiple nops?
         //TODO: does dalvik let a switch instruction point to a non-payload instruction?
@@ -205,7 +228,7 @@ public class MethodDefinition {
                     }
                 }
             }
-            throw new ExceptionWithContext("No switch payload at offset 0x%x", targetOffset);
+            throw new InvalidSwitchPayload(targetOffset);
         } else {
             return targetOffset;
         }
@@ -337,10 +360,16 @@ public class MethodDefinition {
                 Opcode opcode = instruction.getOpcode();
 
                 if (opcode.referenceType == ReferenceType.METHOD) {
-                    MethodReference methodReference =
+                    MethodReference methodReference = null;
-                            (MethodReference)((ReferenceInstruction)instruction).getReference();
+                    try {
+                        methodReference = (MethodReference)((ReferenceInstruction)instruction).getReference();
+                    } catch (InvalidItemIndex ex) {
+                        // just ignore it for now. We'll deal with it later, when processing the instructions
+                        // themselves
+                    }
 
-                    if (SyntheticAccessorResolver.looksLikeSyntheticAccessor(methodReference.getName())) {
+                    if (methodReference != null &&
+                            SyntheticAccessorResolver.looksLikeSyntheticAccessor(methodReference.getName())) {
                         SyntheticAccessorResolver.AccessedMember accessedMember =
                                 classDef.options.syntheticAccessorResolver.getAccessedMember(methodReference);
                         if (accessedMember != null) {
@@ -510,4 +539,17 @@ public class MethodDefinition {
             return labels.values();
         }
     }
+
+    public static class InvalidSwitchPayload extends ExceptionWithContext {
+        private final int payloadOffset;
+
+        public InvalidSwitchPayload(int payloadOffset) {
+            super("No switch payload at offset: %d", payloadOffset);
+            this.payloadOffset = payloadOffset;
+        }
+
+        public int getPayloadOffset() {
+            return payloadOffset;
+        }
+    }
 }

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

@@ -57,6 +57,9 @@ public class ReferenceFormatter {
                 return;
             case ReferenceType.FIELD:
                 ReferenceUtil.writeFieldDescriptor(writer, (FieldReference)reference);
+                return;
+            default:
+                throw new IllegalStateException("Unknown reference type");
         }
     }
 }

baksmali/src/main/java/org/jf/baksmali/baksmaliOptions.java

@@ -67,6 +67,7 @@ public class baksmaliOptions {
     public boolean outputDebugInfo = true;
     public boolean addCodeOffsets = false;
     public boolean noAccessorComments = false;
+    public boolean allowOdex = false;
     public boolean deodex = false;
     public boolean ignoreErrors = false;
     public boolean checkPackagePrivateAccess = false;

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

@@ -255,6 +255,7 @@ public class main {
                 System.err.println("Warning: You are disassembling an odex file without deodexing it. You");
                 System.err.println("won't be able to re-assemble the results unless you deodex it with the -x");
                 System.err.println("option");
+                options.allowOdex = true;
             }
         } else {
             options.deodex = false;

dexlib2/src/main/java/org/jf/dexlib2/VerificationError.java

@@ -34,6 +34,7 @@ package org.jf.dexlib2;
 import com.google.common.collect.Maps;
 import org.jf.util.ExceptionWithContext;
 
+import javax.annotation.Nullable;
 import java.util.HashMap;
 
 public class VerificationError {
@@ -61,6 +62,7 @@ public class VerificationError {
         verificationErrorNames.put("instantiation-error", INSTANTIATION_ERROR);
     }
 
+    @Nullable
     public static String getVerificationErrorName(int verificationError) {
         switch (verificationError) {
             case GENERIC:

dexlib2/src/main/java/org/jf/dexlib2/builder/MutableMethodImplementation.java

@@ -435,6 +435,14 @@ public class MutableMethodImplementation implements MethodImplementation {
                         }
                         case SPARSE_SWITCH_PAYLOAD:
                         case PACKED_SWITCH_PAYLOAD:
+                            if (((BuilderSwitchPayload)instruction).referrer == null) {
+                                // if the switch payload isn't referenced, just remove it
+                                removeInstruction(index);
+                                index--;
+                                madeChanges = true;
+                                break;
+                            }
+                            // intentional fall-through
                         case ARRAY_PAYLOAD: {
                             if ((location.codeAddress & 0x01) != 0) {
                                 int previousIndex = location.index - 1;

dexlib2/src/main/java/org/jf/dexlib2/util/InstructionOffsetMap.java

@@ -59,7 +59,7 @@ public class InstructionOffsetMap {
         int index = Arrays.binarySearch(instructionCodeOffsets, codeOffset);
         if (index < 0) {
             if (exact) {
-                throw new ExceptionWithContext("No instruction at offset %d", codeOffset);
+                throw new InvalidInstructionOffset(codeOffset);
             } else {
                 // This calculation would be incorrect if index was -1 (i.e. insertion point of 0). Luckily, we can
                 // ignore this case, because codeOffset will always be non-negative, and the code offset of the first
@@ -72,8 +72,34 @@ public class InstructionOffsetMap {
 
     public int getInstructionCodeOffset(int index) {
         if (index < 0 || index >= instructionCodeOffsets.length) {
-            throw new ExceptionWithContext("Index out of bounds: %d", index);
+            throw new InvalidInstructionIndex(index);
         }
         return instructionCodeOffsets[index];
     }
+
+    public static class InvalidInstructionOffset extends ExceptionWithContext {
+        private final int instructionOffset;
+
+        public InvalidInstructionOffset(int instructionOffset) {
+            super("No instruction at offset %d", instructionOffset);
+            this.instructionOffset = instructionOffset;
+        }
+
+        public int getInstructionOffset() {
+            return instructionOffset;
+        }
+    }
+
+    public static class InvalidInstructionIndex extends ExceptionWithContext {
+        private final int instructionIndex;
+
+        public InvalidInstructionIndex(int instructionIndex) {
+            super("Instruction index out of bounds: %d", instructionIndex);
+            this.instructionIndex = instructionIndex;
+        }
+
+        public int getInstructionIndex() {
+            return instructionIndex;
+        }
+    }
 }