Add support for alternate field ordering starting at oat version 67

8 files changed, 213 insertions, 60 deletions

diff --git a/baksmali/src/test/java/org/jf/baksmali/DexTest.java b/baksmali/src/test/java/org/jf/baksmali/DexTest.java
new file mode 100644
index 00000000..5a4db658
--- /dev/null
+++ b/baksmali/src/test/java/org/jf/baksmali/DexTest.java
@@ -0,0 +1,78 @@
+/*
+ * Copyright 2015, 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.baksmali;
+
+import org.jf.dexlib2.Opcodes;
+import org.jf.dexlib2.dexbacked.DexBackedDexFile;
+import org.junit.Assert;
+
+import javax.annotation.Nonnull;
+import java.io.File;
+import java.io.IOException;
+
+/**
+ * A base test class for performing a test using a dex file as input
+ */
+/**
+ * A base test class for performing a disassembly on a dex file and verifying the results
+ *
+ * The test accepts a single-class dex file as input. By default, the input dex file should be a resource at
+ * [testDir]/[testName]Input.dex
+ */
+public abstract class DexTest {
+    protected final String testDir;
+
+    protected DexTest(@Nonnull String testDir) {
+        this.testDir = testDir;
+    }
+
+    protected DexTest() {
+        this.testDir = this.getClass().getSimpleName();
+    }
+
+    @Nonnull
+    protected String getInputFilename(@Nonnull String testName) {
+        return String.format("%s%s%sInput.dex", testDir, File.separatorChar, testName);
+    }
+
+    @Nonnull
+    protected DexBackedDexFile getInputDexFile(@Nonnull String testName, @Nonnull baksmaliOptions options) {
+        try {
+            // Load file from resources as a stream
+            byte[] inputBytes = BaksmaliTestUtils.readResourceBytesFully(getInputFilename(testName));
+            return new DexBackedDexFile(Opcodes.forApi(options.apiLevel), inputBytes);
+        } catch (IOException ex) {
+            Assert.fail();
+        }
+        return null;
+    }
+}
diff --git a/baksmali/src/test/java/org/jf/baksmali/DisassemblyTest.java b/baksmali/src/test/java/org/jf/baksmali/DisassemblyTest.java
index 3d26a418..1a34e8c3 100644
--- a/baksmali/src/test/java/org/jf/baksmali/DisassemblyTest.java
+++ b/baksmali/src/test/java/org/jf/baksmali/DisassemblyTest.java
@@ -32,7 +32,6 @@
 package org.jf.baksmali;
 
 import com.google.common.collect.Iterables;
-import org.jf.dexlib2.Opcodes;
 import org.jf.dexlib2.dexbacked.DexBackedDexFile;
 import org.jf.dexlib2.iface.ClassDef;
 import org.junit.Assert;
@@ -50,21 +49,7 @@ import java.io.IOException;
  * By default, the input and output files should be resources at [testDir]/[testName]Input.dex
  * and [testDir]/[testName]Output.smali respectively
  */
