Verify method name in VisibilityBridgeRemover
Kotlin generates bridge methods where the forward method has the same
signature but not the same name. This CL make sure we only remove the
bridge method if it also has the same name.
Also adds a regression test (written in Jasmin).
Bug: 76383728
Change-Id: I21bb613dcda2ea37c8ad4c94b4dfffc84bcddb74
diff --git a/src/main/java/com/android/tools/r8/graph/DexMethod.java b/src/main/java/com/android/tools/r8/graph/DexMethod.java
index c0c2e27..9c3e9a5 100644
--- a/src/main/java/com/android/tools/r8/graph/DexMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexMethod.java
@@ -71,6 +71,14 @@
return false;
}
+ /**
+ * Returns true if the other method has the same name and prototype (including signature and
+ * return type), false otherwise.
+ */
+ public boolean hasSameProtoAndName(DexMethod other) {
+ return name == other.name && proto == other.proto;
+ }
+
@Override
public int compareTo(DexMethod other) {
return sortedCompareTo(other.getSortedIndex());
diff --git a/src/main/java/com/android/tools/r8/optimize/VisibilityBridgeRemover.java b/src/main/java/com/android/tools/r8/optimize/VisibilityBridgeRemover.java
index c815cf8..5847a80 100644
--- a/src/main/java/com/android/tools/r8/optimize/VisibilityBridgeRemover.java
+++ b/src/main/java/com/android/tools/r8/optimize/VisibilityBridgeRemover.java
@@ -33,8 +33,9 @@
method.getCode().registerCodeReferences(targetExtractor);
DexMethod target = targetExtractor.getTarget();
InvokeKind kind = targetExtractor.getKind();
- if (target != null &&
- target.proto == method.method.proto) {
+ // javac-generated visibility forward bridge method has same descriptor (name, signature and
+ // return type).
+ if (target != null && target.hasSameProtoAndName(method.method)) {
assert !method.accessFlags.isPrivate() && !method.accessFlags.isConstructor();
if (kind == InvokeKind.SUPER) {
// This is a visibility forward, so check for the direct target.
diff --git a/src/test/java/com/android/tools/r8/TestBase.java b/src/test/java/com/android/tools/r8/TestBase.java
index d7d8b99..a29d673 100644
--- a/src/test/java/com/android/tools/r8/TestBase.java
+++ b/src/test/java/com/android/tools/r8/TestBase.java
@@ -398,6 +398,13 @@
+ (obfuscate ? "-printmapping\n" : "-dontobfuscate\n");
}
+ public static String keepMainProguardConfiguration(
+ String clazz, boolean allowaccessmodification, boolean obfuscate) {
+ return keepMainProguardConfiguration(clazz)
+ + (allowaccessmodification ? "-allowaccessmodification\n" : "")
+ + (obfuscate ? "-printmapping\n" : "-dontobfuscate\n");
+ }
+
/**
* Run application on the specified version of Art with the specified main class.
*/
diff --git a/src/test/java/com/android/tools/r8/bridgeremoval/RemoveVisibilityBridgeMethodsTest.java b/src/test/java/com/android/tools/r8/bridgeremoval/RemoveVisibilityBridgeMethodsTest.java
index 6e3c114..35a7613 100644
--- a/src/test/java/com/android/tools/r8/bridgeremoval/RemoveVisibilityBridgeMethodsTest.java
+++ b/src/test/java/com/android/tools/r8/bridgeremoval/RemoveVisibilityBridgeMethodsTest.java
@@ -4,14 +4,26 @@
package com.android.tools.r8.bridgeremoval;
+import static com.android.tools.r8.utils.DexInspectorMatchers.isPresent;
+import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
import com.android.tools.r8.TestBase;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.ToolHelper.ProcessResult;
import com.android.tools.r8.bridgeremoval.bridgestoremove.Main;
import com.android.tools.r8.bridgeremoval.bridgestoremove.Outer;
+import com.android.tools.r8.jasmin.JasminBuilder;
+import com.android.tools.r8.jasmin.JasminBuilder.ClassBuilder;
+import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.DexInspector;
+import com.android.tools.r8.utils.DexInspector.ClassSubject;
+import com.android.tools.r8.utils.DexInspector.MethodSubject;
import com.google.common.collect.ImmutableList;
import java.lang.reflect.Method;
+import java.nio.file.Path;
+import java.util.Collections;
import java.util.List;
import org.junit.Test;
@@ -40,4 +52,81 @@
public void testWithoutObfuscation() throws Exception {
run(false);
}
+
+ @Test
+ public void regressionTest_b76383728_WithObfuscation() throws Exception {
+ runRegressionTest_b76383728(true);
+ }
+
+ @Test
+ public void regressionTest_b76383728_WithoutObfuscation() throws Exception {
+ runRegressionTest_b76383728(false);
+ }
+
+ /**
+ * Regression test for b76383728 to make sure we correctly identify and remove real visibility
+ * forward bridge methods synthesized by javac.
+ */
+ private void runRegressionTest_b76383728(boolean obfuscate) throws Exception {
+ JasminBuilder jasminBuilder = new JasminBuilder();
+
+ ClassBuilder superClass = jasminBuilder.addClass("SuperClass");
+ superClass.addDefaultConstructor();
+ superClass.addVirtualMethod("method", Collections.emptyList(), "Ljava/lang/String;",
+ ".limit stack 1",
+ "ldc \"Hello World\"",
+ "areturn");
+
+ // Generate a subclass with a method with same
+ ClassBuilder subclass = jasminBuilder.addClass("SubClass", superClass.name);
+ subclass.addBridgeMethod("getMethod", Collections.emptyList(), "Ljava/lang/String;",
+ ".limit stack 1",
+ "aload_0",
+ "invokespecial " + superClass.name + "/method()Ljava/lang/String;",
+ "areturn");
+
+ ClassBuilder mainClass = jasminBuilder.addClass("Main");
+ mainClass.addMainMethod(
+ ".limit stack 3",
+ ".limit locals 2",
+ "getstatic java/lang/System/out Ljava/io/PrintStream;",
+ "new " + subclass.name,
+ "dup",
+ "invokespecial " + subclass.name + "/<init>()V",
+ "invokevirtual " + subclass.name + "/getMethod()Ljava/lang/String;",
+ "invokevirtual java/io/PrintStream/print(Ljava/lang/String;)V",
+ "return"
+ );
+
+ final String mainClassName = mainClass.name;
+
+ String proguardConfig = keepMainProguardConfiguration(mainClass.name, true, obfuscate);
+
+ // Run input program on java.
+ Path outputDirectory = temp.newFolder().toPath();
+ jasminBuilder.writeClassFiles(outputDirectory);
+ ProcessResult javaResult = ToolHelper.runJava(outputDirectory, mainClassName);
+ assertEquals(0, javaResult.exitCode);
+
+ AndroidApp optimizedApp = compileWithR8(jasminBuilder.build(), proguardConfig,
+ // Disable inlining to avoid the (short) tested method from being inlined then removed.
+ internalOptions -> internalOptions.enableInlining = false);
+
+ // Run optimized (output) program on ART
+ String artResult = runOnArt(optimizedApp, mainClassName);
+ assertEquals(javaResult.stdout, artResult);
+
+ DexInspector inspector = new DexInspector(optimizedApp);
+
+ ClassSubject classSubject = inspector.clazz(superClass.name);
+ assertThat(classSubject, isPresent());
+ MethodSubject methodSubject = classSubject
+ .method("java.lang.String", "method", Collections.emptyList());
+ assertThat(methodSubject, isPresent());
+
+ classSubject = inspector.clazz(subclass.name);
+ assertThat(classSubject, isPresent());
+ methodSubject = classSubject.method("java.lang.String", "getMethod", Collections.emptyList());
+ assertThat(methodSubject, isPresent());
+ }
}
diff --git a/src/test/java/com/android/tools/r8/jasmin/JasminBuilder.java b/src/test/java/com/android/tools/r8/jasmin/JasminBuilder.java
index 64d092b..ff3f81a 100644
--- a/src/test/java/com/android/tools/r8/jasmin/JasminBuilder.java
+++ b/src/test/java/com/android/tools/r8/jasmin/JasminBuilder.java
@@ -120,6 +120,15 @@
return addMethod("public", name, argumentTypes, returnType, lines);
}
+ public MethodSignature addBridgeMethod(
+ String name,
+ List<String> argumentTypes,
+ String returnType,
+ String... lines) {
+ makeInit = true;
+ return addMethod("public bridge", name, argumentTypes, returnType, lines);
+ }
+
public MethodSignature addPrivateVirtualMethod(
String name,
List<String> argumentTypes,
