VarHandle desugar: Enable testing for R8
* Do not read VarHandle and MethodHandles$Lookup from library
* Handle all cases where the synthetic classes can be encountered by R8
* Adjusted stack heights for desugar code
Bug: b/247076137
Change-Id: I54621d9152b066f2c1f66227f372396e65884eac
diff --git a/src/main/java/com/android/tools/r8/graph/ApplicationReaderMap.java b/src/main/java/com/android/tools/r8/graph/ApplicationReaderMap.java
index 34898725..8e51a2e 100644
--- a/src/main/java/com/android/tools/r8/graph/ApplicationReaderMap.java
+++ b/src/main/java/com/android/tools/r8/graph/ApplicationReaderMap.java
@@ -8,8 +8,6 @@
public abstract class ApplicationReaderMap {
- public static ApplicationReaderMap INSTANCE;
-
public abstract String getDescriptor(String descriptor);
public abstract DexType getType(DexType type);
diff --git a/src/main/java/com/android/tools/r8/graph/JarApplicationReader.java b/src/main/java/com/android/tools/r8/graph/JarApplicationReader.java
index 93e56a7..dbdaeb1 100644
--- a/src/main/java/com/android/tools/r8/graph/JarApplicationReader.java
+++ b/src/main/java/com/android/tools/r8/graph/JarApplicationReader.java
@@ -215,6 +215,31 @@
return methodHandlesLookupWitnesses;
}
+ public void checkClassForMethodHandlesLookup(DexClass dexClass, ClassKind<?> classKind) {
+ if (options.shouldDesugarVarHandle()) {
+ if (VarHandleDesugaring.refersToMethodHandlesLookup(dexClass.getType(), getFactory())) {
+ addVarHandleWitness(dexClass.getType(), classKind);
+ }
+ dexClass
+ .getInnerClasses()
+ .forEach(
+ attribute -> {
+ // MethodHandles$Lookup has no inner classes.
+ assert !VarHandleDesugaring.refersToMethodHandlesLookup(
+ attribute.getOuter(), getFactory());
+ if (VarHandleDesugaring.refersToMethodHandlesLookup(
+ attribute.getInner(), getFactory())) {
+ addMethodHandlesLookupWitness(dexClass.getType(), classKind);
+ // When the inner MethodHandles$Lookup is present the outer is MethodHandles, and
+ // in that case the enqueuer will process all methods in the library class
+ // MethodHandles which have references to VarHandle.
+ assert attribute.getOuter() == getFactory().methodHandlesType;
+ addVarHandleWitness(dexClass.getType(), classKind);
+ }
+ });
+ }
+ }
+
public void checkFieldForMethodHandlesLookup(DexField dexField, ClassKind<?> classKind) {
if (options.shouldDesugarVarHandle()
&& VarHandleDesugaring.refersToMethodHandlesLookup(dexField, getFactory())) {
diff --git a/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java b/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
index fe8d9e4..76ca9ac 100644
--- a/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
+++ b/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
@@ -487,6 +487,7 @@
application.getFactory().getSkipNameValidationForTesting(),
getChecksumSupplier(classKind),
syntheticMarker);
+ application.checkClassForMethodHandlesLookup(clazz, classKind);
InnerClassAttribute innerClassAttribute = clazz.getInnerClassAttributeForThisClass();
// A member class should not be a local or anonymous class.
if (innerClassAttribute != null && innerClassAttribute.getOuter() != null) {
diff --git a/src/main/java/com/android/tools/r8/graph/LazyLoadedDexApplication.java b/src/main/java/com/android/tools/r8/graph/LazyLoadedDexApplication.java
index 80bb4cf..ee98d51 100644
--- a/src/main/java/com/android/tools/r8/graph/LazyLoadedDexApplication.java
+++ b/src/main/java/com/android/tools/r8/graph/LazyLoadedDexApplication.java
@@ -22,6 +22,7 @@
import java.util.Set;
import java.util.function.Consumer;
import java.util.function.Function;
+import java.util.function.Predicate;
import java.util.stream.Collectors;
public class LazyLoadedDexApplication extends DexApplication {
@@ -140,10 +141,18 @@
ProgramClassCollection programClassesLoader,
InternalOptions options) {
+ // When desugaring VarHandle do not read the VarHandle and MethodHandles$Lookup classes
+ // from the library as they will be synthesized during desugaring.
+ Predicate<DexType> forceLoadPredicate =
+ type ->
+ !(options.shouldDesugarVarHandle()
+ && (type == options.dexItemFactory().varHandleType
+ || type == options.dexItemFactory().lookupType));
+
// Force-load library classes.
ImmutableMap<DexType, DexLibraryClass> allLibraryClasses;
if (libraryClassesLoader != null) {
- libraryClassesLoader.forceLoad(type -> true);
+ libraryClassesLoader.forceLoad(forceLoadPredicate);
allLibraryClasses = libraryClassesLoader.getAllClassesInMap();
} else {
allLibraryClasses = ImmutableMap.of();
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/varhandle/VarHandleDesugaring.java b/src/main/java/com/android/tools/r8/ir/desugar/varhandle/VarHandleDesugaring.java
index 6aaf59d..3972a88 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/varhandle/VarHandleDesugaring.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/varhandle/VarHandleDesugaring.java
@@ -133,7 +133,7 @@
return refersToVarHandle(field.type, factory);
}
- private static boolean refersToMethodHandlesLookup(DexType type, DexItemFactory factory) {
+ public static boolean refersToMethodHandlesLookup(DexType type, DexItemFactory factory) {
if (type == factory.desugarMethodHandlesLookupType) {
// All references to java.lang.invoke.MethodHandles$Lookup is rewritten during application
// writing.
@@ -251,6 +251,7 @@
&& holder != factory.varHandleType) {
return DesugarDescription.nothing();
}
+
DexMethod method = invoke.getMethod();
if (method.getHolderType() == factory.methodHandlesType) {
if (method.getName().equals(factory.createString("lookup"))
@@ -306,17 +307,20 @@
eventConsumer,
context,
methodProcessingContext,
- dexItemFactory) ->
- ImmutableList.of(
- new CfNew(factory.lookupType),
- new CfStackInstruction(Opcode.Dup),
- new CfInvoke(
- Opcodes.INVOKESPECIAL,
- factory.createMethod(
- factory.lookupType,
- factory.createProto(factory.voidType),
- factory.constructorMethodName),
- false)))
+ dexItemFactory) -> {
+ ensureMethodHandlesLookupClass(eventConsumer, context);
+ localStackAllocator.allocateLocalStack(2);
+ return ImmutableList.of(
+ new CfNew(factory.lookupType),
+ new CfStackInstruction(Opcode.Dup),
+ new CfInvoke(
+ Opcodes.INVOKESPECIAL,
+ factory.createMethod(
+ factory.lookupType,
+ factory.createProto(factory.voidType),
+ factory.constructorMethodName),
+ false));
+ })
.build();
}
@@ -328,18 +332,20 @@
eventConsumer,
context,
methodProcessingContext,
- dexItemFactory) ->
- ImmutableList.of(
- new CfNew(factory.varHandleType),
- new CfStackInstruction(Opcode.DupX1),
- new CfStackInstruction(Opcode.Swap),
- new CfInvoke(
- Opcodes.INVOKESPECIAL,
- factory.createMethod(
- factory.varHandleType,
- factory.createProto(factory.voidType, factory.classType),
- factory.constructorMethodName),
- false)))
+ dexItemFactory) -> {
+ localStackAllocator.allocateLocalStack(2);
+ return ImmutableList.of(
+ new CfNew(factory.varHandleType),
+ new CfStackInstruction(Opcode.DupX1),
+ new CfStackInstruction(Opcode.Swap),
+ new CfInvoke(
+ Opcodes.INVOKESPECIAL,
+ factory.createMethod(
+ factory.varHandleType,
+ factory.createProto(factory.voidType, factory.classType),
+ factory.constructorMethodName),
+ false));
+ })
.build();
}
@@ -352,8 +358,11 @@
eventConsumer,
context,
methodProcessingContext,
- dexItemFactory) ->
- desugarSignaturePolymorphicMethod(invoke, coordinates, freshLocalProvider))
+ dexItemFactory) -> {
+ ensureVarHandleClass(eventConsumer, context);
+ return desugarSignaturePolymorphicMethod(
+ invoke, coordinates, freshLocalProvider, localStackAllocator);
+ })
.build();
}
@@ -370,7 +379,10 @@
}
private Collection<CfInstruction> desugarSignaturePolymorphicMethod(
- CfInvoke invoke, int coordinates, FreshLocalProvider freshLocalProvider) {
+ CfInvoke invoke,
+ int coordinates,
+ FreshLocalProvider freshLocalProvider,
+ LocalStackAllocator localStackAllocator) {
assert invoke.isInvokeVirtual();
// TODO(b/247076137): Support two coordinates (array element VarHandle).
assert (coordinates == 1 || coordinates == 2)
@@ -462,11 +474,13 @@
if (!lastArgument) {
if (hasWideArgument) {
local = freshLocalProvider.getFreshLocal(2);
+ localStackAllocator.allocateLocalStack(1);
builder.add(new CfStore(ValueType.fromDexType(proto.parameters.get(i + 1)), local));
} else {
builder.add(new CfStackInstruction(Opcode.Swap));
}
}
+ localStackAllocator.allocateLocalStack(1);
builder.add(
new CfInvoke(
Opcodes.INVOKESTATIC,
@@ -486,6 +500,7 @@
}
assert newParameters.size() == proto.parameters.size();
if (proto.returnType != returnType) {
+ localStackAllocator.allocateLocalStack(1);
if (proto.returnType.isPrimitiveType()) {
builder.add(new CfConstClass(factory.getBoxedForPrimitiveType(proto.returnType)));
} else {
@@ -498,6 +513,7 @@
builder.add(new CfInvoke(Opcodes.INVOKEVIRTUAL, newMethod, false));
if (proto.returnType.isPrimitiveType() && !newProto.returnType.isPrimitiveType()) {
assert newProto.returnType == factory.objectType;
+ localStackAllocator.allocateLocalStack(2);
builder.add(new CfCheckCast(factory.getBoxedForPrimitiveType(proto.returnType)));
builder.add(
new CfInvoke(
@@ -505,6 +521,7 @@
} else if (proto.returnType.isClassType()
&& proto.returnType != factory.objectType
&& proto.returnType != factory.voidType) {
+ localStackAllocator.allocateLocalStack(1);
builder.add(new CfCheckCast(proto.returnType));
}
return builder.build();
diff --git a/src/test/examplesJava9/varhandle/NoDesugaredTypesInSignatures.java b/src/test/examplesJava9/varhandle/NoDesugaredTypesInSignatures.java
new file mode 100644
index 0000000..0e80d0a
--- /dev/null
+++ b/src/test/examplesJava9/varhandle/NoDesugaredTypesInSignatures.java
@@ -0,0 +1,23 @@
+// Copyright (c) 2023, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package varhandle;
+
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.VarHandle;
+
+public class NoDesugaredTypesInSignatures {
+
+ private int field;
+
+ public static void main(String[] args) throws NoSuchFieldException, IllegalAccessException {
+ // Code where references to VarHandle and MethodHandles$Lookup is only in code and not in any
+ // signature. javac will still add references in InnerClasses attributes.
+ VarHandle varHandle =
+ MethodHandles.lookup()
+ .findVarHandle(NoDesugaredTypesInSignatures.class, "field", int.class);
+
+ NoDesugaredTypesInSignatures instance = new NoDesugaredTypesInSignatures();
+ System.out.println((int) varHandle.get(instance));
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/cf/varhandle/VarHandleDesugaringTestBase.java b/src/test/java/com/android/tools/r8/cf/varhandle/VarHandleDesugaringTestBase.java
index d8b0543..4ea3657 100644
--- a/src/test/java/com/android/tools/r8/cf/varhandle/VarHandleDesugaringTestBase.java
+++ b/src/test/java/com/android/tools/r8/cf/varhandle/VarHandleDesugaringTestBase.java
@@ -10,7 +10,6 @@
import static org.junit.Assume.assumeTrue;
import com.android.tools.r8.JdkClassFileProvider;
-import com.android.tools.r8.R8TestBuilder;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestParametersCollection;
@@ -74,7 +73,7 @@
@Test
public void testReference() throws Throwable {
- assumeTrue(parameters.isCfRuntime());
+ assumeTrue(parameters.isCfRuntime() && parameters.asCfRuntime().isNewerThanOrEqual(CfVm.JDK9));
testForJvm()
.addProgramFiles(VarHandle.jar())
.run(parameters.getRuntime(), getMainClass())
@@ -197,21 +196,8 @@
.inspect(this::inspect);
}
- // TODO(b/247076137: Also turn on VarHandle desugaring for R8 tests.
@Test
public void testR8() throws Throwable {
- // TODO(b/247076137: The "default" VM is acting up on some tests - skip these as they will
- // be fixed when VarHandle desugaring is enabled for R8.
- if (parameters.isDexRuntime()
- && parameters.asDexRuntime().getVersion().isEqualTo(Version.DEFAULT)
- && parameters.getApiLevel().equals(AndroidApiLevel.B)
- && (this instanceof VarHandleDesugaringInstanceBooleanFieldTest
- || this instanceof VarHandleDesugaringInstanceByteFieldTest
- || this instanceof VarHandleDesugaringInstanceShortFieldTest
- || this instanceof VarHandleDesugaringInstanceFloatFieldTest
- || this instanceof VarHandleDesugaringInstanceDoubleFieldTest)) {
- return;
- }
testForR8(parameters.getBackend())
.applyIf(
parameters.isDexRuntime(),
@@ -220,26 +206,25 @@
// Use system JDK to have references types including StringConcatFactory.
b -> b.addLibraryProvider(JdkClassFileProvider.fromSystemJdk()))
.addProgramClassFileData(getProgramClassFileData())
+ .addOptionsModification(options -> options.enableVarHandleDesugaring = true)
.setMinApi(parameters.getApiLevel())
.addKeepMainRule(getMainClass())
.addKeepRules(getKeepRules())
- .applyIf(
- parameters.isDexRuntime() && parameters.getApiLevel().isLessThan(AndroidApiLevel.O),
- R8TestBuilder::allowDiagnosticWarningMessages)
.run(parameters.getRuntime(), getMainClass())
.applyIf(
- // VarHandle is available from Android 9, even though it was not a public API until 13.
parameters.isDexRuntime()
- && parameters.asDexRuntime().getVersion().isOlderThanOrEqual(Version.V7_0_0),
- r -> r.assertFailureWithErrorThatThrows(NoClassDefFoundError.class),
- parameters.isDexRuntime()
- && (parameters.getApiLevel().isLessThan(AndroidApiLevel.P)
- || parameters.asDexRuntime().getVersion().isOlderThanOrEqual(Version.V8_1_0)),
- r -> r.assertFailure(),
+ && parameters.asDexRuntime().getVersion().isOlderThanOrEqual(Version.V4_4_4),
+ // TODO(b/247076137): Running on 4.0.4 and 4.4.4 needs to be checked. Output seems
+ // correct, but at the same time there are VFY errors on stderr.
+ r -> r.assertFailureWithErrorThatThrows(NoSuchFieldException.class),
r ->
r.assertSuccessWithOutput(
- parameters.isCfRuntime()
- ? getExpectedOutputForReferenceImplementation()
- : getExpectedOutputForArtImplementation()));
+ parameters.isDexRuntime()
+ && parameters.getApiLevel().isGreaterThanOrEqualTo(AndroidApiLevel.T)
+ && parameters.getDexRuntimeVersion().isNewerThanOrEqual(Version.V13_0_0)
+ ? getExpectedOutputForArtImplementation()
+ : (parameters.isDexRuntime()
+ ? getExpectedOutputForDesugaringImplementation()
+ : getExpectedOutputForReferenceImplementation())));
}
}
diff --git a/src/test/java/com/android/tools/r8/cf/varhandle/VarHandleNoDesugaredTypesInSignaturesNoAttributesTest.java b/src/test/java/com/android/tools/r8/cf/varhandle/VarHandleNoDesugaredTypesInSignaturesNoAttributesTest.java
new file mode 100644
index 0000000..872febb
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/cf/varhandle/VarHandleNoDesugaredTypesInSignaturesNoAttributesTest.java
@@ -0,0 +1,88 @@
+// Copyright (c) 2023, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.cf.varhandle;
+
+import static org.junit.Assert.assertTrue;
+
+import com.android.tools.r8.JdkClassFileProvider;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.TestRuntime.CfVm;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.ToolHelper.DexVm.Version;
+import com.android.tools.r8.examples.jdk9.VarHandle;
+import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.ZipUtils;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import java.nio.file.Path;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class VarHandleNoDesugaredTypesInSignaturesNoAttributesTest extends TestBase {
+
+ private static final String EXPECTED_OUTPUT = StringUtils.lines("0");
+ private static final String MAIN_CLASS = VarHandle.NoDesugaredTypesInSignatures.typeName();
+ private static final String JAR_ENTRY = "varhandle/NoDesugaredTypesInSignatures.class";
+
+ @Parameter(0)
+ public TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters()
+ .withCfRuntimesStartingFromIncluding(CfVm.JDK9)
+ // Running on 4.0.4 and 4.4.4 needs to be checked. Output seems correct, but at the
+ // same time there are VFY errors on stderr.
+ .withDexRuntimesStartingFromExcluding(Version.V4_4_4)
+ .withAllApiLevels()
+ .build();
+ }
+
+ @Test
+ public void testR8() throws Throwable {
+ // Strip the attributes (including InnerClasses) to ensure that the enqueuer does not
+ // register references to MethodHandles$Lookup until the desugar step.
+ Path programWithoutAttributes =
+ testForR8(Backend.CF)
+ .addLibraryProvider(JdkClassFileProvider.fromSystemJdk())
+ .addProgramClassFileData(ZipUtils.readSingleEntry(VarHandle.jar(), JAR_ENTRY))
+ .addKeepClassAndMembersRules(MAIN_CLASS)
+ .compile()
+ .writeToZip();
+ assertTrue(
+ new CodeInspector(programWithoutAttributes)
+ .clazz(MAIN_CLASS)
+ .getDexProgramClass()
+ .getInnerClasses()
+ .isEmpty());
+
+ testForR8(parameters.getBackend())
+ .applyIf(
+ parameters.isDexRuntime(),
+ // Use android.jar from Android T to get the VarHandle type.
+ b -> b.addLibraryFiles(ToolHelper.getAndroidJar(AndroidApiLevel.T)),
+ // Use system JDK to have references types including StringConcatFactory.
+ b -> b.addLibraryProvider(JdkClassFileProvider.fromSystemJdk()))
+ .addProgramFiles(programWithoutAttributes)
+ .addOptionsModification(options -> options.enableVarHandleDesugaring = true)
+ .setMinApi(parameters.getApiLevel())
+ .addKeepMainRule(MAIN_CLASS)
+ .addKeepRules("-keep class " + MAIN_CLASS + "{ <fields>; }")
+ .run(parameters.getRuntime(), MAIN_CLASS)
+ .applyIf(
+ parameters.isDexRuntime()
+ && parameters.asDexRuntime().getVersion().isOlderThanOrEqual(Version.V4_4_4),
+ // TODO(b/247076137): Running on 4.0.4 and 4.4.4 needs to be checked. Output seems
+ // correct, but at the same time there are VFY errors on stderr.
+ r -> r.assertFailureWithErrorThatThrows(NoSuchFieldException.class),
+ r -> r.assertSuccessWithOutput(EXPECTED_OUTPUT));
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/cf/varhandle/VarHandleNoDesugaredTypesInSignaturesTest.java b/src/test/java/com/android/tools/r8/cf/varhandle/VarHandleNoDesugaredTypesInSignaturesTest.java
new file mode 100644
index 0000000..18ad33b
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/cf/varhandle/VarHandleNoDesugaredTypesInSignaturesTest.java
@@ -0,0 +1,40 @@
+// Copyright (c) 2023, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.cf.varhandle;
+
+import com.android.tools.r8.examples.jdk9.VarHandle;
+import com.android.tools.r8.utils.StringUtils;
+import com.google.common.collect.ImmutableList;
+import java.util.List;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class VarHandleNoDesugaredTypesInSignaturesTest extends VarHandleDesugaringTestBase {
+
+ private static final String EXPECTED_OUTPUT = StringUtils.lines("0");
+ private static final String MAIN_CLASS = VarHandle.NoDesugaredTypesInSignatures.typeName();
+ private static final String JAR_ENTRY = "varhandle/NoDesugaredTypesInSignatures.class";
+
+ @Override
+ protected String getMainClass() {
+ return MAIN_CLASS;
+ }
+
+ @Override
+ protected String getKeepRules() {
+ return "-keep class " + getMainClass() + "{ <fields>; }";
+ }
+
+ @Override
+ protected List<String> getJarEntries() {
+ return ImmutableList.of(JAR_ENTRY);
+ }
+
+ @Override
+ protected String getExpectedOutputForReferenceImplementation() {
+ return EXPECTED_OUTPUT;
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/examples/jdk9/VarHandle.java b/src/test/java/com/android/tools/r8/examples/jdk9/VarHandle.java
index cb2caed..152c6e1 100644
--- a/src/test/java/com/android/tools/r8/examples/jdk9/VarHandle.java
+++ b/src/test/java/com/android/tools/r8/examples/jdk9/VarHandle.java
@@ -52,6 +52,9 @@
public static final JavaExampleClassProxy InstanceStringField =
new JavaExampleClassProxy(EXAMPLE_FILE, "varhandle/InstanceStringField");
+ public static final JavaExampleClassProxy NoDesugaredTypesInSignatures =
+ new JavaExampleClassProxy(EXAMPLE_FILE, "varhandle/NoDesugaredTypesInSignatures");
+
public static Path jar() {
return JavaExampleClassProxy.examplesJar(EXAMPLE_FILE);
}