-public class DisassemblyTest {
-    protected final String testDir;
-
-    protected DisassemblyTest(@Nonnull String testDir) {
-        this.testDir = testDir;
-    }
-
-    protected DisassemblyTest() {
-        this.testDir = this.getClass().getSimpleName();
-    }
-
-    @Nonnull
-    protected String getInputFilename(@Nonnull String testName) {
-        return String.format("%s%s%sInput.dex", testDir, File.separatorChar, testName);
-    }
+public class DisassemblyTest extends DexTest {
 
     @Nonnull
     protected String getOutputFilename(@Nonnull String testName) {
@@ -77,22 +62,13 @@ public class DisassemblyTest {
 
     protected void runTest(@Nonnull String testName, @Nonnull baksmaliOptions options) {
         try {
-            // Load file from resources as a stream
-            String inputFilename = getInputFilename(testName);
-            byte[] inputBytes = BaksmaliTestUtils.readResourceBytesFully(inputFilename);
-
-            DexBackedDexFile inputDex = new DexBackedDexFile(Opcodes.forApi(options.apiLevel), inputBytes);
+            DexBackedDexFile inputDex = getInputDexFile(testName, options);
             Assert.assertEquals(1, inputDex.getClassCount());
             ClassDef inputClass = Iterables.getFirst(inputDex.getClasses(), null);
             Assert.assertNotNull(inputClass);
             String input = BaksmaliTestUtils.getNormalizedSmali(inputClass, options, true);
 
-            String output;
-            if (getOutputFilename(testName).equals(inputFilename)) {
-                output = input;
-            } else {
-                output = BaksmaliTestUtils.readResourceFully(getOutputFilename(testName));
-            }
+            String output = BaksmaliTestUtils.readResourceFully(getOutputFilename(testName));
             output = BaksmaliTestUtils.normalizeSmali(output, true);
 
             // Run smali, baksmali, and then compare strings are equal (minus comments/whitespace)
diff --git a/baksmali/src/test/java/org/jf/baksmali/FieldGapOrderTest.java b/baksmali/src/test/java/org/jf/baksmali/FieldGapOrderTest.java
new file mode 100644
index 00000000..06841310
--- /dev/null
+++ b/baksmali/src/test/java/org/jf/baksmali/FieldGapOrderTest.java
@@ -0,0 +1,69 @@
+/*
+ * Copyright 2015, 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.baksmali;
+
+import com.google.common.collect.Lists;
+import org.jf.dexlib2.analysis.ClassPath;
+import org.jf.dexlib2.analysis.ClassProto;
+import org.jf.dexlib2.iface.DexFile;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class FieldGapOrderTest extends DexTest {
+    @Test
+    public void testOldOrder() {
+        DexFile dexFile = getInputDexFile("FieldGapOrder", new baksmaliOptions());
+        Assert.assertEquals(3, dexFile.getClasses().size());
+
+        ClassPath classPath = new ClassPath(Lists.newArrayList(dexFile), false, 66);
+        ClassProto classProto = (ClassProto)classPath.getClass("LGapOrder;");
+        Assert.assertEquals("r1", classProto.getFieldByOffset(12).getName());
+        Assert.assertEquals("r2", classProto.getFieldByOffset(16).getName());
+        Assert.assertEquals("d", classProto.getFieldByOffset(24).getName());
+        Assert.assertEquals("s", classProto.getFieldByOffset(36).getName());
+        Assert.assertEquals("i", classProto.getFieldByOffset(32).getName());
+    }
+
+    @Test
+    public void testNewOrder() {
+        DexFile dexFile = getInputDexFile("FieldGapOrder", new baksmaliOptions());
+        Assert.assertEquals(3, dexFile.getClasses().size());
+
+        ClassPath classPath = new ClassPath(Lists.newArrayList(dexFile), false, 67);
+        ClassProto classProto = (ClassProto)classPath.getClass("LGapOrder;");
+        Assert.assertEquals("s", classProto.getFieldByOffset(10).getName());
+        Assert.assertEquals("r1", classProto.getFieldByOffset(12).getName());
+        Assert.assertEquals("r2", classProto.getFieldByOffset(16).getName());
+        Assert.assertEquals("i", classProto.getFieldByOffset(20).getName());
+        Assert.assertEquals("d", classProto.getFieldByOffset(24).getName());
+    }
+}
diff --git a/baksmali/src/test/resources/FieldGapOrderTest/FieldGapOrderInput.dex b/baksmali/src/test/resources/FieldGapOrderTest/FieldGapOrderInput.dex
new file mode 100644
index 00000000..4e593516
--- /dev/null
+++ b/baksmali/src/test/resources/FieldGapOrderTest/FieldGapOrderInput.dex
diff --git a/dexlib2/OatVersions.txt b/dexlib2/OatVersions.txt
index e64eccf6..48df39ef 100644
--- a/dexlib2/OatVersions.txt
+++ b/dexlib2/OatVersions.txt
@@ -12,4 +12,7 @@ d7cbf8a6629942e7bd315ffae7e1c77b082f3e11 - 60
 07785bb98dc8bbe192970e0f4c2cafd338a8dc68 - 64
 fa2c054b28d4b540c1b3651401a7a091282a015f - 65
 7070ccd8b6439477eafeea7ed3736645d78e003f - 64 (revert of fa2c054b)
-7bf2b4f1d08050f80782217febac55c8cfc5e4ef - 65 (re-commit of fa2c054b)
\ No newline at end of file
+7bf2b4f1d08050f80782217febac55c8cfc5e4ef - 65 (re-commit of fa2c054b)
+0b71357fb52be9bb06d35396a3042b4381b01041 - 66
+fab6788358dfb64e5c370611ddbbbffab0ed0553 - 67
+- Change in FieldGap priority queue ordering
\ No newline at end of file
diff --git a/dexlib2/src/main/java/org/jf/dexlib2/analysis/ClassPath.java b/dexlib2/src/main/java/org/jf/dexlib2/analysis/ClassPath.java
index 4b8920f4..9d0bb2dc 100644
--- a/dexlib2/src/main/java/org/jf/dexlib2/analysis/ClassPath.java
+++ b/dexlib2/src/main/java/org/jf/dexlib2/analysis/ClassPath.java
@@ -60,8 +60,10 @@ import java.util.regex.Pattern;
 public class ClassPath {
     @Nonnull private final TypeProto unknownClass;
     @Nonnull private HashMap<String, ClassDef> availableClasses = Maps.newHashMap();
-    private boolean checkPackagePrivateAccess;
-    public boolean isArt;
+    private final boolean checkPackagePrivateAccess;
+    public final int oatVersion;
+
+    public static final int NOT_ART = -1;
 
     /**
      * Creates a new ClassPath instance that can load classes from the given dex files
@@ -78,7 +80,7 @@ public class ClassPath {
      * @param classPath An iterable of DexFile objects. When loading a class, these dex files will be searched in order
      * @param api API level
      */
-    public ClassPath(@Nonnull Iterable<DexFile> classPath, int api) {
+    public ClassPath(@Nonnull Iterable<? extends DexFile> classPath, int api) {
         this(Lists.newArrayList(classPath), api == 17);
     }
 
@@ -89,8 +91,8 @@ public class ClassPath {
      * @param checkPackagePrivateAccess Whether checkPackagePrivateAccess is needed, enabled for ONLY early API 17 by
      *                                  default
      */
-    public ClassPath(@Nonnull Iterable<DexFile> classPath, boolean checkPackagePrivateAccess) {
-        this(classPath, checkPackagePrivateAccess, false);
+    public ClassPath(@Nonnull Iterable<? extends DexFile> classPath, boolean checkPackagePrivateAccess) {
+        this(classPath, checkPackagePrivateAccess, NOT_ART);
     }
 
     /**
@@ -99,16 +101,17 @@ public class ClassPath {
      * @param classPath An iterable of DexFile objects. When loading a class, these dex files will be searched in order
      * @param checkPackagePrivateAccess Whether checkPackagePrivateAccess is needed, enabled for ONLY early API 17 by
      *                                  default
-     * @param isArt Whether this is ClassPath is for ART
+     * @param oatVersion The applicable oat version, or NOT_ART
      */
-    public ClassPath(@Nonnull Iterable < DexFile > classPath, boolean checkPackagePrivateAccess, boolean isArt) {
+    public ClassPath(@Nonnull Iterable<? extends DexFile> classPath, boolean checkPackagePrivateAccess,
+                     int oatVersion) {
         // add fallbacks for certain special classes that must be present
         Iterable<DexFile> dexFiles = Iterables.concat(classPath, Lists.newArrayList(getBasicClasses()));
 
         unknownClass = new UnknownClassProto(this);
         loadedClasses.put(unknownClass.getType(), unknownClass);
         this.checkPackagePrivateAccess = checkPackagePrivateAccess;
-        this.isArt = isArt;
+        this.oatVersion = oatVersion;
 
         loadPrimitiveType("Z");
         loadPrimitiveType("B");
@@ -145,6 +148,10 @@ public class ClassPath {
                 new ReflectionClassDef(Throwable.class)));
     }
 
+    public boolean isArt() {
+        return oatVersion != NOT_ART;
+    }
+
     @Nonnull
     public TypeProto getClass(@Nonnull CharSequence type) {
         return loadedClasses.getUnchecked(type.toString());
@@ -191,15 +198,15 @@ public class ClassPath {
                                           int api, boolean checkPackagePrivateAccess, boolean experimental) {
         ArrayList<DexFile> dexFiles = Lists.newArrayList();
 
-        boolean isArt = false;
+        int oatVersion = NOT_ART;
 
         for (String classPathEntry: classPath) {
             List<? extends DexFile> classPathDexFiles =
                     loadClassPathEntry(classPathDirs, classPathEntry, api, experimental);
-            if (!isArt) {
+            if (oatVersion == NOT_ART) {
                 for (DexFile classPathDexFile: classPathDexFiles) {
                     if (classPathDexFile instanceof OatDexFile) {
-                        isArt = true;
+                        oatVersion = ((OatDexFile)classPathDexFile).getOatVersion();
                         break;
                     }
                 }
@@ -207,20 +214,20 @@ public class ClassPath {
             dexFiles.addAll(classPathDexFiles);
         }
         dexFiles.add(dexFile);
-        return new ClassPath(dexFiles, checkPackagePrivateAccess, isArt);
+        return new ClassPath(dexFiles, checkPackagePrivateAccess, oatVersion);
     }
 
     @Nonnull
     public static ClassPath fromClassPath(Iterable<String> classPathDirs, Iterable<String> classPath, DexFile dexFile,
                                           int api, boolean checkPackagePrivateAccess, boolean experimental,
-                                          boolean isArt) {
+                                          int oatVersion) {
         ArrayList<DexFile> dexFiles = Lists.newArrayList();
 
         for (String classPathEntry: classPath) {
             dexFiles.addAll(loadClassPathEntry(classPathDirs, classPathEntry, api, experimental));
         }
         dexFiles.add(dexFile);
-        return new ClassPath(dexFiles, checkPackagePrivateAccess, isArt);
+        return new ClassPath(dexFiles, checkPackagePrivateAccess, oatVersion);
     }
 
     private static final Pattern dalvikCacheOdexPattern = Pattern.compile("@([^@]+)@classes.dex$");
@@ -290,7 +297,7 @@ public class ClassPath {
     private final Supplier<OdexedFieldInstructionMapper> fieldInstructionMapperSupplier = Suppliers.memoize(
             new Supplier<OdexedFieldInstructionMapper>() {
                 @Override public OdexedFieldInstructionMapper get() {
-                    return new OdexedFieldInstructionMapper(isArt);
+                    return new OdexedFieldInstructionMapper(isArt());
                 }
             });
 
diff --git a/dexlib2/src/main/java/org/jf/dexlib2/analysis/ClassProto.java b/dexlib2/src/main/java/org/jf/dexlib2/analysis/ClassProto.java
index d66e8ebd..57aae115 100644
--- a/dexlib2/src/main/java/org/jf/dexlib2/analysis/ClassProto.java
+++ b/dexlib2/src/main/java/org/jf/dexlib2/analysis/ClassProto.java
@@ -374,7 +374,7 @@ public class ClassProto implements TypeProto {
     }
 
     @Nonnull SparseArray<FieldReference> getInstanceFields() {
-        if (classPath.isArt) {
+        if (classPath.isArt()) {
             return artInstanceFieldsSupplier.get();
         } else {
             return dalvikInstanceFieldsSupplier.get();
@@ -548,21 +548,37 @@ public class ClassProto implements TypeProto {
                 }
             });
 
-    private static class FieldGap implements Comparable<FieldGap> {
+    private static abstract class FieldGap implements Comparable<FieldGap> {
         public final int offset;
         public final int size;
 
-        public FieldGap(int offset, int size) {
-            this.offset = offset;
-            this.size = size;
+        public static FieldGap newFieldGap(int offset, int size, int oatVersion) {
+            if (oatVersion >= 67) {
+                return new FieldGap(offset, size) {
+                    @Override public int compareTo(FieldGap o) {
+                        int result = Ints.compare(o.size, size);
+                        if (result != 0) {
+                            return result;
+                        }
+                        return Ints.compare(offset, o.offset);
+                    }
+                };
+            } else {
+                return new FieldGap(offset, size) {
+                    @Override public int compareTo(FieldGap o) {
+                        int result = Ints.compare(size, o.size);
+                        if (result != 0) {
+                            return result;
+                        }
+                        return Ints.compare(o.offset, offset);
+                    }
+                };
+            }
         }
 
-        @Override public int compareTo(@Nonnull FieldGap o) {
-            int result = Ints.compare(o.size, size);
-            if (result != 0) {
-                return result;
-            }
-            return Ints.compare(o.offset, offset);
+        private FieldGap(int offset, int size) {
+            this.offset = offset;
+            this.size = size;
         }
     }
 
@@ -630,13 +646,13 @@ public class ClassProto implements TypeProto {
                         int remaining = gapEnd - offset;
 
                         if ((remaining >= 4) && (offset % 4 == 0)) {
-                            gaps.add(new FieldGap(offset, 4));
+                            gaps.add(FieldGap.newFieldGap(offset, 4, classPath.oatVersion));
                             offset += 4;
                         } else if (remaining >= 2 && (offset % 2 == 0)) {
-                            gaps.add(new FieldGap(offset, 2));
+                            gaps.add(FieldGap.newFieldGap(offset, 2, classPath.oatVersion));
                             offset += 2;
                         } else {
-                            gaps.add(new FieldGap(offset, 1));
+                            gaps.add(FieldGap.newFieldGap(offset, 1, classPath.oatVersion));
                             offset += 1;
                         }
                     }
@@ -703,14 +719,14 @@ public class ClassProto implements TypeProto {
     private int getNextFieldOffset() {
         SparseArray<FieldReference> instanceFields = getInstanceFields();
         if (instanceFields.size() == 0) {
-            return classPath.isArt ? 0 : 8;
+            return classPath.isArt() ? 0 : 8;
         }
 
         int lastItemIndex = instanceFields.size()-1;
         int fieldOffset = instanceFields.keyAt(lastItemIndex);
         FieldReference lastField = instanceFields.valueAt(lastItemIndex);
 
-        if (classPath.isArt) {
+        if (classPath.isArt()) {
             return fieldOffset + getTypeSize(lastField.getType().charAt(0));
         } else {
             switch (lastField.getType().charAt(0)) {
diff --git a/dexlib2/src/main/java/org/jf/dexlib2/dexbacked/OatFile.java b/dexlib2/src/main/java/org/jf/dexlib2/dexbacked/OatFile.java
index b5250d0d..65d682f3 100644
--- a/dexlib2/src/main/java/org/jf/dexlib2/dexbacked/OatFile.java
+++ b/dexlib2/src/main/java/org/jf/dexlib2/dexbacked/OatFile.java
@@ -54,7 +54,7 @@ public class OatFile extends BaseDexBuffer {
     // These are the "known working" versions that I have manually inspected the source for.
     // Later version may or may not work, depending on what changed.
     private static final int MIN_OAT_VERSION = 56;
-    private static final int MAX_OAT_VERSION = 65;
+    private static final int MAX_OAT_VERSION = 67;
 
     public static final int UNSUPPORTED = 0;
     public static final int SUPPORTED = 1;
@@ -192,6 +192,10 @@ public class OatFile extends BaseDexBuffer {
             this.filename = filename;
         }
 
+        public int getOatVersion() {
+            return OatFile.this.getOatVersion();
+        }
+
         @Override public boolean hasOdexOpcodes() {
             return true;
         }