Don't greylist public bridge methods.
We need to be able to apply the @UnsupportedAppUsage annotation to regular
(non-bridged) methods, but due to the SDK visibility rules, the synthetic
bridge is sometimes part of the SDK. We need to know the public API set
in order to exclude these.
Bug: 110868826
Test: atest class2greylisttest
Test: m
Change-Id: I2ac9dfaaf60053762f379a5dc0d81e48a40a4e57
diff --git a/tools/class2greylist/Android.bp b/tools/class2greylist/Android.bp
index 7b1233b..1e3cdff 100644
--- a/tools/class2greylist/Android.bp
+++ b/tools/class2greylist/Android.bp
@@ -20,6 +20,7 @@
static_libs: [
"commons-cli-1.2",
"apache-bcel",
+ "guava",
],
}
diff --git a/tools/class2greylist/src/com/android/class2greylist/AnnotationVisitor.java b/tools/class2greylist/src/com/android/class2greylist/AnnotationVisitor.java
index 6b9ef16..c5c8ef0 100644
--- a/tools/class2greylist/src/com/android/class2greylist/AnnotationVisitor.java
+++ b/tools/class2greylist/src/com/android/class2greylist/AnnotationVisitor.java
@@ -16,6 +16,8 @@
package com.android.class2greylist;
+import com.google.common.annotations.VisibleForTesting;
+
import org.apache.bcel.Const;
import org.apache.bcel.classfile.AnnotationEntry;
import org.apache.bcel.classfile.DescendingVisitor;
@@ -27,6 +29,8 @@
import org.apache.bcel.classfile.Method;
import java.util.Locale;
+import java.util.Set;
+import java.util.function.Predicate;
/**
* Visits a JavaClass instance and pulls out all members annotated with a
@@ -44,13 +48,46 @@
private final JavaClass mClass;
private final String mAnnotationType;
+ private final Predicate<Member> mMemberFilter;
private final Status mStatus;
private final DescendingVisitor mDescendingVisitor;
- public AnnotationVisitor(JavaClass clazz, String annotation, Status d) {
+ /**
+ * Represents a member of a class file (a field or method).
+ */
+ @VisibleForTesting
+ public static class Member {
+
+ /**
+ * Signature of this member.
+ */
+ public final String signature;
+ /**
+ * Indicates if this is a synthetic bridge method.
+ */
+ public final boolean bridge;
+
+ public Member(String signature, boolean bridge) {
+ this.signature = signature;
+ this.bridge = bridge;
+ }
+ }
+
+ public AnnotationVisitor(
+ JavaClass clazz, String annotation, Set<String> publicApis, Status status) {
+ this(clazz,
+ annotation,
+ member -> !(member.bridge && publicApis.contains(member.signature)),
+ status);
+ }
+
+ @VisibleForTesting
+ public AnnotationVisitor(
+ JavaClass clazz, String annotation, Predicate<Member> memberFilter, Status status) {
mClass = clazz;
mAnnotationType = annotation;
- mStatus = d;
+ mMemberFilter = memberFilter;
+ mStatus = status;
mDescendingVisitor = new DescendingVisitor(clazz, this);
}
@@ -102,7 +139,9 @@
break;
}
}
- mStatus.greylistEntry(signature);
+ if (mMemberFilter.test(new Member(signature, bridge))) {
+ mStatus.greylistEntry(signature);
+ }
}
}
}
diff --git a/tools/class2greylist/src/com/android/class2greylist/Class2Greylist.java b/tools/class2greylist/src/com/android/class2greylist/Class2Greylist.java
index 3e9e320..abc9421 100644
--- a/tools/class2greylist/src/com/android/class2greylist/Class2Greylist.java
+++ b/tools/class2greylist/src/com/android/class2greylist/Class2Greylist.java
@@ -16,6 +16,9 @@
package com.android.class2greylist;
+import com.google.common.collect.Sets;
+import com.google.common.io.Files;
+
import org.apache.commons.cli.CommandLine;
import org.apache.commons.cli.CommandLineParser;
import org.apache.commons.cli.GnuParser;
@@ -23,9 +26,12 @@
import org.apache.commons.cli.OptionBuilder;
import org.apache.commons.cli.Options;
import org.apache.commons.cli.ParseException;
-import org.apache.commons.cli.PatternOptionBuilder;
+import java.io.File;
import java.io.IOException;
+import java.nio.charset.Charset;
+import java.util.Collections;
+import java.util.Set;
/**
* Build time tool for extracting a list of members from jar files that have the @UsedByApps
@@ -38,6 +44,11 @@
public static void main(String[] args) {
Options options = new Options();
options.addOption(OptionBuilder
+ .withLongOpt("public-api-list")
+ .hasArgs(1)
+ .withDescription("Public API list file. Used to de-dupe bridge methods.")
+ .create("p"));
+ options.addOption(OptionBuilder
.withLongOpt("debug")
.hasArgs(0)
.withDescription("Enable debug")
@@ -61,6 +72,7 @@
if (cmd.hasOption('h')) {
help(options);
}
+ String publicApiFilename = cmd.getOptionValue('p', null);
String[] jarFiles = cmd.getArgs();
if (jarFiles.length == 0) {
@@ -70,12 +82,26 @@
Status status = new Status(cmd.hasOption('d'));
+ Set<String> publicApis;
+ if (publicApiFilename != null) {
+ try {
+ publicApis = Sets.newHashSet(
+ Files.readLines(new File(publicApiFilename), Charset.forName("UTF-8")));
+ } catch (IOException e) {
+ status.error(e);
+ System.exit(1);
+ return;
+ }
+ } else {
+ publicApis = Collections.emptySet();
+ }
+
for (String jarFile : jarFiles) {
status.debug("Processing jar file %s", jarFile);
try {
JarReader reader = new JarReader(status, jarFile);
- reader.stream().forEach(clazz -> new AnnotationVisitor(
- clazz, ANNOTATION_TYPE, status).visit());
+ reader.stream().forEach(clazz -> new AnnotationVisitor(clazz, ANNOTATION_TYPE,
+ publicApis, status).visit());
reader.close();
} catch (IOException e) {
status.error(e);
diff --git a/tools/class2greylist/test/src/com/android/javac/AnnotationVisitorTest.java b/tools/class2greylist/test/src/com/android/javac/AnnotationVisitorTest.java
index a4ad20c..ff9c265 100644
--- a/tools/class2greylist/test/src/com/android/javac/AnnotationVisitorTest.java
+++ b/tools/class2greylist/test/src/com/android/javac/AnnotationVisitorTest.java
@@ -29,6 +29,7 @@
import com.android.class2greylist.Status;
import com.google.common.base.Joiner;
+import com.google.common.collect.Sets;
import org.junit.Before;
import org.junit.Rule;
@@ -37,6 +38,7 @@
import org.mockito.ArgumentCaptor;
import java.io.IOException;
+import java.util.Set;
public class AnnotationVisitorTest {
@@ -81,7 +83,7 @@
"}"));
assertThat(mJavac.compile()).isTrue();
- new AnnotationVisitor(mJavac.getCompiledClass("a.b.Class"), ANNOTATION, mStatus)
+ new AnnotationVisitor(mJavac.getCompiledClass("a.b.Class"), ANNOTATION, x -> true, mStatus)
.visit();
assertNoErrors();
@@ -101,7 +103,7 @@
"}"));
assertThat(mJavac.compile()).isTrue();
- new AnnotationVisitor(mJavac.getCompiledClass("a.b.Class"), ANNOTATION, mStatus)
+ new AnnotationVisitor(mJavac.getCompiledClass("a.b.Class"), ANNOTATION, x -> true, mStatus)
.visit();
assertNoErrors();
@@ -121,7 +123,7 @@
"}"));
assertThat(mJavac.compile()).isTrue();
- new AnnotationVisitor(mJavac.getCompiledClass("a.b.Class"), ANNOTATION, mStatus)
+ new AnnotationVisitor(mJavac.getCompiledClass("a.b.Class"), ANNOTATION, x -> true, mStatus)
.visit();
assertNoErrors();
@@ -141,7 +143,7 @@
"}"));
assertThat(mJavac.compile()).isTrue();
- new AnnotationVisitor(mJavac.getCompiledClass("a.b.Class"), ANNOTATION, mStatus)
+ new AnnotationVisitor(mJavac.getCompiledClass("a.b.Class"), ANNOTATION, x -> true, mStatus)
.visit();
assertNoErrors();
@@ -161,7 +163,7 @@
"}"));
assertThat(mJavac.compile()).isTrue();
- new AnnotationVisitor(mJavac.getCompiledClass("a.b.Class"), ANNOTATION, mStatus)
+ new AnnotationVisitor(mJavac.getCompiledClass("a.b.Class"), ANNOTATION, x -> true, mStatus)
.visit();
verify(mStatus, times(1)).error(any(String.class));
@@ -180,7 +182,7 @@
"}"));
assertThat(mJavac.compile()).isTrue();
- new AnnotationVisitor(mJavac.getCompiledClass("a.b.Class$Inner"), ANNOTATION,
+ new AnnotationVisitor(mJavac.getCompiledClass("a.b.Class$Inner"), ANNOTATION, x -> true,
mStatus).visit();
assertNoErrors();
@@ -198,7 +200,7 @@
"}"));
assertThat(mJavac.compile()).isTrue();
- new AnnotationVisitor(mJavac.getCompiledClass("a.b.Class"), ANNOTATION, mStatus)
+ new AnnotationVisitor(mJavac.getCompiledClass("a.b.Class"), ANNOTATION, x -> true, mStatus)
.visit();
assertNoErrors();
@@ -216,7 +218,7 @@
"}"));
assertThat(mJavac.compile()).isTrue();
- new AnnotationVisitor(mJavac.getCompiledClass("a.b.Class"), ANNOTATION, mStatus)
+ new AnnotationVisitor(mJavac.getCompiledClass("a.b.Class"), ANNOTATION, x -> true, mStatus)
.visit();
assertNoErrors();
@@ -243,9 +245,9 @@
"}"));
assertThat(mJavac.compile()).isTrue();
- new AnnotationVisitor(mJavac.getCompiledClass("a.b.Base"), ANNOTATION, mStatus)
+ new AnnotationVisitor(mJavac.getCompiledClass("a.b.Base"), ANNOTATION, x -> true, mStatus)
.visit();
- new AnnotationVisitor(mJavac.getCompiledClass("a.b.Class"), ANNOTATION, mStatus)
+ new AnnotationVisitor(mJavac.getCompiledClass("a.b.Class"), ANNOTATION, x -> true, mStatus)
.visit();
assertNoErrors();
@@ -275,9 +277,9 @@
"}"));
assertThat(mJavac.compile()).isTrue();
- new AnnotationVisitor(mJavac.getCompiledClass("a.b.Base"), ANNOTATION, mStatus)
+ new AnnotationVisitor(mJavac.getCompiledClass("a.b.Base"), ANNOTATION, x -> true, mStatus)
.visit();
- new AnnotationVisitor(mJavac.getCompiledClass("a.b.Class"), ANNOTATION, mStatus)
+ new AnnotationVisitor(mJavac.getCompiledClass("a.b.Class"), ANNOTATION, x -> true, mStatus)
.visit();
assertNoErrors();
@@ -312,11 +314,11 @@
assertThat(mJavac.compile()).isTrue();
new AnnotationVisitor(
- mJavac.getCompiledClass("a.b.Interface"), ANNOTATION, mStatus).visit();
+ mJavac.getCompiledClass("a.b.Interface"), ANNOTATION, x -> true, mStatus).visit();
new AnnotationVisitor(
- mJavac.getCompiledClass("a.b.Base"), ANNOTATION, mStatus).visit();
+ mJavac.getCompiledClass("a.b.Base"), ANNOTATION, x -> true, mStatus).visit();
new AnnotationVisitor(
- mJavac.getCompiledClass("a.b.Class"), ANNOTATION, mStatus).visit();
+ mJavac.getCompiledClass("a.b.Class"), ANNOTATION, x -> true, mStatus).visit();
assertNoErrors();
ArgumentCaptor<String> greylist = ArgumentCaptor.forClass(String.class);
@@ -326,4 +328,37 @@
"La/b/Class;->method(Ljava/lang/Object;)V",
"La/b/Base;->method(Ljava/lang/Object;)V");
}
+
+ @Test
+ public void testPublicBridgeExcluded() throws IOException {
+ mJavac.addSource("a.b.Base", Joiner.on('\n').join(
+ "package a.b;",
+ "public abstract class Base<T> {",
+ " public void method(T arg) {}",
+ "}"));
+
+ mJavac.addSource("a.b.Class", Joiner.on('\n').join(
+ "package a.b;",
+ "import annotation.Anno;",
+ "public class Class<T extends String> extends Base<T> {",
+ " @Override",
+ " @Anno",
+ " public void method(T arg) {}",
+ "}"));
+ assertThat(mJavac.compile()).isTrue();
+
+ Set<String> publicApis = Sets.newHashSet(
+ "La/b/Base;->method(Ljava/lang/Object;)V",
+ "La/b/Class;->method(Ljava/lang/Object;)V");
+ new AnnotationVisitor(mJavac.getCompiledClass("a.b.Base"), ANNOTATION, publicApis,
+ mStatus).visit();
+ new AnnotationVisitor(mJavac.getCompiledClass("a.b.Class"), ANNOTATION, publicApis,
+ mStatus).visit();
+
+ assertNoErrors();
+ ArgumentCaptor<String> greylist = ArgumentCaptor.forClass(String.class);
+ // The bridge method generated for the above, is a public API so should be excluded
+ verify(mStatus, times(1)).greylistEntry(greylist.capture());
+ assertThat(greylist.getValue()).isEqualTo("La/b/Class;->method(Ljava/lang/String;)V");
+ }
}