Handle bridge methods correctly in class2greylist.
Don't enforce expectedSignature for bridge methods, as they will not match.
Add a test for this, and another for generics.
Also make the test output more verbose, as the existing output was not
enough to understand what was going on in this case.
Fix the build, so that the test Android.mk is actually included in the
build. It was not before.
Test: java -cp external/jsr330/lib/junit.jar:\
out/host/linux-x86/framework/class2greylisttest.jar:\
out/soong/.intermediates/external/objenesis/objenesis/linux_glibc_common/javac/objenesis.jar \
org.junit.runner.JUnitCore com.android.javac.AnnotationVisitorTest
(atest appears to do nothing in AOSP, so have to invoke it manually)
Bug: 110868826
Change-Id: I1bceae14ffb76aa7cdc58a3b96b3052072f67439
diff --git a/tools/Android.mk b/tools/Android.mk
index 9ecf0cd..e90f5f5 100644
--- a/tools/Android.mk
+++ b/tools/Android.mk
@@ -32,3 +32,5 @@
LOCAL_SRC_FILES := art
LOCAL_MODULE_STEM := art
include $(BUILD_PREBUILT)
+
+include $(LOCAL_PATH)/class2greylist/test/Android.mk
diff --git a/tools/class2greylist/src/com/android/class2greylist/AnnotationVisitor.java b/tools/class2greylist/src/com/android/class2greylist/AnnotationVisitor.java
index 6685752..6b9ef16 100644
--- a/tools/class2greylist/src/com/android/class2greylist/AnnotationVisitor.java
+++ b/tools/class2greylist/src/com/android/class2greylist/AnnotationVisitor.java
@@ -81,14 +81,19 @@
mStatus.debug("Visit member %s : %s", member.getName(), member.getSignature());
for (AnnotationEntry a : member.getAnnotationEntries()) {
if (mAnnotationType.equals(a.getAnnotationType())) {
- mStatus.debug("Method has annotation %s", mAnnotationType);
+ mStatus.debug("Member has annotation %s", mAnnotationType);
+ boolean bridge = (member.getAccessFlags() & Const.ACC_BRIDGE) != 0;
+ if (bridge) {
+ mStatus.debug("Member is a bridge", mAnnotationType);
+ }
String signature = String.format(Locale.US, signatureFormatString,
getClassDescriptor(definingClass), member.getName(), member.getSignature());
for (ElementValuePair property : a.getElementValuePairs()) {
switch (property.getNameString()) {
case EXPECTED_SIGNATURE:
String expected = property.getValue().stringifyValue();
- if (!signature.equals(expected)) {
+ // Don't enforce for bridge methods; they're generated so won't match.
+ if (!bridge && !signature.equals(expected)) {
error(definingClass, member,
"Expected signature does not match generated:\n"
+ "Expected: %s\n"
diff --git a/tools/class2greylist/test/src/com/android/javac/AnnotationVisitorTest.java b/tools/class2greylist/test/src/com/android/javac/AnnotationVisitorTest.java
index 2d97218..a4ad20c 100644
--- a/tools/class2greylist/test/src/com/android/javac/AnnotationVisitorTest.java
+++ b/tools/class2greylist/test/src/com/android/javac/AnnotationVisitorTest.java
@@ -19,20 +19,22 @@
import static com.google.common.truth.Truth.assertThat;
import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
-import static org.mockito.MockitoAnnotations.initMocks;
+import static org.mockito.Mockito.withSettings;
-import com.android.class2greylist.Status;
import com.android.class2greylist.AnnotationVisitor;
+import com.android.class2greylist.Status;
import com.google.common.base.Joiner;
import org.junit.Before;
+import org.junit.Rule;
import org.junit.Test;
+import org.junit.rules.TestName;
import org.mockito.ArgumentCaptor;
-import org.mockito.Mock;
import java.io.IOException;
@@ -40,13 +42,17 @@
private static final String ANNOTATION = "Lannotation/Anno;";
+ @Rule
+ public TestName mTestName = new TestName();
+
private Javac mJavac;
- @Mock
private Status mStatus;
@Before
public void setup() throws IOException {
- initMocks(this);
+ System.out.println(String.format("\n============== STARTING TEST: %s ==============\n",
+ mTestName.getMethodName()));
+ mStatus = mock(Status.class, withSettings().verboseLogging());
mJavac = new Javac();
mJavac.addSource("annotation.Anno", Joiner.on('\n').join(
"package annotation;",
@@ -199,4 +205,125 @@
verify(mStatus, never()).greylistEntry(any(String.class));
}
+ @Test
+ public void testMethodArgGenerics() throws IOException {
+ mJavac.addSource("a.b.Class", Joiner.on('\n').join(
+ "package a.b;",
+ "import annotation.Anno;",
+ "public class Class<T extends String> {",
+ " @Anno(expectedSignature=\"La/b/Class;->method(Ljava/lang/String;)V\")",
+ " public void method(T arg) {}",
+ "}"));
+ assertThat(mJavac.compile()).isTrue();
+
+ new AnnotationVisitor(mJavac.getCompiledClass("a.b.Class"), ANNOTATION, mStatus)
+ .visit();
+
+ assertNoErrors();
+ ArgumentCaptor<String> greylist = ArgumentCaptor.forClass(String.class);
+ verify(mStatus, times(1)).greylistEntry(greylist.capture());
+ assertThat(greylist.getValue()).isEqualTo("La/b/Class;->method(Ljava/lang/String;)V");
+ }
+
+ @Test
+ public void testOverrideMethodWithBridge() throws IOException {
+ mJavac.addSource("a.b.Base", Joiner.on('\n').join(
+ "package a.b;",
+ "abstract class Base<T> {",
+ " protected abstract 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(expectedSignature=\"La/b/Class;->method(Ljava/lang/String;)V\")",
+ " public void method(T arg) {}",
+ "}"));
+ assertThat(mJavac.compile()).isTrue();
+
+ new AnnotationVisitor(mJavac.getCompiledClass("a.b.Base"), ANNOTATION, mStatus)
+ .visit();
+ new AnnotationVisitor(mJavac.getCompiledClass("a.b.Class"), ANNOTATION, mStatus)
+ .visit();
+
+ assertNoErrors();
+ ArgumentCaptor<String> greylist = ArgumentCaptor.forClass(String.class);
+ // A bridge method is generated for the above, so we expect 2 greylist entries.
+ verify(mStatus, times(2)).greylistEntry(greylist.capture());
+ assertThat(greylist.getAllValues()).containsExactly(
+ "La/b/Class;->method(Ljava/lang/Object;)V",
+ "La/b/Class;->method(Ljava/lang/String;)V");
+ }
+
+ @Test
+ public void testOverridePublicMethodWithBridge() 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(expectedSignature=\"La/b/Class;->method(Ljava/lang/String;)V\")",
+ " public void method(T arg) {}",
+ "}"));
+ assertThat(mJavac.compile()).isTrue();
+
+ new AnnotationVisitor(mJavac.getCompiledClass("a.b.Base"), ANNOTATION, mStatus)
+ .visit();
+ new AnnotationVisitor(mJavac.getCompiledClass("a.b.Class"), ANNOTATION, mStatus)
+ .visit();
+
+ assertNoErrors();
+ ArgumentCaptor<String> greylist = ArgumentCaptor.forClass(String.class);
+ // A bridge method is generated for the above, so we expect 2 greylist entries.
+ verify(mStatus, times(2)).greylistEntry(greylist.capture());
+ assertThat(greylist.getAllValues()).containsExactly(
+ "La/b/Class;->method(Ljava/lang/Object;)V",
+ "La/b/Class;->method(Ljava/lang/String;)V");
+ }
+
+ @Test
+ public void testBridgeMethodsFromInterface() throws IOException {
+ mJavac.addSource("a.b.Interface", Joiner.on('\n').join(
+ "package a.b;",
+ "public interface Interface {",
+ " public void method(Object arg);",
+ "}"));
+
+ mJavac.addSource("a.b.Base", Joiner.on('\n').join(
+ "package a.b;",
+ "import annotation.Anno;",
+ "class Base {",
+ " @Anno(expectedSignature=\"La/b/Base;->method(Ljava/lang/Object;)V\")",
+ " public void method(Object arg) {}",
+ "}"));
+
+ mJavac.addSource("a.b.Class", Joiner.on('\n').join(
+ "package a.b;",
+ "public class Class extends Base implements Interface {",
+ "}"));
+ assertThat(mJavac.compile()).isTrue();
+
+ new AnnotationVisitor(
+ mJavac.getCompiledClass("a.b.Interface"), ANNOTATION, mStatus).visit();
+ new AnnotationVisitor(
+ mJavac.getCompiledClass("a.b.Base"), ANNOTATION, mStatus).visit();
+ new AnnotationVisitor(
+ mJavac.getCompiledClass("a.b.Class"), ANNOTATION, mStatus).visit();
+
+ assertNoErrors();
+ ArgumentCaptor<String> greylist = ArgumentCaptor.forClass(String.class);
+ // A bridge method is generated for the above, so we expect 2 greylist entries.
+ verify(mStatus, times(2)).greylistEntry(greylist.capture());
+ assertThat(greylist.getAllValues()).containsExactly(
+ "La/b/Class;->method(Ljava/lang/Object;)V",
+ "La/b/Base;->method(Ljava/lang/Object;)V");
+ }
}