From 4fbfbb3914cfcc6d2da41be818e76208f1e59866 Mon Sep 17 00:00:00 2001 From: Andy McFadden Date: Fri, 19 Feb 2010 16:15:27 -0800 Subject: [PATCH] Minor improvements to apkcheck. It turns out annotation classes aren't spelled out in the public API file, so we now emit warnings instead of errors on unrecognized method calls into annotation classes. The stripper wasn't working quite right, and did the wrong thing on nested brackets and stuff like "java.lang.Class[]". --- tools/apkcheck/README.txt | 23 ++++++++---- .../src/com/android/apkcheck/ApkCheck.java | 12 +++++-- .../src/com/android/apkcheck/ClassInfo.java | 42 ++++++++++++++++++++++ .../src/com/android/apkcheck/TypeUtils.java | 42 +++++++++++++++++----- 4 files changed, 102 insertions(+), 17 deletions(-) diff --git a/tools/apkcheck/README.txt b/tools/apkcheck/README.txt index 1681ed5a..70dcd9a9 100644 --- a/tools/apkcheck/README.txt +++ b/tools/apkcheck/README.txt @@ -92,12 +92,23 @@ and "values"). What isn't included are entries for the fields representing the enumeration values. This makes it look like an APK is referring to non-public fields in the class. -If apkcheck sees a reference to an unknown field, and the superclass of -the field's defining class is java.lang.Enum, we emit a warning instead -of an error. +If apkcheck sees a reference to an unknown field, and the field's defining +class appears to be an Enum (the superclass is java.lang.Enum), we emit +a warning instead of an error. -(3) Covariant return types +(3) Public annotation methods are not listed + +Annotation classes have trivial entries that show only the class name +and "implements java.lang.annotation.Annotation". It is not possible +to verify that a method call on an annotation is valid. + +If apkcheck sees a method call to an unknown method, and the class appears +to be an annotation (extends Object, implements Annotation, defines no +fields or methods), we emit a warning instead of an error. + + +(4) Covariant return types Suppose a class defines a method "public Foo gimmeFoo()". Any subclass that overrides that method must also return Foo, so it would seem that @@ -115,7 +126,7 @@ to verify that the return types are related, but that's more trouble than it's worth.) -(4) Generic signatures +(5) Generic signatures When generic signatures are used, the public API file will contain entries like these: @@ -153,7 +164,7 @@ signatures into the code (--uses-library=BUILTIN). (At some point it may be worthwhile to try a little harder here.) -(5) Use of opaque non-public types +(6) Use of opaque non-public types Some classes are not meant for public consumption, but are still referred to by application code. For example, an opaque type might be passed to diff --git a/tools/apkcheck/src/com/android/apkcheck/ApkCheck.java b/tools/apkcheck/src/com/android/apkcheck/ApkCheck.java index 997fb48d..65d8ca09 100644 --- a/tools/apkcheck/src/com/android/apkcheck/ApkCheck.java +++ b/tools/apkcheck/src/com/android/apkcheck/ApkCheck.java @@ -319,7 +319,7 @@ public class ApkCheck { String nameAndType = apkFieldInfo.getNameAndType(); FieldInfo pubFieldInfo = pubClassInfo.getField(nameAndType); if (pubFieldInfo == null) { - if ("java.lang.Enum".equals(pubClassInfo.getSuperclassName())) { + if (pubClassInfo.isEnum()) { apkWarning("Enum field ref: " + pubPkgInfo.getName() + "." + classInfo.getName() + "." + nameAndType); } else { @@ -337,8 +337,14 @@ public class ApkCheck { if (pubMethodInfo == null) { pubMethodInfo = pubClassInfo.getMethodIgnoringReturn(nameAndDescr); if (pubMethodInfo == null) { - apkError("Illegal method ref: " + pubPkgInfo.getName() + - "." + classInfo.getName() + "." + nameAndDescr); + if (pubClassInfo.isAnnotation()) { + apkWarning("Annotation method ref: " + + pubPkgInfo.getName() + "." + classInfo.getName() + + "." + nameAndDescr); + } else { + apkError("Illegal method ref: " + pubPkgInfo.getName() + + "." + classInfo.getName() + "." + nameAndDescr); + } } else { apkWarning("Possibly covariant method ref: " + pubPkgInfo.getName() + "." + classInfo.getName() + diff --git a/tools/apkcheck/src/com/android/apkcheck/ClassInfo.java b/tools/apkcheck/src/com/android/apkcheck/ClassInfo.java index 6ceb5316..be2c1b1c 100644 --- a/tools/apkcheck/src/com/android/apkcheck/ClassInfo.java +++ b/tools/apkcheck/src/com/android/apkcheck/ClassInfo.java @@ -40,6 +40,11 @@ public class ClassInfo { // holds the name of the superclass and all declared interfaces private ArrayList mSuperNames; + // is this an enumerated type? + private boolean mIsEnum; + // is this an annotation type? + private boolean mIsAnnotation; + private boolean mFlattening = false; private boolean mFlattened = false; @@ -104,6 +109,22 @@ public class ClassInfo { } /** + * Returns whether or not this class is an enumerated type. + */ + public boolean isEnum() { + assert mFlattened; + return mIsEnum; + } + + /** + * Returns whether or not this class is an annotation type. + */ + public boolean isAnnotation() { + assert mFlattened; + return mIsAnnotation; + } + + /** * Adds a field to the list. */ public void addField(FieldInfo fieldInfo) { @@ -194,6 +215,8 @@ public class ClassInfo { * superclasses and interfaces) into the local structure. * * The public API file must be fully parsed before calling here. + * + * This also detects if we're an Enum or Annotation. */ public void flattenClass(ApiList apiList) { if (mFlattened) @@ -217,6 +240,25 @@ public class ClassInfo { normalizeTypes(apiList); /* + * Figure out if this class is an enumerated type. + */ + mIsEnum = "java.lang.Enum".equals(mSuperclassName); + + /* + * Figure out if this class is an annotation type. We expect it + * to extend Object, implement java.lang.annotation.Annotation, + * and declare no fields or methods. (If the API XML file is + * fixed, it will declare methods; but at that point having special + * handling for annotations will be unnecessary.) + */ + if ("java.lang.Object".equals(mSuperclassName) && + mSuperNames.contains("java.lang.annotation.Annotation") && + hasNoFieldMethod()) + { + mIsAnnotation = true; + } + + /* * Flatten our superclass and interfaces. */ for (int i = 0; i < mSuperNames.size(); i++) { diff --git a/tools/apkcheck/src/com/android/apkcheck/TypeUtils.java b/tools/apkcheck/src/com/android/apkcheck/TypeUtils.java index 75e2bcdf..6cfe031d 100644 --- a/tools/apkcheck/src/com/android/apkcheck/TypeUtils.java +++ b/tools/apkcheck/src/com/android/apkcheck/TypeUtils.java @@ -145,12 +145,14 @@ public class TypeUtils { /* * In some cases this can be a generic signature: * + * * * - * If we see a '<', truncate the string at that point. That does - * pretty much the right thing. + * If we see a '<', strip out everything from it to the '>'. That + * does pretty much the right thing, though we have to deal with + * nested stuff like ">". * - * Handling the second item is ugly. If the string is a single + * Handling the third item is ugly. If the string is a single * character, change it to java.lang.Object. This is generally * insufficient and also ambiguous with respect to classes in the * default package, but we don't have much choice here, and it gets @@ -158,11 +160,7 @@ public class TypeUtils { * if somebody tries to normalize a string twice, since we could be * "promoting" a primitive type. */ - int ltOffset = typeName.indexOf('<'); - if (ltOffset >= 0) { - //System.out.println("stripping: " + typeName); - typeName = typeName.substring(0, ltOffset); - } + typeName = stripAngleBrackets(typeName); if (typeName.length() == 1) { //System.out.println("converting X to Object: " + typeName); typeName = "java.lang.Object"; @@ -249,5 +247,33 @@ public class TypeUtils { return typeName; } + + /** + * Strips out everything between '<' and '>'. This will handle + * nested brackets, but we're not expecting to see multiple instances + * in series (i.e. "<foo<bar>>" is expected, but + * "<foo>STUFF<bar> is not). + * + * @return the stripped string + */ + private static String stripAngleBrackets(String str) { + /* + * Since we only expect to see one "run", we can just find the + * first '<' and the last '>'. Ideally we'd verify that they're + * not mismatched, but we're assuming the input file is generally + * correct. + */ + int ltIndex = str.indexOf('<'); + if (ltIndex < 0) + return str; + + int gtIndex = str.lastIndexOf('>'); + if (gtIndex < 0) { + System.err.println("ERROR: found '<' without '>': " + str); + return str; // not much we can do + } + + return str.substring(0, ltIndex) + str.substring(gtIndex+1); + } } -- 2.11.0