From a59fe7e5232eea406a6f7b6055eeb5884683f8b2 Mon Sep 17 00:00:00 2001 From: Ben Gruver Date: Tue, 25 Sep 2012 19:52:04 -0700 Subject: [PATCH] Change how the parent is determined for AnnotationDirectoryItem and ClassDataItem --- .../jf/dexlib/AnnotationDirectoryItem.java | 114 ++++++++++-------- .../java/org/jf/dexlib/ClassDataItem.java | 57 ++++++--- .../main/java/org/jf/dexlib/ClassDefItem.java | 17 +-- 3 files changed, 106 insertions(+), 82 deletions(-) diff --git a/dexlib/src/main/java/org/jf/dexlib/AnnotationDirectoryItem.java b/dexlib/src/main/java/org/jf/dexlib/AnnotationDirectoryItem.java index 3942ce52..3882b25d 100644 --- a/dexlib/src/main/java/org/jf/dexlib/AnnotationDirectoryItem.java +++ b/dexlib/src/main/java/org/jf/dexlib/AnnotationDirectoryItem.java @@ -48,16 +48,6 @@ public class AnnotationDirectoryItem extends Item { @Nullable private ParameterAnnotation[] parameterAnnotations; - /** - * typically each AnnotationDirectoryItem will have a distinct parent. The only case that isn't true is when - * the AnnotationDirectoryItem *only* contains class annotations, with no other type of annotation. In that - * case, the same AnnotationDirectoryItem could be referenced from multiple classes. - * This isn't a problem though, because this field is only used in compareTo to determine the sort order, which - * which knows it should handle a null value as a special case - */ - @Nullable - private ClassDefItem parent = null; - /** * Creates a new uninitialized AnnotationDirectoryItem * @param dexFile The DexFile that this item belongs to @@ -211,8 +201,9 @@ public class AnnotationDirectoryItem extends Item { /** {@inheritDoc} */ protected void writeItem(AnnotatedOutput out) { if (out.annotates()) { - if (!isInternable() && parent != null) { - out.annotate(0, parent.getClassType().getTypeDescriptor()); + TypeIdItem parentType = getParentType(); + if (parentType != null) { + out.annotate(0, parentType.getTypeDescriptor()); } if (classAnnotations != null) { out.annotate(4, "class_annotations_off: 0x" + Integer.toHexString(classAnnotations.getOffset())); @@ -303,29 +294,60 @@ public class AnnotationDirectoryItem extends Item { /** {@inheritDoc} */ public String getConciseIdentity() { - if (parent == null) { + TypeIdItem parentType = getParentType(); + if (parentType == null) { return "annotation_directory_item @0x" + Integer.toHexString(getOffset()); } - return "annotation_directory_item @0x" + Integer.toHexString(getOffset()) + " (" + parent.getClassType() + ")"; + return "annotation_directory_item @0x" + Integer.toHexString(getOffset()) + + " (" + parentType.getTypeDescriptor() + ")"; } /** {@inheritDoc} */ public int compareTo(AnnotationDirectoryItem o) { Preconditions.checkNotNull(o); - if (!isInternable()) { - if (!o.isInternable()) { - Preconditions.checkState(parent != null && o.parent != null, - "Must call setParent before comparing AnnotationDirectoryItem instances"); - return parent.compareTo(o.parent); + + TypeIdItem parentType = getParentType(); + TypeIdItem otherParentType = o.getParentType(); + if (parentType != null) { + if (otherParentType != null) { + return parentType.compareTo(otherParentType); } + return 1; + } + if (otherParentType != null) { return -1; } - if (!o.isInternable()) { + if (classAnnotations != null) { + if (o.classAnnotations != null) { + return classAnnotations.compareTo(o.classAnnotations); + } return 1; } + return -1; + } - return classAnnotations.compareTo(o.classAnnotations); + /** + * Returns the parent type for an AnnotationDirectoryItem that is guaranteed to have a single parent, or null + * for one that may be referenced by multiple classes. + * + * Specifically, the AnnotationDirectoryItem may be referenced by multiple classes if it has only class annotations, + * but not field/method/parameter annotations. + * + * @return The parent type for this AnnotationDirectoryItem, or null if it may have multiple parents + */ + @Nullable + public TypeIdItem getParentType() { + if (fieldAnnotations != null && fieldAnnotations.length > 0) { + return fieldAnnotations[0].field.getContainingClass(); + } + if (methodAnnotations != null && methodAnnotations.length > 0) { + return methodAnnotations[0].method.getContainingClass(); + } + if (parameterAnnotations != null && parameterAnnotations.length > 0) { + return parameterAnnotations[0].method.getContainingClass(); + } + return null; } /** @@ -425,6 +447,17 @@ public class AnnotationDirectoryItem extends Item { return parameterAnnotations[index].annotationSet; } + /** + * + */ + public int getClassAnnotationCount() { + if (classAnnotations == null) { + return 0; + } + AnnotationItem[] annotations = classAnnotations.getAnnotations(); + return annotations.length; + } + /** * @return The number of field annotations in this AnnotationDirectoryItem */ @@ -455,40 +488,17 @@ public class AnnotationDirectoryItem extends Item { return parameterAnnotations.length; } - /** - * @return true if this AnnotationDirectoryItem is internable. It is only internable if it has - * only class annotations, but no field, method or parameter annotations - */ - private boolean isInternable() { - return classAnnotations != null && - (fieldAnnotations == null || fieldAnnotations.length == 0) && - (methodAnnotations == null || methodAnnotations.length == 0) && - (parameterAnnotations == null || parameterAnnotations.length == 0); - } - - /** - * Sets the ClassDefItem that this AnnotationDirectoryItem is associated with. - * - * @param classDefItem the ClassDefItem that this AnnotationDirectoryItem is associated - * with. - */ - protected void setParent(ClassDefItem classDefItem) { - // If this AnnotationDirectoryItem is internable, then setParent may be called multiple times, because it is - // reused for multiple classes. In this case, the parent field isn't used, so it doesn't matter if we overwrite - // it. - // If, on the other hand, it is not internable, we want to make sure that only a single parent is set. parent - // should either be null, or be equal to the new parent - Preconditions.checkState(this.isInternable() || (parent == null || parent.equals(classDefItem))); - this.parent = classDefItem; - } - @Override public int hashCode() { - // An instance is internable only if it has only class annotations, but no other type of annotation - if (!isInternable()) { - return super.hashCode(); + // If the item has a single parent, we can use the re-use the identity (hash) of that parent + TypeIdItem parentType = getParentType(); + if (parentType != null) { + return parentType.hashCode(); } - return classAnnotations.hashCode(); + if (classAnnotations != null) { + return classAnnotations.hashCode(); + } + return 0; } @Override diff --git a/dexlib/src/main/java/org/jf/dexlib/ClassDataItem.java b/dexlib/src/main/java/org/jf/dexlib/ClassDataItem.java index 33710710..9fe605e5 100644 --- a/dexlib/src/main/java/org/jf/dexlib/ClassDataItem.java +++ b/dexlib/src/main/java/org/jf/dexlib/ClassDataItem.java @@ -45,9 +45,6 @@ public class ClassDataItem extends Item { @Nullable private EncodedMethod[] virtualMethods = null; - @Nullable - private ClassDefItem parent = null; - /** * Creates a new uninitialized ClassDataItem * @param dexFile The DexFile that this item belongs to @@ -384,14 +381,17 @@ public class ClassDataItem extends Item { /** {@inheritDoc} */ public String getConciseIdentity() { - if (parent == null) { + TypeIdItem parentType = getParentType(); + if (parentType == null) { return "class_data_item @0x" + Integer.toHexString(getOffset()); } - return "class_data_item @0x" + Integer.toHexString(getOffset()) + " (" + parent.getClassType() +")"; + return "class_data_item @0x" + Integer.toHexString(getOffset()) + " (" + parentType.getTypeDescriptor() +")"; } /** {@inheritDoc} */ public int compareTo(ClassDataItem other) { + Preconditions.checkNotNull(other); + // An empty CodeDataItem may be shared by multiple ClassDefItems, so we can't use parent in this case if (isEmpty()) { if (other.isEmpty()) { @@ -403,25 +403,54 @@ public class ClassDataItem extends Item { return 1; } - if (parent == null) { - if (other.parent == null) { + TypeIdItem parentType = getParentType(); + TypeIdItem otherParentType= other.getParentType(); + if (parentType == null) { + if (otherParentType == null) { return 0; } return -1; } - if (other.parent == null) { + if (otherParentType == null) { return 1; } - return parent.compareTo(other.parent); + return parentType.compareTo(otherParentType); + } + + @Override + public int hashCode() { + // If the item has a single parent, we can use the re-use the identity (hash) of that parent + TypeIdItem parentType = getParentType(); + if (parentType != null) { + return parentType.hashCode(); + } + return 0; } /** - * Sets the ClassDefItem that this ClassDataItem is associated with - * @param classDefItem the ClassDefItem that this ClassDataItem is associated with + * Returns the parent type for a non-empty ClassDataItem, or null for an empty one (which could be referenced by + * multiple ClassDefItem parents) + * + * Specifically, the AnnotationDirectoryItem may be referenced by multiple classes if it has only class annotations, + * but not field/method/parameter annotations. + * + * @return The parent type for this AnnotationDirectoryItem, or null if it may have multiple parents */ - protected void setParent(ClassDefItem classDefItem) { - Preconditions.checkState(parent == null || parent.compareTo(classDefItem) == 0 || isEmpty()); - parent = classDefItem; + @Nullable + public TypeIdItem getParentType() { + if (staticFields != null && staticFields.length > 0) { + return staticFields[0].field.getContainingClass(); + } + if (instanceFields != null && instanceFields.length > 0) { + return instanceFields[0].field.getContainingClass(); + } + if (directMethods != null && directMethods.length > 0) { + return directMethods[0].method.getContainingClass(); + } + if (virtualMethods != null && virtualMethods.length > 0) { + return virtualMethods[0].method.getContainingClass(); + } + return null; } /** diff --git a/dexlib/src/main/java/org/jf/dexlib/ClassDefItem.java b/dexlib/src/main/java/org/jf/dexlib/ClassDefItem.java index 3737adbc..9664b997 100644 --- a/dexlib/src/main/java/org/jf/dexlib/ClassDefItem.java +++ b/dexlib/src/main/java/org/jf/dexlib/ClassDefItem.java @@ -28,7 +28,6 @@ package org.jf.dexlib; -import com.google.common.base.Preconditions; import org.jf.dexlib.EncodedValue.ArrayEncodedSubValue; import org.jf.dexlib.EncodedValue.EncodedValue; import org.jf.dexlib.Util.AccessFlags; @@ -88,13 +87,6 @@ public class ClassDefItem extends Item { this.annotations = annotations; this.classData = classData; this.staticFieldInitializers = staticFieldInitializers; - - if (classData != null) { - classData.setParent(this); - } - if (annotations != null) { - annotations.setParent(this); - } } /** @@ -145,13 +137,6 @@ public class ClassDefItem extends Item { classData = (ClassDataItem)readContext.getOptionalOffsettedItemByOffset(ItemType.TYPE_CLASS_DATA_ITEM, in.readInt()); staticFieldInitializers = (EncodedArrayItem)readContext.getOptionalOffsettedItemByOffset( ItemType.TYPE_ENCODED_ARRAY_ITEM, in.readInt()); - - if (classData != null) { - classData.setParent(this); - } - if (annotations != null) { - annotations.setParent(this); - } } /** {@inheritDoc} */ @@ -344,7 +329,7 @@ public class ClassDefItem extends Item { */ private static EncodedArrayItem makeStaticFieldInitializersItem(DexFile dexFile, @Nonnull List staticFieldInitializers) { - if (staticFieldInitializers == null || staticFieldInitializers.size() == 0) { + if (staticFieldInitializers.size() == 0) { return null; }