From 89ef73b927d64cadd8a724a6e26f290223ea1966 Mon Sep 17 00:00:00 2001 From: Alex Light Date: Wed, 23 Sep 2015 16:55:13 -0700 Subject: [PATCH] Make smali respect order of implements Previously smali would reorder implements directives to be alphabetical in the dex file. In the Java 8 Language the order of interfaces may affect the order of initializer invocation when using default methods. Therefore we will preserve this order in smali/baksmai. Bug: 24338722 Change-Id: I6851b02a5402c7d4cd4b60af54052c320e48d0bf --- .../jf/baksmali/Adaptors/ClassDefinition.java | 3 +- .../org/jf/baksmali/InterfaceOrderTest.java | 41 +++++++++++++++++++ .../InterfaceOrderTest/InterfaceOrder.smali | 37 +++++++++++++++++ .../reflection/ReflectionClassDef.java | 28 ++++++------- .../dexlib2/dexbacked/DexBackedClassDef.java | 13 +++--- .../java/org/jf/dexlib2/iface/ClassDef.java | 7 ++-- .../dexlib2/immutable/ImmutableClassDef.java | 13 +++--- .../jf/dexlib2/rewriter/ClassDefRewriter.java | 5 ++- .../org/jf/dexlib2/writer/ClassSection.java | 2 +- .../java/org/jf/dexlib2/writer/DexWriter.java | 4 +- .../writer/builder/BuilderClassDef.java | 13 ++---- .../writer/builder/BuilderClassPool.java | 2 +- .../jf/dexlib2/writer/builder/DexBuilder.java | 10 ++--- .../org/jf/dexlib2/writer/pool/ClassPool.java | 2 +- .../jf/dexlib2/writer/pool/PoolClassDef.java | 6 +-- 15 files changed, 129 insertions(+), 57 deletions(-) create mode 100644 brut.apktool.smali/baksmali/src/test/java/org/jf/baksmali/InterfaceOrderTest.java create mode 100644 brut.apktool.smali/baksmali/src/test/resources/InterfaceOrderTest/InterfaceOrder.smali diff --git a/brut.apktool.smali/baksmali/src/main/java/org/jf/baksmali/Adaptors/ClassDefinition.java b/brut.apktool.smali/baksmali/src/main/java/org/jf/baksmali/Adaptors/ClassDefinition.java index 9c171f49..2529af8a 100644 --- a/brut.apktool.smali/baksmali/src/main/java/org/jf/baksmali/Adaptors/ClassDefinition.java +++ b/brut.apktool.smali/baksmali/src/main/java/org/jf/baksmali/Adaptors/ClassDefinition.java @@ -146,8 +146,7 @@ public class ClassDefinition { } private void writeInterfaces(IndentingWriter writer) throws IOException { - List interfaces = Lists.newArrayList(classDef.getInterfaces()); - Collections.sort(interfaces); + List interfaces = classDef.getInterfaces(); if (interfaces.size() != 0) { writer.write('\n'); diff --git a/brut.apktool.smali/baksmali/src/test/java/org/jf/baksmali/InterfaceOrderTest.java b/brut.apktool.smali/baksmali/src/test/java/org/jf/baksmali/InterfaceOrderTest.java new file mode 100644 index 00000000..d85d7913 --- /dev/null +++ b/brut.apktool.smali/baksmali/src/test/java/org/jf/baksmali/InterfaceOrderTest.java @@ -0,0 +1,41 @@ +/* + * 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.junit.Test; + +public class InterfaceOrderTest extends IdenticalRoundtripTest { + @Test + public void testInterfaceOrder() { + runTest("InterfaceOrder", new baksmaliOptions()); + } +} diff --git a/brut.apktool.smali/baksmali/src/test/resources/InterfaceOrderTest/InterfaceOrder.smali b/brut.apktool.smali/baksmali/src/test/resources/InterfaceOrderTest/InterfaceOrder.smali new file mode 100644 index 00000000..b4745cb3 --- /dev/null +++ b/brut.apktool.smali/baksmali/src/test/resources/InterfaceOrderTest/InterfaceOrder.smali @@ -0,0 +1,37 @@ +.class public LInterfaceOrder; +.super Ljava/lang/Object; + +# Note how these two interfaces are not in alphabetical order +.implements Ljava/io/Serializable; +.implements Ljava/util/EventListener; +.implements Ljava/lang/Runnable; +.implements Ljava/io/Flushable; +.implements Ljava/lang/Clonable; +.implements Ljava/util/Observer; +.implements Ljava/io/Closeable; + +# direct methods +.method public constructor ()V + .registers 1 + return-void +.end method + +.method public close()V + .registers 1 + return-void +.end method + +.method public flush()V + .registers 1 + return-void +.end method + +.method public run()V + .registers 1 + return-void +.end method + +.method public update(Ljava/util/Observable;Ljava/lang/Object;)V + .registers 3 + return-void +.end method diff --git a/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/analysis/reflection/ReflectionClassDef.java b/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/analysis/reflection/ReflectionClassDef.java index fc1dc62f..7e8a2829 100644 --- a/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/analysis/reflection/ReflectionClassDef.java +++ b/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/analysis/reflection/ReflectionClassDef.java @@ -33,6 +33,7 @@ package org.jf.dexlib2.analysis.reflection; import com.google.common.base.Function; import com.google.common.base.Predicate; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterators; import org.jf.dexlib2.analysis.reflection.util.ReflectionUtils; @@ -48,6 +49,7 @@ import java.lang.reflect.Constructor; import java.lang.reflect.Modifier; import java.util.AbstractSet; import java.util.Iterator; +import java.util.List; import java.util.Set; /** @@ -78,23 +80,17 @@ public class ReflectionClassDef extends BaseTypeReference implements ClassDef { return ReflectionUtils.javaToDexName(superClass.getName()); } - @Nonnull @Override public Set getInterfaces() { - return new AbstractSet() { - @Nonnull @Override public Iterator iterator() { - return Iterators.transform(Iterators.forArray(cls.getInterfaces()), new Function() { - @Nullable @Override public String apply(@Nullable Class input) { - if (input == null) { - return null; - } - return ReflectionUtils.javaToDexName(input.getName()); - } - }); + @Nonnull @Override public List getInterfaces() { + return ImmutableList.copyOf(Iterators.transform(Iterators.forArray(cls.getInterfaces()), new Function() { + @Nullable + @Override + public String apply(@Nullable Class input) { + if (input == null) { + return null; + } + return ReflectionUtils.javaToDexName(input.getName()); } - - @Override public int size() { - return cls.getInterfaces().length; - } - }; + })); } @Nullable @Override public String getSourceFile() { diff --git a/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/dexbacked/DexBackedClassDef.java b/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/dexbacked/DexBackedClassDef.java index 9596a278..88f1dce9 100644 --- a/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/dexbacked/DexBackedClassDef.java +++ b/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/dexbacked/DexBackedClassDef.java @@ -31,6 +31,7 @@ package org.jf.dexlib2.dexbacked; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import org.jf.dexlib2.base.reference.BaseTypeReference; @@ -47,7 +48,9 @@ import org.jf.dexlib2.immutable.reference.ImmutableMethodReference; import javax.annotation.Nonnull; import javax.annotation.Nullable; +import java.util.AbstractList; import java.util.Iterator; +import java.util.List; import java.util.Set; public class DexBackedClassDef extends BaseTypeReference implements ClassDef { @@ -114,21 +117,21 @@ public class DexBackedClassDef extends BaseTypeReference implements ClassDef { @Nonnull @Override - public Set getInterfaces() { + public List getInterfaces() { final int interfacesOffset = dexFile.readSmallUint(classDefOffset + ClassDefItem.INTERFACES_OFFSET); if (interfacesOffset > 0) { final int size = dexFile.readSmallUint(interfacesOffset); - return new FixedSizeSet() { - @Nonnull + return new AbstractList() { @Override - public String readItem(int index) { + @Nonnull + public String get(int index) { return dexFile.getType(dexFile.readUshort(interfacesOffset + 4 + (2*index))); } @Override public int size() { return size; } }; } - return ImmutableSet.of(); + return ImmutableList.of(); } @Nonnull diff --git a/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/iface/ClassDef.java b/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/iface/ClassDef.java index 31d7fb99..78e17b15 100644 --- a/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/iface/ClassDef.java +++ b/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/iface/ClassDef.java @@ -35,6 +35,7 @@ import org.jf.dexlib2.iface.reference.TypeReference; import javax.annotation.Nonnull; import javax.annotation.Nullable; +import java.util.List; import java.util.Set; /** @@ -72,11 +73,11 @@ public interface ClassDef extends TypeReference, Annotatable { @Nullable String getSuperclass(); /** - * Gets a set of the interfaces that this class implements. + * Gets a list of the interfaces that this class implements. * - * @return A set of the interfaces that this class implements + * @return A list of the interfaces that this class implements */ - @Nonnull Set getInterfaces(); + @Nonnull List getInterfaces(); /** * Gets the name of the primary source file that this class is defined in, if available. diff --git a/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/immutable/ImmutableClassDef.java b/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/immutable/ImmutableClassDef.java index 42e14549..8bdfff26 100644 --- a/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/immutable/ImmutableClassDef.java +++ b/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/immutable/ImmutableClassDef.java @@ -47,12 +47,13 @@ import javax.annotation.Nullable; import java.util.AbstractCollection; import java.util.Collection; import java.util.Iterator; +import java.util.List; public class ImmutableClassDef extends BaseTypeReference implements ClassDef { @Nonnull protected final String type; protected final int accessFlags; @Nullable protected final String superclass; - @Nonnull protected final ImmutableSet interfaces; + @Nonnull protected final ImmutableList interfaces; @Nullable protected final String sourceFile; @Nonnull protected final ImmutableSet annotations; @Nonnull protected final ImmutableSortedSet staticFields; @@ -78,7 +79,7 @@ public class ImmutableClassDef extends BaseTypeReference implements ClassDef { this.type = type; this.accessFlags = accessFlags; this.superclass = superclass; - this.interfaces = interfaces==null ? ImmutableSet.of() : ImmutableSet.copyOf(interfaces); + this.interfaces = interfaces==null ? ImmutableList.of() : ImmutableList.copyOf(interfaces); this.sourceFile = sourceFile; this.annotations = ImmutableAnnotation.immutableSetOf(annotations); this.staticFields = ImmutableField.immutableSetOf(Iterables.filter(fields, FieldUtil.FIELD_IS_STATIC)); @@ -100,7 +101,7 @@ public class ImmutableClassDef extends BaseTypeReference implements ClassDef { this.type = type; this.accessFlags = accessFlags; this.superclass = superclass; - this.interfaces = interfaces==null ? ImmutableSet.of() : ImmutableSet.copyOf(interfaces); + this.interfaces = interfaces==null ? ImmutableList.of() : ImmutableList.copyOf(interfaces); this.sourceFile = sourceFile; this.annotations = ImmutableAnnotation.immutableSetOf(annotations); this.staticFields = ImmutableField.immutableSetOf(staticFields); @@ -112,7 +113,7 @@ public class ImmutableClassDef extends BaseTypeReference implements ClassDef { public ImmutableClassDef(@Nonnull String type, int accessFlags, @Nullable String superclass, - @Nullable ImmutableSet interfaces, + @Nullable ImmutableList interfaces, @Nullable String sourceFile, @Nullable ImmutableSet annotations, @Nullable ImmutableSortedSet staticFields, @@ -122,7 +123,7 @@ public class ImmutableClassDef extends BaseTypeReference implements ClassDef { this.type = type; this.accessFlags = accessFlags; this.superclass = superclass; - this.interfaces = ImmutableUtils.nullToEmptySet(interfaces); + this.interfaces = ImmutableUtils.nullToEmptyList(interfaces); this.sourceFile = sourceFile; this.annotations = ImmutableUtils.nullToEmptySet(annotations); this.staticFields = ImmutableUtils.nullToEmptySortedSet(staticFields); @@ -151,7 +152,7 @@ public class ImmutableClassDef extends BaseTypeReference implements ClassDef { @Nonnull @Override public String getType() { return type; } @Override public int getAccessFlags() { return accessFlags; } @Nullable @Override public String getSuperclass() { return superclass; } - @Nonnull @Override public ImmutableSet getInterfaces() { return interfaces; } + @Nonnull @Override public ImmutableList getInterfaces() { return interfaces; } @Nullable @Override public String getSourceFile() { return sourceFile; } @Nonnull @Override public ImmutableSet getAnnotations() { return annotations; } @Nonnull @Override public ImmutableSet getStaticFields() { return staticFields; } diff --git a/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/rewriter/ClassDefRewriter.java b/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/rewriter/ClassDefRewriter.java index ad246e5b..7e34ee33 100644 --- a/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/rewriter/ClassDefRewriter.java +++ b/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/rewriter/ClassDefRewriter.java @@ -41,6 +41,7 @@ import org.jf.dexlib2.iface.Method; import javax.annotation.Nonnull; import javax.annotation.Nullable; import java.util.Iterator; +import java.util.List; import java.util.Set; public class ClassDefRewriter implements Rewriter { @@ -73,8 +74,8 @@ public class ClassDefRewriter implements Rewriter { return RewriterUtils.rewriteNullable(rewriters.getTypeRewriter(), classDef.getSuperclass()); } - @Override @Nonnull public Set getInterfaces() { - return RewriterUtils.rewriteSet(rewriters.getTypeRewriter(), classDef.getInterfaces()); + @Override @Nonnull public List getInterfaces() { + return RewriterUtils.rewriteList(rewriters.getTypeRewriter(), classDef.getInterfaces()); } @Override @Nullable public String getSourceFile() { diff --git a/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/writer/ClassSection.java b/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/writer/ClassSection.java index bb2e4a72..d28dd444 100644 --- a/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/writer/ClassSection.java +++ b/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/writer/ClassSection.java @@ -53,7 +53,7 @@ public interface ClassSection getStaticInitializers(@Nonnull ClassKey key); diff --git a/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/writer/DexWriter.java b/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/writer/DexWriter.java index 916147ec..be23978e 100644 --- a/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/writer/DexWriter.java +++ b/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/writer/DexWriter.java @@ -433,7 +433,7 @@ public abstract class DexWriter< nextIndex = writeClass(indexWriter, offsetWriter, nextIndex, superEntry); // then, try to write interfaces - for (TypeKey interfaceTypeKey: typeListSection.getTypes(classSection.getSortedInterfaces(key))) { + for (TypeKey interfaceTypeKey: typeListSection.getTypes(classSection.getInterfaces(key))) { Map.Entry interfaceEntry = classSection.getClassEntryByType(interfaceTypeKey); nextIndex = writeClass(indexWriter, offsetWriter, nextIndex, interfaceEntry); } @@ -446,7 +446,7 @@ public abstract class DexWriter< indexWriter.writeInt(typeSection.getItemIndex(classSection.getType(key))); indexWriter.writeInt(classSection.getAccessFlags(key)); indexWriter.writeInt(typeSection.getNullableItemIndex(classSection.getSuperclass(key))); - indexWriter.writeInt(typeListSection.getNullableItemOffset(classSection.getSortedInterfaces(key))); + indexWriter.writeInt(typeListSection.getNullableItemOffset(classSection.getInterfaces(key))); indexWriter.writeInt(stringSection.getNullableItemIndex(classSection.getSourceFile(key))); indexWriter.writeInt(classSection.getAnnotationDirectoryOffset(key)); diff --git a/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/writer/builder/BuilderClassDef.java b/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/writer/builder/BuilderClassDef.java index f19a2e7f..10215925 100644 --- a/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/writer/builder/BuilderClassDef.java +++ b/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/writer/builder/BuilderClassDef.java @@ -31,6 +31,7 @@ package org.jf.dexlib2.writer.builder; +import com.google.common.base.Function; import com.google.common.base.Functions; import com.google.common.collect.*; import org.jf.dexlib2.base.reference.BaseTypeReference; @@ -101,16 +102,8 @@ public class BuilderClassDef extends BaseTypeReference implements ClassDef { @Nonnull @Override public SortedSet getVirtualMethods() { return virtualMethods; } @Nonnull @Override - public Set getInterfaces() { - return new AbstractSet() { - @Nonnull @Override public Iterator iterator() { - return Iterators.transform(interfaces.iterator(), Functions.toStringFunction()); - } - - @Override public int size() { - return interfaces.size(); - } - }; + public List getInterfaces() { + return Lists.transform(this.interfaces, Functions.toStringFunction()); } @Nonnull @Override public Collection getFields() { diff --git a/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/writer/builder/BuilderClassPool.java b/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/writer/builder/BuilderClassPool.java index 76fb9e03..695109fa 100644 --- a/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/writer/builder/BuilderClassPool.java +++ b/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/writer/builder/BuilderClassPool.java @@ -122,7 +122,7 @@ public class BuilderClassPool implements ClassSection interfaces_copy = Sets.newHashSet(interfaces); Iterator interfaceIterator = interfaces.iterator(); while (interfaceIterator.hasNext()) { String iface = interfaceIterator.next(); - if (prev != null && iface.equals(prev)) { + if (!interfaces_copy.contains(iface)) { interfaceIterator.remove(); + } else { + interfaces_copy.remove(iface); } - prev = iface; } } diff --git a/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/writer/pool/ClassPool.java b/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/writer/pool/ClassPool.java index f2440cdb..25cf4da8 100644 --- a/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/writer/pool/ClassPool.java +++ b/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/writer/pool/ClassPool.java @@ -250,7 +250,7 @@ public class ClassPool implements ClassSection> getSortedInterfaces(@Nonnull PoolClassDef classDef) { + @Nullable @Override public TypeListPool.Key> getInterfaces(@Nonnull PoolClassDef classDef) { return classDef.interfaces; } diff --git a/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/writer/pool/PoolClassDef.java b/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/writer/pool/PoolClassDef.java index fe897d26..00958fb8 100644 --- a/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/writer/pool/PoolClassDef.java +++ b/brut.apktool.smali/dexlib2/src/main/java/org/jf/dexlib2/writer/pool/PoolClassDef.java @@ -43,7 +43,7 @@ import java.util.*; class PoolClassDef extends BaseTypeReference implements ClassDef { @Nonnull final ClassDef classDef; - @Nonnull final TypeListPool.Key> interfaces; + @Nonnull final TypeListPool.Key> interfaces; @Nonnull final ImmutableSortedSet staticFields; @Nonnull final ImmutableSortedSet instanceFields; @Nonnull final ImmutableSortedSet directMethods; @@ -56,7 +56,7 @@ class PoolClassDef extends BaseTypeReference implements ClassDef { PoolClassDef(@Nonnull ClassDef classDef) { this.classDef = classDef; - interfaces = new TypeListPool.Key>(ImmutableSortedSet.copyOf(classDef.getInterfaces())); + interfaces = new TypeListPool.Key>(ImmutableList.copyOf(classDef.getInterfaces())); staticFields = ImmutableSortedSet.copyOf(classDef.getStaticFields()); instanceFields = ImmutableSortedSet.copyOf(classDef.getInstanceFields()); directMethods = ImmutableSortedSet.copyOf( @@ -77,7 +77,7 @@ class PoolClassDef extends BaseTypeReference implements ClassDef { return classDef.getSuperclass(); } - @Nonnull @Override public SortedSet getInterfaces() { + @Nonnull @Override public List getInterfaces() { return interfaces.types; }