Desugaring of try-with-resources. Adds desugaring of try-with-resources for pre-19 min-sdk version, it essentially boils down to removing all calls to Throwable::addSuppressed(...) and replacing all calls to Throwable::getSuppressed() with empty array of Throwable. The desugaring requires library and class path since it needs to inspect/check exception class hierarchy. Disabling it by default until we measure performance impact. Bug:37744723 BUG= Change-Id: If2781ec68cab3a2b38127ee20fffb94ceac3157a
diff --git a/src/main/java/com/android/tools/r8/D8Command.java b/src/main/java/com/android/tools/r8/D8Command.java index 48d8690..a3d92ac 100644 --- a/src/main/java/com/android/tools/r8/D8Command.java +++ b/src/main/java/com/android/tools/r8/D8Command.java
@@ -143,6 +143,7 @@ assert internal.useTreeShaking; internal.useTreeShaking = false; assert internal.interfaceMethodDesugaring == OffOrAuto.Off; + assert internal.tryWithResourcesDesugaring == OffOrAuto.Off; assert internal.allowAccessModification; internal.allowAccessModification = false; assert internal.inlineAccessors;
diff --git a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java index 5bff70d..299be81 100644 --- a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java +++ b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
@@ -86,6 +86,7 @@ public DexString stringDescriptor = createString("Ljava/lang/String;"); public DexString objectDescriptor = createString("Ljava/lang/Object;"); public DexString classDescriptor = createString("Ljava/lang/Class;"); + public DexString throwableDescriptor = createString("Ljava/lang/Throwable;"); public DexString objectsDescriptor = createString("Ljava/util/Objects;"); public DexString constructorMethodName = createString(Constants.INSTANCE_INITIALIZER_NAME); @@ -94,6 +95,7 @@ public DexString thisName = createString("this"); private DexString charArrayDescriptor = createString("[C"); + public DexString throwableArrayDescriptor = createString("[Ljava/lang/Throwable;"); public DexType booleanType = createType(booleanDescriptor); public DexType byteType = createType(byteDescriptor); @@ -117,6 +119,7 @@ public DexType stringType = createType(stringDescriptor); public DexType objectType = createType(objectDescriptor); + public DexType throwableType = createType(throwableDescriptor); public StringBuildingMethods stringBuilderMethods = new StringBuildingMethods(createString("Ljava/lang/StringBuilder;")); @@ -125,6 +128,7 @@ public ObjectsMethods objectsMethods = new ObjectsMethods(); public ObjectMethods objectMethods = new ObjectMethods(); public LongMethods longMethods = new LongMethods(); + public ThrowableMethods throwableMethods = new ThrowableMethods(); public void clearSubtypeInformation() { types.values().forEach(DexType::clearSubtypeInformation); @@ -139,6 +143,18 @@ } } + public class ThrowableMethods { + public final DexMethod addSuppressed; + public final DexMethod getSuppressed; + + private ThrowableMethods() { + addSuppressed = createMethod(throwableDescriptor, + createString("addSuppressed"), voidDescriptor, new DexString[] { throwableDescriptor }); + getSuppressed = createMethod(throwableDescriptor, + createString("getSuppressed"), throwableArrayDescriptor, DexString.EMPTY_ARRAY); + } + } + public class ObjectMethods { public DexMethod getClass; @@ -410,18 +426,18 @@ private DexString shorty(DexType returnType, DexType[] parameters) { StringBuilder builder = new StringBuilder(); - builder.append(returnType.toDescriptorString().charAt(0)); + addToShorty(builder, returnType); for (DexType parameter : parameters) { - String descriptor = parameter.toDescriptorString(); - if (descriptor.charAt(0) == '[') { - builder.append('L'); - } else { - builder.append(descriptor.charAt(0)); - } + addToShorty(builder, parameter); } return createString(builder.toString()); } + private void addToShorty(StringBuilder builder, DexType type) { + char first = type.toDescriptorString().charAt(0); + builder.append(first == '[' ? 'L' : first); + } + private static <S extends PresortedComparable<S>> void assignSortedIndices(Collection<S> items, NamingLens namingLens) { List<S> sorted = new ArrayList<>(items);
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 fa7ee4d..22ce470 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
@@ -132,6 +132,16 @@ throw new Unreachable(); } + private boolean enableTryWithResourcesDesugaring() { + switch (options.tryWithResourcesDesugaring) { + case Off: + return false; + case Auto: + return !options.canUseSuppressedExceptions(); + } + throw new Unreachable(); + } + private void markLibraryMethodsReturningReceiver() { DexItemFactory dexItemFactory = appInfo.dexItemFactory; dexItemFactory.stringBuilderMethods.forEeachAppendMethod(this::markReturnsReceiver); @@ -427,6 +437,10 @@ DeadCodeRemover.removeDeadCode(code, codeRewriter, options); assert code.isConsistentSSA(); + if (enableTryWithResourcesDesugaring()) { + codeRewriter.rewriteThrowableAddAndGetSuppressed(code); + } + if (lambdaRewriter != null) { lambdaRewriter.desugarLambdas(method, code); assert code.isConsistentSSA();
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java index 5e701ed..81a9e55 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
@@ -5,7 +5,9 @@ package com.android.tools.r8.ir.optimize; import com.android.tools.r8.dex.Constants; +import com.android.tools.r8.errors.CompilationError; import com.android.tools.r8.graph.AppInfo; +import com.android.tools.r8.graph.DexClass; import com.android.tools.r8.graph.DexEncodedMethod; import com.android.tools.r8.graph.DexItemFactory; import com.android.tools.r8.graph.DexMethod; @@ -17,6 +19,7 @@ import com.android.tools.r8.ir.code.Cmp; import com.android.tools.r8.ir.code.Cmp.Bias; import com.android.tools.r8.ir.code.ConstNumber; +import com.android.tools.r8.ir.code.ConstType; import com.android.tools.r8.ir.code.DominatorTree; import com.android.tools.r8.ir.code.Goto; import com.android.tools.r8.ir.code.IRCode; @@ -938,4 +941,88 @@ } assert code.isConsistentSSA(); } + + // Removes calls to Throwable.addSuppressed(Throwable) and rewrites + // Throwable.getSuppressed() into new Throwable[0]. + // + // Note that addSuppressed() and getSuppressed() methods are final in + // Throwable, so these changes don't have to worry about overrides. + public void rewriteThrowableAddAndGetSuppressed(IRCode code) { + boolean removeUnneededCatchHandlers = false; + DexItemFactory.ThrowableMethods throwableMethods = dexItemFactory.throwableMethods; + + for (BasicBlock block : code.blocks) { + InstructionListIterator iterator = block.listIterator(); + while (iterator.hasNext()) { + Instruction current = iterator.next(); + if (current.isInvokeMethod()) { + DexMethod invokedMethod = current.asInvokeMethod().getInvokedMethod(); + + if (matchesMethodOfThrowable(invokedMethod, throwableMethods.addSuppressed)) { + // Remove Throwable::addSuppressed(Throwable) call. + iterator.remove(); + removeUnneededCatchHandlers = true; + + } else if (matchesMethodOfThrowable(invokedMethod, throwableMethods.getSuppressed)) { + Value destValue = current.outValue(); + if (destValue == null) { + // If the result of the call was not used we don't create + // an empty array and just remove the call. + iterator.remove(); + removeUnneededCatchHandlers = true; + continue; + } + + // Replace call to Throwable::getSuppressed() with new Throwable[0]. + + // First insert the constant value *before* the current instruction. + Value zero = new Value(code.valueNumberGenerator.next(), -1, MoveType.SINGLE, null); + assert iterator.hasPrevious(); + iterator.previous(); + iterator.add(new ConstNumber(ConstType.INT, zero, 0)); + + // Then replace the invoke instruction with NewArrayEmpty instruction. + Instruction next = iterator.next(); + assert current == next; + NewArrayEmpty newArray = new NewArrayEmpty(destValue, zero, + dexItemFactory.createType(dexItemFactory.throwableArrayDescriptor)); + iterator.replaceCurrentInstruction(newArray); + + // NOTE: nothing needs to be changed in catch handlers since we replace + // one throwable instruction with another. + } + } + } + } + + // If at least one addSuppressed(...) call was removed, or we were able + // to remove getSuppressed() call without replacing it with a new empty array, + // we need to deal with possible unreachable catch handlers. + if (removeUnneededCatchHandlers) { + removeUnneededCatchHandlers(code); + } + + assert code.isConsistentSSA(); + } + + private boolean matchesMethodOfThrowable(DexMethod invoked, DexMethod expected) { + return invoked.name == expected.name + && invoked.proto == expected.proto + && isSubtypeOfThrowable(invoked.holder); + } + + private boolean isSubtypeOfThrowable(DexType type) { + while (type != null && type != dexItemFactory.objectType) { + if (type == dexItemFactory.throwableType) { + return true; + } + DexClass dexClass = appInfo.definitionFor(type); + if (dexClass == null) { + throw new CompilationError("Class or interface " + type.toSourceString() + + " required for desugaring of try-with-resources is not found."); + } + type = dexClass.superType; + } + 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 1fd72f8..f80f947 100644 --- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java +++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -54,6 +54,8 @@ // Defines interface method rewriter behavior. public OffOrAuto interfaceMethodDesugaring = OffOrAuto.Off; + // Defines try-with-resources rewriter behavior. + public OffOrAuto tryWithResourcesDesugaring = OffOrAuto.Off; public boolean useTreeShaking = true; @@ -254,6 +256,10 @@ return minApiLevel >= Constants.ANDROID_K_API; } + public boolean canUseSuppressedExceptions() { + return minApiLevel >= Constants.ANDROID_K_API; + } + public boolean canUsePrivateInterfaceMethods() { return minApiLevel >= Constants.ANDROID_N_API; }
diff --git a/src/test/examplesAndroidO/trywithresources/TryWithResources.java b/src/test/examplesAndroidO/trywithresources/TryWithResources.java new file mode 100644 index 0000000..ac779d6 --- /dev/null +++ b/src/test/examplesAndroidO/trywithresources/TryWithResources.java
@@ -0,0 +1,237 @@ +// Copyright (c) 2017, 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 trywithresources; + +import java.io.Closeable; +import java.io.IOException; + +public abstract class TryWithResources { + // --- TEST SUPPORT --- + + interface Test { + void test() throws Throwable; + } + + private void test(Test test) { + try { + test.test(); + } catch (Throwable e) { + dumpException(e); + } + } + + private void dumpException(Throwable e) { + dumpException(e, "Exception: "); + } + + private void dumpException(Throwable e, String indent) { + assert e != null; + System.out.println(indent + e.getMessage()); + + indent = indent.replaceAll("[^:]", " "); + + Throwable cause = e.getCause(); + if (cause != null) { + dumpException(cause, indent + " cause: "); + } + + // Dump suppressed UNLESS it is a desugared code running + // on JVM, in which case we avoid dumping suppressed, since + // the output will be used for comparison with desugared code + // running on device. + if (!desugaredCodeRunningOnJvm()) { + Throwable[] suppressed = e.getSuppressed(); + for (int i = 0; i < suppressed.length; i++) { + dumpException(suppressed[i], indent + "supp[" + i + "]: "); + } + } + } + + abstract boolean desugaredCodeRunningOnJvm(); + + // --- TEST SYMBOLS --- + + static class Resource implements Closeable { + final String tag; + + Resource(String tag) { + this.tag = tag; + } + + @Override + public void close() throws IOException { + Class<? extends Resource> cls = this.getClass(); + System.out.println("Closing " + tag + " (" + + cls.getName().substring(TryWithResources.class.getName().length() + 1) + ")"); + } + } + + // --- TEST --- + + class RegularTryWithResources { + class RegularResource extends Resource { + RegularResource(String tag) { + super(tag); + } + } + + private void test() throws Throwable { + test(2); + } + + private void test(int level) throws Throwable { + try (RegularResource a = new RegularResource("a" + level); + RegularResource b = new RegularResource("b" + level)) { + if (level > 0) { + try { + test(level - 1); + } catch (Throwable e) { + throw new RuntimeException("e" + level, e); + } + } + throw new RuntimeException("primary cause"); + } + } + } + + // --- TEST --- + + class FailingTryWithResources { + class FailingResource extends Resource { + FailingResource(String tag) { + super(tag); + } + + @Override + public void close() throws IOException { + super.close(); + throw new RuntimeException("failed to close '" + tag + "'"); + } + } + + private void test() throws Throwable { + test(2); + } + + private void test(int level) throws Throwable { + try (FailingResource a = new FailingResource("a" + level); + FailingResource b = new FailingResource("b" + level)) { + if (level > 0) { + try { + test(level - 1); + } catch (Throwable e) { + throw new RuntimeException("e" + level, e); + } + } + throw new RuntimeException("primary cause"); + } + } + } + + // --- TEST --- + + class ExplicitAddGetSuppressed { + class RegularResource extends Resource { + RegularResource(String tag) { + super(tag); + } + + @Override + public void close() throws IOException { + super.close(); + throw new RuntimeException("failed to close '" + tag + "'"); + } + } + + private void test() throws Throwable { + test(2); + } + + private void test(int level) throws RuntimeException { + try (RegularResource a = new RegularResource("a" + level); + RegularResource b = new RegularResource("b" + level)) { + if (level > 0) { + try { + test(level - 1); + } catch (RuntimeException e) { + // Just collect suppressed, but throw away the exception. + RuntimeException re = new RuntimeException("e" + level); + for (Throwable suppressed : e.getSuppressed()) { + re.addSuppressed(suppressed); + } + throw re; + } + } + throw new RuntimeException("primary cause"); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + } + + // --- TEST --- + + interface Consumer { + void act(RuntimeException re); + } + + interface Supplier { + Throwable[] get(); + } + + class AddGetSuppressedRoundTrip { + private void test() throws Throwable { + RuntimeException carrier = new RuntimeException("carrier"); + Consumer packer = carrier::addSuppressed; + Supplier unpacker = carrier::getSuppressed; + + packer.act(new RuntimeException("original exception A")); + packer.act(new RuntimeException("original exception Z")); + + for (Throwable unpacked : unpacker.get()) { + if (!desugaredCodeRunningOnJvm()) { + dumpException(unpacked); + } + } + } + } + + // --- TEST --- + + class UnreachableCatchAfterCallsRemoved { + private void test() throws Throwable { + RuntimeException main = new RuntimeException("main"); + RuntimeException origA = new RuntimeException("original exception A"); + RuntimeException origB = new RuntimeException("original exception Z"); + + try { + // After both calls below are removed, the whole catch + // handler should be removed. + main.addSuppressed(origA); + main.addSuppressed(origB); + } catch (Throwable t) { + throw new RuntimeException("UNREACHABLE"); + } + + // Return value not used. + main.getSuppressed(); + } + } + + // --- MAIN TEST --- + + void test() throws Exception { + System.out.println("----- TEST 1 -----"); + test(new RegularTryWithResources()::test); + System.out.println("----- TEST 2 -----"); + test(new FailingTryWithResources()::test); + System.out.println("----- TEST 3 -----"); + test(new ExplicitAddGetSuppressed()::test); + System.out.println("----- TEST 4 -----"); + test(new AddGetSuppressedRoundTrip()::test); + System.out.println("----- TEST 5 -----"); + test(new UnreachableCatchAfterCallsRemoved()::test); + System.out.println("------------------"); + } +}
diff --git a/src/test/examplesAndroidO/trywithresources/TryWithResourcesDesugaredTests.java b/src/test/examplesAndroidO/trywithresources/TryWithResourcesDesugaredTests.java new file mode 100644 index 0000000..10a96f9 --- /dev/null +++ b/src/test/examplesAndroidO/trywithresources/TryWithResourcesDesugaredTests.java
@@ -0,0 +1,24 @@ +// Copyright (c) 2017, 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 trywithresources; + +public class TryWithResourcesDesugaredTests extends TryWithResources { + private boolean isAndroid() { + try { + Class.forName("dalvik.system.VMRuntime"); + return true; + } catch (Exception ignored) { + } + return false; + } + + @Override + boolean desugaredCodeRunningOnJvm() { + return !isAndroid(); + } + + public static void main(String[] args) throws Exception { + new TryWithResourcesDesugaredTests().test(); + } +}
diff --git a/src/test/examplesAndroidO/trywithresources/TryWithResourcesNotDesugaredTests.java b/src/test/examplesAndroidO/trywithresources/TryWithResourcesNotDesugaredTests.java new file mode 100644 index 0000000..d2a05c3 --- /dev/null +++ b/src/test/examplesAndroidO/trywithresources/TryWithResourcesNotDesugaredTests.java
@@ -0,0 +1,15 @@ +// Copyright (c) 2017, 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 trywithresources; + +public class TryWithResourcesNotDesugaredTests extends TryWithResources { + @Override + boolean desugaredCodeRunningOnJvm() { + return false; + } + + public static void main(String[] args) throws Exception { + new TryWithResourcesNotDesugaredTests().test(); + } +}
diff --git a/src/test/java/com/android/tools/r8/RunExamplesAndroidOTest.java b/src/test/java/com/android/tools/r8/RunExamplesAndroidOTest.java index 3f63319..457e7f4 100644 --- a/src/test/java/com/android/tools/r8/RunExamplesAndroidOTest.java +++ b/src/test/java/com/android/tools/r8/RunExamplesAndroidOTest.java
@@ -13,6 +13,11 @@ import com.android.tools.r8.ToolHelper.DexVm; import com.android.tools.r8.errors.CompilationError; +import com.android.tools.r8.utils.DexInspector; +import com.android.tools.r8.utils.DexInspector.FoundClassSubject; +import com.android.tools.r8.utils.DexInspector.FoundMethodSubject; +import com.android.tools.r8.utils.DexInspector.InstructionSubject; +import com.android.tools.r8.utils.DexInspector.InvokeInstructionSubject; import com.android.tools.r8.utils.InternalOptions; import com.android.tools.r8.utils.OffOrAuto; import com.google.common.collect.ImmutableList; @@ -20,10 +25,13 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.function.Consumer; +import java.util.function.Predicate; import java.util.function.UnaryOperator; +import org.junit.Assert; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -38,6 +46,7 @@ final String mainClass; final List<Consumer<InternalOptions>> optionConsumers = new ArrayList<>(); + final List<Consumer<DexInspector>> dexInspectorChecks = new ArrayList<>(); final List<UnaryOperator<B>> builderTransformations = new ArrayList<>(); TestRunner(String testName, String packageName, String mainClass) { @@ -46,6 +55,35 @@ this.mainClass = mainClass; } + TestRunner withDexCheck(Consumer<DexInspector> check) { + dexInspectorChecks.add(check); + return this; + } + + TestRunner withClassCheck(Consumer<FoundClassSubject> check) { + withDexCheck(inspector -> inspector.forAllClasses(check)); + return this; + } + + TestRunner withMethodCheck(Consumer<FoundMethodSubject> check) { + withClassCheck(clazz -> clazz.forAllMethods(check)); + return this; + } + + <T extends InstructionSubject> TestRunner + withInstructionCheck(Predicate<InstructionSubject> filter, Consumer<T> check) { + withMethodCheck(method -> { + if (method.isAbstract()) { + return; + } + Iterator<T> iterator = method.iterateInstructions(filter); + while (iterator.hasNext()) { + check.accept(iterator.next()); + } + }); + return this; + } + TestRunner withOptionConsumer(Consumer<InternalOptions> consumer) { optionConsumers.add(consumer); return this; @@ -55,6 +93,10 @@ return withOptionConsumer(o -> o.interfaceMethodDesugaring = behavior); } + TestRunner withTryWithResourcesDesugaring(OffOrAuto behavior) { + return withOptionConsumer(o -> o.tryWithResourcesDesugaring = behavior); + } + TestRunner withBuilderTransformation(UnaryOperator<B> builderTransformation) { builderTransformations.add(builderTransformation); return this; @@ -86,6 +128,13 @@ thrown.expect(Throwable.class); } + if (!dexInspectorChecks.isEmpty()) { + DexInspector inspector = new DexInspector(out); + for (Consumer<DexInspector> check : dexInspectorChecks) { + check.accept(inspector); + } + } + String output = ToolHelper.runArtNoVerificationErrors(out.toString(), qualifiedMainClass); if (!expectedToFail) { ToolHelper.ProcessResult javaResult = @@ -238,5 +287,24 @@ test("repeat_annotations", "repeat_annotations", "RepeatAnnotations").run(); } + @Test + public void testTryWithResources() throws Throwable { + test("try-with-resources-simplified", "trywithresources", "TryWithResourcesNotDesugaredTests") + .withTryWithResourcesDesugaring(OffOrAuto.Off) + .run(); + } + + @Test + public void testTryWithResourcesDesugared() throws Throwable { + test("try-with-resources-simplified", "trywithresources", "TryWithResourcesDesugaredTests") + .withTryWithResourcesDesugaring(OffOrAuto.Auto) + .withInstructionCheck(InstructionSubject::isInvoke, + (InvokeInstructionSubject invoke) -> { + Assert.assertFalse(invoke.invokedMethod().name.toString().equals("addSuppressed")); + Assert.assertFalse(invoke.invokedMethod().name.toString().equals("getSuppressed")); + }) + .run(); + } + abstract TestRunner test(String testName, String packageName, String mainClass); }