Ensure member classes are not non-member classes.
For anonymous classes (non-null outer and null inner-name), we can
recover by erasing outer-class references.
For local classes (non-null inner-name and enclosing member), though,
it is unclear whether the intention was a member class or local class.
So we can't remove outer-class references in this case, and rather fail
hard.
Bug: 137881258
Change-Id: I4f1159fe978729bc72d959575b3707172ab2edb0
diff --git a/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java b/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
index eae05d2..ee88c49 100644
--- a/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
+++ b/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
@@ -417,6 +417,33 @@
directMethods.toArray(DexEncodedMethod.EMPTY_ARRAY),
virtualMethods.toArray(DexEncodedMethod.EMPTY_ARRAY),
application.getFactory().getSkipNameValidationForTesting());
+ InnerClassAttribute innerClassAttribute = clazz.getInnerClassAttributeForThisClass();
+ // A member class should not be a local or anonymous class.
+ if (innerClassAttribute != null && innerClassAttribute.getOuter() != null) {
+ if (innerClassAttribute.isAnonymous()) {
+ assert innerClassAttribute.getInnerName() == null;
+ // If the enclosing member is not null, the intention would be an anonymous class, and
+ // thus the outer-class reference should have been null.
+ // If the enclosing member is null, it is likely due to the missing enclosing member, and
+ // we will warn that below. In either case, we can recover InnerClasses attribute by
+ // erasing the outer-class reference.
+ application.options.warningInvalidNonMemberClasses(
+ origin, enclosingMember, innerClassAttribute);
+ InnerClassAttribute recoveredAttribute = new InnerClassAttribute(
+ innerClassAttribute.getAccess(), innerClassAttribute.getInner(), null, null);
+ clazz.replaceInnerClassAttributeForThisClass(recoveredAttribute);
+ } else if (enclosingMember != null) {
+ assert innerClassAttribute.isNamed();
+ // It is unclear whether the intention was a member class or a local class. Fail hard.
+ throw new CompilationError(
+ StringUtils.lines(
+ "A member class should be a (non-member) local class at the same time.",
+ "This is likely due to invalid EnclosingMethod and InnerClasses attributes:",
+ enclosingMember.toString(),
+ innerClassAttribute.toString()),
+ origin);
+ }
+ }
if (enclosingMember == null
&& (clazz.isLocalClass() || clazz.isAnonymousClass())
&& getMajorVersion() > V1_6) {
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 cccd363..e4b84f6 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -30,6 +30,8 @@
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.EnclosingMethodAttribute;
+import com.android.tools.r8.graph.InnerClassAttribute;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.optimize.Inliner;
import com.android.tools.r8.origin.Origin;
@@ -538,6 +540,30 @@
return ImmutableSet.of();
}
+ private static class InvalidNonMemberClassInfo {
+ final EnclosingMethodAttribute enclosingMember;
+ final InnerClassAttribute innerClassAttribute;
+
+ InvalidNonMemberClassInfo(
+ EnclosingMethodAttribute enclosingMember,
+ InnerClassAttribute innerClassAttribute) {
+ this.enclosingMember = enclosingMember;
+ this.innerClassAttribute = innerClassAttribute;
+ }
+
+ @Override
+ public String toString() {
+ List<String> attrs = new ArrayList<>();
+ if (enclosingMember != null) {
+ attrs.add(" EnclosingMethod: " + enclosingMember.toString());
+ }
+ if (innerClassAttribute != null) {
+ attrs.add(" InnerClasses: " + innerClassAttribute.toString());
+ }
+ return StringUtils.join(attrs, System.lineSeparator());
+ }
+ }
+
public static class InvalidParameterAnnotationInfo {
final DexMethod method;
@@ -565,6 +591,9 @@
private final Map<Origin, List<TypeVersionPair>> missingEnclosingMembers = new HashMap<>();
+ private final Map<Origin, InvalidNonMemberClassInfo> warningInvalidNonMemberClasses =
+ new HashMap<>();
+
private final Map<Origin, List<InvalidParameterAnnotationInfo>> warningInvalidParameterAnnotations
= new HashMap<>();
@@ -771,6 +800,17 @@
}
}
+ public void warningInvalidNonMemberClasses(
+ Origin origin,
+ EnclosingMethodAttribute enclosingMethodAttribute,
+ InnerClassAttribute innerClassAttribute) {
+ InvalidNonMemberClassInfo info =
+ new InvalidNonMemberClassInfo(enclosingMethodAttribute, innerClassAttribute);
+ synchronized (warningInvalidNonMemberClasses) {
+ warningInvalidNonMemberClasses.put(origin, info);
+ }
+ }
+
public void warningInvalidParameterAnnotations(
DexMethod method, Origin origin, int expected, int actual) {
InvalidParameterAnnotationInfo info =
@@ -837,6 +877,21 @@
printed = true;
printOutdatedToolchain = true;
}
+ if (warningInvalidNonMemberClasses.size() > 0) {
+ reporter.info(
+ new StringDiagnostic(
+ "A member class should not be a local or anonymous class at the same time."
+ + " Such InnerClasses attributes are recovered."));
+ for (Origin origin : new TreeSet<>(warningInvalidNonMemberClasses.keySet())) {
+ StringBuilder builder =
+ new StringBuilder("Invalid EnclosingMethod/InnerClasses attributes:");
+ InvalidNonMemberClassInfo info = warningInvalidNonMemberClasses.get(origin);
+ builder.append(System.lineSeparator());
+ builder.append(info.toString());
+ reporter.info(new StringDiagnostic(builder.toString(), origin));
+ }
+ printed = true;
+ }
if (missingEnclosingMembers.size() > 0) {
reporter.info(
new StringDiagnostic(
diff --git a/src/test/java/com/android/tools/r8/d8/MemberAndLocalClassTest.java b/src/test/java/com/android/tools/r8/d8/MemberAndLocalClassTest.java
new file mode 100644
index 0000000..e4afafd
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/d8/MemberAndLocalClassTest.java
@@ -0,0 +1,135 @@
+// Copyright (c) 2019, 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.d8;
+
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.fail;
+
+import com.android.tools.r8.CompilationFailedException;
+import com.android.tools.r8.TestBase;
+import org.junit.Test;
+import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.Label;
+import org.objectweb.asm.MethodVisitor;
+import org.objectweb.asm.Opcodes;
+
+public class MemberAndLocalClassTest extends TestBase {
+
+ @Test
+ public void testD8() throws Exception {
+ try {
+ testForD8()
+ .addProgramClassFileData(Dump.dump())
+ .compile();
+ fail("Expected to fail due to invalid EnclosingMethod attribute");
+ } catch (CompilationFailedException e) {
+ String message = e.getCause().getMessage();
+
+ // We want to avoid showing mysterious messages.
+ assertThat(message, not(containsString("Unsorted annotation set")));
+ assertThat(message, not(containsString("dalvik.annotation.EnclosingClass")));
+
+ assertThat(message, containsString("invalid EnclosingMethod"));
+ }
+ }
+
+ // Compiled the following kt code:
+ // class WebContext {
+ // companion object {
+ // val debug = true
+ // }
+ // }
+ // and added EnclosingMethod attribute as described at b/137881258#comment7
+ private static class Dump implements Opcodes {
+
+ public static byte[] dump () throws Exception {
+
+ ClassWriter classWriter = new ClassWriter(0);
+ MethodVisitor methodVisitor;
+
+ classWriter.visit(
+ V1_6,
+ ACC_PUBLIC | ACC_FINAL | ACC_SUPER,
+ "WebContext$Companion",
+ null,
+ "java/lang/Object",
+ null);
+
+ classWriter.visitSource("WebContext.kt", null);
+
+ // Manually added, indicating this could be a local class.
+ classWriter.visitOuterClass("WebContext", null, null);
+
+ // But, it's actually a member class.
+ classWriter.visitInnerClass(
+ "WebContext$Companion", "WebContext", "Companion", ACC_PUBLIC | ACC_FINAL | ACC_STATIC);
+
+ {
+ methodVisitor =
+ classWriter.visitMethod(ACC_PUBLIC | ACC_FINAL, "getDebug", "()Z", null, null);
+ methodVisitor.visitCode();
+ Label label0 = new Label();
+ methodVisitor.visitLabel(label0);
+ methodVisitor.visitLineNumber(3, label0);
+ methodVisitor.visitMethodInsn(
+ INVOKESTATIC, "WebContext", "access$getDebug$cp", "()Z", false);
+ methodVisitor.visitInsn(IRETURN);
+ Label label1 = new Label();
+ methodVisitor.visitLabel(label1);
+ methodVisitor.visitLocalVariable("this", "LWebContext$Companion;", null, label0, label1, 0);
+ methodVisitor.visitMaxs(1, 1);
+ methodVisitor.visitEnd();
+ }
+ {
+ methodVisitor = classWriter.visitMethod(ACC_PRIVATE, "<init>", "()V", null, null);
+ methodVisitor.visitCode();
+ Label label0 = new Label();
+ methodVisitor.visitLabel(label0);
+ methodVisitor.visitLineNumber(2, label0);
+ methodVisitor.visitVarInsn(ALOAD, 0);
+ methodVisitor.visitMethodInsn(INVOKESPECIAL, "java/lang/Object", "<init>", "()V", false);
+ methodVisitor.visitInsn(RETURN);
+ Label label1 = new Label();
+ methodVisitor.visitLabel(label1);
+ methodVisitor.visitLocalVariable("this", "LWebContext$Companion;", null, label0, label1, 0);
+ methodVisitor.visitMaxs(1, 1);
+ methodVisitor.visitEnd();
+ }
+ {
+ methodVisitor =
+ classWriter.visitMethod(
+ ACC_PUBLIC | ACC_SYNTHETIC,
+ "<init>",
+ "(Lkotlin/jvm/internal/DefaultConstructorMarker;)V",
+ null,
+ null);
+ methodVisitor.visitCode();
+ Label label0 = new Label();
+ methodVisitor.visitLabel(label0);
+ methodVisitor.visitLineNumber(2, label0);
+ methodVisitor.visitVarInsn(ALOAD, 0);
+ methodVisitor.visitMethodInsn(
+ INVOKESPECIAL, "WebContext$Companion", "<init>", "()V", false);
+ methodVisitor.visitInsn(RETURN);
+ Label label1 = new Label();
+ methodVisitor.visitLabel(label1);
+ methodVisitor.visitLocalVariable("this", "LWebContext$Companion;", null, label0, label1, 0);
+ methodVisitor.visitLocalVariable(
+ "$constructor_marker",
+ "Lkotlin/jvm/internal/DefaultConstructorMarker;",
+ null,
+ label0,
+ label1,
+ 1);
+ methodVisitor.visitMaxs(1, 2);
+ methodVisitor.visitEnd();
+ }
+ classWriter.visitEnd();
+
+ return classWriter.toByteArray();
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/d8/NonNamedMemberClassTest.java b/src/test/java/com/android/tools/r8/d8/NonNamedMemberClassTest.java
new file mode 100644
index 0000000..4ba6754
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/d8/NonNamedMemberClassTest.java
@@ -0,0 +1,122 @@
+// Copyright (c) 2019, 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.d8;
+
+import static org.hamcrest.CoreMatchers.containsString;
+
+import com.android.tools.r8.TestBase;
+import org.junit.Test;
+import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.Label;
+import org.objectweb.asm.MethodVisitor;
+import org.objectweb.asm.Opcodes;
+
+public class NonNamedMemberClassTest extends TestBase {
+
+ @Test
+ public void testD8() throws Exception {
+ testForD8()
+ .addProgramClassFileData(Dump.dump())
+ .compile()
+ .assertOnlyInfos()
+ .assertInfoMessageThatMatches(containsString("InnerClasses attributes are recovered"))
+ .assertInfoMessageThatMatches(containsString("inner: LWebContext$Companion;"))
+ .assertInfoMessageThatMatches(containsString("outer: LWebContext;"))
+ .assertInfoMessageThatMatches(containsString("innerName: (anonymous)"));
+ }
+
+ // Compiled the following kt code:
+ // class WebContext {
+ // companion object {
+ // val debug = true
+ // }
+ // }
+ // and removed inner-name, Companion.
+ private static class Dump implements Opcodes {
+
+ public static byte[] dump () throws Exception {
+
+ ClassWriter classWriter = new ClassWriter(0);
+ MethodVisitor methodVisitor;
+
+ classWriter.visit(
+ V1_6,
+ ACC_PUBLIC | ACC_FINAL | ACC_SUPER,
+ "WebContext$Companion",
+ null,
+ "java/lang/Object",
+ null);
+
+ classWriter.visitSource("WebContext.kt", null);
+
+ // Removed the inner-name, Companion.
+ classWriter.visitInnerClass(
+ "WebContext$Companion", "WebContext", null, ACC_PUBLIC | ACC_FINAL | ACC_STATIC);
+
+ {
+ methodVisitor =
+ classWriter.visitMethod(ACC_PUBLIC | ACC_FINAL, "getDebug", "()Z", null, null);
+ methodVisitor.visitCode();
+ Label label0 = new Label();
+ methodVisitor.visitLabel(label0);
+ methodVisitor.visitLineNumber(3, label0);
+ methodVisitor.visitMethodInsn(
+ INVOKESTATIC, "WebContext", "access$getDebug$cp", "()Z", false);
+ methodVisitor.visitInsn(IRETURN);
+ Label label1 = new Label();
+ methodVisitor.visitLabel(label1);
+ methodVisitor.visitLocalVariable("this", "LWebContext$Companion;", null, label0, label1, 0);
+ methodVisitor.visitMaxs(1, 1);
+ methodVisitor.visitEnd();
+ }
+ {
+ methodVisitor = classWriter.visitMethod(ACC_PRIVATE, "<init>", "()V", null, null);
+ methodVisitor.visitCode();
+ Label label0 = new Label();
+ methodVisitor.visitLabel(label0);
+ methodVisitor.visitLineNumber(2, label0);
+ methodVisitor.visitVarInsn(ALOAD, 0);
+ methodVisitor.visitMethodInsn(INVOKESPECIAL, "java/lang/Object", "<init>", "()V", false);
+ methodVisitor.visitInsn(RETURN);
+ Label label1 = new Label();
+ methodVisitor.visitLabel(label1);
+ methodVisitor.visitLocalVariable("this", "LWebContext$Companion;", null, label0, label1, 0);
+ methodVisitor.visitMaxs(1, 1);
+ methodVisitor.visitEnd();
+ }
+ {
+ methodVisitor =
+ classWriter.visitMethod(
+ ACC_PUBLIC | ACC_SYNTHETIC,
+ "<init>",
+ "(Lkotlin/jvm/internal/DefaultConstructorMarker;)V",
+ null,
+ null);
+ methodVisitor.visitCode();
+ Label label0 = new Label();
+ methodVisitor.visitLabel(label0);
+ methodVisitor.visitLineNumber(2, label0);
+ methodVisitor.visitVarInsn(ALOAD, 0);
+ methodVisitor.visitMethodInsn(
+ INVOKESPECIAL, "WebContext$Companion", "<init>", "()V", false);
+ methodVisitor.visitInsn(RETURN);
+ Label label1 = new Label();
+ methodVisitor.visitLabel(label1);
+ methodVisitor.visitLocalVariable("this", "LWebContext$Companion;", null, label0, label1, 0);
+ methodVisitor.visitLocalVariable(
+ "$constructor_marker",
+ "Lkotlin/jvm/internal/DefaultConstructorMarker;",
+ null,
+ label0,
+ label1,
+ 1);
+ methodVisitor.visitMaxs(1, 2);
+ methodVisitor.visitEnd();
+ }
+ classWriter.visitEnd();
+
+ return classWriter.toByteArray();
+ }
+ }
+}
