Lens rewrite const-method handles and types.
Fixes: b/235810300
Change-Id: I79832258f0ee78bdf765e49c12f2977a4f8422ff
diff --git a/src/main/java/com/android/tools/r8/cf/code/CfInvoke.java b/src/main/java/com/android/tools/r8/cf/code/CfInvoke.java
index 285ebe5..8cc10c2 100644
--- a/src/main/java/com/android/tools/r8/cf/code/CfInvoke.java
+++ b/src/main/java/com/android/tools/r8/cf/code/CfInvoke.java
@@ -111,12 +111,25 @@
LensCodeRewriterUtils rewriter,
MethodVisitor visitor) {
Invoke.Type invokeType = Invoke.Type.fromCfOpcode(opcode, method, context, appView);
- MethodLookupResult lookup = graphLens.lookupMethod(method, context.getReference(), invokeType);
- DexMethod rewrittenMethod = lookup.getReference();
+ Invoke.Type rewrittenType;
+ DexMethod rewrittenMethod;
+ if (invokeType == Type.POLYMORPHIC) {
+ // The method is one of the java.lang.MethodHandle invokes.
+ // Only the argument signature (getProto()) is to be type rewritten.
+ rewrittenType = Type.POLYMORPHIC;
+ DexProto rewrittenProto = rewriter.rewriteProto(method.getProto());
+ rewrittenMethod =
+ dexItemFactory.createMethod(method.getHolderType(), rewrittenProto, method.getName());
+ } else {
+ MethodLookupResult lookup =
+ graphLens.lookupMethod(method, context.getReference(), invokeType);
+ rewrittenType = lookup.getType();
+ rewrittenMethod = lookup.getReference();
+ }
String owner = namingLens.lookupInternalName(rewrittenMethod.holder);
String name = namingLens.lookupName(rewrittenMethod).toString();
String desc = rewrittenMethod.proto.toDescriptorString(namingLens);
- visitor.visitMethodInsn(lookup.getType().getCfOpcode(), owner, name, desc, itf);
+ visitor.visitMethodInsn(rewrittenType.getCfOpcode(), owner, name, desc, itf);
}
@Override
diff --git a/src/main/java/com/android/tools/r8/dex/code/DexFormat45cc.java b/src/main/java/com/android/tools/r8/dex/code/DexFormat45cc.java
index e658265..8e9a2b0 100644
--- a/src/main/java/com/android/tools/r8/dex/code/DexFormat45cc.java
+++ b/src/main/java/com/android/tools/r8/dex/code/DexFormat45cc.java
@@ -124,7 +124,10 @@
LensCodeRewriterUtils rewriter) {
MethodLookupResult lookup =
graphLens.lookupMethod(getMethod(), context.getReference(), Type.POLYMORPHIC);
+ // The method is one of the java.lang.MethodHandle invokes.
+ // Only the argument (getProto()) signature is to be type rewritten.
assert lookup.getType() == Type.POLYMORPHIC;
+ assert getMethod() == lookup.getReference();
writeFirst(A, G, dest);
write16BitReference(lookup.getReference(), dest, mapping);
write16BitValue(combineBytes(makeByte(F, E), makeByte(D, C)), dest);
diff --git a/src/main/java/com/android/tools/r8/dex/code/DexFormat4rcc.java b/src/main/java/com/android/tools/r8/dex/code/DexFormat4rcc.java
index ace2725..94b043a 100644
--- a/src/main/java/com/android/tools/r8/dex/code/DexFormat4rcc.java
+++ b/src/main/java/com/android/tools/r8/dex/code/DexFormat4rcc.java
@@ -61,7 +61,10 @@
LensCodeRewriterUtils rewriter) {
MethodLookupResult lookup =
graphLens.lookupMethod(getMethod(), context.getReference(), Type.POLYMORPHIC);
+ // The method is one of the java.lang.MethodHandle invokes.
+ // Only the argument (getProto()) signature is to be type rewritten.
assert lookup.getType() == Type.POLYMORPHIC;
+ assert getMethod() == lookup.getReference();
writeFirst(AA, dest);
write16BitReference(lookup.getReference(), dest, mapping);
write16BitValue(CCCC, dest);
diff --git a/src/main/java/com/android/tools/r8/graph/DexMethodHandle.java b/src/main/java/com/android/tools/r8/graph/DexMethodHandle.java
index 8859c3c..610a0c8 100644
--- a/src/main/java/com/android/tools/r8/graph/DexMethodHandle.java
+++ b/src/main/java/com/android/tools/r8/graph/DexMethodHandle.java
@@ -326,10 +326,6 @@
.withNullableItem(m -> m.rewrittenTarget);
}
- public Handle toAsmHandle() {
- return toAsmHandle(NamingLens.getIdentityLens());
- }
-
public Handle toAsmHandle(NamingLens lens) {
String owner;
String name;
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java b/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
index 2154c8c..207da48 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
@@ -10,6 +10,7 @@
import static com.android.tools.r8.ir.code.Opcodes.CHECK_CAST;
import static com.android.tools.r8.ir.code.Opcodes.CONST_CLASS;
import static com.android.tools.r8.ir.code.Opcodes.CONST_METHOD_HANDLE;
+import static com.android.tools.r8.ir.code.Opcodes.CONST_METHOD_TYPE;
import static com.android.tools.r8.ir.code.Opcodes.INIT_CLASS;
import static com.android.tools.r8.ir.code.Opcodes.INSTANCE_GET;
import static com.android.tools.r8.ir.code.Opcodes.INSTANCE_OF;
@@ -46,6 +47,7 @@
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexMethodHandle;
+import com.android.tools.r8.graph.DexProto;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.GraphLens;
import com.android.tools.r8.graph.GraphLens.FieldLookupResult;
@@ -70,6 +72,7 @@
import com.android.tools.r8.ir.code.CheckCast;
import com.android.tools.r8.ir.code.ConstClass;
import com.android.tools.r8.ir.code.ConstMethodHandle;
+import com.android.tools.r8.ir.code.ConstMethodType;
import com.android.tools.r8.ir.code.FieldInstruction;
import com.android.tools.r8.ir.code.FieldPut;
import com.android.tools.r8.ir.code.IRCode;
@@ -85,6 +88,7 @@
import com.android.tools.r8.ir.code.InvokeMethod;
import com.android.tools.r8.ir.code.InvokeMultiNewArray;
import com.android.tools.r8.ir.code.InvokeNewArray;
+import com.android.tools.r8.ir.code.InvokePolymorphic;
import com.android.tools.r8.ir.code.MoveException;
import com.android.tools.r8.ir.code.NewArrayEmpty;
import com.android.tools.r8.ir.code.NewInstance;
@@ -277,11 +281,18 @@
.computeIfAbsent()
.rewriteDexMethodHandle(handle, NOT_ARGUMENT_TO_LAMBDA_METAFACTORY, method);
if (newHandle != handle) {
- Value newOutValue = makeOutValue(current, code, graphLens, codeLens);
- iterator.replaceCurrentInstruction(new ConstMethodHandle(newOutValue, newHandle));
- if (newOutValue != null && newOutValue.getType() != current.getOutType()) {
- affectedPhis.addAll(newOutValue.uniquePhiUsers());
- }
+ iterator.replaceCurrentInstruction(
+ new ConstMethodHandle(current.outValue(), newHandle));
+ }
+ }
+ break;
+ case CONST_METHOD_TYPE:
+ {
+ ConstMethodType constType = current.asConstMethodType();
+ DexProto rewrittenProto = helper.computeIfAbsent().rewriteProto(constType.getValue());
+ if (constType.getValue() != rewrittenProto) {
+ iterator.replaceCurrentInstruction(
+ new ConstMethodType(constType.outValue(), rewrittenProto));
}
}
break;
@@ -298,9 +309,25 @@
}
break;
+ case INVOKE_POLYMORPHIC:
+ {
+ InvokePolymorphic invoke = current.asInvokePolymorphic();
+ // The invoked method is on java.lang.invoke.MethodHandle and always remains as is.
+ assert factory.polymorphicMethods.isPolymorphicInvoke(invoke.getInvokedMethod());
+ // Rewrite the signature of the handles actual target.
+ DexProto rewrittenProto = helper.computeIfAbsent().rewriteProto(invoke.getProto());
+ if (invoke.getProto() != rewrittenProto) {
+ iterator.replaceCurrentInstruction(
+ new InvokePolymorphic(
+ invoke.getInvokedMethod(),
+ rewrittenProto,
+ invoke.outValue(),
+ invoke.arguments()));
+ }
+ }
+ break;
case INVOKE_DIRECT:
case INVOKE_INTERFACE:
- case INVOKE_POLYMORPHIC:
case INVOKE_STATIC:
case INVOKE_SUPER:
case INVOKE_VIRTUAL:
diff --git a/src/test/java/com/android/tools/r8/cf/methodhandles/MethodHandleTest.java b/src/test/java/com/android/tools/r8/cf/methodhandles/MethodHandleTest.java
index 497e19f..423ffcd 100644
--- a/src/test/java/com/android/tools/r8/cf/methodhandles/MethodHandleTest.java
+++ b/src/test/java/com/android/tools/r8/cf/methodhandles/MethodHandleTest.java
@@ -126,8 +126,8 @@
divicMethod().invoke(i, 15, 'x');
assertEquals(42L, (long) dijicMethod().invoke(i, 16, 'x'));
constructorMethod().invoke(21);
- System.out.println(veType().parameterType(0).getName().lastIndexOf('.'));
- System.out.println(fType().returnType().getName().lastIndexOf('.'));
+ System.out.println(veType().parameterType(0).equals(E.class));
+ System.out.println(fType().returnType().equals(F.class));
} catch (Throwable e) {
throw new RuntimeException(e);
}
diff --git a/src/test/java/com/android/tools/r8/cf/methodhandles/MethodHandleTestRunner.java b/src/test/java/com/android/tools/r8/cf/methodhandles/MethodHandleTestRunner.java
index 1812854..4760c61 100644
--- a/src/test/java/com/android/tools/r8/cf/methodhandles/MethodHandleTestRunner.java
+++ b/src/test/java/com/android/tools/r8/cf/methodhandles/MethodHandleTestRunner.java
@@ -5,10 +5,8 @@
import static com.android.tools.r8.DiagnosticsMatcher.diagnosticType;
import static org.hamcrest.CoreMatchers.containsString;
-import static org.junit.Assert.assertThrows;
import static org.junit.Assume.assumeTrue;
-import com.android.tools.r8.CompilationFailedException;
import com.android.tools.r8.R8TestBuilder;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestDiagnosticMessages;
@@ -49,8 +47,8 @@
private String getExpected() {
return StringUtils.lines(
"C 42", "svi 1", "sji 2", "svic 3", "sjic 4", "vvi 5", "vji 6", "vvic 7", "vjic 8", "svi 9",
- "sji 10", "svic 11", "sjic 12", "dvi 13", "dji 14", "dvic 15", "djic 16", "C 21", "37",
- "37");
+ "sji 10", "svic 11", "sjic 12", "dvi 13", "dji 14", "dvic 15", "djic 16", "C 21", "true",
+ "true");
}
private final TestParameters parameters;
@@ -118,28 +116,34 @@
.addProgramClassFileData(getTransformedClasses())
.addLibraryFiles(ToolHelper.getMostRecentAndroidJar())
.addNoVerticalClassMergingAnnotations();
+ // TODO(b/237750295): The CONSTANT case should not need rules as they are not reflective uses.
if (minifyMode == MinifyMode.MINIFY) {
builder
.enableProguardTestOptions()
.addKeepMainRule(MethodHandleTest.class)
.addKeepRules(
- // Prevent the second argument of C.svic(), C.sjic(), I.sjic() and I.svic() from
- // being removed although they are never used unused. This is needed since these
- // methods are accessed reflectively.
"-keep,allowobfuscation public class " + typeName(C.class) + " {",
+ " void <init>(int);",
+ " static void svi(int);",
+ " static long sji(int);",
" static void svic(int, char);",
" static long sjic(int, char);",
+ " void vvi(int);",
+ " long vji(int);",
+ " void vvic(int, char);",
+ " long vjic(int, char);",
"}",
"-keep,allowobfuscation public interface " + typeName(I.class) + " {",
- " static long sjic(int, char);",
+ " static void svi(int);",
+ " static long sji(int);",
" static void svic(int, char);",
+ " static long sjic(int, char);",
+ " void dvi(int);",
+ " long dji(int);",
+ " void dvic(int, char);",
+ " long djic(int, char);",
"}");
- // TODO(b/235810300): The compiler fails with assertion in AppInfoWithLiveness.
- if (lookupType == LookupType.CONSTANT && hasConstMethodCompileSupport()) {
- builder.allowDiagnosticMessages();
- assertThrows(CompilationFailedException.class, builder::compile);
- return;
- }
+
} else {
builder.noTreeShaking();
builder.noMinification();
