Separate direct/virtual methods and static/instance fields in the ClassDef interface

This is unfortunately required to support not-quite-well-formed dex files
containing duplicate static/instance fields, or duplicate direct/virtual methods,
which dalvik inadvertently allows.

In cases when there are duplicate fields/methods in the same category, we
unambiguously remove/hide the latter duplicate fields/methods.

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

@@ -67,7 +67,7 @@ public class ClassDefinition {
     private HashSet<String> findFieldsSetInStaticConstructor() {
         HashSet<String> fieldsSetInStaticConstructor = new HashSet<String>();
 
-        for (Method method: classDef.getMethods()) {
+        for (Method method: classDef.getDirectMethods()) {
             if (method.getName().equals("<clinit>")) {
                 MethodImplementation impl = method.getImplementation();
                 if (impl != null) {
@@ -165,93 +165,77 @@ public class ClassDefinition {
 
     private void writeStaticFields(IndentingWriter writer) throws IOException {
         boolean wroteHeader = false;
-        for (Field field: classDef.getFields()) {
+        for (Field field: classDef.getStaticFields()) {
-            if (AccessFlags.STATIC.isSet(field.getAccessFlags())) {
+            if (!wroteHeader) {
-                if (!wroteHeader) {
+                writer.write("\n\n");
-                    writer.write("\n\n");
+                writer.write("# static fields");
-                    writer.write("# static fields");
+                wroteHeader = true;
-                    wroteHeader = true;
-                }
-                writer.write('\n');
-                // TODO: detect duplicate fields.
-
-                boolean setInStaticConstructor =
-                        fieldsSetInStaticConstructor.contains(ReferenceUtil.getShortFieldDescriptor(field));
-
-                FieldDefinition.writeTo(writer, field, setInStaticConstructor);
             }
+            writer.write('\n');
+            // TODO: detect duplicate fields.
+
+            boolean setInStaticConstructor =
+                    fieldsSetInStaticConstructor.contains(ReferenceUtil.getShortFieldDescriptor(field));
+
+            FieldDefinition.writeTo(writer, field, setInStaticConstructor);
         }
     }
 
     private void writeInstanceFields(IndentingWriter writer) throws IOException {
         boolean wroteHeader = false;
-        for (Field field: classDef.getFields()) {
+        for (Field field: classDef.getInstanceFields()) {
-            if (!AccessFlags.STATIC.isSet(field.getAccessFlags())) {
+            if (!wroteHeader) {
-                if (!wroteHeader) {
+                writer.write("\n\n");
-                    writer.write("\n\n");
+                writer.write("# instance fields");
-                    writer.write("# instance fields");
+                wroteHeader = true;
-                    wroteHeader = true;
-                }
-                writer.write('\n');
-                // TODO: detect duplicate fields.
-
-                FieldDefinition.writeTo(writer, field, false);
             }
+            writer.write('\n');
+            // TODO: detect duplicate fields.
+
+            FieldDefinition.writeTo(writer, field, false);
         }
     }
 
     private void writeDirectMethods(IndentingWriter writer) throws IOException {
         boolean wroteHeader = false;
-        for (Method method: classDef.getMethods()) {
+        for (Method method: classDef.getDirectMethods()) {
-            int accessFlags = method.getAccessFlags();
+            if (!wroteHeader) {
+                writer.write("\n\n");
+                writer.write("# direct methods");
+                wroteHeader = true;
+            }
+            writer.write('\n');
+            // TODO: detect duplicate methods.
+            // TODO: check for method validation errors
 
-            if (AccessFlags.STATIC.isSet(accessFlags) ||
+            MethodImplementation methodImpl = method.getImplementation();
-                    AccessFlags.PRIVATE.isSet(accessFlags) ||
+            if (methodImpl == null) {
-                    AccessFlags.CONSTRUCTOR.isSet(accessFlags)) {
+                MethodDefinition.writeEmptyMethodTo(writer, method);
-                if (!wroteHeader) {
+            } else {
-                    writer.write("\n\n");
+                MethodDefinition methodDefinition = new MethodDefinition(this, method, methodImpl);
-                    writer.write("# direct methods");
+                methodDefinition.writeTo(writer);
-                    wroteHeader = true;
-                }
-                writer.write('\n');
-                // TODO: detect duplicate methods.
-                // TODO: check for method validation errors
-
-                MethodImplementation methodImpl = method.getImplementation();
-                if (methodImpl == null) {
-                    MethodDefinition.writeEmptyMethodTo(writer, method);
-                } else {
-                    MethodDefinition methodDefinition = new MethodDefinition(this, method, methodImpl);
-                    methodDefinition.writeTo(writer);
-                }
             }
         }
     }
 
     private void writeVirtualMethods(IndentingWriter writer) throws IOException {
         boolean wroteHeader = false;
-        for (Method method: classDef.getMethods()) {
+        for (Method method: classDef.getVirtualMethods()) {
-            int accessFlags = method.getAccessFlags();
+            if (!wroteHeader) {
+                writer.write("\n\n");
+                writer.write("# virtual methods");
+                wroteHeader = true;
+            }
+            writer.write('\n');
+            // TODO: detect duplicate methods.
+            // TODO: check for method validation errors
 
-            if (!AccessFlags.STATIC.isSet(accessFlags) &&
+            MethodImplementation methodImpl = method.getImplementation();
-                    !AccessFlags.PRIVATE.isSet(accessFlags) &&
+            if (methodImpl == null) {
-                    !AccessFlags.CONSTRUCTOR.isSet(accessFlags)) {
+                MethodDefinition.writeEmptyMethodTo(writer, method);
-                if (!wroteHeader) {
+            } else {
-                    writer.write("\n\n");
+                MethodDefinition methodDefinition = new MethodDefinition(this, method, methodImpl);
-                    writer.write("# virtual methods");
+                methodDefinition.writeTo(writer);
-                    wroteHeader = true;
-                }
-                writer.write('\n');
-                // TODO: detect duplicate methods.
-                // TODO: check for method validation errors
-
-                MethodImplementation methodImpl = method.getImplementation();
-                if (methodImpl == null) {
-                    MethodDefinition.writeEmptyMethodTo(writer, method);
-                } else {
-                    MethodDefinition methodDefinition = new MethodDefinition(this, method, methodImpl);
-                    methodDefinition.writeTo(writer);
-                }
             }
         }
     }

baksmali/src/test/resources/DuplicateTest/DuplicateDirectMethods.smali

@@ -0,0 +1,22 @@
+.class public LDuplicateStaticFields;
+.super Ljava/lang/Object;
+
+.method private alah()V
+    .registers 1
+    return-void
+.end method
+
+.method private blah()V
+    .registers 1
+    return-void
+.end method
+
+.method private blah()V
+    .registers 1
+    return-void
+.end method
+
+.method private clah()V
+    .registers 1
+    return-void
+.end method

baksmali/src/test/resources/DuplicateTest/DuplicateDirectVirtualMethods.smali

@@ -0,0 +1,32 @@
+.class public LDuplicateStaticFields;
+.super Ljava/lang/Object;
+
+.method public alah()V
+    .registers 1
+    return-void
+.end method
+
+.method private blah()V
+    .registers 1
+    return-void
+.end method
+
+.method private blah()V
+    .registers 1
+    return-void
+.end method
+
+.method public blah()V
+    .registers 1
+    return-void
+.end method
+
+.method public blah()V
+    .registers 1
+    return-void
+.end method
+
+.method public clah()V
+    .registers 1
+    return-void
+.end method

baksmali/src/test/resources/DuplicateTest/DuplicateInstanceFields.smali

@@ -0,0 +1,9 @@
+.class public LDuplicateInstanceFields;
+.super Ljava/lang/Object;
+
+.field public alah:I
+
+.field public blah:I
+.field public blah:I
+
+.field public clah:I

baksmali/src/test/resources/DuplicateTest/DuplicateStaticFields.smali

@@ -0,0 +1,9 @@
+.class public LDuplicateStaticFields;
+.super Ljava/lang/Object;
+
+.field public static alah:I
+
+.field public static blah:I
+.field public static blah:I
+
+.field public static clah:I

baksmali/src/test/resources/DuplicateTest/DuplicateVirtualMethods.smali

@@ -0,0 +1,22 @@
+.class public LDuplicateStaticFields;
+.super Ljava/lang/Object;
+
+.method public alah()V
+    .registers 1
+    return-void
+.end method
+
+.method public blah()V
+    .registers 1
+    return-void
+.end method
+
+.method public blah()V
+    .registers 1
+    return-void
+.end method
+
+.method public clah()V
+    .registers 1
+    return-void
+.end method

dexlib2/src/main/java/org/jf/dexlib2/analysis/reflection/ReflectionClassDef.java

@@ -32,6 +32,7 @@
 package org.jf.dexlib2.analysis.reflection;
 
 import com.google.common.base.Function;
+import com.google.common.base.Predicate;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterators;
 import org.jf.dexlib2.analysis.reflection.util.ReflectionUtils;
@@ -104,6 +105,50 @@ public class ReflectionClassDef extends BaseTypeReference implements ClassDef {
         return ImmutableSet.of();
     }
 
+    @Nonnull @Override public Iterable<? extends Field> getStaticFields() {
+        return new Iterable<Field>() {
+            @Nonnull @Override public Iterator<Field> iterator() {
+                Iterator<java.lang.reflect.Field> staticFields = Iterators.filter(
+                        Iterators.forArray(cls.getDeclaredFields()),
+                        new Predicate<java.lang.reflect.Field>() {
+                            @Override public boolean apply(@Nullable java.lang.reflect.Field input) {
+                                return input!=null && Modifier.isStatic(input.getModifiers());
+                            }
+                        });
+
+                return Iterators.transform(staticFields,
+                        new Function<java.lang.reflect.Field, Field>() {
+                            @Nullable @Override public Field apply(@Nullable java.lang.reflect.Field input) {
+                                return new ReflectionField(input);
+                            }
+                        }
+                );
+            }
+        };
+    }
+
+    @Nonnull @Override public Iterable<? extends Field> getInstanceFields() {
+        return new Iterable<Field>() {
+            @Nonnull @Override public Iterator<Field> iterator() {
+                Iterator<java.lang.reflect.Field> staticFields = Iterators.filter(
+                        Iterators.forArray(cls.getDeclaredFields()),
+                        new Predicate<java.lang.reflect.Field>() {
+                            @Override public boolean apply(@Nullable java.lang.reflect.Field input) {
+                                return input!=null && !Modifier.isStatic(input.getModifiers());
+                            }
+                        });
+
+                return Iterators.transform(staticFields,
+                        new Function<java.lang.reflect.Field, Field>() {
+                            @Nullable @Override public Field apply(@Nullable java.lang.reflect.Field input) {
+                                return new ReflectionField(input);
+                            }
+                        }
+                );
+            }
+        };
+    }
+
     @Nonnull @Override public Set<? extends Field> getFields() {
         return new AbstractSet<Field>() {
             @Nonnull @Override public Iterator<Field> iterator() {
@@ -121,6 +166,58 @@ public class ReflectionClassDef extends BaseTypeReference implements ClassDef {
         };
     }
 
+    private static final int DIRECT_MODIFIERS = Modifier.PRIVATE | Modifier.STATIC;
+    @Nonnull @Override public Iterable<? extends Method> getDirectMethods() {
+        return new Iterable<Method>() {
+            @Nonnull @Override public Iterator<Method> iterator() {
+                Iterator<Method> constructorIterator =
+                        Iterators.transform(Iterators.forArray(cls.getDeclaredConstructors()),
+                                new Function<Constructor, Method>() {
+                                    @Nullable @Override public Method apply(@Nullable Constructor input) {
+                                        return new ReflectionConstructor(input);
+                                    }
+                                });
+
+                Iterator<java.lang.reflect.Method> directMethods = Iterators.filter(
+                        Iterators.forArray(cls.getDeclaredMethods()),
+                        new Predicate<java.lang.reflect.Method>() {
+                            @Override public boolean apply(@Nullable java.lang.reflect.Method input) {
+                                return input != null && (input.getModifiers() & DIRECT_MODIFIERS) != 0;
+                            }
+                        });
+
+                Iterator<Method> methodIterator = Iterators.transform(directMethods,
+                        new Function<java.lang.reflect.Method, Method>() {
+                            @Nullable @Override public Method apply(@Nullable java.lang.reflect.Method input) {
+                                return new ReflectionMethod(input);
+                            }
+                        });
+                return Iterators.concat(constructorIterator, methodIterator);
+            }
+        };
+    }
+
+    @Nonnull @Override public Iterable<? extends Method> getVirtualMethods() {
+        return new Iterable<Method>() {
+            @Nonnull @Override public Iterator<Method> iterator() {
+                Iterator<java.lang.reflect.Method> directMethods = Iterators.filter(
+                        Iterators.forArray(cls.getDeclaredMethods()),
+                        new Predicate<java.lang.reflect.Method>() {
+                            @Override public boolean apply(@Nullable java.lang.reflect.Method input) {
+                                return input != null && (input.getModifiers() & DIRECT_MODIFIERS) == 0;
+                            }
+                        });
+
+                return Iterators.transform(directMethods,
+                        new Function<java.lang.reflect.Method, Method>() {
+                            @Nullable @Override public Method apply(@Nullable java.lang.reflect.Method input) {
+                                return new ReflectionMethod(input);
+                            }
+                        });
+            }
+        };
+    }
+
     @Nonnull @Override public Set<? extends Method> getMethods() {
         return new AbstractSet<Method>() {
             @Nonnull @Override public Iterator<Method> iterator() {

dexlib2/src/main/java/org/jf/dexlib2/dexbacked/DexBackedClassDef.java

@@ -32,17 +32,21 @@
 package org.jf.dexlib2.dexbacked;
 
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
 import org.jf.dexlib2.base.reference.BaseTypeReference;
 import org.jf.dexlib2.dexbacked.raw.ClassDefItem;
 import org.jf.dexlib2.dexbacked.util.AnnotationsDirectory;
 import org.jf.dexlib2.dexbacked.util.FixedSizeSet;
 import org.jf.dexlib2.dexbacked.util.StaticInitialValueIterator;
-import org.jf.dexlib2.dexbacked.util.VariableSizeIterator;
+import org.jf.dexlib2.dexbacked.util.VariableSizeLookaheadIterator;
 import org.jf.dexlib2.iface.ClassDef;
+import org.jf.dexlib2.iface.reference.FieldReference;
+import org.jf.dexlib2.iface.reference.MethodReference;
+import org.jf.dexlib2.immutable.reference.ImmutableFieldReference;
+import org.jf.dexlib2.immutable.reference.ImmutableMethodReference;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
-import java.util.AbstractSet;
 import java.util.Iterator;
 import java.util.Set;
 
@@ -50,7 +54,16 @@ public class DexBackedClassDef extends BaseTypeReference implements ClassDef {
     @Nonnull public final DexBackedDexFile dexFile;
     private final int classDefOffset;
 
-    private int classDataOffset = -1;
+    private final int classDataOffset;
+    private final int staticFieldsOffset;
+    private int instanceFieldsOffset = 0;
+    private int directMethodsOffset = 0;
+    private int virtualMethodsOffset = 0;
+
+    private final int staticFieldCount;
+    private final int instanceFieldCount;
+    private final int directMethodCount;
+    private final int virtualMethodCount;
 
     @Nullable private AnnotationsDirectory annotationsDirectory;
 
@@ -58,6 +71,23 @@ public class DexBackedClassDef extends BaseTypeReference implements ClassDef {
                              int classDefOffset) {
         this.dexFile = dexFile;
         this.classDefOffset = classDefOffset;
+
+        this.classDataOffset = dexFile.readSmallUint(classDefOffset + ClassDefItem.CLASS_DATA_OFFSET);
+        if (classDataOffset == 0) {
+            staticFieldsOffset = -1;
+            staticFieldCount = 0;
+            instanceFieldCount = 0;
+            directMethodCount = 0;
+            virtualMethodCount = 0;
+        } else {
+            DexReader reader = dexFile.readerAt(classDataOffset);
+            staticFieldCount = reader.readSmallUleb128();
+            instanceFieldCount = reader.readSmallUleb128();
+            directMethodCount = reader.readSmallUleb128();
+            virtualMethodCount = reader.readSmallUleb128();
+            staticFieldsOffset = reader.getOffset();
+        }
+
     }
 
     @Nonnull
@@ -110,115 +140,249 @@ public class DexBackedClassDef extends BaseTypeReference implements ClassDef {
 
     @Nonnull
     @Override
-    public Set<? extends DexBackedField> getFields() {
+    public Iterable<? extends DexBackedField> getStaticFields() {
-        int classDataOffset = getClassDataOffset();
+        if (staticFieldCount > 0) {
-        if (getClassDataOffset() != 0) {
+            DexReader reader = dexFile.readerAt(staticFieldsOffset);
-            DexReader reader = dexFile.readerAt(classDataOffset);
-            final int staticFieldCount = reader.readSmallUleb128();
-            int instanceFieldCount = reader.readSmallUleb128();
-            final int fieldCount = staticFieldCount + instanceFieldCount;
-            if (fieldCount > 0) {
-                reader.skipUleb128(); //direct_methods_size
-                reader.skipUleb128(); //virtual_methods_size
 
-                final AnnotationsDirectory annotationsDirectory = getAnnotationsDirectory();
+            final AnnotationsDirectory annotationsDirectory = getAnnotationsDirectory();
-                final int staticInitialValuesOffset =
+            final int staticInitialValuesOffset =
-                        dexFile.readSmallUint(classDefOffset + ClassDefItem.STATIC_VALUES_OFFSET);
+                    dexFile.readSmallUint(classDefOffset + ClassDefItem.STATIC_VALUES_OFFSET);
-                final int fieldsStartOffset = reader.getOffset();
+            final int fieldsStartOffset = reader.getOffset();
+            final AnnotationsDirectory.AnnotationIterator annotationIterator =
+                    annotationsDirectory.getFieldAnnotationIterator();
+            final StaticInitialValueIterator staticInitialValueIterator =
+                    StaticInitialValueIterator.newOrEmpty(dexFile, staticInitialValuesOffset);
 
-                return new AbstractSet<DexBackedField>() {
-                    @Nonnull
-                    @Override
-                    public Iterator<DexBackedField> iterator() {
-                        return new VariableSizeIterator<DexBackedField>(dexFile, fieldsStartOffset, fieldCount) {
-                            private int previousFieldIndex = 0;
-                            @Nonnull private final AnnotationsDirectory.AnnotationIterator annotationIterator =
-                                    annotationsDirectory.getFieldAnnotationIterator();
-                            @Nonnull private final StaticInitialValueIterator staticInitialValueIterator =
-                                    StaticInitialValueIterator.newOrEmpty(dexFile, staticInitialValuesOffset);
 
-                            @Nonnull
+            return new Iterable<DexBackedField>() {
-                            @Override
+                @Nonnull
-                            protected DexBackedField readNextItem(@Nonnull DexReader reader, int index) {
+                @Override
-                                if (index == staticFieldCount) {
+                public Iterator<DexBackedField> iterator() {
-                                    // We reached the end of the static field, restart the numbering for
+                    return new VariableSizeLookaheadIterator<DexBackedField>(dexFile, fieldsStartOffset) {
-                                    // instance fields
+                        private int count;
-                                    previousFieldIndex = 0;
+                        // TODO: consider implementing and using two MutableFieldReference classes.
-                                    annotationIterator.reset();
+                        // this would prevent creating a lot of throw-away ImmutableFieldReference objects while
+                        // iterating
+                        @Nullable private FieldReference previousField;
+                        private int previousIndex;
+
+                        @Nullable
+                        @Override
+                        protected DexBackedField readNextItem(@Nonnull DexReader reader) {
+                            while (true) {
+                                if (++count > staticFieldCount) {
+                                    instanceFieldsOffset = reader.getOffset();
+                                    return null;
                                 }
+
                                 DexBackedField item = new DexBackedField(reader, DexBackedClassDef.this,
-                                        previousFieldIndex, staticInitialValueIterator, annotationIterator);
+                                        previousIndex, staticInitialValueIterator, annotationIterator);
-                                previousFieldIndex = item.fieldIndex;
+                                FieldReference currentField = previousField;
+                                FieldReference nextField = ImmutableFieldReference.of(item);
+
+                                previousField = nextField;
+                                previousIndex = item.fieldIndex;
+
+                                if (currentField != null && currentField.equals(nextField)) {
+                                    continue;
+                                }
+
                                 return item;
                             }
-                        };
+                        }
-                    }
+                    };
-
+                }
-                    @Override public int size() { return fieldCount; }
+            };
-                };
+        } else {
-            }
+            instanceFieldsOffset = staticFieldsOffset;
+            return ImmutableSet.of();
         }
-        return ImmutableSet.of();
     }
 
     @Nonnull
     @Override
-    public Set<? extends DexBackedMethod> getMethods() {
+    public Iterable<? extends DexBackedField> getInstanceFields() {
-        int classDataOffset = getClassDataOffset();
+        if (instanceFieldCount > 0) {
-        if (classDataOffset > 0) {
+            DexReader reader = dexFile.readerAt(getInstanceFieldsOffset());
-            DexReader reader = dexFile.readerAt(classDataOffset);
-            int staticFieldCount = reader.readSmallUleb128();
-            int instanceFieldCount = reader.readSmallUleb128();
-            final int directMethodCount = reader.readSmallUleb128();
-            int virtualMethodCount = reader.readSmallUleb128();
-            final int methodCount = directMethodCount + virtualMethodCount;
-            if (methodCount > 0) {
-                DexBackedField.skipAllFields(reader, staticFieldCount + instanceFieldCount);
 
-                final AnnotationsDirectory annotationsDirectory = getAnnotationsDirectory();
+            final AnnotationsDirectory annotationsDirectory = getAnnotationsDirectory();
-                final int methodsStartOffset = reader.getOffset();
+            final AnnotationsDirectory.AnnotationIterator annotationIterator =
+                    annotationsDirectory.getFieldAnnotationIterator();
+            final int fieldsStartOffset = reader.getOffset();
 
-                return new AbstractSet<DexBackedMethod>() {
+            return new Iterable<DexBackedField>() {
-                    @Nonnull
+                @Nonnull
-                    @Override
+                @Override
-                    public Iterator<DexBackedMethod> iterator() {
+                public Iterator<DexBackedField> iterator() {
-                        return new VariableSizeIterator<DexBackedMethod>(dexFile, methodsStartOffset, methodCount) {
+                    return new VariableSizeLookaheadIterator<DexBackedField>(dexFile, fieldsStartOffset) {
-                            private int previousMethodIndex = 0;
+                        private int count;
-                            @Nonnull private final AnnotationsDirectory.AnnotationIterator methodAnnotationIterator =
+                        // TODO: consider implementing and using two MutableFieldReference classes.
-                                    annotationsDirectory.getMethodAnnotationIterator();
+                        // this would prevent creating a lot of throw-away ImmutableFieldReference objects while
-                            @Nonnull private final AnnotationsDirectory.AnnotationIterator parameterAnnotationIterator =
+                        // iterating
-                                    annotationsDirectory.getParameterAnnotationIterator();
+                        @Nullable private FieldReference previousField;
+                        private int previousIndex;
 
-                            @Nonnull
+                        @Nullable
-                            @Override
+                        @Override
-                            protected DexBackedMethod readNextItem(@Nonnull DexReader reader, int index) {
+                        protected DexBackedField readNextItem(@Nonnull DexReader reader) {
-                                if (index == directMethodCount) {
+                            while (true) {
-                                    // We reached the end of the direct methods, restart the numbering for
+                                if (++count > instanceFieldCount) {
-                                    // virtual methods
+                                    directMethodsOffset = reader.getOffset();
-                                    previousMethodIndex = 0;
+                                    return null;
-                                    methodAnnotationIterator.reset();
-                                    parameterAnnotationIterator.reset();
                                 }
-                                DexBackedMethod item = new DexBackedMethod(reader, DexBackedClassDef.this,
+
-                                        previousMethodIndex, methodAnnotationIterator, parameterAnnotationIterator);
+                                DexBackedField item = new DexBackedField(reader, DexBackedClassDef.this,
-                                previousMethodIndex = item.methodIndex;
+                                        previousIndex, annotationIterator);
+                                FieldReference currentField = previousField;
+                                FieldReference nextField = ImmutableFieldReference.of(item);
+
+                                previousField = nextField;
+                                previousIndex = item.fieldIndex;
+
+                                if (currentField != null && currentField.equals(nextField)) {
+                                    continue;
+                                }
+
                                 return item;
                             }
-                        };
+                        }
-                    }
+                    };
-
+                }
-                    @Override public int size() { return methodCount; }
+            };
-                };
+        } else {
+            if (instanceFieldsOffset > 0) {
+                directMethodsOffset = instanceFieldsOffset;
             }
+            return ImmutableSet.of();
         }
-        return ImmutableSet.of();
     }
 
-    private int getClassDataOffset() {
+
-        if (classDataOffset == -1) {
+    @Nonnull
-            classDataOffset = dexFile.readSmallUint(classDefOffset + ClassDefItem.CLASS_DATA_OFFSET);
+    @Override
+    public Iterable<? extends DexBackedField> getFields() {
+        return Iterables.concat(getStaticFields(), getInstanceFields());
+    }
+
+    @Nonnull
+    @Override
+    public Iterable<? extends DexBackedMethod> getDirectMethods() {
+        if (directMethodCount > 0) {
+            DexReader reader = dexFile.readerAt(getDirectMethodsOffset());
+
+            final AnnotationsDirectory annotationsDirectory = getAnnotationsDirectory();
+            final AnnotationsDirectory.AnnotationIterator methodAnnotationIterator =
+                    annotationsDirectory.getMethodAnnotationIterator();
+            final AnnotationsDirectory.AnnotationIterator parameterAnnotationIterator =
+                    annotationsDirectory.getParameterAnnotationIterator();
+            final int methodsStartOffset = reader.getOffset();
+
+            return new Iterable<DexBackedMethod>() {
+                @Nonnull
+                @Override
+                public Iterator<DexBackedMethod> iterator() {
+                    return new VariableSizeLookaheadIterator<DexBackedMethod>(dexFile, methodsStartOffset) {
+                        private int count;
+                        // TODO: consider implementing and using two MutableMethodReference classes.
+                        // this would prevent creating a lot of throw-away ImmutableMethodReference objects while
+                        // iterating
+                        @Nullable private MethodReference previousMethod;
+                        private int previousIndex;
+
+                        @Nullable
+                        @Override
+                        protected DexBackedMethod readNextItem(@Nonnull DexReader reader) {
+                            while (true) {
+                                if (++count > directMethodCount) {
+                                    virtualMethodsOffset = reader.getOffset();
+                                    return null;
+                                }
+
+                                DexBackedMethod item = new DexBackedMethod(reader, DexBackedClassDef.this,
+                                        previousIndex, methodAnnotationIterator, parameterAnnotationIterator);
+                                MethodReference currentMethod = previousMethod;
+                                MethodReference nextMethod = ImmutableMethodReference.of(item);
+
+                                previousMethod = nextMethod;
+                                previousIndex = item.methodIndex;
+
+                                if (currentMethod != null && currentMethod.equals(nextMethod)) {
+                                    continue;
+
+                                }
+                                return item;
+                            }
+                        }
+                    };
+                }
+            };
+        } else {
+            if (directMethodsOffset > 0) {
+                virtualMethodsOffset = directMethodsOffset;
+            }
+            return ImmutableSet.of();
         }
-        return classDataOffset;
+    }
+
+    @Nonnull
+    @Override
+    public Iterable<? extends DexBackedMethod> getVirtualMethods() {
+        if (virtualMethodCount > 0) {
+            DexReader reader = dexFile.readerAt(getVirtualMethodsOffset());
+
+            final AnnotationsDirectory annotationsDirectory = getAnnotationsDirectory();
+            final AnnotationsDirectory.AnnotationIterator methodAnnotationIterator =
+                    annotationsDirectory.getMethodAnnotationIterator();
+            final AnnotationsDirectory.AnnotationIterator parameterAnnotationIterator =
+                    annotationsDirectory.getParameterAnnotationIterator();
+            final int methodsStartOffset = reader.getOffset();
+
+            return new Iterable<DexBackedMethod>() {
+                @Nonnull
+                @Override
+                public Iterator<DexBackedMethod> iterator() {
+                    return new VariableSizeLookaheadIterator<DexBackedMethod>(dexFile, methodsStartOffset) {
+                        private int count;
+                        // TODO: consider implementing and using two MutableMethodReference classes.
+                        // this would prevent creating a lot of throw-away ImmutableMethodReference objects while
+                        // iterating
+                        @Nullable private MethodReference previousMethod;
+                        private int previousIndex;
+
+                        @Nullable
+                        @Override
+                        protected DexBackedMethod readNextItem(@Nonnull DexReader reader) {
+                            while (true) {
+                                if (++count > virtualMethodCount) {
+                                    return null;
+                                }
+
+                                DexBackedMethod item = new DexBackedMethod(reader, DexBackedClassDef.this,
+                                        previousIndex, methodAnnotationIterator, parameterAnnotationIterator);
+                                MethodReference currentMethod = previousMethod;
+                                MethodReference nextMethod = ImmutableMethodReference.of(item);
+
+                                previousMethod = nextMethod;
+                                previousIndex = item.methodIndex;
+
+                                if (currentMethod != null && currentMethod.equals(nextMethod)) {
+                                    continue;
+                                }
+                                return item;
+                            }
+                        }
+                    };
+                }
+            };
+        } else {
+            return ImmutableSet.of();
+        }
+    }
+
+    @Nonnull
+    @Override
+    public Iterable<? extends DexBackedMethod> getMethods() {
+        return Iterables.concat(getDirectMethods(), getVirtualMethods());
     }
 
     private AnnotationsDirectory getAnnotationsDirectory() {
@@ -228,4 +392,34 @@ public class DexBackedClassDef extends BaseTypeReference implements ClassDef {
         }
         return annotationsDirectory;
     }
+
+    private int getInstanceFieldsOffset() {
+        if (instanceFieldsOffset > 0) {
+            return instanceFieldsOffset;
+        }
+        DexReader reader = new DexReader(dexFile, staticFieldsOffset);
+        DexBackedField.skipAllFields(reader, staticFieldCount);
+        instanceFieldsOffset = reader.getOffset();
+        return instanceFieldsOffset;
+    }
+
+    private int getDirectMethodsOffset() {
+        if (directMethodsOffset > 0) {
+            return directMethodsOffset;
+        }
+        DexReader reader = dexFile.readerAt(getInstanceFieldsOffset());
+        DexBackedField.skipAllFields(reader, instanceFieldCount);
+        directMethodsOffset = reader.getOffset();
+        return directMethodsOffset;
+    }
+
+    private int getVirtualMethodsOffset() {
+        if (virtualMethodsOffset > 0) {
+            return virtualMethodsOffset;
+        }
+        DexReader reader = dexFile.readerAt(getDirectMethodsOffset());
+        DexBackedField.skipAllFields(reader, instanceFieldCount);
+        virtualMethodsOffset = reader.getOffset();
+        return virtualMethodsOffset;
+    }
 }

dexlib2/src/main/java/org/jf/dexlib2/dexbacked/DexBackedField.java

@@ -71,6 +71,21 @@ public class DexBackedField extends BaseFieldReference implements Field {
         this.initialValue = staticInitialValueIterator.getNextOrNull();
     }
 
+    public DexBackedField(@Nonnull DexReader reader,
+                          @Nonnull DexBackedClassDef classDef,
+                          int previousFieldIndex,
+                          @Nonnull AnnotationsDirectory.AnnotationIterator annotationIterator) {
+        this.dexFile = reader.dexBuf;
+        this.classDef = classDef;
+
+        int fieldIndexDiff = reader.readSmallUleb128();
+        this.fieldIndex = fieldIndexDiff + previousFieldIndex;
+        this.accessFlags = reader.readSmallUleb128();
+
+        this.annotationSetOffset = annotationIterator.seekTo(fieldIndex);
+        this.initialValue = null;
+    }
+
     @Nonnull
     @Override
     public String getName() {

dexlib2/src/main/java/org/jf/dexlib2/iface/ClassDef.java

@@ -98,20 +98,68 @@ public interface ClassDef extends TypeReference {
     @Nonnull Set<? extends Annotation> getAnnotations();
 
     /**
-     * Gets a set of the fields that are defined by this class.
+     * Gets the static fields that are defined by this class.
      *
-     * TODO: uniqueness?
+     * The static fields that are returned must have no duplicates.
+     *
+     * @return The static fields that are defined by this class
+     */
+    @Nonnull Iterable<? extends Field> getStaticFields();
+
+    /**
+     * Gets the instance fields that are defined by this class.
+     *
+     * The instance fields that are returned must have no duplicates.
+     *
+     * @return The instance fields that are defined by this class
+     */
+    @Nonnull Iterable<? extends Field> getInstanceFields();
+
+    /**
+     * Gets all the fields that are defined by this class.
+     *
+     * This is a convenience method that combines getStaticFields() and getInstanceFields()
+     *
+     * The returned fields may be in any order. I.e. It's not safe to assume that all instance fields will come after
+     * all static fields.
+     *
+     * Note that there typically should not be any duplicate fields between the two, but some versions of
+     * dalvik inadvertently allow duplicate static/instance fields, and are supported here for completeness
      *
      * @return A set of the fields that are defined by this class
      */
-    @Nonnull Set<? extends Field> getFields();
+    @Nonnull Iterable<? extends Field> getFields();
 
     /**
-     * Gets a set of the methods that are defined by this class.
+     * Gets the direct methods that are defined by this class.
      *
-     * TODO: uniqueness?
+     * The direct methods that are returned must have no duplicates.
      *
-     * @return A set of the methods that are defined by this class.
+     * @return The direct methods that are defined by this class.
      */
-    @Nonnull Set<? extends Method> getMethods();
+    @Nonnull Iterable<? extends Method> getDirectMethods();
+
+    /**
+     * Gets the virtual methods that are defined by this class.
+     *
+     * The virtual methods that are returned must have no duplicates.
+     *
+     * @return The virtual methods that are defined by this class.
+     */
+    @Nonnull Iterable<? extends Method> getVirtualMethods();
+
+    /**
+     * Gets all the methods that are defined by this class.
+     *
+     * This is a convenience method that combines getDirectMethods() and getVirtualMethods().
+     *
+     * The returned methods may be in any order. I.e. It's not safe to assume that all virtual methods will come after
+     * all direct methods.
+     *
+     * Note that there typically should not be any duplicate methods between the two, but some versions of
+     * dalvik inadvertently allow duplicate direct/virtual methods, and are supported here for completeness
+     *
+     * @return An iterable of the methods that are defined by this class.
+     */
+    @Nonnull Iterable<? extends Method> getMethods();
 }

dexlib2/src/main/java/org/jf/dexlib2/immutable/ImmutableClassDef.java

@@ -32,6 +32,8 @@
 package org.jf.dexlib2.immutable;
 
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSortedSet;
+import com.google.common.collect.Iterators;
 import org.jf.dexlib2.base.reference.BaseTypeReference;
 import org.jf.dexlib2.iface.Annotation;
 import org.jf.dexlib2.iface.ClassDef;
@@ -42,7 +44,9 @@ import org.jf.util.ImmutableUtils;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
+import java.util.AbstractCollection;
 import java.util.Collection;
+import java.util.Iterator;
 
 public class ImmutableClassDef extends BaseTypeReference implements ClassDef {
     @Nonnull protected final String type;
@@ -51,8 +55,10 @@ public class ImmutableClassDef extends BaseTypeReference implements ClassDef {
     @Nonnull protected final ImmutableSet<String> interfaces;
     @Nullable protected final String sourceFile;
     @Nonnull protected final ImmutableSet<? extends ImmutableAnnotation> annotations;
-    @Nonnull protected final ImmutableSet<? extends ImmutableField> fields;
+    @Nonnull protected final ImmutableSortedSet<? extends ImmutableField> staticFields;
-    @Nonnull protected final ImmutableSet<? extends ImmutableMethod> methods;
+    @Nonnull protected final ImmutableSortedSet<? extends ImmutableField> instanceFields;
+    @Nonnull protected final ImmutableSortedSet<? extends ImmutableMethod> directMethods;
+    @Nonnull protected final ImmutableSortedSet<? extends ImmutableMethod> virtualMethods;
 
     public ImmutableClassDef(@Nonnull String type,
                              int accessFlags,
@@ -60,16 +66,20 @@ public class ImmutableClassDef extends BaseTypeReference implements ClassDef {
                              @Nullable Collection<String> interfaces,
                              @Nullable String sourceFile,
                              @Nullable Collection<? extends Annotation> annotations,
-                             @Nullable Collection<? extends Field> fields,
+                             @Nullable Iterable<? extends Field> staticFields,
-                             @Nullable Collection<? extends Method> methods) {
+                             @Nullable Iterable<? extends Field> instanceFields,
+                             @Nullable Iterable<? extends Method> directMethods,
+                             @Nullable Iterable<? extends Method> virtualMethods) {
         this.type = type;
         this.accessFlags = accessFlags;
         this.superclass = superclass;
         this.interfaces = interfaces==null ? ImmutableSet.<String>of() : ImmutableSet.copyOf(interfaces);
         this.sourceFile = sourceFile;
         this.annotations = ImmutableAnnotation.immutableSetOf(annotations);
-        this.fields = ImmutableField.immutableSetOf(fields);
+        this.staticFields = ImmutableField.immutableSetOf(staticFields);
-        this.methods = ImmutableMethod.immutableSetOf(methods);
+        this.instanceFields = ImmutableField.immutableSetOf(instanceFields);
+        this.directMethods = ImmutableMethod.immutableSetOf(directMethods);
+        this.virtualMethods = ImmutableMethod.immutableSetOf(virtualMethods);
     }
 
     public ImmutableClassDef(@Nonnull String type,
@@ -78,16 +88,20 @@ public class ImmutableClassDef extends BaseTypeReference implements ClassDef {
                              @Nullable ImmutableSet<String> interfaces,
                              @Nullable String sourceFile,
                              @Nullable ImmutableSet<? extends ImmutableAnnotation> annotations,
-                             @Nullable ImmutableSet<? extends ImmutableField> fields,
+                             @Nullable ImmutableSortedSet<? extends ImmutableField> staticFields,
-                             @Nullable ImmutableSet<? extends ImmutableMethod> methods) {
+                             @Nullable ImmutableSortedSet<? extends ImmutableField> instanceFields,
+                             @Nullable ImmutableSortedSet<? extends ImmutableMethod> directMethods,
+                             @Nullable ImmutableSortedSet<? extends ImmutableMethod> virtualMethods) {
         this.type = type;
         this.accessFlags = accessFlags;
         this.superclass = superclass;
         this.interfaces = ImmutableUtils.nullToEmptySet(interfaces);
         this.sourceFile = sourceFile;
         this.annotations = ImmutableUtils.nullToEmptySet(annotations);
-        this.fields = ImmutableUtils.nullToEmptySet(fields);
+        this.staticFields = ImmutableUtils.nullToEmptySortedSet(staticFields);
-        this.methods = ImmutableUtils.nullToEmptySet(methods);
+        this.instanceFields = ImmutableUtils.nullToEmptySortedSet(instanceFields);
+        this.directMethods = ImmutableUtils.nullToEmptySortedSet(directMethods);
+        this.virtualMethods = ImmutableUtils.nullToEmptySortedSet(virtualMethods);
     }
 
     public static ImmutableClassDef of(ClassDef classDef) {
@@ -101,8 +115,10 @@ public class ImmutableClassDef extends BaseTypeReference implements ClassDef {
                 classDef.getInterfaces(),
                 classDef.getSourceFile(),
                 classDef.getAnnotations(),
-                classDef.getFields(),
+                classDef.getStaticFields(),
-                classDef.getMethods());
+                classDef.getInstanceFields(),
+                classDef.getDirectMethods(),
+                classDef.getVirtualMethods());
     }
 
     @Nonnull @Override public String getType() { return type; }
@@ -111,8 +127,42 @@ public class ImmutableClassDef extends BaseTypeReference implements ClassDef {
     @Nonnull @Override public ImmutableSet<String> getInterfaces() { return interfaces; }
     @Nullable @Override public String getSourceFile() { return sourceFile; }
     @Nonnull @Override public ImmutableSet<? extends ImmutableAnnotation> getAnnotations() { return annotations; }
-    @Nonnull @Override public ImmutableSet<? extends ImmutableField> getFields() { return fields; }
+    @Nonnull @Override public ImmutableSet<? extends ImmutableField> getStaticFields() { return staticFields; }
-    @Nonnull @Override public ImmutableSet<? extends ImmutableMethod> getMethods() { return methods; }
+    @Nonnull @Override public ImmutableSet<? extends ImmutableField> getInstanceFields() { return instanceFields; }
+    @Nonnull @Override public ImmutableSet<? extends ImmutableMethod> getDirectMethods() { return directMethods; }
+    @Nonnull @Override public ImmutableSet<? extends ImmutableMethod> getVirtualMethods() { return virtualMethods; }
+
+    @Nonnull
+    @Override
+    public Collection<? extends ImmutableField> getFields() {
+        return new AbstractCollection<ImmutableField>() {
+            @Nonnull
+            @Override
+            public Iterator<ImmutableField> iterator() {
+                return Iterators.concat(staticFields.iterator(), instanceFields.iterator());
+            }
+
+            @Override public int size() {
+                return staticFields.size() + instanceFields.size();
+            }
+        };
+    }
+
+    @Nonnull
+    @Override
+    public Collection<? extends ImmutableMethod> getMethods() {
+        return new AbstractCollection<ImmutableMethod>() {
+            @Nonnull
+            @Override
+            public Iterator<ImmutableMethod> iterator() {
+                return Iterators.concat(directMethods.iterator(), virtualMethods.iterator());
+            }
+
+            @Override public int size() {
+                return directMethods.size() + virtualMethods.size();
+            }
+        };
+    }
 
     @Nonnull
     public static ImmutableSet<ImmutableClassDef> immutableSetOf(@Nullable Iterable<? extends ClassDef> iterable) {

dexlib2/src/main/java/org/jf/dexlib2/immutable/ImmutableField.java

@@ -32,6 +32,8 @@
 package org.jf.dexlib2.immutable;
 
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSortedSet;
+import com.google.common.collect.Ordering;
 import org.jf.dexlib2.base.reference.BaseFieldReference;
 import org.jf.dexlib2.iface.Annotation;
 import org.jf.dexlib2.iface.Field;
@@ -102,8 +104,8 @@ public class ImmutableField extends BaseFieldReference implements Field {
     @Nonnull @Override public ImmutableSet<? extends ImmutableAnnotation> getAnnotations() { return annotations; }
 
     @Nonnull
-    public static ImmutableSet<ImmutableField> immutableSetOf(@Nullable Iterable<? extends Field> list) {
+    public static ImmutableSortedSet<ImmutableField> immutableSetOf(@Nullable Iterable<? extends Field> list) {
-        return CONVERTER.toSet(list);
+        return CONVERTER.toSortedSet(Ordering.natural(), list);
     }
 
     private static final ImmutableConverter<ImmutableField, Field> CONVERTER =

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

@@ -39,6 +39,19 @@ import java.io.IOException;
 import java.io.Writer;
 
 public final class ReferenceUtil {
+    public static String getShortMethodDescriptor(MethodReference methodReference) {
+        // TODO: try using a thread local StringBuilder
+        StringBuilder sb = new StringBuilder();
+        sb.append(methodReference.getName());
+        sb.append('(');
+        for (CharSequence paramType: methodReference.getParameterTypes()) {
+            sb.append(paramType);
+        }
+        sb.append(')');
+        sb.append(methodReference.getReturnType());
+        return sb.toString();
+    }
+
     public static String getMethodDescriptor(MethodReference methodReference) {
         // TODO: try using a thread local StringBuilder
         StringBuilder sb = new StringBuilder();

dexlib2/src/main/java/org/jf/dexlib2/writer/AnnotationDirectoryPool.java

@@ -32,6 +32,7 @@
 package org.jf.dexlib2.writer;
 
 import com.google.common.base.Predicate;
+import com.google.common.base.Predicates;
 import com.google.common.collect.*;
 import org.jf.dexlib2.iface.ClassDef;
 import org.jf.dexlib2.iface.Field;
@@ -42,7 +43,9 @@ import org.jf.util.ExceptionWithContext;
 
 import javax.annotation.Nonnull;
 import java.io.IOException;
-import java.util.*;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
 
 public class AnnotationDirectoryPool {
     @Nonnull private final Map<Key, Integer> internedAnnotationDirectoryItems = Maps.newHashMap();
@@ -72,16 +75,17 @@ public class AnnotationDirectoryPool {
         }
 
         dexFile.annotationSetPool.intern(classDef.getAnnotations());
-        for (Field field: key.getFieldsWithAnnotations()) {
+        for (Field field: Iterables.filter(classDef.getFields(), FIELD_HAS_ANNOTATION)) {
             dexFile.annotationSetPool.intern(field.getAnnotations());
         }
 
-        for (Method method: key.getMethodsWithAnnotations()) {
+        for (Method method: classDef.getMethods()) {
-            dexFile.annotationSetPool.intern(method.getAnnotations());
+            if (METHOD_HAS_ANNOTATION.apply(method)) {
-        }
+                dexFile.annotationSetPool.intern(method.getAnnotations());
-
+            }
-        for (Method method: key.getMethodsWithParameterAnnotations()) {
+            if (METHOD_HAS_PARAMETER_ANNOTATION.apply(method)) {
-            dexFile.annotationSetRefPool.intern(method);
+                dexFile.annotationSetRefPool.intern(method);
+            }
         }
     }
 
@@ -140,39 +144,33 @@ public class AnnotationDirectoryPool {
             writer.writeInt(key.methodAnnotationCount);
             writer.writeInt(key.parameterAnnotationCount);
 
-            Iterable<? extends Field> fieldsWithAnnotations;
+            List<? extends Field> sortedFieldsWithAnnotations = Ordering.natural().immutableSortedCopy(
-            if (CollectionUtils.isNaturalSortedSet(key.classDef.getFields())) {
+                    Iterables.filter(key.classDef.getFields(), FIELD_HAS_ANNOTATION));
-                fieldsWithAnnotations = key.getFieldsWithAnnotations();
+
-            } else {
+            for (Field field: sortedFieldsWithAnnotations) {
-                fieldsWithAnnotations = Ordering.natural().immutableSortedCopy(key.getFieldsWithAnnotations());
-            }
-            for (Field field: fieldsWithAnnotations) {
                 writer.writeInt(dexFile.fieldPool.getIndex(field));
                 writer.writeInt(dexFile.annotationSetPool.getOffset(field.getAnnotations()));
             }
 
-            boolean sortMethods = !CollectionUtils.isNaturalSortedSet(key.classDef.getMethods());
+            List<? extends Method> sortedMethods = Ordering.natural().immutableSortedCopy(
-            Iterable<? extends Method> methodsWithAnnotations;
+                    Iterables.filter(
-            if (sortMethods) {
+                            key.classDef.getMethods(),
-                methodsWithAnnotations = Ordering.natural().immutableSortedCopy(key.getMethodsWithAnnotations());
+                            Predicates.or(METHOD_HAS_ANNOTATION, METHOD_HAS_PARAMETER_ANNOTATION)));
-            } else {
+
-                methodsWithAnnotations = key.getMethodsWithAnnotations();
+            // It's safe to assume that we don't have any duplicate methods here. We would have already caught that and
-            }
+            // thrown an exception
-            for (Method method: methodsWithAnnotations) {
+            for (Method method: sortedMethods) {
-                writer.writeInt(dexFile.methodPool.getIndex(method));
+                if (METHOD_HAS_ANNOTATION.apply(method)) {
-                writer.writeInt(dexFile.annotationSetPool.getOffset(method.getAnnotations()));
+                    writer.writeInt(dexFile.methodPool.getIndex(method));
+                    writer.writeInt(dexFile.annotationSetPool.getOffset(method.getAnnotations()));
+                }
             }
 
-            Iterable<? extends Method> methodsWithParameterAnnotations;
+            for (Method method: sortedMethods) {
-            if (sortMethods) {
+                if (METHOD_HAS_PARAMETER_ANNOTATION.apply(method)) {
-                methodsWithParameterAnnotations = Ordering.natural().immutableSortedCopy(
+                    writer.writeInt(dexFile.methodPool.getIndex(method));
-                        key.getMethodsWithParameterAnnotations());
+                    writer.writeInt(dexFile.annotationSetRefPool.getOffset(method));
-            } else {
+                }
-                methodsWithParameterAnnotations = key.getMethodsWithParameterAnnotations();
-            }
-            for (Method method: methodsWithParameterAnnotations) {
-                writer.writeInt(dexFile.methodPool.getIndex(method));
-                writer.writeInt(dexFile.annotationSetRefPool.getOffset(method));
             }
         }
     }
@@ -207,28 +205,19 @@ public class AnnotationDirectoryPool {
 
         public Key(@Nonnull ClassDef classDef) {
             this.classDef = classDef;
-            this.fieldAnnotationCount = Iterables.size(getFieldsWithAnnotations());
+            this.fieldAnnotationCount = Iterables.size(Iterables.filter(classDef.getFields(), FIELD_HAS_ANNOTATION));
-            this.methodAnnotationCount = Iterables.size(getMethodsWithAnnotations());
+            int methodAnnotationCount = 0;
-            this.parameterAnnotationCount = Iterables.size(getMethodsWithParameterAnnotations());
+            int parameterAnnotationCount = 0;
-        }
+            for (Method method: classDef.getMethods()) {
-
+                if (METHOD_HAS_ANNOTATION.apply(method)) {
-        public int getFieldAnnotationCount() { return fieldAnnotationCount; }
+                    methodAnnotationCount++;
-        public int getMethodAnnotationCount() { return methodAnnotationCount; }
+                }
-        public int getParameterAnnotationCount() { return parameterAnnotationCount; }
+                if (METHOD_HAS_PARAMETER_ANNOTATION.apply(method)) {
-
+                    parameterAnnotationCount++;
-        @Nonnull
+                }
-        public Iterable<? extends Field> getFieldsWithAnnotations() {
+            }
-            return FluentIterable.from(classDef.getFields()).filter(FIELD_HAS_ANNOTATION);
+            this.methodAnnotationCount = methodAnnotationCount;
-        }
+            this.parameterAnnotationCount = parameterAnnotationCount;
-
-        @Nonnull
-        public Iterable<? extends Method> getMethodsWithAnnotations() {
-            return FluentIterable.from(classDef.getMethods()).filter(METHOD_HAS_ANNOTATION);
-        }
-
-        @Nonnull
-        public Iterable<? extends Method> getMethodsWithParameterAnnotations() {
-            return FluentIterable.from(classDef.getMethods()).filter(METHOD_HAS_PARAMETER_ANNOTATION);
         }
 
         public boolean hasClassAnnotations() {

dexlib2/src/main/java/org/jf/dexlib2/writer/ClassDefPool.java

@@ -31,17 +31,16 @@
 
 package org.jf.dexlib2.writer;
 
-import com.google.common.collect.ImmutableSortedSet;
+import com.google.common.collect.*;
-import com.google.common.collect.Lists;
+import org.jf.dexlib2.iface.ClassDef;
-import com.google.common.collect.Maps;
+import org.jf.dexlib2.iface.Field;
-import org.jf.dexlib2.iface.*;
+import org.jf.dexlib2.iface.Method;
-import org.jf.dexlib2.util.FieldUtil;
+import org.jf.dexlib2.util.ReferenceUtil;
-import org.jf.dexlib2.util.MethodUtil;
 import org.jf.util.ExceptionWithContext;
 
 import javax.annotation.Nonnull;
 import java.io.IOException;
-import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 
@@ -73,17 +72,32 @@ public class ClassDefPool {
         dexFile.typeListPool.intern(ImmutableSortedSet.copyOf(classDef.getInterfaces()));
         dexFile.stringPool.internNullable(classDef.getSourceFile());
         dexFile.encodedArrayPool.intern(classDef);
-        dexFile.annotationDirectoryPool.intern(classDef);
         boolean hasClassData = false;
+
+        HashSet<String> fields = new HashSet<String>();
         for (Field field: classDef.getFields()) {
             hasClassData = true;
+            String fieldDescriptor = ReferenceUtil.getShortFieldDescriptor(field);
+            if (!fields.add(fieldDescriptor)) {
+                throw new ExceptionWithContext("Multiple definitions for field %s->%s",
+                        classDef.getType(), fieldDescriptor);
+            }
             dexFile.fieldPool.intern(field);
         }
+
+        HashSet<String> methods = new HashSet<String>();
         for (Method method: classDef.getMethods()) {
             hasClassData = true;
+            String methodDescriptor = ReferenceUtil.getShortMethodDescriptor(method);
+            if (!methods.add(methodDescriptor)) {
+                throw new ExceptionWithContext("Multiple definitions for method %s->%s",
+                        classDef.getType(), methodDescriptor);
+            }
             dexFile.methodPool.intern(method);
             dexFile.codeItemPool.intern(method);
         }
+
+        dexFile.annotationDirectoryPool.intern(classDef);
         if (hasClassData) {
             classDataCount++;
         }
@@ -187,8 +201,6 @@ public class ClassDefPool {
 
     private class ClassDataItem {
         ClassDef classDef;
-        List<Field> fields;
-        List<Method> methods;
 
         int numStaticFields = 0;
         int numInstanceFields = 0;
@@ -198,26 +210,10 @@ public class ClassDefPool {
         private ClassDataItem(ClassDef classDef) {
             this.classDef = classDef;
 
-            fields = Lists.newArrayList(classDef.getFields());
+            numStaticFields = Iterables.size(classDef.getStaticFields());
-            Collections.sort(fields);
+            numInstanceFields = Iterables.size(classDef.getInstanceFields());
-
+            numDirectMethods = Iterables.size(classDef.getDirectMethods());
-            methods = Lists.newArrayList(classDef.getMethods());
+            numVirtualMethods = Iterables.size(classDef.getVirtualMethods());
-            Collections.sort(methods);
-
-            for (Field field: fields) {
-                if (FieldUtil.isStatic(field)) {
-                    numStaticFields++;
-                } else {
-                    numInstanceFields++;
-                }
-            }
-            for (Method method: methods) {
-                if (MethodUtil.isDirect(method)) {
-                    numDirectMethods++;
-                } else {
-                    numVirtualMethods++;
-                }
-            }
         }
 
         private boolean hasData() {
@@ -230,55 +226,63 @@ public class ClassDefPool {
 
         private void writeStaticFields(DexWriter writer) throws IOException {
             int lastIdx = 0;
-            for (Field field: fields) {
-                if (FieldUtil.isStatic(field)) {
-                    int idx = dexFile.fieldPool.getIndex(field);
-                    writer.writeUleb128(idx - lastIdx);
-                    lastIdx = idx;
 
-                    writer.writeUleb128(field.getAccessFlags());
+            Iterable<? extends Field> sortedStaticFields =
-                }
+                    Ordering.natural().immutableSortedCopy(classDef.getStaticFields());
+
+            for (Field field: sortedStaticFields) {
+                int idx = dexFile.fieldPool.getIndex(field);
+                writer.writeUleb128(idx - lastIdx);
+                lastIdx = idx;
+
+                writer.writeUleb128(field.getAccessFlags());
             }
         }
 
         private void writeInstanceFields(DexWriter writer) throws IOException {
             int lastIdx = 0;
-            for (Field field: fields) {
-                if (!FieldUtil.isStatic(field)) {
-                    int idx = dexFile.fieldPool.getIndex(field);
-                    writer.writeUleb128(idx - lastIdx);
-                    lastIdx = idx;
 
-                    writer.writeUleb128(field.getAccessFlags());
+            Iterable<? extends Field> sortedInstanceFields =
-                }
+                    Ordering.natural().immutableSortedCopy(classDef.getInstanceFields());
+
+            for (Field field: sortedInstanceFields) {
+                int idx = dexFile.fieldPool.getIndex(field);
+                writer.writeUleb128(idx - lastIdx);
+                lastIdx = idx;
+
+                writer.writeUleb128(field.getAccessFlags());
             }
         }
 
         private void writeDirectMethods(DexWriter writer) throws IOException {
             int lastIdx = 0;
-            for (Method method: methods) {
-                if (MethodUtil.isDirect(method)) {
-                    int idx = dexFile.methodPool.getIndex(method);
-                    writer.writeUleb128(idx - lastIdx);
-                    lastIdx = idx;
 
-                    writer.writeUleb128(method.getAccessFlags());
+            Iterable<? extends Method> sortedDirectMethods =
-                    writer.writeUleb128(dexFile.codeItemPool.getOffset(method));
+                    Ordering.natural().immutableSortedCopy(classDef.getDirectMethods());
-                }
+
+            for (Method method: sortedDirectMethods) {
+                int idx = dexFile.methodPool.getIndex(method);
+                writer.writeUleb128(idx - lastIdx);
+                lastIdx = idx;
+
+                writer.writeUleb128(method.getAccessFlags());
+                writer.writeUleb128(dexFile.codeItemPool.getOffset(method));
             }
         }
 
         private void writeVirtualMethods(DexWriter writer) throws IOException {
             int lastIdx = 0;
-            for (Method method: methods) {
-                if (!MethodUtil.isDirect(method)) {
-                    int idx = dexFile.methodPool.getIndex(method);
-                    writer.writeUleb128(idx - lastIdx);
-                    lastIdx = idx;
 
-                    writer.writeUleb128(method.getAccessFlags());
+            Iterable<? extends Method> sortedVirtualMethods =
-                    writer.writeUleb128(dexFile.codeItemPool.getOffset(method));
+                    Ordering.natural().immutableSortedCopy(classDef.getVirtualMethods());
-                }
+            
+            for (Method method: sortedVirtualMethods) {
+                int idx = dexFile.methodPool.getIndex(method);
+                writer.writeUleb128(idx - lastIdx);
+                lastIdx = idx;
+
+                writer.writeUleb128(method.getAccessFlags());
+                writer.writeUleb128(dexFile.codeItemPool.getOffset(method));
             }
         }
 

dexlib2/src/main/java/org/jf/dexlib2/writer/EncodedArrayPool.java

@@ -33,17 +33,13 @@ package org.jf.dexlib2.writer;
 
 import com.google.common.base.Function;
 import com.google.common.base.Predicate;
-import com.google.common.collect.FluentIterable;
+import com.google.common.collect.*;
-import com.google.common.collect.Iterables;
-import com.google.common.collect.Lists;
-import com.google.common.collect.Maps;
 import com.google.common.primitives.Ints;
 import org.jf.dexlib2.iface.ClassDef;
 import org.jf.dexlib2.iface.Field;
 import org.jf.dexlib2.iface.value.EncodedValue;
 import org.jf.dexlib2.immutable.value.ImmutableEncodedValueFactory;
 import org.jf.dexlib2.util.EncodedValueUtils;
-import org.jf.dexlib2.util.FieldUtil;
 import org.jf.util.CollectionUtils;
 import org.jf.util.ExceptionWithContext;
 
@@ -57,7 +53,7 @@ public class EncodedArrayPool {
     @Nonnull private final DexFile dexFile;
     private int sectionOffset = -1;
 
-    public EncodedArrayPool(DexFile dexFile) {
+    public EncodedArrayPool(@Nonnull DexFile dexFile) {
         this.dexFile = dexFile;
     }
 
@@ -111,16 +107,9 @@ public class EncodedArrayPool {
     }
 
     public static class Key implements Comparable<Key> {
-        private final Set<? extends Field> fields;
+        private final List<? extends Field> fields;
         private final int size;
 
-        private static class FieldComparator implements Comparator<Field> {
-            @Override
-            public int compare(Field o1, Field o2) {
-                return o1.compareTo(o2);
-            }
-        }
-
         private static final Function<Field, EncodedValue> GET_INITIAL_VALUE =
                 new Function<Field, EncodedValue>() {
                     @Override
@@ -133,16 +122,15 @@ public class EncodedArrayPool {
                     }
                 };
 
-        private Key(@Nonnull Set<? extends Field> fields, int size) {
+        private Key(@Nonnull List<? extends Field> fields, int size) {
             this.fields = fields;
             this.size = size;
         }
 
         @Nullable
         public static Key of(@Nonnull ClassDef classDef) {
-            Set<? extends Field> staticFieldsSorted = FluentIterable.from(classDef.getFields())
+            List<? extends Field> staticFieldsSorted = Ordering.natural().immutableSortedCopy(
-                    .filter(IS_STATIC_FIELD)
+                    classDef.getStaticFields());
-                    .toSortedSet(new FieldComparator());
 
             int lastIndex = CollectionUtils.lastIndexOf(staticFieldsSorted, HAS_INITIALIZER);
             if (lastIndex > -1) {
@@ -187,13 +175,6 @@ public class EncodedArrayPool {
             }
         };
 
-        private static final Predicate<Field> IS_STATIC_FIELD = new Predicate<Field>() {
-            @Override
-            public boolean apply(Field input) {
-                return FieldUtil.isStatic(input);
-            }
-        };
-
         @Override
         public int compareTo(Key o) {
             int res = Ints.compare(size, o.size);

dexlib2/src/test/java/org/jf/dexlib2/analysis/CustomMethodInlineTableTest.java

@@ -64,7 +64,7 @@ public class CustomMethodInlineTableTest {
                 methodImpl);
 
         ClassDef classDef = new ImmutableClassDef("Lblah;", AccessFlags.PUBLIC.getValue(), "Ljava/lang/Object;", null,
-                null, null, null, ImmutableList.of(method));
+                null, null, null, null, null, ImmutableList.of(method));
 
         DexFile dexFile = new ImmutableDexFile(ImmutableList.of(classDef));
 
@@ -90,7 +90,7 @@ public class CustomMethodInlineTableTest {
                 methodImpl);
 
         ClassDef classDef = new ImmutableClassDef("Lblah;", AccessFlags.PUBLIC.getValue(), "Ljava/lang/Object;", null,
-                null, null, null, ImmutableList.of(method));
+                null, null, null, null, ImmutableList.of(method), null);
 
         DexFile dexFile = new ImmutableDexFile(ImmutableList.of(classDef));
 
@@ -116,7 +116,7 @@ public class CustomMethodInlineTableTest {
                 methodImpl);
 
         ClassDef classDef = new ImmutableClassDef("Lblah;", AccessFlags.PUBLIC.getValue(), "Ljava/lang/Object;", null,
-                null, null, null, ImmutableList.of(method));
+                null, null, null, null, ImmutableList.of(method), null);
 
         DexFile dexFile = new ImmutableDexFile(ImmutableList.of(classDef));
 

dexlib2/src/test/java/org/jf/dexlib2/analysis/TestUtils.java

@@ -41,11 +41,12 @@ import javax.annotation.Nullable;
 
 public class TestUtils {
     public static ClassDef makeClassDef(@Nonnull String classType, @Nullable String superType, String... interfaces) {
-        return new ImmutableClassDef(classType, 0, superType, ImmutableSet.copyOf(interfaces), null, null, null, null);
+        return new ImmutableClassDef(classType, 0, superType, ImmutableSet.copyOf(interfaces),
+                null, null, null, null, null, null);
     }
 
     public static ClassDef makeInterfaceDef(@Nonnull String classType, String... interfaces) {
         return new ImmutableClassDef(classType, AccessFlags.INTERFACE.getValue(), "Ljava/lang/Object;",
-                ImmutableSet.copyOf(interfaces), null, null, null, null);
+                ImmutableSet.copyOf(interfaces), null, null, null, null, null, null);
     }
 }