Only allow instances of final classes in filled-new-array
Bug: b/293501981
Change-Id: I0554aaaba74509a5afff9147ba11e6cfd68c7e14
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/passes/ArrayConstructionSimplifier.java b/src/main/java/com/android/tools/r8/ir/conversion/passes/ArrayConstructionSimplifier.java
index 7ce1b99..03da617 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/passes/ArrayConstructionSimplifier.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/passes/ArrayConstructionSimplifier.java
@@ -4,12 +4,12 @@
package com.android.tools.r8.ir.conversion.passes;
+import static com.android.tools.r8.graph.DexProgramClass.asProgramClassOrNull;
+
import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.AppView;
-import com.android.tools.r8.graph.DexClass;
+import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexType;
-import com.android.tools.r8.ir.analysis.type.ArrayTypeElement;
-import com.android.tools.r8.ir.analysis.type.TypeElement;
import com.android.tools.r8.ir.code.ArrayPut;
import com.android.tools.r8.ir.code.BasicBlock;
import com.android.tools.r8.ir.code.IRCode;
@@ -117,7 +117,7 @@
RewriteArrayOptions rewriteOptions = options.rewriteArrayOptions();
InstructionListIterator it = block.listIterator(code);
while (it.hasNext()) {
- FilledArrayCandidate candidate = computeFilledArrayCandidate(it.next(), rewriteOptions);
+ FilledArrayCandidate candidate = computeFilledArrayCandidate(code, it.next(), rewriteOptions);
if (candidate == null) {
continue;
}
@@ -367,7 +367,7 @@
}
private FilledArrayCandidate computeFilledArrayCandidate(
- Instruction instruction, RewriteArrayOptions options) {
+ IRCode code, Instruction instruction, RewriteArrayOptions options) {
NewArrayEmpty newArrayEmpty = instruction.asNewArrayEmpty();
if (newArrayEmpty == null) {
return null;
@@ -392,52 +392,22 @@
&& arrayType != dexItemFactory.objectArrayType
&& !arrayType.isPrimitiveArrayType()) {
DexType elementType = arrayType.toArrayElementType(dexItemFactory);
- for (Instruction uniqueUser : newArrayEmpty.outValue().uniqueUsers()) {
- if (uniqueUser.isArrayPut()
- && uniqueUser.asArrayPut().array() == newArrayEmpty.outValue()
- && !checkTypeOfArrayPut(uniqueUser.asArrayPut(), elementType)) {
- return null;
- }
+ if (!elementType.isClassType()) {
+ return null;
+ }
+ DexProgramClass clazz = null;
+ if (appView.enableWholeProgramOptimizations()) {
+ clazz = asProgramClassOrNull(appView.definitionFor(elementType, code.context()));
+ } else if (elementType == code.context().getHolderType()) {
+ clazz = code.context().getHolder();
+ }
+ if (clazz == null || !clazz.isFinal()) {
+ return null;
}
}
return new FilledArrayCandidate(newArrayEmpty, size, encodeAsFilledNewArray);
}
- private boolean checkTypeOfArrayPut(ArrayPut arrayPut, DexType elementType) {
- TypeElement valueType = arrayPut.value().getType();
- if (!valueType.isPrimitiveType() && elementType == dexItemFactory.objectType) {
- return true;
- }
- if (valueType.isNullType() && !elementType.isPrimitiveType()) {
- return true;
- }
- if (elementType.isArrayType()) {
- if (valueType.isNullType()) {
- return true;
- }
- ArrayTypeElement arrayTypeElement = valueType.asArrayType();
- if (arrayTypeElement == null
- || arrayTypeElement.getNesting() != elementType.getNumberOfLeadingSquareBrackets()) {
- return false;
- }
- valueType = arrayTypeElement.getBaseType();
- elementType = elementType.toBaseType(dexItemFactory);
- }
- assert !valueType.isArrayType();
- assert !elementType.isArrayType();
- if (valueType.isPrimitiveType() && !elementType.isPrimitiveType()) {
- return false;
- }
- if (valueType.isPrimitiveType()) {
- return true;
- }
- DexClass clazz = appView.definitionFor(elementType);
- if (clazz == null) {
- return false;
- }
- return valueType.isClassType(elementType);
- }
-
private boolean canUseFilledNewArray(DexType arrayType, int size, RewriteArrayOptions options) {
if (size < options.minSizeForFilledNewArray) {
return false;
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index 03a8229..0360999 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -1555,7 +1555,7 @@
// issues. See b/283715197.
public boolean canUseSubTypesInFilledNewArray() {
assert isGeneratingDex();
- return !canHaveBugPresentUntil(AndroidApiLevel.U);
+ return !canHaveBugPresentUntilInclusive(AndroidApiLevel.U);
}
// Dalvik doesn't handle new-filled-array with arrays as values. It fails with:
@@ -2486,6 +2486,14 @@
return true;
}
+ private boolean canHaveBugPresentUntilInclusive(AndroidApiLevel level) {
+ if (desugarState.isOn() || isGeneratingDex()) {
+ return level == null || !getMinApiLevel().isGreaterThan(level);
+ }
+ assert minApiLevel.equals(B);
+ return true;
+ }
+
/**
* Allow access modification of synthetic lambda implementation methods in D8 to avoid generating
* an excessive amount of accessibility bridges. In R8, the lambda implementation methods are
diff --git a/src/test/java/com/android/tools/r8/workaround/FilledNewArrayFromSubtypeWithMissingInterfaceWorkaroundTest.java b/src/test/java/com/android/tools/r8/workaround/FilledNewArrayFromSubtypeWithMissingInterfaceWorkaroundTest.java
index de98a44..53794e3 100644
--- a/src/test/java/com/android/tools/r8/workaround/FilledNewArrayFromSubtypeWithMissingInterfaceWorkaroundTest.java
+++ b/src/test/java/com/android/tools/r8/workaround/FilledNewArrayFromSubtypeWithMissingInterfaceWorkaroundTest.java
@@ -6,16 +6,13 @@
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
-import com.android.tools.r8.Dex2OatTestRunResult;
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.NoVerticalClassMerging;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestParametersCollection;
-import com.android.tools.r8.ToolHelper.DexVm.Version;
-import com.android.tools.r8.utils.AndroidApiLevel;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
import com.android.tools.r8.utils.codeinspector.InstructionSubject;
import com.android.tools.r8.utils.codeinspector.MethodSubject;
@@ -70,25 +67,15 @@
compileResult
.inspect(this::inspect)
.runDex2Oat(parameters.getRuntime())
- // TODO(b/293501981): Should not fail verification.
- .applyIf(
- parameters.getApiLevel().isGreaterThanOrEqualTo(AndroidApiLevel.N),
- Dex2OatTestRunResult::assertVerificationErrors,
- Dex2OatTestRunResult::assertNoVerificationErrors))
+ .assertNoVerificationErrors())
.run(parameters.getRuntime(), Main.class)
- .applyIf(
- parameters.isCfRuntime()
- || !parameters.getDexRuntimeVersion().isEqualTo(Version.V13_0_0)
- || parameters.getApiLevel() == AndroidApiLevel.B,
- runResult -> runResult.assertFailureWithErrorThatThrows(NoClassDefFoundError.class),
- runResult -> runResult.assertFailureWithErrorThatThrows(VerifyError.class));
+ .assertFailureWithErrorThatThrows(NoClassDefFoundError.class);
}
private void inspect(CodeInspector inspector) {
MethodSubject mainMethodSubject = inspector.clazz(Main.class).mainMethod();
assertThat(mainMethodSubject, isPresent());
- assertEquals(
- parameters.getApiLevel().isGreaterThanOrEqualTo(AndroidApiLevel.N),
+ assertFalse(
mainMethodSubject.streamInstructions().anyMatch(InstructionSubject::isFilledNewArray));
}
