Mitigate issues from Dalvik array field get verifier bug
Bug: 206891715
Change-Id: Id3f50f13242a73159cc82be9c3ada3309403c5a6
diff --git a/src/main/java/com/android/tools/r8/graph/analysis/EnqueuerFieldAccessAnalysis.java b/src/main/java/com/android/tools/r8/graph/analysis/EnqueuerFieldAccessAnalysis.java
new file mode 100644
index 0000000..eee1f34
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/graph/analysis/EnqueuerFieldAccessAnalysis.java
@@ -0,0 +1,24 @@
+// Copyright (c) 2021, 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.graph.analysis;
+
+import com.android.tools.r8.graph.DexField;
+import com.android.tools.r8.graph.FieldResolutionResult;
+import com.android.tools.r8.graph.ProgramMethod;
+
+public interface EnqueuerFieldAccessAnalysis {
+
+ void traceInstanceFieldRead(
+ DexField field, FieldResolutionResult resolutionResult, ProgramMethod context);
+
+ void traceInstanceFieldWrite(
+ DexField field, FieldResolutionResult resolutionResult, ProgramMethod context);
+
+ void traceStaticFieldRead(
+ DexField field, FieldResolutionResult resolutionResult, ProgramMethod context);
+
+ void traceStaticFieldWrite(
+ DexField field, FieldResolutionResult resolutionResult, ProgramMethod context);
+}
diff --git a/src/main/java/com/android/tools/r8/graph/analysis/GetArrayOfMissingTypeVerifyErrorWorkaround.java b/src/main/java/com/android/tools/r8/graph/analysis/GetArrayOfMissingTypeVerifyErrorWorkaround.java
new file mode 100644
index 0000000..1243e98
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/graph/analysis/GetArrayOfMissingTypeVerifyErrorWorkaround.java
@@ -0,0 +1,122 @@
+// Copyright (c) 2021, 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.graph.analysis;
+
+import com.android.tools.r8.graph.AppInfoWithClassHierarchy;
+import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.DexClass;
+import com.android.tools.r8.graph.DexField;
+import com.android.tools.r8.graph.DexItemFactory;
+import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.FieldResolutionResult;
+import com.android.tools.r8.graph.ProgramMethod;
+import com.android.tools.r8.shaking.Enqueuer;
+import com.android.tools.r8.shaking.KeepInfo.Joiner;
+import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.InternalOptions;
+import com.google.common.collect.ImmutableSet;
+import java.util.Set;
+
+/**
+ * In Dalvik it is a verification error to read and use a field of type Missing[].
+ *
+ * <p>Example:
+ *
+ * <pre>
+ * Consumer<?>[] consumer = this.field;
+ * acceptConsumer(consumer); // acceptConsumer(Consumer[])
+ * </pre>
+ *
+ * <p>To avoid that the compiler moves such code into other contexts (e.g., as a result of inlining
+ * or class merging), and thereby causes new classes to no longer verify on Dalvik, we soft-pin
+ * methods that reads such fields.
+ */
+public class GetArrayOfMissingTypeVerifyErrorWorkaround implements EnqueuerFieldAccessAnalysis {
+
+ private final DexItemFactory dexItemFactory;
+ private final Enqueuer enqueuer;
+ private final Set<DexType> knownToBePresentOnDalvik;
+
+ public GetArrayOfMissingTypeVerifyErrorWorkaround(
+ AppView<? extends AppInfoWithClassHierarchy> appView, Enqueuer enqueuer) {
+ this.dexItemFactory = appView.dexItemFactory();
+ this.enqueuer = enqueuer;
+ this.knownToBePresentOnDalvik =
+ ImmutableSet.<DexType>builder()
+ .add(dexItemFactory.boxedBooleanType)
+ .add(dexItemFactory.boxedByteType)
+ .add(dexItemFactory.boxedCharType)
+ .add(dexItemFactory.boxedDoubleType)
+ .add(dexItemFactory.boxedFloatType)
+ .add(dexItemFactory.boxedIntType)
+ .add(dexItemFactory.boxedLongType)
+ .add(dexItemFactory.boxedShortType)
+ .add(dexItemFactory.classType)
+ .add(dexItemFactory.objectType)
+ .add(dexItemFactory.enumType)
+ .add(dexItemFactory.stringType)
+ .build();
+ }
+
+ public static void register(
+ AppView<? extends AppInfoWithClassHierarchy> appView, Enqueuer enqueuer) {
+ if (!isNoop(appView)) {
+ enqueuer.registerFieldAccessAnalysis(
+ new GetArrayOfMissingTypeVerifyErrorWorkaround(appView, enqueuer));
+ }
+ }
+
+ private static boolean isNoop(AppView<? extends AppInfoWithClassHierarchy> appView) {
+ InternalOptions options = appView.options();
+ return options.isGeneratingDex()
+ && options.getMinApiLevel().isGreaterThanOrEqualTo(AndroidApiLevel.L);
+ }
+
+ @Override
+ public void traceInstanceFieldRead(
+ DexField field, FieldResolutionResult resolutionResult, ProgramMethod context) {
+ if (isUnsafeToUseFieldOnDalvik(field, context)) {
+ enqueuer.getKeepInfo().joinMethod(context, Joiner::disallowOptimization);
+ }
+ }
+
+ @Override
+ public void traceStaticFieldRead(
+ DexField field, FieldResolutionResult resolutionResult, ProgramMethod context) {
+ if (isUnsafeToUseFieldOnDalvik(field, context)) {
+ enqueuer.getKeepInfo().joinMethod(context, Joiner::disallowOptimization);
+ }
+ }
+
+ private boolean isUnsafeToUseFieldOnDalvik(DexField field, ProgramMethod context) {
+ DexType fieldType = field.getType();
+ if (!fieldType.isArrayType()) {
+ return false;
+ }
+ DexType baseType = fieldType.toBaseType(dexItemFactory);
+ if (!baseType.isClassType()) {
+ return false;
+ }
+ if (knownToBePresentOnDalvik.contains(baseType)) {
+ return false;
+ }
+ // TODO(b/206891715): Use the API database to determine if the given type is introduced in API
+ // level L or later.
+ DexClass baseClass = enqueuer.definitionFor(baseType, context);
+ return baseClass != null && baseClass.isLibraryClass();
+ }
+
+ @Override
+ public void traceInstanceFieldWrite(
+ DexField field, FieldResolutionResult resolutionResult, ProgramMethod context) {
+ // Intentionally empty.
+ }
+
+ @Override
+ public void traceStaticFieldWrite(
+ DexField field, FieldResolutionResult resolutionResult, ProgramMethod context) {
+ // Intentionally empty.
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
index 02c18fc..6ca6a5c 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -79,8 +79,10 @@
import com.android.tools.r8.graph.analysis.EnqueuerAnalysis;
import com.android.tools.r8.graph.analysis.EnqueuerCheckCastAnalysis;
import com.android.tools.r8.graph.analysis.EnqueuerExceptionGuardAnalysis;
+import com.android.tools.r8.graph.analysis.EnqueuerFieldAccessAnalysis;
import com.android.tools.r8.graph.analysis.EnqueuerInstanceOfAnalysis;
import com.android.tools.r8.graph.analysis.EnqueuerInvokeAnalysis;
+import com.android.tools.r8.graph.analysis.GetArrayOfMissingTypeVerifyErrorWorkaround;
import com.android.tools.r8.graph.analysis.InvokeVirtualToInterfaceVerifyErrorWorkaround;
import com.android.tools.r8.ir.analysis.proto.ProtoEnqueuerUseRegistry;
import com.android.tools.r8.ir.analysis.proto.schema.ProtoEnqueuerExtension;
@@ -232,11 +234,12 @@
private final boolean forceProguardCompatibility;
private final Mode mode;
- private Set<EnqueuerAnalysis> analyses = new LinkedHashSet<>();
- private Set<EnqueuerInvokeAnalysis> invokeAnalyses = new LinkedHashSet<>();
- private Set<EnqueuerInstanceOfAnalysis> instanceOfAnalyses = new LinkedHashSet<>();
- private Set<EnqueuerExceptionGuardAnalysis> exceptionGuardAnalyses = new LinkedHashSet<>();
- private Set<EnqueuerCheckCastAnalysis> checkCastAnalyses = new LinkedHashSet<>();
+ private final Set<EnqueuerAnalysis> analyses = new LinkedHashSet<>();
+ private final Set<EnqueuerFieldAccessAnalysis> fieldAccessAnalyses = new LinkedHashSet<>();
+ private final Set<EnqueuerInvokeAnalysis> invokeAnalyses = new LinkedHashSet<>();
+ private final Set<EnqueuerInstanceOfAnalysis> instanceOfAnalyses = new LinkedHashSet<>();
+ private final Set<EnqueuerExceptionGuardAnalysis> exceptionGuardAnalyses = new LinkedHashSet<>();
+ private final Set<EnqueuerCheckCastAnalysis> checkCastAnalyses = new LinkedHashSet<>();
// Don't hold a direct pointer to app info (use appView).
private AppInfoWithClassHierarchy appInfo;
@@ -471,6 +474,7 @@
: null;
if (mode.isInitialOrFinalTreeShaking()) {
+ GetArrayOfMissingTypeVerifyErrorWorkaround.register(appView, this);
InvokeVirtualToInterfaceVerifyErrorWorkaround.register(appView, this);
if (options.protoShrinking().enableGeneratedMessageLiteShrinking) {
registerAnalysis(new ProtoEnqueuerExtension(appView));
@@ -530,6 +534,11 @@
return this;
}
+ public Enqueuer registerFieldAccessAnalysis(EnqueuerFieldAccessAnalysis analysis) {
+ fieldAccessAnalyses.add(analysis);
+ return this;
+ }
+
public Enqueuer registerInvokeAnalysis(EnqueuerInvokeAnalysis analysis) {
invokeAnalyses.add(analysis);
return this;
@@ -1432,6 +1441,10 @@
}
FieldResolutionResult resolutionResult = resolveField(fieldReference, currentMethod);
+ fieldAccessAnalyses.forEach(
+ analysis ->
+ analysis.traceInstanceFieldRead(fieldReference, resolutionResult, currentMethod));
+
if (resolutionResult.isFailedOrUnknownResolution()) {
// Must trace the types from the field reference even if it does not exist.
traceFieldReference(fieldReference, resolutionResult, currentMethod);
@@ -1484,6 +1497,10 @@
}
FieldResolutionResult resolutionResult = resolveField(fieldReference, currentMethod);
+ fieldAccessAnalyses.forEach(
+ analysis ->
+ analysis.traceInstanceFieldWrite(fieldReference, resolutionResult, currentMethod));
+
if (resolutionResult.isFailedOrUnknownResolution()) {
// Must trace the types from the field reference even if it does not exist.
traceFieldReference(fieldReference, resolutionResult, currentMethod);
@@ -1536,6 +1553,9 @@
}
FieldResolutionResult resolutionResult = resolveField(fieldReference, currentMethod);
+ fieldAccessAnalyses.forEach(
+ analysis -> analysis.traceStaticFieldRead(fieldReference, resolutionResult, currentMethod));
+
if (resolutionResult.isFailedOrUnknownResolution()) {
// Must trace the types from the field reference even if it does not exist.
traceFieldReference(fieldReference, resolutionResult, currentMethod);
@@ -1604,6 +1624,10 @@
}
FieldResolutionResult resolutionResult = resolveField(fieldReference, currentMethod);
+ fieldAccessAnalyses.forEach(
+ analysis ->
+ analysis.traceStaticFieldWrite(fieldReference, resolutionResult, currentMethod));
+
if (resolutionResult.isFailedOrUnknownResolution()) {
// Must trace the types from the field reference even if it does not exist.
traceFieldReference(fieldReference, resolutionResult, currentMethod);
diff --git a/src/test/java/com/android/tools/r8/workaround/ArrayFieldGetWithMissingBaseTypeTest.java b/src/test/java/com/android/tools/r8/workaround/ArrayFieldGetWithMissingBaseTypeTest.java
index 87627bc..b28721d 100644
--- a/src/test/java/com/android/tools/r8/workaround/ArrayFieldGetWithMissingBaseTypeTest.java
+++ b/src/test/java/com/android/tools/r8/workaround/ArrayFieldGetWithMissingBaseTypeTest.java
@@ -10,6 +10,7 @@
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.utils.AndroidApiLevel;
import java.util.function.Consumer;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -39,21 +40,22 @@
.addKeepClassAndMembersRules(Utils.class)
.addKeepRules(
"-keep class " + Main.class.getTypeName() + " { void notUsedDuringLaunch(); }")
- // TODO(b/206891715): Should not allow merging when compiling for Dalvik.
.addHorizontallyMergedClassesInspector(
inspector ->
inspector
- .assertIsCompleteMergeGroup(UsedDuringLaunch.class, NotUsedDuringLaunch.class)
+ .applyIf(
+ parameters.isDexRuntime()
+ && parameters.getApiLevel().isGreaterThanOrEqualTo(AndroidApiLevel.L),
+ i ->
+ i.assertIsCompleteMergeGroup(
+ UsedDuringLaunch.class, NotUsedDuringLaunch.class))
.assertNoOtherClassesMerged())
.enableInliningAnnotations()
.enableNeverClassInliningAnnotations()
.setMinApi(parameters.getApiLevel())
.compile()
.run(parameters.getRuntime(), Main.class)
- .applyIf(
- parameters.isCfRuntime() || !parameters.getDexRuntimeVersion().isDalvik(),
- runResult -> runResult.assertSuccessWithOutputLines("Hello world!"),
- runResult -> runResult.assertFailureWithErrorThatThrows(VerifyError.class));
+ .assertSuccessWithOutputLines("Hello world!");
}
static class Main {
