Handle distinct symbolic reference to a dynamically-computed constants
A class file can have multiple dynamically computed constants using the
exact same BSM and arguments.
Keep track of the individual symbolic references instead of comparing
BSM and arguments.
Fixes: b/272346803
Change-Id: I068c4009628cba5ca88c0d049023d7505f06adcd
diff --git a/src/main/java/com/android/tools/r8/cf/code/CfConstDynamic.java b/src/main/java/com/android/tools/r8/cf/code/CfConstDynamic.java
index 6c379bb..9b9b2b1 100644
--- a/src/main/java/com/android/tools/r8/cf/code/CfConstDynamic.java
+++ b/src/main/java/com/android/tools/r8/cf/code/CfConstDynamic.java
@@ -46,17 +46,21 @@
private final ConstantDynamicReference reference;
public CfConstDynamic(
+ int symbolicReferenceId,
DexString name,
DexType type,
DexMethodHandle bootstrapMethod,
List<DexValue> bootstrapMethodArguments) {
+ assert symbolicReferenceId >= 0;
assert name != null;
assert type != null;
assert bootstrapMethod != null;
assert bootstrapMethodArguments != null;
assert bootstrapMethodArguments.isEmpty();
- reference = new ConstantDynamicReference(name, type, bootstrapMethod, bootstrapMethodArguments);
+ reference =
+ new ConstantDynamicReference(
+ symbolicReferenceId, name, type, bootstrapMethod, bootstrapMethodArguments);
}
@Override
@@ -86,7 +90,10 @@
}
public static CfConstDynamic fromAsmConstantDynamic(
- ConstantDynamic insn, JarApplicationReader application, DexType clazz) {
+ int symbolicReferenceId,
+ ConstantDynamic insn,
+ JarApplicationReader application,
+ DexType clazz) {
String constantName = insn.getName();
String constantDescriptor = insn.getDescriptor();
DexMethodHandle bootstrapMethodHandle =
@@ -99,6 +106,7 @@
bootstrapMethodArguments.add(dexValue);
}
return new CfConstDynamic(
+ symbolicReferenceId,
application.getString(constantName),
application.getTypeFromDescriptor(constantDescriptor),
bootstrapMethodHandle,
diff --git a/src/main/java/com/android/tools/r8/graph/LazyCfCode.java b/src/main/java/com/android/tools/r8/graph/LazyCfCode.java
index 45fed00..153f85f 100644
--- a/src/main/java/com/android/tools/r8/graph/LazyCfCode.java
+++ b/src/main/java/com/android/tools/r8/graph/LazyCfCode.java
@@ -79,6 +79,8 @@
import it.unimi.dsi.fastutil.ints.Int2ReferenceMap;
import it.unimi.dsi.fastutil.ints.IntArrayList;
import it.unimi.dsi.fastutil.ints.IntList;
+import it.unimi.dsi.fastutil.objects.Reference2IntArrayMap;
+import it.unimi.dsi.fastutil.objects.Reference2IntMap;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
@@ -86,6 +88,7 @@
import java.util.List;
import java.util.Map;
import java.util.function.BiFunction;
+import java.util.function.Supplier;
import org.objectweb.asm.ClassReader;
import org.objectweb.asm.ClassVisitor;
import org.objectweb.asm.ConstantDynamic;
@@ -334,6 +337,7 @@
private boolean usrJsrInliner;
private final Origin origin;
private final DebugParsingOptions debugParsingOptions;
+ private Reference2IntMap<ConstantDynamic> constantDynamicSymbolicReferences;
ClassCodeVisitor(
DexClass clazz,
@@ -351,6 +355,13 @@
this.debugParsingOptions = debugParsingOptions;
}
+ private Reference2IntMap<ConstantDynamic> getConstantDynamicSymbolicReferences() {
+ if (constantDynamicSymbolicReferences == null) {
+ constantDynamicSymbolicReferences = new Reference2IntArrayMap<>();
+ }
+ return constantDynamicSymbolicReferences;
+ }
+
@Override
public MethodVisitor visitMethod(
int access, String name, String desc, String signature, String[] exceptions) {
@@ -360,7 +371,13 @@
if (code != null) {
DexMethod method = application.getMethod(clazz.type, name, desc);
MethodCodeVisitor methodVisitor =
- new MethodCodeVisitor(application, method, code, origin, debugParsingOptions);
+ new MethodCodeVisitor(
+ application,
+ method,
+ code,
+ origin,
+ debugParsingOptions,
+ this::getConstantDynamicSymbolicReferences);
if (!usrJsrInliner) {
return methodVisitor;
}
@@ -391,13 +408,16 @@
private final Origin origin;
private int minLine = Integer.MAX_VALUE;
private int maxLine = -1;
+ private final Supplier<Reference2IntMap<ConstantDynamic>>
+ constantDynamicSymbolicReferencesSupplier;
MethodCodeVisitor(
JarApplicationReader application,
DexMethod method,
LazyCfCode code,
Origin origin,
- DebugParsingOptions debugParsingOptions) {
+ DebugParsingOptions debugParsingOptions,
+ Supplier<Reference2IntMap<ConstantDynamic>> constantDynamicSymbolicReferencesSupplier) {
super(InternalOptions.ASM_VERSION);
this.debugParsingOptions = debugParsingOptions;
assert code != null;
@@ -406,6 +426,7 @@
this.code = code;
this.method = method;
this.origin = origin;
+ this.constantDynamicSymbolicReferencesSupplier = constantDynamicSymbolicReferencesSupplier;
}
private void addInstruction(CfInstruction instruction) {
@@ -1004,9 +1025,20 @@
new CfConstMethodHandle(
DexMethodHandle.fromAsmHandle((Handle) cst, application, method.holder)));
} else if (cst instanceof ConstantDynamic) {
+ // Each symbolic reference to a dynamically-computed constant has a unique ConstantDynamic
+ // instance from ASM, even when they are equal (i.e. all their components are equal). See
+ // ConstantDynamicMultipleConstantsWithDifferentSymbolicReferenceUsingSameBSMAndArgumentsTest
+ // for an example.
+ Reference2IntMap<ConstantDynamic> constantDynamicSymbolicReferences =
+ constantDynamicSymbolicReferencesSupplier.get();
+ int symbolicReferenceId = constantDynamicSymbolicReferences.getOrDefault(cst, -1);
+ if (symbolicReferenceId == -1) {
+ symbolicReferenceId = constantDynamicSymbolicReferences.size();
+ constantDynamicSymbolicReferences.put((ConstantDynamic) cst, symbolicReferenceId);
+ }
addInstruction(
CfConstDynamic.fromAsmConstantDynamic(
- (ConstantDynamic) cst, application, method.holder));
+ symbolicReferenceId, (ConstantDynamic) cst, application, method.holder));
} else {
throw new CompilationError("Unsupported constant: " + cst.toString());
}
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/constantdynamic/ConstantDynamicReference.java b/src/main/java/com/android/tools/r8/ir/desugar/constantdynamic/ConstantDynamicReference.java
index ed08c4d..478e4cf 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/constantdynamic/ConstantDynamicReference.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/constantdynamic/ConstantDynamicReference.java
@@ -15,24 +15,24 @@
import java.util.Objects;
public class ConstantDynamicReference implements StructuralItem<ConstantDynamicReference> {
+ private final int symbolicReferenceId;
private final DexString name;
private final DexType type;
private final DexMethodHandle bootstrapMethod;
private final List<DexValue> bootstrapMethodArguments;
private static void specify(StructuralSpecification<ConstantDynamicReference, ?> spec) {
- spec.withItem(ConstantDynamicReference::getName)
- .withItem(ConstantDynamicReference::getType)
- .withItem(ConstantDynamicReference::getBootstrapMethod)
- .withItemCollection(ConstantDynamicReference::getBootstrapMethodArguments);
+ spec.withInt(c -> c.symbolicReferenceId);
}
public ConstantDynamicReference(
+ int symbolicReferenceId,
DexString name,
DexType type,
DexMethodHandle bootstrapMethod,
List<DexValue> bootstrapMethodArguments) {
assert bootstrapMethodArguments.isEmpty();
+ this.symbolicReferenceId = symbolicReferenceId;
this.name = name;
this.type = type;
this.bootstrapMethod = bootstrapMethod;
diff --git a/src/main/java/com/android/tools/r8/synthesis/SyntheticNaming.java b/src/main/java/com/android/tools/r8/synthesis/SyntheticNaming.java
index 95105f8..301d7b4 100644
--- a/src/main/java/com/android/tools/r8/synthesis/SyntheticNaming.java
+++ b/src/main/java/com/android/tools/r8/synthesis/SyntheticNaming.java
@@ -65,7 +65,7 @@
// TODO(b/214901256): Sharing of synthetic classes may lead to duplicate method errors.
public final SyntheticKind NON_FIXED_INIT_TYPE_ARGUMENT =
generator.forNonSharableInstanceClass("$IA");
- public final SyntheticKind CONST_DYNAMIC = generator.forInstanceClass("$Condy");
+ public final SyntheticKind CONST_DYNAMIC = generator.forNonSharableInstanceClass("$Condy");
// Method synthetics.
public final SyntheticKind ENUM_UNBOXING_CHECK_NOT_ZERO_METHOD =
diff --git a/src/test/java/com/android/tools/r8/desugar/constantdynamic/ConstantDynamicMultipleConstantsUsingSameSymbolicReferenceTest.java b/src/test/java/com/android/tools/r8/desugar/constantdynamic/ConstantDynamicMultipleConstantsUsingSameSymbolicReferenceTest.java
new file mode 100644
index 0000000..5ba0396
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/desugar/constantdynamic/ConstantDynamicMultipleConstantsUsingSameSymbolicReferenceTest.java
@@ -0,0 +1,141 @@
+// 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.desugar.constantdynamic;
+
+import static com.android.tools.r8.DiagnosticsMatcher.diagnosticMessage;
+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.DesugarTestConfiguration;
+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.cf.CfVersion;
+import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.StringUtils;
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+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 ConstantDynamicMultipleConstantsUsingSameSymbolicReferenceTest extends TestBase {
+
+ @Parameter() public TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimes().withAllApiLevelsAlsoForCf().build();
+ }
+
+ private static final Class<?> MAIN_CLASS = Main.class;
+ private static final String EXPECTED_OUTPUT = StringUtils.lines("true");
+
+ @Test
+ public void testReference() throws Exception {
+ parameters.assumeJvmTestParameters();
+ assumeTrue(parameters.getRuntime().asCf().isNewerThanOrEqual(CfVm.JDK11));
+ testForJvm(parameters)
+ .addProgramClassFileData(getTransformedClasses())
+ .addProgramClasses(Main.class)
+ .run(parameters.getRuntime(), MAIN_CLASS)
+ .assertSuccessWithOutput(EXPECTED_OUTPUT);
+ }
+
+ @Test
+ public void testDesugaring() throws Exception {
+ testForDesugaring(parameters)
+ .addProgramClassFileData(getTransformedClasses())
+ .addProgramClasses(Main.class)
+ .run(parameters.getRuntime(), MAIN_CLASS)
+ .applyIf(
+ // When not desugaring the CF code requires JDK 11.
+ DesugarTestConfiguration::isNotDesugared,
+ r -> {
+ if (parameters.isCfRuntime()
+ && parameters.getRuntime().asCf().isNewerThanOrEqual(CfVm.JDK11)) {
+ r.assertSuccessWithOutput(EXPECTED_OUTPUT);
+ } else {
+ r.assertFailureWithErrorThatThrows(UnsupportedClassVersionError.class);
+ }
+ })
+ .applyIf(
+ DesugarTestConfiguration::isDesugared, r -> r.assertSuccessWithOutput(EXPECTED_OUTPUT));
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ parameters.assumeR8TestParameters();
+ testForR8(parameters.getBackend())
+ .addProgramClassFileData(getTransformedClasses())
+ .addProgramClasses(Main.class)
+ .setMinApi(parameters)
+ .addKeepMainRule(MAIN_CLASS)
+ // TODO(b/198142613): There should not be a warnings on class references which are
+ // desugared away.
+ .applyIf(
+ parameters.getApiLevel().isLessThan(AndroidApiLevel.O),
+ b -> b.addDontWarn("java.lang.invoke.MethodHandles$Lookup"))
+ // TODO(b/198142625): Support CONSTANT_Dynamic output for class files.
+ .applyIf(
+ parameters.isCfRuntime(),
+ r -> {
+ assertThrows(
+ CompilationFailedException.class,
+ () ->
+ r.compileWithExpectedDiagnostics(
+ diagnostics -> {
+ diagnostics.assertOnlyErrors();
+ diagnostics.assertErrorsMatch(
+ diagnosticMessage(
+ containsString(
+ "Unsupported dynamic constant (not desugaring)")));
+ }));
+ },
+ r ->
+ r.run(parameters.getRuntime(), MAIN_CLASS)
+ .assertSuccessWithOutput(EXPECTED_OUTPUT));
+ }
+
+ // When ASM writes two dynamic constants with the same BSM and arguments they are canonicialized
+ // into the same symbolic reference.
+ private byte[] getTransformedClasses() throws IOException {
+ return transformer(A.class)
+ .setVersion(CfVersion.V11)
+ .transformConstStringToConstantDynamic(
+ "condy1", A.class, "myConstant", false, "constantName", Object.class)
+ .transformConstStringToConstantDynamic(
+ "condy2", A.class, "myConstant", false, "constantName", Object.class)
+ .transform();
+ }
+
+ public static class Main {
+
+ public static void main(String[] args) {
+ // This prints "true" due to the ASM canonicalization of the ConstantDynamic when writing.
+ System.out.println(A.getConstant1() == A.getConstant2());
+ }
+ }
+
+ public static class A {
+
+ public static Object getConstant1() {
+ return "condy1"; // Will be transformed to Constant_DYNAMIC.
+ }
+
+ public static Object getConstant2() {
+ return "condy2"; // Will be transformed to Constant_DYNAMIC.
+ }
+
+ private static Object myConstant(MethodHandles.Lookup lookup, String name, Class<?> type) {
+ return new Object();
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/desugar/constantdynamic/ConstantDynamicMultipleConstantsWithDifferentSymbolicReferenceUsingSameBSMAndArgumentsTest.java b/src/test/java/com/android/tools/r8/desugar/constantdynamic/ConstantDynamicMultipleConstantsWithDifferentSymbolicReferenceUsingSameBSMAndArgumentsTest.java
index 9947216..3bf967d 100644
--- a/src/test/java/com/android/tools/r8/desugar/constantdynamic/ConstantDynamicMultipleConstantsWithDifferentSymbolicReferenceUsingSameBSMAndArgumentsTest.java
+++ b/src/test/java/com/android/tools/r8/desugar/constantdynamic/ConstantDynamicMultipleConstantsWithDifferentSymbolicReferenceUsingSameBSMAndArgumentsTest.java
@@ -38,7 +38,6 @@
private static final String MAIN_CLASS = "A";
private static final String EXPECTED_OUTPUT = StringUtils.lines("false");
- private static final String UNEXPECTED_OUTPUT = StringUtils.lines("true");
@Test
public void testReference() throws Exception {
@@ -68,7 +67,7 @@
}
})
.applyIf(
- DesugarTestConfiguration::isDesugared, r -> r.assertSuccessWithOutput(UNEXPECTED_OUTPUT));
+ DesugarTestConfiguration::isDesugared, r -> r.assertSuccessWithOutput(EXPECTED_OUTPUT));
}
@Test
@@ -101,7 +100,7 @@
},
r ->
r.run(parameters.getRuntime(), MAIN_CLASS)
- .assertSuccessWithOutput(UNEXPECTED_OUTPUT));
+ .assertSuccessWithOutput(EXPECTED_OUTPUT));
}
/*
diff --git a/src/test/java/com/android/tools/r8/desugar/constantdynamic/ConstantDynamicRegress272346803Test.java b/src/test/java/com/android/tools/r8/desugar/constantdynamic/ConstantDynamicRegress272346803Test.java
index 4d8d477..bec0148 100644
--- a/src/test/java/com/android/tools/r8/desugar/constantdynamic/ConstantDynamicRegress272346803Test.java
+++ b/src/test/java/com/android/tools/r8/desugar/constantdynamic/ConstantDynamicRegress272346803Test.java
@@ -39,7 +39,6 @@
private static final Class<?> MAIN_CLASS = Main.class;
private static final String EXPECTED_OUTPUT = StringUtils.lines("A", "B");
- private static final String UNEXPECTED_OUTPUT = StringUtils.lines("A", "A");
@Test
public void testReference() throws Exception {
@@ -107,7 +106,7 @@
},
r ->
r.run(parameters.getRuntime(), MAIN_CLASS)
- .assertSuccessWithOutput(UNEXPECTED_OUTPUT));
+ .assertSuccessWithOutput(EXPECTED_OUTPUT));
}
private List<byte[]> getTransformedClasses() throws IOException {
@@ -126,7 +125,7 @@
// When R8 optimize this code the getter for the two constant dynamics will be inlined into
// Main.main. This leaves the synthetic constant dynamic classes with just two static fields.
- // The synthetic sharing then share these two synthetics leaving only one constant.
+ // The synthetic sharing cannot share these two synthetics as that would leave only one constant.
// See b/272346803 for details.
public static class Main {
