From 15ae0affc492cab13b19241c9723019b7b80c859 Mon Sep 17 00:00:00 2001 From: Izzat Bahadirov Date: Wed, 20 Feb 2013 23:28:37 -0500 Subject: [PATCH 01/13] Handling jumbo string conversions and consequent offset adjustments for branch target instructions and 4-byte alignment enforcement for payload instructions (by prepending them with nops). --- .../org/jf/dexlib2/writer/CodeItemPool.java | 37 +- .../writer/util/InstructionWriteUtil.java | 375 ++++++++++++++++++ 2 files changed, 390 insertions(+), 22 deletions(-) create mode 100644 dexlib2/src/main/java/org/jf/dexlib2/writer/util/InstructionWriteUtil.java diff --git a/dexlib2/src/main/java/org/jf/dexlib2/writer/CodeItemPool.java b/dexlib2/src/main/java/org/jf/dexlib2/writer/CodeItemPool.java index 909aa177..3a67ad3f 100644 --- a/dexlib2/src/main/java/org/jf/dexlib2/writer/CodeItemPool.java +++ b/dexlib2/src/main/java/org/jf/dexlib2/writer/CodeItemPool.java @@ -43,9 +43,9 @@ import org.jf.dexlib2.iface.instruction.ReferenceInstruction; import org.jf.dexlib2.iface.instruction.SwitchElement; import org.jf.dexlib2.iface.instruction.formats.*; import org.jf.dexlib2.iface.reference.*; -import org.jf.dexlib2.util.InstructionUtil; import org.jf.dexlib2.util.MethodUtil; import org.jf.dexlib2.util.ReferenceUtil; +import org.jf.dexlib2.writer.util.InstructionWriteUtil; import org.jf.util.ExceptionWithContext; import javax.annotation.Nonnull; @@ -149,30 +149,15 @@ public class CodeItemPool { writer.writeUshort(methodImpl.getRegisterCount()); writer.writeUshort(MethodUtil.getParameterRegisterCount(method, MethodUtil.isStatic(method))); - int maxOutParamCount = 0; - int codeUnitCount = 0; - for (Instruction instruction: methodImpl.getInstructions()) { - codeUnitCount += instruction.getCodeUnits(); - if (instruction.getOpcode().referenceType == ReferenceType.METHOD) { - ReferenceInstruction refInsn = (ReferenceInstruction)instruction; - MethodReference methodRef = (MethodReference)refInsn.getReference(); - int paramCount = MethodUtil.getParameterRegisterCount(methodRef, - InstructionUtil.isInvokeStatic(instruction.getOpcode())); - if (paramCount > maxOutParamCount) { - maxOutParamCount = paramCount; - } - } - } - writer.writeUshort(maxOutParamCount); + InstructionWriteUtil instrWriteUtil = new InstructionWriteUtil(methodImpl, dexFile.stringPool); + writer.writeUshort(instrWriteUtil.getOutParamCount()); List tryBlocks = methodImpl.getTryBlocks(); writer.writeUshort(tryBlocks.size()); writer.writeInt(dexFile.debugInfoPool.getOffset(method)); - writer.writeInt(codeUnitCount); + writer.writeInt(instrWriteUtil.getCodeUnitCount()); - // TODO: need to fix up instructions. Add alignment nops, convert to const-string/jumbos, etc. - - for (Instruction instruction: methodImpl.getInstructions()) { + for (Instruction instruction: instrWriteUtil.getInstructions()) { switch (instruction.getOpcode().format) { case Format10t: writeFormat10t(writer, (Instruction10t)instruction); @@ -274,8 +259,15 @@ public class CodeItemPool { DexWriter.writeUleb128(ehBuf, exceptionHandlerOffsetMap.size()); for (TryBlock tryBlock: tryBlocks) { - writer.writeInt(tryBlock.getStartCodeAddress()); - writer.writeUshort(tryBlock.getCodeUnitCount()); + int startAddress = tryBlock.getStartCodeAddress(); + int endAddress = startAddress + tryBlock.getCodeUnitCount(); + + startAddress += instrWriteUtil.codeOffsetShift(startAddress); + endAddress += instrWriteUtil.codeOffsetShift(endAddress); + int tbCodeUnitCount = endAddress - startAddress; + + writer.writeInt(startAddress); + writer.writeUshort(tbCodeUnitCount); if (tryBlock.getExceptionHandlers().size() == 0) { throw new ExceptionWithContext("No exception handlers for the try block!"); @@ -303,6 +295,7 @@ public class CodeItemPool { for (ExceptionHandler eh : tryBlock.getExceptionHandlers()) { String exceptionType = eh.getExceptionType(); int codeAddress = eh.getHandlerCodeAddress(); + codeAddress += instrWriteUtil.codeOffsetShift(codeAddress); if (exceptionType != null) { //regular exception handling diff --git a/dexlib2/src/main/java/org/jf/dexlib2/writer/util/InstructionWriteUtil.java b/dexlib2/src/main/java/org/jf/dexlib2/writer/util/InstructionWriteUtil.java new file mode 100644 index 00000000..438a5c57 --- /dev/null +++ b/dexlib2/src/main/java/org/jf/dexlib2/writer/util/InstructionWriteUtil.java @@ -0,0 +1,375 @@ +/* + * Copyright 2013, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package org.jf.dexlib2.writer.util; + +import com.google.common.collect.Lists; +import org.jf.dexlib2.Format; +import org.jf.dexlib2.Opcode; +import org.jf.dexlib2.ReferenceType; +import org.jf.dexlib2.iface.MethodImplementation; +import org.jf.dexlib2.iface.instruction.Instruction; +import org.jf.dexlib2.iface.instruction.ReferenceInstruction; +import org.jf.dexlib2.iface.instruction.SwitchElement; +import org.jf.dexlib2.iface.instruction.SwitchPayload; +import org.jf.dexlib2.iface.instruction.formats.*; +import org.jf.dexlib2.iface.reference.*; +import org.jf.dexlib2.immutable.instruction.*; +import org.jf.dexlib2.util.InstructionUtil; +import org.jf.dexlib2.util.MethodUtil; +import org.jf.dexlib2.writer.StringPool; +import org.jf.util.ExceptionWithContext; + +import javax.annotation.Nonnull; +import java.util.ArrayList; +import java.util.List; + +public class InstructionWriteUtil { + private final StringPool stringPool; + MethodImplementation methodImplementation; + + private List instructions; + private ArrayList codeOffsetShifts; + + private int codeUnitCount; + private int outParamCount; + + public InstructionWriteUtil(@Nonnull MethodImplementation methodImpl, @Nonnull StringPool stringPool) { + this.stringPool = stringPool; + methodImplementation = methodImpl; + + calculateMaxOutParamCount(); + findCodeOffsetShifts(); + modifyInstructions(); + } + + private void calculateMaxOutParamCount() { + for (Instruction instruction: methodImplementation.getInstructions()) { + codeUnitCount += instruction.getCodeUnits(); + if (instruction.getOpcode().referenceType == ReferenceType.METHOD) { + ReferenceInstruction refInsn = (ReferenceInstruction)instruction; + MethodReference methodRef = (MethodReference)refInsn.getReference(); + int paramCount = MethodUtil.getParameterRegisterCount(methodRef, InstructionUtil.isInvokeStatic(instruction.getOpcode())); + if (paramCount > outParamCount) { + outParamCount = paramCount; + } + } + } + } + + public Iterable getInstructions() { + if (instructions != null) { + return instructions; + } else { + return methodImplementation.getInstructions(); + } + } + + public int getCodeUnitCount() { + return codeUnitCount; + } + + public int getOutParamCount() { + return outParamCount; + } + + private int targetOffsetShift(int instrOffset, int targetOffset) { + int targetOffsetShift = 0; + if (codeOffsetShifts != null) { + int instrShift = codeOffsetShift(instrOffset); + int targetShift = codeOffsetShift(instrOffset+targetOffset); + targetOffsetShift = targetShift - instrShift; + } + return targetOffsetShift; + } + + public int codeOffsetShift(int offset) { + int shift = 0; + if (codeOffsetShifts != null) { + int numCodeOffsetShifts = codeOffsetShifts.size(); + if (numCodeOffsetShifts > 0) { + if (offset >= codeOffsetShifts.get(numCodeOffsetShifts-1)) { + shift = numCodeOffsetShifts; + } else if (numCodeOffsetShifts>1) { + for (int i=1;i= codeOffsetShifts.get(i-1) && offset < codeOffsetShifts.get(i)) { + shift = i; + break; + } + } + } + } + } + return shift; + } + + /** + * This method creates a list of code offsets of instructions, whose (and subsequent instructions') + * code offset will get shifted by one code unit with respect to previous instruction(s). + * This happens when the previous instruction has to be changed to a larger sized one + * to fit the new value or payload instruction has to be prepended by nop to ensure alignment. + */ + private void findCodeOffsetShifts() { + // first, process const-string to const-string/jumbo conversions + int currentCodeOffset = 0; + for (Instruction instruction: methodImplementation.getInstructions()) { + if (instruction.getOpcode().equals(Opcode.CONST_STRING)) { + ReferenceInstruction refInstr = (ReferenceInstruction) instruction; + int referenceIndex = stringPool.getIndex((StringReference)refInstr.getReference()); + if (referenceIndex > 0xFFFF) { + if (codeOffsetShifts == null) { + codeOffsetShifts = new ArrayList(); + } + codeOffsetShifts.add(currentCodeOffset+instruction.getCodeUnits()); + } + } + currentCodeOffset += instruction.getCodeUnits(); + } + + if (codeOffsetShifts == null) { + return; + } + + // next, let's check if this caused any conversions in goto instructions due to changes in offset values + // since code offset delta is equivalent to the position of instruction's code offset in the shift list, + // we use it as a position here + // we also check if we will have to insert nops to ensure 4-byte alignment for switch statements and packed arrays + currentCodeOffset = 0; + for (Instruction instruction: methodImplementation.getInstructions()) { + if (instruction.getOpcode().format.equals(Format.Format10t)) { + int targetOffset = ((Instruction10t)instruction).getCodeOffset(); + int codeOffsetDelta = codeOffsetShift(currentCodeOffset); + int newTargetOffset = targetOffset + targetOffsetShift(currentCodeOffset, targetOffset); + if ((byte)newTargetOffset != newTargetOffset) { + if ((short)newTargetOffset != newTargetOffset) { + // handling very small (negligible) possibility of goto becoming goto/32 + // we insert extra 1 code unit shift referring to the same position + // this will cause subsequent code offsets to be shifted by 2 code units + codeOffsetShifts.add(codeOffsetDelta, currentCodeOffset+instruction.getCodeUnits()); + } + codeOffsetShifts.add(codeOffsetDelta, currentCodeOffset+instruction.getCodeUnits()); + } + } else if (instruction.getOpcode().format.equals(Format.Format20t)) { + int targetOffset = ((Instruction20t)instruction).getCodeOffset(); + int codeOffsetDelta = codeOffsetShift(currentCodeOffset); + int newTargetOffset = targetOffsetShift(currentCodeOffset, targetOffset); + if ((short)newTargetOffset != newTargetOffset) { + codeOffsetShifts.add(codeOffsetDelta, currentCodeOffset+instruction.getCodeUnits()); + } + } else if (instruction.getOpcode().format.equals(Format.ArrayPayload) + || instruction.getOpcode().format.equals(Format.SparseSwitchPayload) + || instruction.getOpcode().format.equals(Format.PackedSwitchPayload)) { + int codeOffsetDelta = codeOffsetShift(currentCodeOffset); + if ((currentCodeOffset+codeOffsetDelta)%2 != 0) { + codeOffsetShifts.add(codeOffsetDelta, currentCodeOffset); + } + } + currentCodeOffset += instruction.getCodeUnits(); + } + + codeUnitCount += codeOffsetShifts.size(); + } + + private void modifyInstructions() { + if (codeOffsetShifts == null) { + return; + } + + instructions = Lists.newArrayList(); + int currentCodeOffset = 0; + for (Instruction instruction: methodImplementation.getInstructions()) { + Instruction modifiedInstruction = null; + switch (instruction.getOpcode().format) { + case Format10t: { + Instruction10t instr = (Instruction10t)instruction; + int targetOffset = instr.getCodeOffset(); + int newTargetOffset = targetOffset + targetOffsetShift(currentCodeOffset, targetOffset); + if (newTargetOffset != targetOffset) { + if ((byte)newTargetOffset != newTargetOffset) { + if ((short)newTargetOffset != newTargetOffset) { + modifiedInstruction = new ImmutableInstruction30t(Opcode.GOTO_32, newTargetOffset); + } else { + modifiedInstruction = new ImmutableInstruction20t(Opcode.GOTO_16, newTargetOffset); + } + } else { + modifiedInstruction = new ImmutableInstruction10t(instr.getOpcode(), newTargetOffset); + } + } + break; + } + case Format20t: { + Instruction20t instr = (Instruction20t)instruction; + int targetOffset = instr.getCodeOffset(); + int newTargetOffset = targetOffset + targetOffsetShift(currentCodeOffset, targetOffset); + if (newTargetOffset != targetOffset) { + if ((short)newTargetOffset != newTargetOffset) { + modifiedInstruction = new ImmutableInstruction30t(Opcode.GOTO_32, newTargetOffset); + } else { + modifiedInstruction = new ImmutableInstruction20t(Opcode.GOTO_16, newTargetOffset); + } + } + break; + } + case Format21c: { + Instruction21c instr = (Instruction21c)instruction; + if (instr.getOpcode().equals(Opcode.CONST_STRING)) { + int referenceIndex = stringPool.getIndex((StringReference)instr.getReference()); + if (referenceIndex > 0xFFFF) { + modifiedInstruction = new ImmutableInstruction31c(Opcode.CONST_STRING_JUMBO, instr.getRegisterA(), instr.getReference()); + } + } + break; + } + case Format21t: { + Instruction21t instr = (Instruction21t)instruction; + int targetOffset = instr.getCodeOffset(); + int newTargetOffset = targetOffset + targetOffsetShift(currentCodeOffset, targetOffset); + if (newTargetOffset != targetOffset) { + modifiedInstruction = new ImmutableInstruction21t(instr.getOpcode(), instr.getRegisterA(), newTargetOffset); + } + break; + } + case Format22t: { + Instruction22t instr = (Instruction22t)instruction; + int targetOffset = instr.getCodeOffset(); + int newTargetOffset = targetOffset + targetOffsetShift(currentCodeOffset, targetOffset); + if (newTargetOffset != targetOffset) { + modifiedInstruction = new ImmutableInstruction22t(instr.getOpcode(), instr.getRegisterA(), instr.getRegisterB(), newTargetOffset); + } + break; + } + case Format30t: { + Instruction30t instr = (Instruction30t)instruction; + int targetOffset = instr.getCodeOffset(); + int newTargetOffset = targetOffset + targetOffsetShift(currentCodeOffset, targetOffset); + if (newTargetOffset != targetOffset) { + modifiedInstruction = new ImmutableInstruction30t(instr.getOpcode(), newTargetOffset); + } + break; + } + case Format31t: { + Instruction31t instr = (Instruction31t)instruction; + int targetOffset = instr.getCodeOffset(); + int newTargetOffset = targetOffset + targetOffsetShift(currentCodeOffset, targetOffset); + if (newTargetOffset != targetOffset) { + modifiedInstruction = new ImmutableInstruction31t(instr.getOpcode(), instr.getRegisterA(), newTargetOffset); + } + break; + } + case SparseSwitchPayload: { + alignPayload(currentCodeOffset); + int switchInstructionOffset = findSwitchInstructionOffset(currentCodeOffset); + SwitchPayload payload = (SwitchPayload)instruction; + if (isSwitchTargetOffsetChanged(payload, switchInstructionOffset)) { + List newSwitchElements = modifySwitchElements(payload, switchInstructionOffset); + modifiedInstruction = new ImmutableSparseSwitchPayload(newSwitchElements); + } + break; + } + case PackedSwitchPayload: { + alignPayload(currentCodeOffset); + int switchInstructionOffset = findSwitchInstructionOffset(currentCodeOffset); + SwitchPayload payload = (SwitchPayload)instruction; + if (isSwitchTargetOffsetChanged(payload, switchInstructionOffset)) { + List newSwitchElements = modifySwitchElements(payload, switchInstructionOffset); + modifiedInstruction = new ImmutablePackedSwitchPayload(newSwitchElements); + } + break; + } + case ArrayPayload: { + alignPayload(currentCodeOffset); + break; + } + } + + if (modifiedInstruction != null) { + instructions.add(modifiedInstruction); + } else { + instructions.add(instruction); + } + + currentCodeOffset += instruction.getCodeUnits(); + } + } + + private void alignPayload(int codeOffset) { + if (codeOffsetShifts.contains(codeOffset)) { + instructions.add(new ImmutableInstruction10x(Opcode.NOP)); + } + } + + private int findSwitchInstructionOffset(int payloadOffset) { + int currentCodeOffset = 0; + int switchInstructionOffset = -1; + for (Instruction instruction: methodImplementation.getInstructions()) { + if (instruction.getOpcode().equals(Opcode.PACKED_SWITCH) + || instruction.getOpcode().equals(Opcode.SPARSE_SWITCH)) { + int targetOffset = currentCodeOffset + ((Instruction31t)instruction).getCodeOffset(); + if (targetOffset == payloadOffset) { + if (switchInstructionOffset < 0) { + switchInstructionOffset = currentCodeOffset; + } else { + throw new ExceptionWithContext("Multiple switch instructions refer to the same switch payload!"); + } + } + } + currentCodeOffset += instruction.getCodeUnits(); + } + return switchInstructionOffset; + } + + private boolean isSwitchTargetOffsetChanged(SwitchPayload payload, int switchInstructionOffset) { + for (SwitchElement switchElement: payload.getSwitchElements()) { + if (targetOffsetShift(switchInstructionOffset, switchElement.getOffset()) != 0) { + return true; + } + } + return false; + } + + private ArrayList modifySwitchElements(SwitchPayload payload, int switchInstructionOffset) { + ArrayList switchElements = Lists.newArrayList(); + for (SwitchElement switchElement: payload.getSwitchElements()) { + int targetOffset = switchElement.getOffset(); + int newTargetOffset = targetOffset + targetOffsetShift(switchInstructionOffset, targetOffset); + if (newTargetOffset != targetOffset) { + ImmutableSwitchElement immuSwitchElement = new ImmutableSwitchElement(switchElement.getKey(), newTargetOffset); + switchElements.add(immuSwitchElement); + } else { + switchElements.add(switchElement); + } + } + return switchElements; + } + +} + + From 5df16db1081313e7bb8b0336167a74404b64c38b Mon Sep 17 00:00:00 2001 From: Izzat Bahadirov Date: Fri, 8 Mar 2013 19:22:34 -0500 Subject: [PATCH 02/13] Iterative logic to handle embiggening of goto instructions, when subsequent code shifts are inserted. Layout logic creates a HashMap of predicted instruction replacements and nop insertions to be used later in writing method, which is more cromulent solution than simply replicating the logic, given its iterativeness. --- .../writer/util/InstructionWriteUtil.java | 114 +++++++++++------- 1 file changed, 68 insertions(+), 46 deletions(-) diff --git a/dexlib2/src/main/java/org/jf/dexlib2/writer/util/InstructionWriteUtil.java b/dexlib2/src/main/java/org/jf/dexlib2/writer/util/InstructionWriteUtil.java index 438a5c57..a0b48be3 100644 --- a/dexlib2/src/main/java/org/jf/dexlib2/writer/util/InstructionWriteUtil.java +++ b/dexlib2/src/main/java/org/jf/dexlib2/writer/util/InstructionWriteUtil.java @@ -50,6 +50,7 @@ import org.jf.util.ExceptionWithContext; import javax.annotation.Nonnull; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; public class InstructionWriteUtil { @@ -58,6 +59,7 @@ public class InstructionWriteUtil { private List instructions; private ArrayList codeOffsetShifts; + private HashMap offsetToNewInstructionMap; private int codeUnitCount; private int outParamCount; @@ -148,7 +150,11 @@ public class InstructionWriteUtil { if (codeOffsetShifts == null) { codeOffsetShifts = new ArrayList(); } + if (offsetToNewInstructionMap == null) { + offsetToNewInstructionMap = new HashMap(); + } codeOffsetShifts.add(currentCodeOffset+instruction.getCodeUnits()); + offsetToNewInstructionMap.put(currentCodeOffset, instruction.getOpcode().format); } } currentCodeOffset += instruction.getCodeUnits(); @@ -162,38 +168,55 @@ public class InstructionWriteUtil { // since code offset delta is equivalent to the position of instruction's code offset in the shift list, // we use it as a position here // we also check if we will have to insert nops to ensure 4-byte alignment for switch statements and packed arrays - currentCodeOffset = 0; - for (Instruction instruction: methodImplementation.getInstructions()) { - if (instruction.getOpcode().format.equals(Format.Format10t)) { - int targetOffset = ((Instruction10t)instruction).getCodeOffset(); - int codeOffsetDelta = codeOffsetShift(currentCodeOffset); - int newTargetOffset = targetOffset + targetOffsetShift(currentCodeOffset, targetOffset); - if ((byte)newTargetOffset != newTargetOffset) { - if ((short)newTargetOffset != newTargetOffset) { - // handling very small (negligible) possibility of goto becoming goto/32 - // we insert extra 1 code unit shift referring to the same position - // this will cause subsequent code offsets to be shifted by 2 code units + boolean shiftsInserted; + do { + currentCodeOffset = 0; + shiftsInserted = false; + for (Instruction instruction: methodImplementation.getInstructions()) { + if (instruction.getOpcode().format.equals(Format.Format10t) && !offsetToNewInstructionMap.containsKey(currentCodeOffset)) { + int targetOffset = ((Instruction10t)instruction).getCodeOffset(); + int codeOffsetDelta = codeOffsetShift(currentCodeOffset); + int newTargetOffset = targetOffset + targetOffsetShift(currentCodeOffset, targetOffset); + if ((byte)newTargetOffset != newTargetOffset) { + if ((short)newTargetOffset != newTargetOffset) { + // handling very small (negligible) possibility of goto becoming goto/32 + // we insert extra 1 code unit shift referring to the same position + // this will cause subsequent code offsets to be shifted by 2 code units + codeOffsetShifts.add(codeOffsetDelta, currentCodeOffset+instruction.getCodeUnits()); + offsetToNewInstructionMap.put(currentCodeOffset, Format.Format30t); + } else { + offsetToNewInstructionMap.put(currentCodeOffset, Format.Format20t); + } codeOffsetShifts.add(codeOffsetDelta, currentCodeOffset+instruction.getCodeUnits()); - } - codeOffsetShifts.add(codeOffsetDelta, currentCodeOffset+instruction.getCodeUnits()); + shiftsInserted = true; + } + } else if (instruction.getOpcode().format.equals(Format.Format20t) && !offsetToNewInstructionMap.containsKey(currentCodeOffset)) { + int targetOffset = ((Instruction20t)instruction).getCodeOffset(); + int codeOffsetDelta = codeOffsetShift(currentCodeOffset); + int newTargetOffset = targetOffsetShift(currentCodeOffset, targetOffset); + if ((short)newTargetOffset != newTargetOffset) { + codeOffsetShifts.add(codeOffsetDelta, currentCodeOffset+instruction.getCodeUnits()); + offsetToNewInstructionMap.put(currentCodeOffset, Format.Format30t); + shiftsInserted = true; + } + } else if (instruction.getOpcode().format.equals(Format.ArrayPayload) + || instruction.getOpcode().format.equals(Format.SparseSwitchPayload) + || instruction.getOpcode().format.equals(Format.PackedSwitchPayload)) { + int codeOffsetDelta = codeOffsetShift(currentCodeOffset); + if ((currentCodeOffset+codeOffsetDelta)%2 != 0) { + if (codeOffsetShifts.contains(currentCodeOffset)) { + codeOffsetShifts.remove(codeOffsetDelta); + offsetToNewInstructionMap.remove(currentCodeOffset); + } else { + codeOffsetShifts.add(codeOffsetDelta, currentCodeOffset); + offsetToNewInstructionMap.put(currentCodeOffset, Format.Format10x); + shiftsInserted = true; + } + } } - } else if (instruction.getOpcode().format.equals(Format.Format20t)) { - int targetOffset = ((Instruction20t)instruction).getCodeOffset(); - int codeOffsetDelta = codeOffsetShift(currentCodeOffset); - int newTargetOffset = targetOffsetShift(currentCodeOffset, targetOffset); - if ((short)newTargetOffset != newTargetOffset) { - codeOffsetShifts.add(codeOffsetDelta, currentCodeOffset+instruction.getCodeUnits()); - } - } else if (instruction.getOpcode().format.equals(Format.ArrayPayload) - || instruction.getOpcode().format.equals(Format.SparseSwitchPayload) - || instruction.getOpcode().format.equals(Format.PackedSwitchPayload)) { - int codeOffsetDelta = codeOffsetShift(currentCodeOffset); - if ((currentCodeOffset+codeOffsetDelta)%2 != 0) { - codeOffsetShifts.add(codeOffsetDelta, currentCodeOffset); - } - } - currentCodeOffset += instruction.getCodeUnits(); - } + currentCodeOffset += instruction.getCodeUnits(); + } + } while (shiftsInserted); codeUnitCount += codeOffsetShifts.size(); } @@ -212,16 +235,15 @@ public class InstructionWriteUtil { Instruction10t instr = (Instruction10t)instruction; int targetOffset = instr.getCodeOffset(); int newTargetOffset = targetOffset + targetOffsetShift(currentCodeOffset, targetOffset); - if (newTargetOffset != targetOffset) { - if ((byte)newTargetOffset != newTargetOffset) { - if ((short)newTargetOffset != newTargetOffset) { - modifiedInstruction = new ImmutableInstruction30t(Opcode.GOTO_32, newTargetOffset); - } else { - modifiedInstruction = new ImmutableInstruction20t(Opcode.GOTO_16, newTargetOffset); - } - } else { - modifiedInstruction = new ImmutableInstruction10t(instr.getOpcode(), newTargetOffset); + Format newInstructionFormat = offsetToNewInstructionMap.get(currentCodeOffset); + if (newInstructionFormat != null) { + if (newInstructionFormat.equals(Format.Format30t)) { + modifiedInstruction = new ImmutableInstruction30t(Opcode.GOTO_32, newTargetOffset); + } else if (newInstructionFormat.equals(Format.Format20t)) { + modifiedInstruction = new ImmutableInstruction20t(Opcode.GOTO_16, newTargetOffset); } + } else if (newTargetOffset != targetOffset) { + modifiedInstruction = new ImmutableInstruction10t(instr.getOpcode(), newTargetOffset); } break; } @@ -229,12 +251,11 @@ public class InstructionWriteUtil { Instruction20t instr = (Instruction20t)instruction; int targetOffset = instr.getCodeOffset(); int newTargetOffset = targetOffset + targetOffsetShift(currentCodeOffset, targetOffset); - if (newTargetOffset != targetOffset) { - if ((short)newTargetOffset != newTargetOffset) { - modifiedInstruction = new ImmutableInstruction30t(Opcode.GOTO_32, newTargetOffset); - } else { - modifiedInstruction = new ImmutableInstruction20t(Opcode.GOTO_16, newTargetOffset); - } + Format newInstructionFormat = offsetToNewInstructionMap.get(currentCodeOffset); + if (newInstructionFormat != null && newInstructionFormat.equals(Format.Format30t)) { + modifiedInstruction = new ImmutableInstruction30t(Opcode.GOTO_32, newTargetOffset); + } else if (newTargetOffset != targetOffset) { + modifiedInstruction = new ImmutableInstruction20t(Opcode.GOTO_16, newTargetOffset); } break; } @@ -321,7 +342,8 @@ public class InstructionWriteUtil { } private void alignPayload(int codeOffset) { - if (codeOffsetShifts.contains(codeOffset)) { + Format newInstructionFormat = offsetToNewInstructionMap.get(codeOffset); + if (newInstructionFormat != null && newInstructionFormat.equals(Format.Format10x)) { instructions.add(new ImmutableInstruction10x(Opcode.NOP)); } } From f598b96244c124cb8147867ab680bba18e3fa735 Mon Sep 17 00:00:00 2001 From: Izzat Bahadirov Date: Mon, 25 Mar 2013 11:39:22 -0400 Subject: [PATCH 03/13] Proper opcode for const-string instruction modification prediction. --- .../java/org/jf/dexlib2/writer/util/InstructionWriteUtil.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dexlib2/src/main/java/org/jf/dexlib2/writer/util/InstructionWriteUtil.java b/dexlib2/src/main/java/org/jf/dexlib2/writer/util/InstructionWriteUtil.java index a0b48be3..f729dd72 100644 --- a/dexlib2/src/main/java/org/jf/dexlib2/writer/util/InstructionWriteUtil.java +++ b/dexlib2/src/main/java/org/jf/dexlib2/writer/util/InstructionWriteUtil.java @@ -154,7 +154,7 @@ public class InstructionWriteUtil { offsetToNewInstructionMap = new HashMap(); } codeOffsetShifts.add(currentCodeOffset+instruction.getCodeUnits()); - offsetToNewInstructionMap.put(currentCodeOffset, instruction.getOpcode().format); + offsetToNewInstructionMap.put(currentCodeOffset, Opcode.CONST_STRING_JUMBO.format); } } currentCodeOffset += instruction.getCodeUnits(); From e05e01eee08cb78748a657e73f2444884f3a1663 Mon Sep 17 00:00:00 2001 From: Izzat Bahadirov Date: Mon, 25 Mar 2013 11:41:38 -0400 Subject: [PATCH 04/13] Simple unit tests for const-string to const-string/jumbo conversions, as well as target offset modifications for goto and branch target instructions. --- .../org/jf/dexlib2/writer/StringPool.java | 2 +- .../writer/JumboStringConversionTest.java | 222 ++++++++++++++++++ .../org/jf/dexlib2/writer/MockStringPool.java | 40 ++++ 3 files changed, 263 insertions(+), 1 deletion(-) create mode 100644 dexlib2/src/test/java/org/jf/dexlib2/writer/JumboStringConversionTest.java create mode 100644 dexlib2/src/test/java/org/jf/dexlib2/writer/MockStringPool.java diff --git a/dexlib2/src/main/java/org/jf/dexlib2/writer/StringPool.java b/dexlib2/src/main/java/org/jf/dexlib2/writer/StringPool.java index 5af1a276..7a31f2df 100644 --- a/dexlib2/src/main/java/org/jf/dexlib2/writer/StringPool.java +++ b/dexlib2/src/main/java/org/jf/dexlib2/writer/StringPool.java @@ -46,7 +46,7 @@ import java.util.Map; public class StringPool { public final static int STRING_ID_ITEM_SIZE = 0x04; - @Nonnull private final Map internedStringIdItems = Maps.newHashMap(); + @Nonnull protected final Map internedStringIdItems = Maps.newHashMap(); private int indexSectionOffset = -1; private int dataSectionOffset = -1; diff --git a/dexlib2/src/test/java/org/jf/dexlib2/writer/JumboStringConversionTest.java b/dexlib2/src/test/java/org/jf/dexlib2/writer/JumboStringConversionTest.java new file mode 100644 index 00000000..52c804fd --- /dev/null +++ b/dexlib2/src/test/java/org/jf/dexlib2/writer/JumboStringConversionTest.java @@ -0,0 +1,222 @@ +/* + * Copyright 2012, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package org.jf.dexlib2.writer; + +import com.google.common.collect.Lists; +import junit.framework.Assert; +import org.jf.dexlib2.Opcode; +import org.jf.dexlib2.iface.instruction.Instruction; +import org.jf.dexlib2.iface.instruction.formats.*; +import org.jf.dexlib2.immutable.ImmutableMethodImplementation; +import org.jf.dexlib2.immutable.instruction.*; +import org.jf.dexlib2.immutable.reference.ImmutableStringReference; +import org.jf.dexlib2.writer.util.InstructionWriteUtil; +import org.junit.Before; +import org.junit.Test; + +import java.util.ArrayList; + +public class JumboStringConversionTest { + private static final int MIN_NUM_JUMBO_STRINGS = 256; + + private MockStringPool mStringPool; + ArrayList mJumboStrings; + + @Before + public void setup() { + mStringPool = new MockStringPool(); + StringBuilder stringBuilder = new StringBuilder("a"); + mJumboStrings = Lists.newArrayList(); + int index = 0; + + // populate StringPool, make sure there are more than 64k+MIN_NUM_JUMBO_STRINGS strings + while (mJumboStrings.size()=0;pos--) { + for (char ch='a';ch<='z';ch++) { + stringBuilder.setCharAt(pos, ch); + mStringPool.intern(stringBuilder.toString(), index++); + if (mStringPool.getNumItems()>0xFFFF) { + mJumboStrings.add(stringBuilder.toString()); + } + } + } + + stringBuilder.setLength(stringBuilder.length()+1); + for (int pos=0;pos instructions = Lists.newArrayList(); + + ImmutableStringReference reference = new ImmutableStringReference(mJumboStrings.get(0)); + ImmutableInstruction21c instruction = new ImmutableInstruction21c(Opcode.CONST_STRING, 0, reference); + instructions.add(instruction); + + ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); + InstructionWriteUtil writeUtil = new InstructionWriteUtil(methodImplementation, mStringPool); + + for (Instruction instr: writeUtil.getInstructions()) { + Assert.assertEquals("Jumbo string conversion was not performed!", instr.getOpcode(), Opcode.CONST_STRING_JUMBO); + } + } + + private ArrayList createSimpleInstructionList() { + ArrayList instructions = Lists.newArrayList(); + + ImmutableStringReference reference = new ImmutableStringReference(mJumboStrings.get(0)); + ImmutableInstruction21c stringInstr = new ImmutableInstruction21c(Opcode.CONST_STRING, 0, reference); + instructions.add(stringInstr); + + ImmutableInstruction10x nopInstr = new ImmutableInstruction10x(Opcode.NOP); + instructions.add(nopInstr); + + return instructions; + } + + @Test + public void testInstruction10tSimple() { + ArrayList instructions = createSimpleInstructionList(); + + ImmutableInstruction10t gotoInstr = new ImmutableInstruction10t(Opcode.GOTO, 3); + instructions.add(0, gotoInstr); + + ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); + InstructionWriteUtil writeUtil = new InstructionWriteUtil(methodImplementation, mStringPool); + + Instruction10t instruction = (Instruction10t) writeUtil.getInstructions().iterator().next(); + Assert.assertEquals("goto (Format10t) target was not modified properly", instruction.getCodeOffset(), 4); + } + + @Test + public void testInstruction20tSimple() { + ArrayList instructions = createSimpleInstructionList(); + + ImmutableInstruction20t gotoInstr = new ImmutableInstruction20t(Opcode.GOTO_16, 4); + instructions.add(0, gotoInstr); + + ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); + InstructionWriteUtil writeUtil = new InstructionWriteUtil(methodImplementation, mStringPool); + + Instruction20t instruction = (Instruction20t) writeUtil.getInstructions().iterator().next(); + Assert.assertEquals("goto/16 (Format20t) target was not modified properly", instruction.getCodeOffset(), 5); + } + + @Test + public void testInstruction30t() { + ArrayList instructions = createSimpleInstructionList(); + + ImmutableInstruction30t gotoInstr = new ImmutableInstruction30t(Opcode.GOTO_32, 5); + instructions.add(0, gotoInstr); + + ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); + InstructionWriteUtil writeUtil = new InstructionWriteUtil(methodImplementation, mStringPool); + + Instruction30t instruction = (Instruction30t) writeUtil.getInstructions().iterator().next(); + Assert.assertEquals("goto/32 (Format30t) target was not modified properly", instruction.getCodeOffset(), 6); + } + + @Test + public void testInstruction21t() { + ArrayList instructions = createSimpleInstructionList(); + + ImmutableInstruction21t branchInstr = new ImmutableInstruction21t(Opcode.IF_EQZ, 0, 4); + instructions.add(0, branchInstr); + + // jam a jumbo string before the branch target instruction, to make sure that relative offset is modified + ImmutableStringReference reference = new ImmutableStringReference(mJumboStrings.get(1)); + ImmutableInstruction21c stringInstr = new ImmutableInstruction21c(Opcode.CONST_STRING, 0, reference); + instructions.add(0, stringInstr); + + ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); + InstructionWriteUtil writeUtil = new InstructionWriteUtil(methodImplementation, mStringPool); + + for (Instruction instr: writeUtil.getInstructions()) { + if (instr instanceof Instruction21t) { + Instruction21t instruction = (Instruction21t) instr; + Assert.assertEquals("branch instruction (Format21t) target was not modified properly", instruction.getCodeOffset(), 5); + break; + } + } + } + + @Test + public void testInstruction22t() { + ArrayList instructions = createSimpleInstructionList(); + + ImmutableInstruction22t branchInstr = new ImmutableInstruction22t(Opcode.IF_EQ, 0, 1, 4); + instructions.add(0, branchInstr); + + // jam a jumbo string before the branch target instruction, to make sure that relative offset is modified + ImmutableStringReference reference = new ImmutableStringReference(mJumboStrings.get(1)); + ImmutableInstruction21c stringInstr = new ImmutableInstruction21c(Opcode.CONST_STRING, 0, reference); + instructions.add(0, stringInstr); + + ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); + InstructionWriteUtil writeUtil = new InstructionWriteUtil(methodImplementation, mStringPool); + + for (Instruction instr: writeUtil.getInstructions()) { + if (instr instanceof Instruction22t) { + Instruction22t instruction = (Instruction22t) instr; + Assert.assertEquals("branch instruction (Format22t) target was not modified properly", instruction.getCodeOffset(), 5); + break; + } + } + } + + @Test + public void testInstruction31t() { + ArrayList instructions = createSimpleInstructionList(); + + ImmutableInstruction31t branchInstr = new ImmutableInstruction31t(Opcode.PACKED_SWITCH, 0, 5); + instructions.add(0, branchInstr); + + // jam a jumbo string before the branch target instruction, to make sure that relative offset is modified + ImmutableStringReference reference = new ImmutableStringReference(mJumboStrings.get(1)); + ImmutableInstruction21c stringInstr = new ImmutableInstruction21c(Opcode.CONST_STRING, 0, reference); + instructions.add(0, stringInstr); + + ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); + InstructionWriteUtil writeUtil = new InstructionWriteUtil(methodImplementation, mStringPool); + + for (Instruction instr: writeUtil.getInstructions()) { + if (instr instanceof Instruction31t) { + Instruction31t instruction = (Instruction31t) instr; + Assert.assertEquals("branch instruction (Format31t) target was not modified properly", instruction.getCodeOffset(), 6); + break; + } + } + } +} diff --git a/dexlib2/src/test/java/org/jf/dexlib2/writer/MockStringPool.java b/dexlib2/src/test/java/org/jf/dexlib2/writer/MockStringPool.java new file mode 100644 index 00000000..01d47789 --- /dev/null +++ b/dexlib2/src/test/java/org/jf/dexlib2/writer/MockStringPool.java @@ -0,0 +1,40 @@ +/* + * Copyright 2012, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package org.jf.dexlib2.writer; + +import javax.annotation.Nonnull; + +public class MockStringPool extends StringPool { + public void intern(@Nonnull CharSequence string, int index) { + internedStringIdItems.put(string.toString(), index); + } +} From 8309057de3791e442a6323cde83e0b8855a0e5db Mon Sep 17 00:00:00 2001 From: Izzat Bahadirov Date: Mon, 25 Mar 2013 12:28:43 -0400 Subject: [PATCH 05/13] Minor refactoring of simple unit tests, using the same test implementation for all branch target instructions. --- .../writer/JumboStringConversionTest.java | 58 ++++++++++--------- 1 file changed, 31 insertions(+), 27 deletions(-) diff --git a/dexlib2/src/test/java/org/jf/dexlib2/writer/JumboStringConversionTest.java b/dexlib2/src/test/java/org/jf/dexlib2/writer/JumboStringConversionTest.java index 52c804fd..5a5abe07 100644 --- a/dexlib2/src/test/java/org/jf/dexlib2/writer/JumboStringConversionTest.java +++ b/dexlib2/src/test/java/org/jf/dexlib2/writer/JumboStringConversionTest.java @@ -100,6 +100,10 @@ public class JumboStringConversionTest { ImmutableInstruction21c stringInstr = new ImmutableInstruction21c(Opcode.CONST_STRING, 0, reference); instructions.add(stringInstr); + reference = new ImmutableStringReference(mJumboStrings.get(1)); + stringInstr = new ImmutableInstruction21c(Opcode.CONST_STRING, 0, reference); + instructions.add(stringInstr); + ImmutableInstruction10x nopInstr = new ImmutableInstruction10x(Opcode.NOP); instructions.add(nopInstr); @@ -111,13 +115,18 @@ public class JumboStringConversionTest { ArrayList instructions = createSimpleInstructionList(); ImmutableInstruction10t gotoInstr = new ImmutableInstruction10t(Opcode.GOTO, 3); - instructions.add(0, gotoInstr); + instructions.add(1, gotoInstr); ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); InstructionWriteUtil writeUtil = new InstructionWriteUtil(methodImplementation, mStringPool); - Instruction10t instruction = (Instruction10t) writeUtil.getInstructions().iterator().next(); - Assert.assertEquals("goto (Format10t) target was not modified properly", instruction.getCodeOffset(), 4); + for (Instruction instr: writeUtil.getInstructions()) { + if (instr instanceof Instruction10t) { + Instruction10t instruction = (Instruction10t) instr; + Assert.assertEquals("goto (Format10t) target was not modified properly", instruction.getCodeOffset(), 4); + break; + } + } } @Test @@ -125,13 +134,18 @@ public class JumboStringConversionTest { ArrayList instructions = createSimpleInstructionList(); ImmutableInstruction20t gotoInstr = new ImmutableInstruction20t(Opcode.GOTO_16, 4); - instructions.add(0, gotoInstr); + instructions.add(1, gotoInstr); ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); InstructionWriteUtil writeUtil = new InstructionWriteUtil(methodImplementation, mStringPool); - Instruction20t instruction = (Instruction20t) writeUtil.getInstructions().iterator().next(); - Assert.assertEquals("goto/16 (Format20t) target was not modified properly", instruction.getCodeOffset(), 5); + for (Instruction instr: writeUtil.getInstructions()) { + if (instr instanceof Instruction20t) { + Instruction20t instruction = (Instruction20t) instr; + Assert.assertEquals("goto/16 (Format20t) target was not modified properly", instruction.getCodeOffset(), 5); + break; + } + } } @Test @@ -139,13 +153,18 @@ public class JumboStringConversionTest { ArrayList instructions = createSimpleInstructionList(); ImmutableInstruction30t gotoInstr = new ImmutableInstruction30t(Opcode.GOTO_32, 5); - instructions.add(0, gotoInstr); + instructions.add(1, gotoInstr); ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); InstructionWriteUtil writeUtil = new InstructionWriteUtil(methodImplementation, mStringPool); - Instruction30t instruction = (Instruction30t) writeUtil.getInstructions().iterator().next(); - Assert.assertEquals("goto/32 (Format30t) target was not modified properly", instruction.getCodeOffset(), 6); + for (Instruction instr: writeUtil.getInstructions()) { + if (instr instanceof Instruction30t) { + Instruction30t instruction = (Instruction30t) instr; + Assert.assertEquals("goto/32 (Format30t) target was not modified properly", instruction.getCodeOffset(), 6); + break; + } + } } @Test @@ -153,12 +172,7 @@ public class JumboStringConversionTest { ArrayList instructions = createSimpleInstructionList(); ImmutableInstruction21t branchInstr = new ImmutableInstruction21t(Opcode.IF_EQZ, 0, 4); - instructions.add(0, branchInstr); - - // jam a jumbo string before the branch target instruction, to make sure that relative offset is modified - ImmutableStringReference reference = new ImmutableStringReference(mJumboStrings.get(1)); - ImmutableInstruction21c stringInstr = new ImmutableInstruction21c(Opcode.CONST_STRING, 0, reference); - instructions.add(0, stringInstr); + instructions.add(1, branchInstr); ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); InstructionWriteUtil writeUtil = new InstructionWriteUtil(methodImplementation, mStringPool); @@ -177,12 +191,7 @@ public class JumboStringConversionTest { ArrayList instructions = createSimpleInstructionList(); ImmutableInstruction22t branchInstr = new ImmutableInstruction22t(Opcode.IF_EQ, 0, 1, 4); - instructions.add(0, branchInstr); - - // jam a jumbo string before the branch target instruction, to make sure that relative offset is modified - ImmutableStringReference reference = new ImmutableStringReference(mJumboStrings.get(1)); - ImmutableInstruction21c stringInstr = new ImmutableInstruction21c(Opcode.CONST_STRING, 0, reference); - instructions.add(0, stringInstr); + instructions.add(1, branchInstr); ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); InstructionWriteUtil writeUtil = new InstructionWriteUtil(methodImplementation, mStringPool); @@ -201,12 +210,7 @@ public class JumboStringConversionTest { ArrayList instructions = createSimpleInstructionList(); ImmutableInstruction31t branchInstr = new ImmutableInstruction31t(Opcode.PACKED_SWITCH, 0, 5); - instructions.add(0, branchInstr); - - // jam a jumbo string before the branch target instruction, to make sure that relative offset is modified - ImmutableStringReference reference = new ImmutableStringReference(mJumboStrings.get(1)); - ImmutableInstruction21c stringInstr = new ImmutableInstruction21c(Opcode.CONST_STRING, 0, reference); - instructions.add(0, stringInstr); + instructions.add(1, branchInstr); ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); InstructionWriteUtil writeUtil = new InstructionWriteUtil(methodImplementation, mStringPool); From e7ab4b681b456cb2b337ef8b31eb51746168f260 Mon Sep 17 00:00:00 2001 From: Izzat Bahadirov Date: Mon, 25 Mar 2013 13:14:14 -0400 Subject: [PATCH 06/13] Unit tests for offset modification in SparseSwitchPayload and PackedSwitchPayload. --- .../writer/JumboStringConversionTest.java | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/dexlib2/src/test/java/org/jf/dexlib2/writer/JumboStringConversionTest.java b/dexlib2/src/test/java/org/jf/dexlib2/writer/JumboStringConversionTest.java index 5a5abe07..35036fec 100644 --- a/dexlib2/src/test/java/org/jf/dexlib2/writer/JumboStringConversionTest.java +++ b/dexlib2/src/test/java/org/jf/dexlib2/writer/JumboStringConversionTest.java @@ -35,6 +35,7 @@ import com.google.common.collect.Lists; import junit.framework.Assert; import org.jf.dexlib2.Opcode; import org.jf.dexlib2.iface.instruction.Instruction; +import org.jf.dexlib2.iface.instruction.SwitchElement; import org.jf.dexlib2.iface.instruction.formats.*; import org.jf.dexlib2.immutable.ImmutableMethodImplementation; import org.jf.dexlib2.immutable.instruction.*; @@ -107,6 +108,16 @@ public class JumboStringConversionTest { ImmutableInstruction10x nopInstr = new ImmutableInstruction10x(Opcode.NOP); instructions.add(nopInstr); + ArrayList switchElements = Lists.newArrayList(); + ImmutableSwitchElement switchElement = new ImmutableSwitchElement(0, 5); + switchElements.add(switchElement); + + ImmutablePackedSwitchPayload packedSwitchInstr = new ImmutablePackedSwitchPayload(switchElements); + instructions.add(packedSwitchInstr); + + ImmutableSparseSwitchPayload sparseSwitchPayload = new ImmutableSparseSwitchPayload(switchElements); + instructions.add(sparseSwitchPayload); + return instructions; } @@ -223,4 +234,46 @@ public class JumboStringConversionTest { } } } + + @Test + public void testPackedSwitchPayload() { + ArrayList instructions = createSimpleInstructionList(); + + ImmutableInstruction31t branchInstr = new ImmutableInstruction31t(Opcode.PACKED_SWITCH, 0, 6); + instructions.add(1, branchInstr); + + ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); + InstructionWriteUtil writeUtil = new InstructionWriteUtil(methodImplementation, mStringPool); + + for (Instruction instr: writeUtil.getInstructions()) { + if (instr instanceof PackedSwitchPayload) { + PackedSwitchPayload instruction = (PackedSwitchPayload) instr; + for (SwitchElement switchElement: instruction.getSwitchElements()) { + Assert.assertEquals("packed switch payload affset was not modified properly", switchElement.getOffset(), 6); + } + break; + } + } + } + + @Test + public void testSparseSwitchPayload() { + ArrayList instructions = createSimpleInstructionList(); + + ImmutableInstruction31t branchInstr = new ImmutableInstruction31t(Opcode.SPARSE_SWITCH, 0, 12); + instructions.add(1, branchInstr); + + ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); + InstructionWriteUtil writeUtil = new InstructionWriteUtil(methodImplementation, mStringPool); + + for (Instruction instr: writeUtil.getInstructions()) { + if (instr instanceof SparseSwitchPayload) { + SparseSwitchPayload instruction = (SparseSwitchPayload) instr; + for (SwitchElement switchElement: instruction.getSwitchElements()) { + Assert.assertEquals("packed switch payload affset was not modified properly", switchElement.getOffset(), 6); + } + break; + } + } + } } From add494945d9879944a2eaa6db7bcf322bae9e27e Mon Sep 17 00:00:00 2001 From: Izzat Bahadirov Date: Mon, 25 Mar 2013 14:07:58 -0400 Subject: [PATCH 07/13] Unit tests for payload alignment. Also, typo correction. --- .../writer/JumboStringConversionTest.java | 65 ++++++++++++++++++- 1 file changed, 63 insertions(+), 2 deletions(-) diff --git a/dexlib2/src/test/java/org/jf/dexlib2/writer/JumboStringConversionTest.java b/dexlib2/src/test/java/org/jf/dexlib2/writer/JumboStringConversionTest.java index 35036fec..6244d96a 100644 --- a/dexlib2/src/test/java/org/jf/dexlib2/writer/JumboStringConversionTest.java +++ b/dexlib2/src/test/java/org/jf/dexlib2/writer/JumboStringConversionTest.java @@ -249,7 +249,7 @@ public class JumboStringConversionTest { if (instr instanceof PackedSwitchPayload) { PackedSwitchPayload instruction = (PackedSwitchPayload) instr; for (SwitchElement switchElement: instruction.getSwitchElements()) { - Assert.assertEquals("packed switch payload affset was not modified properly", switchElement.getOffset(), 6); + Assert.assertEquals("packed switch payload offset was not modified properly", switchElement.getOffset(), 6); } break; } @@ -270,10 +270,71 @@ public class JumboStringConversionTest { if (instr instanceof SparseSwitchPayload) { SparseSwitchPayload instruction = (SparseSwitchPayload) instr; for (SwitchElement switchElement: instruction.getSwitchElements()) { - Assert.assertEquals("packed switch payload affset was not modified properly", switchElement.getOffset(), 6); + Assert.assertEquals("packed switch payload offset was not modified properly", switchElement.getOffset(), 6); } break; } } } + + @Test + public void testArrayPayloadAlignment() { + ArrayList instructions = createSimpleInstructionList(); + + // add misaligned array payload + ImmutableInstruction10x nopInstr = new ImmutableInstruction10x(Opcode.NOP); + instructions.add(nopInstr); + + ImmutableArrayPayload arrayPayload = new ImmutableArrayPayload(4, null); + instructions.add(arrayPayload); + + ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); + InstructionWriteUtil writeUtil = new InstructionWriteUtil(methodImplementation, mStringPool); + + int codeOffset = 0; + for (Instruction instr: writeUtil.getInstructions()) { + if (codeOffset == 21) { + Assert.assertEquals("array payload was not aligned properly", instr.getOpcode(), Opcode.NOP); + } + codeOffset += instr.getCodeUnits(); + } + } + + @Test + public void testPackedSwitchAlignment() { + ArrayList instructions = createSimpleInstructionList(); + + // packed switch instruction is already misaligned + + ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); + InstructionWriteUtil writeUtil = new InstructionWriteUtil(methodImplementation, mStringPool); + + int codeOffset = 0; + for (Instruction instr: writeUtil.getInstructions()) { + if (codeOffset == 7) { + Assert.assertEquals("packed switch payload was not aligned properly", instr.getOpcode(), Opcode.NOP); + } + codeOffset += instr.getCodeUnits(); + } + } + + @Test + public void testSparseSwitchAlignment() { + ArrayList instructions = createSimpleInstructionList(); + + // insert a nop to mis-align sparse switch payload + ImmutableInstruction10x nopInstr = new ImmutableInstruction10x(Opcode.NOP); + instructions.add(4, nopInstr); + + ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); + InstructionWriteUtil writeUtil = new InstructionWriteUtil(methodImplementation, mStringPool); + + int codeOffset = 0; + for (Instruction instr: writeUtil.getInstructions()) { + if (codeOffset == 15) { + Assert.assertEquals("packed switch payload was not aligned properly", instr.getOpcode(), Opcode.NOP); + } + codeOffset += instr.getCodeUnits(); + } + } } From 6e524ece32784272da29c657c264ea38083c6cf1 Mon Sep 17 00:00:00 2001 From: Izzat Bahadirov Date: Mon, 25 Mar 2013 16:44:32 -0400 Subject: [PATCH 08/13] Fix for incorrect goto/16 to goto/32 conversion. --- .../java/org/jf/dexlib2/writer/util/InstructionWriteUtil.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dexlib2/src/main/java/org/jf/dexlib2/writer/util/InstructionWriteUtil.java b/dexlib2/src/main/java/org/jf/dexlib2/writer/util/InstructionWriteUtil.java index f729dd72..4f238a8a 100644 --- a/dexlib2/src/main/java/org/jf/dexlib2/writer/util/InstructionWriteUtil.java +++ b/dexlib2/src/main/java/org/jf/dexlib2/writer/util/InstructionWriteUtil.java @@ -193,7 +193,7 @@ public class InstructionWriteUtil { } else if (instruction.getOpcode().format.equals(Format.Format20t) && !offsetToNewInstructionMap.containsKey(currentCodeOffset)) { int targetOffset = ((Instruction20t)instruction).getCodeOffset(); int codeOffsetDelta = codeOffsetShift(currentCodeOffset); - int newTargetOffset = targetOffsetShift(currentCodeOffset, targetOffset); + int newTargetOffset = targetOffset + targetOffsetShift(currentCodeOffset, targetOffset); if ((short)newTargetOffset != newTargetOffset) { codeOffsetShifts.add(codeOffsetDelta, currentCodeOffset+instruction.getCodeUnits()); offsetToNewInstructionMap.put(currentCodeOffset, Format.Format30t); From c995fb5086a1a2989baf3680c1f4ba1c1f697dda Mon Sep 17 00:00:00 2001 From: Izzat Bahadirov Date: Mon, 25 Mar 2013 17:59:21 -0400 Subject: [PATCH 09/13] Unit tests for goto and goto/16 instructions embiggening. --- .../writer/JumboStringConversionTest.java | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/dexlib2/src/test/java/org/jf/dexlib2/writer/JumboStringConversionTest.java b/dexlib2/src/test/java/org/jf/dexlib2/writer/JumboStringConversionTest.java index 6244d96a..abef9af8 100644 --- a/dexlib2/src/test/java/org/jf/dexlib2/writer/JumboStringConversionTest.java +++ b/dexlib2/src/test/java/org/jf/dexlib2/writer/JumboStringConversionTest.java @@ -337,4 +337,51 @@ public class JumboStringConversionTest { codeOffset += instr.getCodeUnits(); } } + + @Test + public void testGotoToGoto16() { + ArrayList instructions = Lists.newArrayList(); + + ImmutableInstruction10t gotoInstr = new ImmutableInstruction10t(Opcode.GOTO, 127); + instructions.add(gotoInstr); + + ImmutableStringReference reference = new ImmutableStringReference(mJumboStrings.get(0)); + ImmutableInstruction21c stringInstr = new ImmutableInstruction21c(Opcode.CONST_STRING, 0, reference); + instructions.add(stringInstr); + + for (int i=0;i<127;i++) { + ImmutableInstruction10x nopInstr = new ImmutableInstruction10x(Opcode.NOP); + instructions.add(nopInstr); + } + + ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); + InstructionWriteUtil writeUtil = new InstructionWriteUtil(methodImplementation, mStringPool); + + Instruction instr = writeUtil.getInstructions().iterator().next(); + Assert.assertEquals("goto was not converted to goto/16 properly", instr.getOpcode(), Opcode.GOTO_16); + } + + @Test + public void testGoto16ToGoto32() { + ArrayList instructions = Lists.newArrayList(); + + ImmutableInstruction20t gotoInstr = new ImmutableInstruction20t(Opcode.GOTO_16, Short.MAX_VALUE); + instructions.add(gotoInstr); + + ImmutableStringReference reference = new ImmutableStringReference(mJumboStrings.get(0)); + ImmutableInstruction21c stringInstr = new ImmutableInstruction21c(Opcode.CONST_STRING, 0, reference); + instructions.add(stringInstr); + + for (int i=0;i Date: Mon, 25 Mar 2013 19:04:39 -0400 Subject: [PATCH 10/13] Fix for bug with incorrect removal of previously inserted alignment nop. --- .../java/org/jf/dexlib2/writer/util/InstructionWriteUtil.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dexlib2/src/main/java/org/jf/dexlib2/writer/util/InstructionWriteUtil.java b/dexlib2/src/main/java/org/jf/dexlib2/writer/util/InstructionWriteUtil.java index 4f238a8a..d44644db 100644 --- a/dexlib2/src/main/java/org/jf/dexlib2/writer/util/InstructionWriteUtil.java +++ b/dexlib2/src/main/java/org/jf/dexlib2/writer/util/InstructionWriteUtil.java @@ -205,7 +205,7 @@ public class InstructionWriteUtil { int codeOffsetDelta = codeOffsetShift(currentCodeOffset); if ((currentCodeOffset+codeOffsetDelta)%2 != 0) { if (codeOffsetShifts.contains(currentCodeOffset)) { - codeOffsetShifts.remove(codeOffsetDelta); + codeOffsetShifts.remove(codeOffsetDelta-1); offsetToNewInstructionMap.remove(currentCodeOffset); } else { codeOffsetShifts.add(codeOffsetDelta, currentCodeOffset); From 3d721348c55b6b7b68d48dafb9829adb5f1829d2 Mon Sep 17 00:00:00 2001 From: Izzat Bahadirov Date: Mon, 25 Mar 2013 19:06:07 -0400 Subject: [PATCH 11/13] Unit test for iterative goto embiggening and consequent payload re-alignment. --- .../writer/JumboStringConversionTest.java | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/dexlib2/src/test/java/org/jf/dexlib2/writer/JumboStringConversionTest.java b/dexlib2/src/test/java/org/jf/dexlib2/writer/JumboStringConversionTest.java index abef9af8..ce3b5f9d 100644 --- a/dexlib2/src/test/java/org/jf/dexlib2/writer/JumboStringConversionTest.java +++ b/dexlib2/src/test/java/org/jf/dexlib2/writer/JumboStringConversionTest.java @@ -384,4 +384,35 @@ public class JumboStringConversionTest { Assert.assertEquals("goto/16 was not converted to goto/32 properly", instr.getOpcode(), Opcode.GOTO_32); } + @Test + public void testGotoIterative() { + ArrayList instructions = Lists.newArrayList(); + + instructions.add(new ImmutableInstruction10t(Opcode.GOTO, 126)); + instructions.add(new ImmutableInstruction10t(Opcode.GOTO, 127)); + instructions.add(new ImmutableInstruction21c(Opcode.CONST_STRING, 0, new ImmutableStringReference(mJumboStrings.get(0)))); + for (int i=0;i<122;i++) { + instructions.add(new ImmutableInstruction10x(Opcode.NOP)); + } + instructions.add(new ImmutableInstruction21c(Opcode.CONST_STRING, 0, new ImmutableStringReference(mJumboStrings.get(1)))); + instructions.add(new ImmutableInstruction10x(Opcode.NOP)); + + // this misaligned array payload will cause nop insertion on the first pass and its removal on the second pass + instructions.add(new ImmutableInstruction10x(Opcode.NOP)); + instructions.add(new ImmutableArrayPayload(4, null)); + + ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); + InstructionWriteUtil writeUtil = new InstructionWriteUtil(methodImplementation, mStringPool); + + Instruction instr = writeUtil.getInstructions().iterator().next(); + Assert.assertEquals("goto was not converted to goto/16 properly", instr.getOpcode(), Opcode.GOTO_16); + + int codeOffset = 0; + for (Instruction instruction: writeUtil.getInstructions()) { + if (instruction instanceof ArrayPayload) { + Assert.assertEquals("packed switch payload was not aligned properly", codeOffset%2, 0); + } + codeOffset += instruction.getCodeUnits(); + } + } } From bad3ae4dd90966580ea7bfe689ea013f6ac7d635 Mon Sep 17 00:00:00 2001 From: Izzat Bahadirov Date: Mon, 25 Mar 2013 21:32:49 -0400 Subject: [PATCH 12/13] Refactoring unit tests. --- .../writer/JumboStringConversionTest.java | 103 +++++------------- 1 file changed, 25 insertions(+), 78 deletions(-) diff --git a/dexlib2/src/test/java/org/jf/dexlib2/writer/JumboStringConversionTest.java b/dexlib2/src/test/java/org/jf/dexlib2/writer/JumboStringConversionTest.java index ce3b5f9d..5e0cf8f7 100644 --- a/dexlib2/src/test/java/org/jf/dexlib2/writer/JumboStringConversionTest.java +++ b/dexlib2/src/test/java/org/jf/dexlib2/writer/JumboStringConversionTest.java @@ -47,7 +47,7 @@ import org.junit.Test; import java.util.ArrayList; public class JumboStringConversionTest { - private static final int MIN_NUM_JUMBO_STRINGS = 256; + private static final int MIN_NUM_JUMBO_STRINGS = 2; private MockStringPool mStringPool; ArrayList mJumboStrings; @@ -81,10 +81,7 @@ public class JumboStringConversionTest { @Test public void testInstruction21c() { ArrayList instructions = Lists.newArrayList(); - - ImmutableStringReference reference = new ImmutableStringReference(mJumboStrings.get(0)); - ImmutableInstruction21c instruction = new ImmutableInstruction21c(Opcode.CONST_STRING, 0, reference); - instructions.add(instruction); + instructions.add(new ImmutableInstruction21c(Opcode.CONST_STRING, 0, new ImmutableStringReference(mJumboStrings.get(0)))); ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); InstructionWriteUtil writeUtil = new InstructionWriteUtil(methodImplementation, mStringPool); @@ -96,27 +93,14 @@ public class JumboStringConversionTest { private ArrayList createSimpleInstructionList() { ArrayList instructions = Lists.newArrayList(); - - ImmutableStringReference reference = new ImmutableStringReference(mJumboStrings.get(0)); - ImmutableInstruction21c stringInstr = new ImmutableInstruction21c(Opcode.CONST_STRING, 0, reference); - instructions.add(stringInstr); - - reference = new ImmutableStringReference(mJumboStrings.get(1)); - stringInstr = new ImmutableInstruction21c(Opcode.CONST_STRING, 0, reference); - instructions.add(stringInstr); - - ImmutableInstruction10x nopInstr = new ImmutableInstruction10x(Opcode.NOP); - instructions.add(nopInstr); + instructions.add(new ImmutableInstruction21c(Opcode.CONST_STRING, 0, new ImmutableStringReference(mJumboStrings.get(0)))); + instructions.add(new ImmutableInstruction21c(Opcode.CONST_STRING, 0, new ImmutableStringReference(mJumboStrings.get(1)))); + instructions.add(new ImmutableInstruction10x(Opcode.NOP)); ArrayList switchElements = Lists.newArrayList(); - ImmutableSwitchElement switchElement = new ImmutableSwitchElement(0, 5); - switchElements.add(switchElement); - - ImmutablePackedSwitchPayload packedSwitchInstr = new ImmutablePackedSwitchPayload(switchElements); - instructions.add(packedSwitchInstr); - - ImmutableSparseSwitchPayload sparseSwitchPayload = new ImmutableSparseSwitchPayload(switchElements); - instructions.add(sparseSwitchPayload); + switchElements.add(new ImmutableSwitchElement(0, 5)); + instructions.add(new ImmutablePackedSwitchPayload(switchElements)); + instructions.add(new ImmutableSparseSwitchPayload(switchElements)); return instructions; } @@ -124,9 +108,7 @@ public class JumboStringConversionTest { @Test public void testInstruction10tSimple() { ArrayList instructions = createSimpleInstructionList(); - - ImmutableInstruction10t gotoInstr = new ImmutableInstruction10t(Opcode.GOTO, 3); - instructions.add(1, gotoInstr); + instructions.add(1, new ImmutableInstruction10t(Opcode.GOTO, 3)); ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); InstructionWriteUtil writeUtil = new InstructionWriteUtil(methodImplementation, mStringPool); @@ -143,9 +125,7 @@ public class JumboStringConversionTest { @Test public void testInstruction20tSimple() { ArrayList instructions = createSimpleInstructionList(); - - ImmutableInstruction20t gotoInstr = new ImmutableInstruction20t(Opcode.GOTO_16, 4); - instructions.add(1, gotoInstr); + instructions.add(1, new ImmutableInstruction20t(Opcode.GOTO_16, 4)); ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); InstructionWriteUtil writeUtil = new InstructionWriteUtil(methodImplementation, mStringPool); @@ -162,9 +142,7 @@ public class JumboStringConversionTest { @Test public void testInstruction30t() { ArrayList instructions = createSimpleInstructionList(); - - ImmutableInstruction30t gotoInstr = new ImmutableInstruction30t(Opcode.GOTO_32, 5); - instructions.add(1, gotoInstr); + instructions.add(1, new ImmutableInstruction30t(Opcode.GOTO_32, 5)); ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); InstructionWriteUtil writeUtil = new InstructionWriteUtil(methodImplementation, mStringPool); @@ -181,9 +159,7 @@ public class JumboStringConversionTest { @Test public void testInstruction21t() { ArrayList instructions = createSimpleInstructionList(); - - ImmutableInstruction21t branchInstr = new ImmutableInstruction21t(Opcode.IF_EQZ, 0, 4); - instructions.add(1, branchInstr); + instructions.add(1, new ImmutableInstruction21t(Opcode.IF_EQZ, 0, 4)); ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); InstructionWriteUtil writeUtil = new InstructionWriteUtil(methodImplementation, mStringPool); @@ -200,9 +176,7 @@ public class JumboStringConversionTest { @Test public void testInstruction22t() { ArrayList instructions = createSimpleInstructionList(); - - ImmutableInstruction22t branchInstr = new ImmutableInstruction22t(Opcode.IF_EQ, 0, 1, 4); - instructions.add(1, branchInstr); + instructions.add(1, new ImmutableInstruction22t(Opcode.IF_EQ, 0, 1, 4)); ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); InstructionWriteUtil writeUtil = new InstructionWriteUtil(methodImplementation, mStringPool); @@ -219,9 +193,7 @@ public class JumboStringConversionTest { @Test public void testInstruction31t() { ArrayList instructions = createSimpleInstructionList(); - - ImmutableInstruction31t branchInstr = new ImmutableInstruction31t(Opcode.PACKED_SWITCH, 0, 5); - instructions.add(1, branchInstr); + instructions.add(1, new ImmutableInstruction31t(Opcode.PACKED_SWITCH, 0, 5)); ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); InstructionWriteUtil writeUtil = new InstructionWriteUtil(methodImplementation, mStringPool); @@ -238,9 +210,7 @@ public class JumboStringConversionTest { @Test public void testPackedSwitchPayload() { ArrayList instructions = createSimpleInstructionList(); - - ImmutableInstruction31t branchInstr = new ImmutableInstruction31t(Opcode.PACKED_SWITCH, 0, 6); - instructions.add(1, branchInstr); + instructions.add(1, new ImmutableInstruction31t(Opcode.PACKED_SWITCH, 0, 6)); ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); InstructionWriteUtil writeUtil = new InstructionWriteUtil(methodImplementation, mStringPool); @@ -259,9 +229,7 @@ public class JumboStringConversionTest { @Test public void testSparseSwitchPayload() { ArrayList instructions = createSimpleInstructionList(); - - ImmutableInstruction31t branchInstr = new ImmutableInstruction31t(Opcode.SPARSE_SWITCH, 0, 12); - instructions.add(1, branchInstr); + instructions.add(1, new ImmutableInstruction31t(Opcode.SPARSE_SWITCH, 0, 12)); ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); InstructionWriteUtil writeUtil = new InstructionWriteUtil(methodImplementation, mStringPool); @@ -280,13 +248,9 @@ public class JumboStringConversionTest { @Test public void testArrayPayloadAlignment() { ArrayList instructions = createSimpleInstructionList(); - // add misaligned array payload - ImmutableInstruction10x nopInstr = new ImmutableInstruction10x(Opcode.NOP); - instructions.add(nopInstr); - - ImmutableArrayPayload arrayPayload = new ImmutableArrayPayload(4, null); - instructions.add(arrayPayload); + instructions.add(new ImmutableInstruction10x(Opcode.NOP)); + instructions.add(new ImmutableArrayPayload(4, null)); ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); InstructionWriteUtil writeUtil = new InstructionWriteUtil(methodImplementation, mStringPool); @@ -303,7 +267,6 @@ public class JumboStringConversionTest { @Test public void testPackedSwitchAlignment() { ArrayList instructions = createSimpleInstructionList(); - // packed switch instruction is already misaligned ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); @@ -321,10 +284,8 @@ public class JumboStringConversionTest { @Test public void testSparseSwitchAlignment() { ArrayList instructions = createSimpleInstructionList(); - // insert a nop to mis-align sparse switch payload - ImmutableInstruction10x nopInstr = new ImmutableInstruction10x(Opcode.NOP); - instructions.add(4, nopInstr); + instructions.add(4, new ImmutableInstruction10x(Opcode.NOP)); ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); InstructionWriteUtil writeUtil = new InstructionWriteUtil(methodImplementation, mStringPool); @@ -341,17 +302,10 @@ public class JumboStringConversionTest { @Test public void testGotoToGoto16() { ArrayList instructions = Lists.newArrayList(); - - ImmutableInstruction10t gotoInstr = new ImmutableInstruction10t(Opcode.GOTO, 127); - instructions.add(gotoInstr); - - ImmutableStringReference reference = new ImmutableStringReference(mJumboStrings.get(0)); - ImmutableInstruction21c stringInstr = new ImmutableInstruction21c(Opcode.CONST_STRING, 0, reference); - instructions.add(stringInstr); - + instructions.add(new ImmutableInstruction10t(Opcode.GOTO, 127)); + instructions.add(new ImmutableInstruction21c(Opcode.CONST_STRING, 0, new ImmutableStringReference(mJumboStrings.get(0)))); for (int i=0;i<127;i++) { - ImmutableInstruction10x nopInstr = new ImmutableInstruction10x(Opcode.NOP); - instructions.add(nopInstr); + instructions.add(new ImmutableInstruction10x(Opcode.NOP)); } ImmutableMethodImplementation methodImplementation = new ImmutableMethodImplementation(1, instructions, null, null); @@ -364,17 +318,10 @@ public class JumboStringConversionTest { @Test public void testGoto16ToGoto32() { ArrayList instructions = Lists.newArrayList(); - - ImmutableInstruction20t gotoInstr = new ImmutableInstruction20t(Opcode.GOTO_16, Short.MAX_VALUE); - instructions.add(gotoInstr); - - ImmutableStringReference reference = new ImmutableStringReference(mJumboStrings.get(0)); - ImmutableInstruction21c stringInstr = new ImmutableInstruction21c(Opcode.CONST_STRING, 0, reference); - instructions.add(stringInstr); - + instructions.add(new ImmutableInstruction20t(Opcode.GOTO_16, Short.MAX_VALUE)); + instructions.add(new ImmutableInstruction21c(Opcode.CONST_STRING, 0, new ImmutableStringReference(mJumboStrings.get(0)))); for (int i=0;i Date: Thu, 28 Mar 2013 18:23:37 -0400 Subject: [PATCH 13/13] Fixing comment alignment. --- .../jf/dexlib2/writer/util/InstructionWriteUtil.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/dexlib2/src/main/java/org/jf/dexlib2/writer/util/InstructionWriteUtil.java b/dexlib2/src/main/java/org/jf/dexlib2/writer/util/InstructionWriteUtil.java index d44644db..3323b4d3 100644 --- a/dexlib2/src/main/java/org/jf/dexlib2/writer/util/InstructionWriteUtil.java +++ b/dexlib2/src/main/java/org/jf/dexlib2/writer/util/InstructionWriteUtil.java @@ -133,12 +133,12 @@ public class InstructionWriteUtil { return shift; } - /** - * This method creates a list of code offsets of instructions, whose (and subsequent instructions') - * code offset will get shifted by one code unit with respect to previous instruction(s). - * This happens when the previous instruction has to be changed to a larger sized one - * to fit the new value or payload instruction has to be prepended by nop to ensure alignment. - */ + /* + * This method creates a list of code offsets of instructions, whose (and subsequent instructions') + * code offset will get shifted by one code unit with respect to previous instruction(s). + * This happens when the previous instruction has to be changed to a larger sized one + * to fit the new value or payload instruction has to be prepended by nop to ensure alignment. + */ private void findCodeOffsetShifts() { // first, process const-string to const-string/jumbo conversions int currentCodeOffset = 0;