Track exception guard types to prevent merging.
Bug: 163311975
Change-Id: I4f77dc23f09d402dbcb957d60ce074777378af79
diff --git a/src/main/java/com/android/tools/r8/cf/code/CfTryCatch.java b/src/main/java/com/android/tools/r8/cf/code/CfTryCatch.java
index 8e9752b..01ff28f 100644
--- a/src/main/java/com/android/tools/r8/cf/code/CfTryCatch.java
+++ b/src/main/java/com/android/tools/r8/cf/code/CfTryCatch.java
@@ -4,7 +4,9 @@
package com.android.tools.r8.cf.code;
import com.android.tools.r8.graph.CfCompareHelper;
+import com.android.tools.r8.graph.DexClassAndMethod;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.UseRegistry;
import com.android.tools.r8.ir.code.BasicBlock;
import com.android.tools.r8.ir.code.CatchHandlers;
import com.android.tools.r8.ir.conversion.CfBuilder;
@@ -54,4 +56,8 @@
.thenComparing(c -> c.targets, ComparatorUtils.listComparator(helper::compareLabels))
.compare(this, other);
}
+
+ public void internalRegisterUse(UseRegistry registry, DexClassAndMethod context) {
+ guards.forEach(registry::registerExceptionGuard);
+ }
}
diff --git a/src/main/java/com/android/tools/r8/graph/CfCode.java b/src/main/java/com/android/tools/r8/graph/CfCode.java
index 141484c..a08d65e 100644
--- a/src/main/java/com/android/tools/r8/graph/CfCode.java
+++ b/src/main/java/com/android/tools/r8/graph/CfCode.java
@@ -508,7 +508,7 @@
for (CfInstruction instruction : instructions) {
instruction.registerUse(registry, method);
}
- tryCatchRanges.forEach(tryCatch -> tryCatch.guards.forEach(registry::registerTypeReference));
+ tryCatchRanges.forEach(tryCatch -> tryCatch.internalRegisterUse(registry, method));
}
@Override
diff --git a/src/main/java/com/android/tools/r8/graph/UseRegistry.java b/src/main/java/com/android/tools/r8/graph/UseRegistry.java
index 033182e..4a3327a 100644
--- a/src/main/java/com/android/tools/r8/graph/UseRegistry.java
+++ b/src/main/java/com/android/tools/r8/graph/UseRegistry.java
@@ -75,6 +75,10 @@
registerTypeReference(type);
}
+ public void registerExceptionGuard(DexType guard) {
+ registerTypeReference(guard);
+ }
+
public void registerMethodHandle(DexMethodHandle methodHandle, MethodHandleUse use) {
switch (methodHandle.type) {
case INSTANCE_GET:
diff --git a/src/main/java/com/android/tools/r8/graph/analysis/EnqueuerExceptionGuardAnalysis.java b/src/main/java/com/android/tools/r8/graph/analysis/EnqueuerExceptionGuardAnalysis.java
new file mode 100644
index 0000000..1bf392e
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/graph/analysis/EnqueuerExceptionGuardAnalysis.java
@@ -0,0 +1,12 @@
+// 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.graph.analysis;
+
+import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.ProgramMethod;
+
+public interface EnqueuerExceptionGuardAnalysis {
+ void traceExceptionGuard(DexType guard, ProgramMethod context);
+}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/NoRuntimeTypeChecks.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/NoRuntimeTypeChecks.java
index dff9967..23f10b2 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/NoRuntimeTypeChecks.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/NoRuntimeTypeChecks.java
@@ -18,7 +18,6 @@
@Override
public boolean canMerge(DexProgramClass clazz) {
// We currently assume we only merge classes that implement the same set of interfaces.
- return !(this.classMergingEnqueuerExtension.isCheckCastType(clazz)
- || this.classMergingEnqueuerExtension.isInstanceOfType(clazz));
+ return !classMergingEnqueuerExtension.isRuntimeCheckType(clazz);
}
}
diff --git a/src/main/java/com/android/tools/r8/shaking/ClassMergingEnqueuerExtension.java b/src/main/java/com/android/tools/r8/shaking/ClassMergingEnqueuerExtension.java
index a6d7326..6a2151b 100644
--- a/src/main/java/com/android/tools/r8/shaking/ClassMergingEnqueuerExtension.java
+++ b/src/main/java/com/android/tools/r8/shaking/ClassMergingEnqueuerExtension.java
@@ -9,15 +9,19 @@
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.graph.analysis.EnqueuerCheckCastAnalysis;
+import com.android.tools.r8.graph.analysis.EnqueuerExceptionGuardAnalysis;
import com.android.tools.r8.graph.analysis.EnqueuerInstanceOfAnalysis;
import com.google.common.collect.Sets;
import java.util.Set;
public class ClassMergingEnqueuerExtension
- implements EnqueuerInstanceOfAnalysis, EnqueuerCheckCastAnalysis {
+ implements EnqueuerInstanceOfAnalysis,
+ EnqueuerCheckCastAnalysis,
+ EnqueuerExceptionGuardAnalysis {
private final Set<DexType> instanceOfTypes = Sets.newIdentityHashSet();
private final Set<DexType> checkCastTypes = Sets.newIdentityHashSet();
+ private final Set<DexType> exceptionGuardTypes = Sets.newIdentityHashSet();
private final DexItemFactory factory;
public ClassMergingEnqueuerExtension(DexItemFactory factory) {
@@ -34,6 +38,11 @@
instanceOfTypes.add(type.toBaseType(factory));
}
+ @Override
+ public void traceExceptionGuard(DexType guard, ProgramMethod context) {
+ exceptionGuardTypes.add(guard);
+ }
+
public boolean isCheckCastType(DexProgramClass clazz) {
return checkCastTypes.contains(clazz.type);
}
@@ -42,7 +51,18 @@
return instanceOfTypes.contains(clazz.type);
}
+ public boolean isExceptionGuardType(DexProgramClass clazz) {
+ return exceptionGuardTypes.contains(clazz.type);
+ }
+
+ public boolean isRuntimeCheckType(DexProgramClass clazz) {
+ return isInstanceOfType(clazz) || isCheckCastType(clazz) || isExceptionGuardType(clazz);
+ }
+
public void attach(Enqueuer enqueuer) {
- enqueuer.registerInstanceOfAnalysis(this).registerCheckCastAnalysis(this);
+ enqueuer
+ .registerInstanceOfAnalysis(this)
+ .registerCheckCastAnalysis(this)
+ .registerExceptionGuardAnalysis(this);
}
}
diff --git a/src/main/java/com/android/tools/r8/shaking/DefaultEnqueuerUseRegistry.java b/src/main/java/com/android/tools/r8/shaking/DefaultEnqueuerUseRegistry.java
index 34fb593..b67d90d 100644
--- a/src/main/java/com/android/tools/r8/shaking/DefaultEnqueuerUseRegistry.java
+++ b/src/main/java/com/android/tools/r8/shaking/DefaultEnqueuerUseRegistry.java
@@ -134,6 +134,11 @@
}
@Override
+ public void registerExceptionGuard(DexType guard) {
+ enqueuer.traceExceptionGuard(guard, context);
+ }
+
+ @Override
public void registerMethodHandle(DexMethodHandle methodHandle, MethodHandleUse use) {
super.registerMethodHandle(methodHandle, use);
enqueuer.traceMethodHandle(methodHandle, use, context);
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 21bf839..230e5ca 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -65,6 +65,7 @@
import com.android.tools.r8.graph.analysis.DesugaredLibraryConversionWrapperAnalysis;
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.EnqueuerInstanceOfAnalysis;
import com.android.tools.r8.graph.analysis.EnqueuerInvokeAnalysis;
import com.android.tools.r8.ir.analysis.proto.ProtoEnqueuerUseRegistry;
@@ -178,6 +179,7 @@
private Set<EnqueuerAnalysis> analyses = Sets.newIdentityHashSet();
private Set<EnqueuerInvokeAnalysis> invokeAnalyses = Sets.newIdentityHashSet();
private Set<EnqueuerInstanceOfAnalysis> instanceOfAnalyses = Sets.newIdentityHashSet();
+ private Set<EnqueuerExceptionGuardAnalysis> exceptionGuardAnalyses = Sets.newIdentityHashSet();
private Set<EnqueuerCheckCastAnalysis> checkCastAnalyses = Sets.newIdentityHashSet();
// Don't hold a direct pointer to app info (use appView).
@@ -436,6 +438,11 @@
return this;
}
+ public Enqueuer registerExceptionGuardAnalysis(EnqueuerExceptionGuardAnalysis analysis) {
+ exceptionGuardAnalyses.add(analysis);
+ return this;
+ }
+
public void setAnnotationRemoverBuilder(AnnotationRemover.Builder annotationRemoverBuilder) {
this.annotationRemoverBuilder = annotationRemoverBuilder;
}
@@ -1027,6 +1034,11 @@
traceTypeReference(type, currentMethod);
}
+ void traceExceptionGuard(DexType guard, ProgramMethod currentMethod) {
+ exceptionGuardAnalyses.forEach(analysis -> analysis.traceExceptionGuard(guard, currentMethod));
+ traceTypeReference(guard, currentMethod);
+ }
+
void traceInvokeDirect(DexMethod invokedMethod, ProgramMethod context) {
boolean skipTracing =
registerDeferredActionForDeadProtoBuilder(
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/DistinguishExceptionClassesTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/DistinguishExceptionClassesTest.java
new file mode 100644
index 0000000..c751735
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/DistinguishExceptionClassesTest.java
@@ -0,0 +1,57 @@
+// 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.classmerging.horizontal;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.android.tools.r8.TestParameters;
+import org.junit.Test;
+
+public class DistinguishExceptionClassesTest extends HorizontalClassMergingTestBase {
+ public DistinguishExceptionClassesTest(
+ TestParameters parameters, boolean enableHorizontalClassMerging) {
+ super(parameters, enableHorizontalClassMerging);
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepMainRule(Main.class)
+ .addOptionsModification(
+ options -> options.enableHorizontalClassMerging = enableHorizontalClassMerging)
+ .setMinApi(parameters.getApiLevel())
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines("test success")
+ .inspect(
+ codeInspector -> {
+ assertThat(codeInspector.clazz(Exception1.class), isPresent());
+ assertThat(codeInspector.clazz(Exception2.class), isPresent());
+ });
+ }
+
+ public static class Exception1 extends Exception {}
+
+ public static class Exception2 extends Exception {}
+
+ public static class Main {
+ public static void main(String[] args) {
+ try {
+ try {
+ if (System.currentTimeMillis() > 0) {
+ throw new Exception2();
+ } else {
+ throw new Exception1();
+ }
+ } catch (Exception1 ex) {
+ System.out.println("test failed");
+ }
+ } catch (Exception2 ex) {
+ System.out.println("test success");
+ }
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/classmerging/vertical/ExceptionTablesTest.java b/src/test/java/com/android/tools/r8/classmerging/vertical/ExceptionTablesTest.java
index 167c6c1..ababe2f 100644
--- a/src/test/java/com/android/tools/r8/classmerging/vertical/ExceptionTablesTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/vertical/ExceptionTablesTest.java
@@ -38,7 +38,6 @@
@Test
public void testClassesHaveBeenMerged() throws Exception {
- expectThrowsWithHorizontalClassMerging();
testForR8(parameters.getBackend())
.addInnerClasses(ExceptionTablesTest.class)
.addKeepMainRule(TestClass.class)
