Remove abstract method to empty throwing method transformation in dex parser
Bug: 158069726
Change-Id: I5eec984fd87c031d7fd58678c92d0c489151ab3c
diff --git a/src/main/java/com/android/tools/r8/dex/DexParser.java b/src/main/java/com/android/tools/r8/dex/DexParser.java
index 26dca03..a416e51 100644
--- a/src/main/java/com/android/tools/r8/dex/DexParser.java
+++ b/src/main/java/com/android/tools/r8/dex/DexParser.java
@@ -642,8 +642,7 @@
int size,
DexMethodAnnotation[] annotations,
DexParameterAnnotation[] parameters,
- boolean skipCodes,
- boolean ensureNonAbstract) {
+ boolean skipCodes) {
DexEncodedMethod[] methods = new DexEncodedMethod[size];
int methodIndex = 0;
MemberAnnotationIterator<DexMethod, DexAnnotationSet> annotationIterator =
@@ -661,19 +660,13 @@
code = codes.get(codeOff);
}
DexMethod method = indexedItems.getMethod(methodIndex);
- DexEncodedMethod encodedMethod =
+ methods[i] =
new DexEncodedMethod(
method,
accessFlags,
annotationIterator.getNextFor(method),
parameterAnnotationsIterator.getNextFor(method),
code);
- if (accessFlags.isAbstract() && ensureNonAbstract) {
- accessFlags.unsetAbstract();
- assert !options.isGeneratingClassFiles();
- encodedMethod = encodedMethod.toEmptyThrowingMethodDex(false);
- }
- methods[i] = encodedMethod;
}
return methods;
}
@@ -754,17 +747,13 @@
directMethodsSize,
annotationsDirectory.methods,
annotationsDirectory.parameters,
- classKind != ClassKind.PROGRAM,
- options.canHaveDalvikAbstractMethodOnNonAbstractClassVerificationBug()
- && !flags.isAbstract());
+ classKind != ClassKind.PROGRAM);
virtualMethods =
readMethods(
virtualMethodsSize,
annotationsDirectory.methods,
annotationsDirectory.parameters,
- classKind != ClassKind.PROGRAM,
- options.canHaveDalvikAbstractMethodOnNonAbstractClassVerificationBug()
- && !flags.isAbstract());
+ classKind != ClassKind.PROGRAM);
}
AttributesAndAnnotations attrs =
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
index ae412b5..c12f3f4 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -771,11 +771,6 @@
null);
}
- public DexCode buildEmptyThrowingDexCode() {
- Instruction insn[] = {new Const(0, 0), new Throw(0)};
- return generateCodeFromTemplate(1, 0, insn);
- }
-
public DexEncodedMethod toEmptyThrowingMethod(InternalOptions options) {
return options.isGeneratingClassFiles()
? toEmptyThrowingMethodCf()
@@ -795,17 +790,6 @@
return result;
}
- public CfCode buildEmptyThrowingCfCode() {
- CfInstruction insn[] = {new CfConstNull(), new CfThrow()};
- return new CfCode(
- method.holder,
- 1,
- method.proto.parameters.size() + 1,
- Arrays.asList(insn),
- Collections.emptyList(),
- Collections.emptyList());
- }
-
private DexEncodedMethod toEmptyThrowingMethodCf() {
checkIfObsolete();
assert !shouldNotHaveCode();
@@ -819,6 +803,28 @@
return result;
}
+ public Code buildEmptyThrowingCode(InternalOptions options) {
+ return options.isGeneratingClassFiles()
+ ? buildEmptyThrowingCfCode()
+ : buildEmptyThrowingDexCode();
+ }
+
+ public CfCode buildEmptyThrowingCfCode() {
+ CfInstruction insn[] = {new CfConstNull(), new CfThrow()};
+ return new CfCode(
+ method.holder,
+ 1,
+ method.proto.parameters.size() + 1,
+ Arrays.asList(insn),
+ Collections.emptyList(),
+ Collections.emptyList());
+ }
+
+ public DexCode buildEmptyThrowingDexCode() {
+ Instruction insn[] = {new Const(0, 0), new Throw(0)};
+ return generateCodeFromTemplate(1, 0, insn);
+ }
+
public DexEncodedMethod toMethodThatLogsError(AppView<?> appView) {
if (appView.options().isGeneratingDex()) {
return toMethodThatLogsErrorDexCode(appView.dexItemFactory());
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
index 4c127fa..35760b4 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
@@ -473,6 +473,8 @@
public DexApplication convert(DexApplication application, ExecutorService executor)
throws ExecutionException {
removeLambdaDeserializationMethods();
+ workaroundAbstractMethodOnNonAbstractClassVerificationBug(
+ executor, OptimizationFeedbackIgnore.getInstance());
timing.begin("IR conversion");
ThreadUtils.processItems(application.classes(), this::convertMethods, executor);
@@ -636,6 +638,28 @@
}
}
+ private void workaroundAbstractMethodOnNonAbstractClassVerificationBug(
+ ExecutorService executorService, OptimizationFeedback feedback) throws ExecutionException {
+ if (!options.canHaveDalvikAbstractMethodOnNonAbstractClassVerificationBug()) {
+ return;
+ }
+ assert delayedOptimizationFeedback.noUpdatesLeft();
+ ThreadUtils.processItems(
+ appView.appInfo().classes(),
+ clazz -> {
+ if (!clazz.isAbstract()) {
+ clazz.forEachMethod(
+ method -> {
+ if (method.isAbstract()) {
+ method.accessFlags.unsetAbstract();
+ finalizeEmptyThrowingCode(method, feedback);
+ }
+ });
+ }
+ },
+ executorService);
+ }
+
public DexApplication optimize() throws ExecutionException {
ExecutorService executor = Executors.newSingleThreadExecutor();
try {
@@ -651,6 +675,8 @@
computeReachabilitySensitivity(application);
collectLambdaMergingCandidates(application);
collectStaticizerCandidates(application);
+ workaroundAbstractMethodOnNonAbstractClassVerificationBug(
+ executorService, simpleOptimizationFeedback);
// The process is in two phases in general.
// 1) Subject all DexEncodedMethods to optimization, except some optimizations that require
@@ -1681,10 +1707,7 @@
private void finalizeEmptyThrowingCode(DexEncodedMethod method, OptimizationFeedback feedback) {
assert options.isGeneratingClassFiles() || options.isGeneratingDex();
- Code emptyThrowingCode =
- options.isGeneratingClassFiles()
- ? method.buildEmptyThrowingCfCode()
- : method.buildEmptyThrowingDexCode();
+ Code emptyThrowingCode = method.buildEmptyThrowingCode(options);
method.setCode(emptyThrowingCode, appView);
feedback.markProcessed(method, ConstraintWithTarget.ALWAYS);
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxer.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxer.java
index 9c5e2e4..c5cd89d 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxer.java
@@ -754,11 +754,7 @@
// enumUnboxerRewriter will generate invalid code.
// To work around this problem we clear such methods, i.e., we replace the code object by
// an empty throwing code object, so reprocessing won't take time and will be valid.
- enumMethod.setCode(
- appView.options().isGeneratingClassFiles()
- ? enumMethod.buildEmptyThrowingCfCode()
- : enumMethod.buildEmptyThrowingDexCode(),
- appView);
+ enumMethod.setCode(enumMethod.buildEmptyThrowingCode(appView.options()), appView);
}
private DexEncodedMethod fixupEncodedMethodToUtility(
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackSimple.java b/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackSimple.java
index 221fbd9..822e0c5 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackSimple.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackSimple.java
@@ -135,8 +135,7 @@
@Override
public void markProcessed(DexEncodedMethod method, ConstraintWithTarget state) {
- // Just as processed, don't provide any inlining constraints.
- method.markProcessed(ConstraintWithTarget.NEVER);
+ method.markProcessed(state);
}
@Override
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 5a286b7..d7ba750 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -1669,7 +1669,7 @@
//
// See b/132953944.
public boolean canHaveDalvikAbstractMethodOnNonAbstractClassVerificationBug() {
- return minApiLevel < AndroidApiLevel.L.getLevel();
+ return isGeneratingDex() && minApiLevel < AndroidApiLevel.L.getLevel();
}
// On dalvik we see issues when using an int value in places where a boolean, byte, char, or short
diff --git a/src/test/java/com/android/tools/r8/transformers/ClassFileTransformer.java b/src/test/java/com/android/tools/r8/transformers/ClassFileTransformer.java
index 3f1ad69..1f1d744 100644
--- a/src/test/java/com/android/tools/r8/transformers/ClassFileTransformer.java
+++ b/src/test/java/com/android/tools/r8/transformers/ClassFileTransformer.java
@@ -337,6 +337,10 @@
});
}
+ public ClassFileTransformer unsetAbstract() {
+ return setAccessFlags(ClassAccessFlags::unsetAbstract);
+ }
+
public ClassFileTransformer setAnnotation() {
return setAccessFlags(
accessFlags -> {
diff --git a/src/test/java/com/android/tools/r8/workaround/InputWithAbstractMethodOnNonAbstractClassTest.java b/src/test/java/com/android/tools/r8/workaround/InputWithAbstractMethodOnNonAbstractClassTest.java
new file mode 100644
index 0000000..aca08d4
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/workaround/InputWithAbstractMethodOnNonAbstractClassTest.java
@@ -0,0 +1,99 @@
+// Copyright (c) 2020, 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.workaround;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isAbstract;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assume.assumeTrue;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class InputWithAbstractMethodOnNonAbstractClassTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public InputWithAbstractMethodOnNonAbstractClassTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void testD8() throws Exception {
+ assumeTrue(parameters.isDexRuntime());
+ testForD8()
+ .addProgramClasses(TestClass.class)
+ .addProgramClassFileData(transformer(Greeter.class).unsetAbstract().transform())
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .inspect(this::inspect)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("Hello world!");
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ testForR8(parameters.getBackend())
+ .addProgramClasses(TestClass.class)
+ .addProgramClassFileData(transformer(Greeter.class).unsetAbstract().transform())
+ .addKeepMainRule(TestClass.class)
+ .addKeepClassAndMembersRules(Greeter.class)
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .inspect(this::inspect)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("Hello world!");
+ }
+
+ @Test
+ public void testJVM() throws Exception {
+ assumeTrue(parameters.isCfRuntime());
+ testForJvm()
+ .addProgramClasses(TestClass.class)
+ .addProgramClassFileData(transformer(Greeter.class).unsetAbstract().transform())
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("Hello world!");
+ }
+
+ private void inspect(CodeInspector inspector) {
+ MethodSubject methodOfInterest = inspector.clazz(Greeter.class).uniqueMethodWithName("dead");
+ assertThat(methodOfInterest, isPresent());
+ if (parameters.isDexRuntime() && parameters.getApiLevel().isLessThan(AndroidApiLevel.L)) {
+ assertThat(methodOfInterest, not(isAbstract()));
+ } else {
+ assertThat(methodOfInterest, isAbstract());
+ }
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ Greeter.greet();
+ }
+ }
+
+ /*not-*/ abstract static class Greeter {
+
+ static void greet() {
+ System.out.println("Hello world!");
+ }
+
+ abstract void dead();
+ }
+}
