Fix stack overflow
Stack overflow was caused by input with "loops" in the
EnclosingMethod/InnerClasses attributes. A concrete case for the
EnclosingMethod case has been seen in class files generated by
the Kotlin compiler.
Bug: b/366140351
Change-Id: I6ee0f9af52fb578235f5e031762b44e67d140026
diff --git a/src/main/java/com/android/tools/r8/graph/GenericSignatureContextBuilder.java b/src/main/java/com/android/tools/r8/graph/GenericSignatureContextBuilder.java
index 0cc9356..84bd5a1 100644
--- a/src/main/java/com/android/tools/r8/graph/GenericSignatureContextBuilder.java
+++ b/src/main/java/com/android/tools/r8/graph/GenericSignatureContextBuilder.java
@@ -10,6 +10,7 @@
import com.android.tools.r8.graph.GenericSignature.FormalTypeParameter;
import com.android.tools.r8.graph.GenericSignature.MethodTypeSignature;
import com.android.tools.r8.utils.WorkList;
+import com.google.common.collect.Sets;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
@@ -53,7 +54,6 @@
}
public static class TypeParameterContext {
-
private static final TypeParameterContext EMPTY =
new TypeParameterContext(Collections.emptyMap(), Collections.emptySet());
@@ -207,17 +207,38 @@
public TypeParameterContext computeTypeParameterContext(
AppView<?> appView, DexReference reference, Predicate<DexType> wasPruned) {
assert !wasPruned.test(reference.getContextType()) : "Building context for pruned type";
- return computeTypeParameterContext(appView, reference, wasPruned, false);
+ return computeTypeParameterContext(appView, reference, wasPruned, false, null);
}
private TypeParameterContext computeTypeParameterContext(
AppView<?> appView,
DexReference reference,
Predicate<DexType> wasPruned,
- boolean seenPruned) {
+ boolean seenPruned,
+ Object seen) {
if (reference == null) {
return empty();
}
+
+ // Optimize seen set to be null when empty and a DexReference when it only has one element.
+ if (seen == null) {
+ seen = reference;
+ } else if (seen instanceof DexReference) {
+ if (seen == reference) {
+ return empty();
+ }
+ DexReference tmp = (DexReference) seen;
+ seen = Sets.newIdentityHashSet();
+ ((Set<DexReference>) seen).add(tmp);
+ }
+ assert seen != null;
+ assert seen instanceof DexReference || seen instanceof Set;
+ if (seen instanceof Set) {
+ if (!((Set<DexReference>) seen).add(reference)) {
+ return empty();
+ }
+ }
+
DexType contextType = reference.getContextType();
// TODO(b/187035453): We should visit generic signatures in the enqueuer.
DexClass clazz = appView.appInfo().definitionForWithoutExistenceAssert(contextType);
@@ -242,7 +263,8 @@
enclosingReference,
wasPruned,
prunedHere
- || hasPrunedRelationship(appView, enclosingReference, contextType, wasPruned))
+ || hasPrunedRelationship(appView, enclosingReference, contextType, wasPruned),
+ seen)
// Add formals for the context
.combine(formalsInfo, prunedHere);
if (!reference.isDexMethod()) {
diff --git a/src/test/java/com/android/tools/r8/graph/genericsignature/GenericSignatureSelfReferencingEnclosingConstructorTest.java b/src/test/java/com/android/tools/r8/graph/genericsignature/GenericSignatureSelfReferencingEnclosingConstructorTest.java
index d9bbc23..79c68c2 100644
--- a/src/test/java/com/android/tools/r8/graph/genericsignature/GenericSignatureSelfReferencingEnclosingConstructorTest.java
+++ b/src/test/java/com/android/tools/r8/graph/genericsignature/GenericSignatureSelfReferencingEnclosingConstructorTest.java
@@ -3,19 +3,14 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.graph.genericsignature;
-import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertThrows;
-import static org.junit.Assert.assertTrue;
import static org.junit.Assume.assumeTrue;
-import com.android.tools.r8.CompilationFailedException;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.references.Reference;
-import com.android.tools.r8.utils.ExceptionDiagnostic;
import com.android.tools.r8.utils.StringUtils;
import java.io.IOException;
import java.nio.file.Path;
@@ -40,6 +35,7 @@
private static final String CONSTRUCTOR =
TestClass.class.getTypeName() + "$1A(" + TestClass.class.getTypeName() + ")";
private static final String EXPECTED_OUTPUT = StringUtils.lines(CONSTRUCTOR);
+ private static final String EXPECTED_OUTPUT_R8_DEX = StringUtils.lines("public " + CONSTRUCTOR);
@Test
public void testJvm() throws Exception {
@@ -65,24 +61,27 @@
@Test
public void testR8() throws Exception {
- // TODO(b/366140351): This should not crash the compiler.
- assertThrows(
- CompilationFailedException.class,
- () ->
- testForR8(parameters.getBackend())
- .addProgramClasses(TestClass.class)
- .addProgramClassFileData(
- getProgramClassFileDataWithSelfReferencingEnclosingMethod())
- .addKeepMainRule(TestClass.class)
- .setMinApi(parameters)
- .compileWithExpectedDiagnostics(
- diagnostics -> {
- assertEquals(1, diagnostics.getErrors().size());
- assertTrue(diagnostics.getErrors().get(0) instanceof ExceptionDiagnostic);
- assertTrue(
- ((ExceptionDiagnostic) diagnostics.getErrors().get(0)).getCause()
- instanceof StackOverflowError);
- }));
+ testForR8(parameters.getBackend())
+ .addProgramClasses(TestClass.class)
+ .addProgramClassFileData(getProgramClassFileDataWithSelfReferencingEnclosingMethod())
+ .addKeepMainRule(TestClass.class)
+ .addKeepAttributeInnerClassesAndEnclosingMethod()
+ .addKeepRules("-keep class **$1A { <methods>; }")
+ .setMinApi(parameters)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutput(
+ parameters.isDexRuntime() ? EXPECTED_OUTPUT_R8_DEX : EXPECTED_OUTPUT);
+ }
+
+ @Test
+ public void testR8NoKeep() throws Exception {
+ testForR8(parameters.getBackend())
+ .addProgramClasses(TestClass.class)
+ .addProgramClassFileData(getProgramClassFileDataWithSelfReferencingEnclosingMethod())
+ .addKeepMainRule(TestClass.class)
+ .setMinApi(parameters)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("null");
}
// Change the EnclosingMethod attribute for the local class A to be a constructor of A itself.
diff --git a/src/test/java/com/android/tools/r8/graph/genericsignature/GenericSignatureSelfReferencingEnclosingMethodTest.java b/src/test/java/com/android/tools/r8/graph/genericsignature/GenericSignatureSelfReferencingEnclosingMethodTest.java
index b3a2931..92685d4 100644
--- a/src/test/java/com/android/tools/r8/graph/genericsignature/GenericSignatureSelfReferencingEnclosingMethodTest.java
+++ b/src/test/java/com/android/tools/r8/graph/genericsignature/GenericSignatureSelfReferencingEnclosingMethodTest.java
@@ -3,19 +3,14 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.graph.genericsignature;
-import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertThrows;
-import static org.junit.Assert.assertTrue;
import static org.junit.Assume.assumeTrue;
-import com.android.tools.r8.CompilationFailedException;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.references.Reference;
-import com.android.tools.r8.utils.ExceptionDiagnostic;
import com.android.tools.r8.utils.StringUtils;
import java.io.IOException;
import java.nio.file.Path;
@@ -63,24 +58,26 @@
@Test
public void testR8() throws Exception {
- // TODO(b/366140351): This should not crash the compiler.
- assertThrows(
- CompilationFailedException.class,
- () ->
- testForR8(parameters.getBackend())
- .addProgramClasses(TestClass.class)
- .addProgramClassFileData(
- getProgramClassFileDataWithSelfReferencingEnclosingMethod())
- .addKeepMainRule(TestClass.class)
- .setMinApi(parameters)
- .compileWithExpectedDiagnostics(
- diagnostics -> {
- assertEquals(1, diagnostics.getErrors().size());
- assertTrue(diagnostics.getErrors().get(0) instanceof ExceptionDiagnostic);
- assertTrue(
- ((ExceptionDiagnostic) diagnostics.getErrors().get(0)).getCause()
- instanceof StackOverflowError);
- }));
+ testForR8(parameters.getBackend())
+ .addProgramClasses(TestClass.class)
+ .addProgramClassFileData(getProgramClassFileDataWithSelfReferencingEnclosingMethod())
+ .addKeepMainRule(TestClass.class)
+ .addKeepAttributeInnerClassesAndEnclosingMethod()
+ .addKeepRules("-keep class **$1A { <methods>; }")
+ .setMinApi(parameters)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutput(EXPECTED_OUTPUT);
+ }
+
+ @Test
+ public void testR8NoKeep() throws Exception {
+ testForR8(parameters.getBackend())
+ .addProgramClasses(TestClass.class)
+ .addProgramClassFileData(getProgramClassFileDataWithSelfReferencingEnclosingMethod())
+ .addKeepMainRule(TestClass.class)
+ .setMinApi(parameters)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("No method m", "null");
}
// Change the EnclosingMethod attribute for the local class A to be method m of A itself.
@@ -101,7 +98,11 @@
class A {
public void m() {
try {
- System.out.println(getClass().getMethod("m"));
+ try {
+ System.out.println(getClass().getMethod("m"));
+ } catch (NoSuchMethodException e) {
+ System.out.println("No method m");
+ }
System.out.println(A.class.getEnclosingMethod());
} catch (Exception e) {
System.out.println("Unexpected " + e);
diff --git a/src/test/java/com/android/tools/r8/graph/genericsignature/GenericSignatureSelfReferencingInnerClassesTest.java b/src/test/java/com/android/tools/r8/graph/genericsignature/GenericSignatureSelfReferencingInnerClassesTest.java
index f0384ac..446978f 100644
--- a/src/test/java/com/android/tools/r8/graph/genericsignature/GenericSignatureSelfReferencingInnerClassesTest.java
+++ b/src/test/java/com/android/tools/r8/graph/genericsignature/GenericSignatureSelfReferencingInnerClassesTest.java
@@ -3,20 +3,16 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.graph.genericsignature;
-import static org.junit.Assert.assertEquals;
+import static org.hamcrest.CoreMatchers.containsString;
import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertThrows;
-import static org.junit.Assert.assertTrue;
import static org.junit.Assume.assumeTrue;
-import com.android.tools.r8.CompilationFailedException;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.graph.genericsignature.GenericSignatureSelfReferencingInnerClassesTest.A.B;
import com.android.tools.r8.references.Reference;
-import com.android.tools.r8.utils.ExceptionDiagnostic;
import com.android.tools.r8.utils.StringUtils;
import java.io.IOException;
import java.nio.file.Path;
@@ -64,23 +60,35 @@
@Test
public void testR8() throws Exception {
- // TODO(b/366140351): This should not crash the compiler.
- assertThrows(
- CompilationFailedException.class,
- () ->
- testForR8(parameters.getBackend())
- .addProgramClasses(TestClass.class)
- .addProgramClassFileData(getProgramClassFileDataWithSelfReferencingInnerClass())
- .addKeepMainRule(TestClass.class)
- .setMinApi(parameters)
- .compileWithExpectedDiagnostics(
- diagnostics -> {
- assertEquals(1, diagnostics.getErrors().size());
- assertTrue(diagnostics.getErrors().get(0) instanceof ExceptionDiagnostic);
- assertTrue(
- ((ExceptionDiagnostic) diagnostics.getErrors().get(0)).getCause()
- instanceof StackOverflowError);
- }));
+ testForR8(parameters.getBackend())
+ .addProgramClasses(TestClass.class)
+ .addProgramClassFileData(getProgramClassFileDataWithSelfReferencingInnerClass())
+ .addKeepMainRule(TestClass.class)
+ .addKeepAttributeInnerClassesAndEnclosingMethod()
+ .addKeepRules("-keep class **B { <methods>; }")
+ .setMinApi(parameters)
+ .allowDiagnosticInfoMessages()
+ .compile()
+ .assertAllInfoMessagesMatch(containsString("Malformed inner-class attribute"))
+ .run(parameters.getRuntime(), TestClass.class)
+ .applyIf(
+ parameters.isCfRuntime(),
+ r -> r.assertFailureWithErrorThatThrows(ClassFormatError.class),
+ r -> r.assertSuccessWithOutput(EXPECTED_OUTPUT));
+ }
+
+ @Test
+ public void testR8NoKeep() throws Exception {
+ testForR8(parameters.getBackend())
+ .addProgramClasses(TestClass.class)
+ .addProgramClassFileData(getProgramClassFileDataWithSelfReferencingInnerClass())
+ .addKeepMainRule(TestClass.class)
+ .setMinApi(parameters)
+ .allowDiagnosticInfoMessages()
+ .compile()
+ .assertAllInfoMessagesMatch(containsString("Malformed inner-class attribute"))
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("class " + getClass().getPackage().getName() + ".a", "null");
}
// Change the InnerClasses attribute for the inner class B to be B itself.
