Avoid NPE while computing inner-name separator from missing attributes.
Bug: 131210377, 131140696
Change-Id: I2006177073ccc34e39e7607778194063aa21f0f6
diff --git a/src/main/java/com/android/tools/r8/graph/DexClass.java b/src/main/java/com/android/tools/r8/graph/DexClass.java
index d0eeaad..16cd797 100644
--- a/src/main/java/com/android/tools/r8/graph/DexClass.java
+++ b/src/main/java/com/android/tools/r8/graph/DexClass.java
@@ -806,24 +806,23 @@
public boolean isLocalClass() {
InnerClassAttribute innerClass = getInnerClassAttributeForThisClass();
+ // The corresponding enclosing-method attribute might be not available, e.g., CF version 50.
return innerClass != null
- && innerClass.isNamed()
- && getEnclosingMethod() != null;
+ && innerClass.getOuter() == null
+ && innerClass.isNamed();
}
public boolean isMemberClass() {
InnerClassAttribute innerClass = getInnerClassAttributeForThisClass();
return innerClass != null
&& innerClass.getOuter() != null
- && innerClass.isNamed()
- && getEnclosingMethod() == null;
+ && innerClass.isNamed();
}
public boolean isAnonymousClass() {
InnerClassAttribute innerClass = getInnerClassAttributeForThisClass();
- return innerClass != null
- && innerClass.isAnonymous()
- && getEnclosingMethod() != null;
+ // The corresponding enclosing-method attribute might be not available, e.g., CF version 50.
+ return innerClass != null && innerClass.isAnonymous();
}
public boolean isInANest() {
diff --git a/src/main/java/com/android/tools/r8/graph/EnclosingMethodAttribute.java b/src/main/java/com/android/tools/r8/graph/EnclosingMethodAttribute.java
index 0fe8c04..5457311 100644
--- a/src/main/java/com/android/tools/r8/graph/EnclosingMethodAttribute.java
+++ b/src/main/java/com/android/tools/r8/graph/EnclosingMethodAttribute.java
@@ -72,4 +72,13 @@
enclosingMethod.collectIndexedItems(indexedItems);
}
}
+
+ @Override
+ public String toString() {
+ return "[enclosingClass: "
+ + (enclosingClass == null ? "null" : enclosingClass.toDescriptorString())
+ + ", enclosingMethod: "
+ + (enclosingMethod == null ? "null" : enclosingMethod.toSourceString())
+ + "]";
+ }
}
diff --git a/src/main/java/com/android/tools/r8/graph/InnerClassAttribute.java b/src/main/java/com/android/tools/r8/graph/InnerClassAttribute.java
index 04feae7..b40f328 100644
--- a/src/main/java/com/android/tools/r8/graph/InnerClassAttribute.java
+++ b/src/main/java/com/android/tools/r8/graph/InnerClassAttribute.java
@@ -102,4 +102,13 @@
}
return context;
}
+
+ @Override
+ public String toString() {
+ return "[access : " + Integer.toHexString(access)
+ + ", inner: " + inner.toDescriptorString()
+ + ", outer: " + (outer == null ? "null" : outer.toDescriptorString())
+ + ", innerName: " + (innerName == null ? "(anonymous)" : innerName.toString())
+ + "]";
+ }
}
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 5a80fe0..665e3d5 100644
--- a/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
+++ b/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
@@ -8,6 +8,8 @@
import static org.objectweb.asm.ClassReader.SKIP_DEBUG;
import static org.objectweb.asm.ClassReader.SKIP_FRAMES;
import static org.objectweb.asm.Opcodes.ACC_DEPRECATED;
+import static org.objectweb.asm.Opcodes.V1_6;
+import static org.objectweb.asm.Opcodes.V9;
import com.android.tools.r8.ProgramResource.Kind;
import com.android.tools.r8.dex.Constants;
@@ -240,10 +242,10 @@
@Override
public void visitInnerClass(String name, String outerName, String innerName, int access) {
if (outerName != null && innerName != null) {
- String separator =
- DescriptorUtils.computeInnerClassSeparator(
- outerName, name, innerName, application.options.isMinifying());
- if (separator == null) {
+ String separator = DescriptorUtils.computeInnerClassSeparator(outerName, name, innerName);
+ if (separator == null
+ && !application.options.isMinifying()
+ && getMajorVersion() < V9) {
application.options.reporter.info(
new StringDiagnostic(
StringUtils.lines(
@@ -419,6 +421,11 @@
directMethods.toArray(DexEncodedMethod.EMPTY_ARRAY),
virtualMethods.toArray(DexEncodedMethod.EMPTY_ARRAY),
application.getFactory().getSkipNameValidationForTesting());
+ if (enclosingMember == null
+ && (clazz.isLocalClass() || clazz.isAnonymousClass())
+ && getMajorVersion() > V1_6) {
+ application.options.warningMissingEnclosingMember(clazz.type, clazz.origin, version);
+ }
if (clazz.isProgramClass()) {
context.owner = clazz.asProgramClass();
clazz.asProgramClass().setInitialClassFileVersion(version);
diff --git a/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java b/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
index f37e60f..04cc0ba 100644
--- a/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
@@ -249,14 +249,14 @@
return attribute.getOuter();
}
- private String getInnerNameForType(DexType type) {
+ private DexString getInnerNameForType(DexType type) {
// This util is used only after the corresponding outer-class is recognized.
// Therefore, the definition for the type and its inner-class attribute should be found.
DexClass clazz = appView.definitionFor(type);
assert clazz != null;
InnerClassAttribute attribute = clazz.getInnerClassAttributeForThisClass();
assert attribute != null;
- return attribute.getInnerName().toString();
+ return attribute.getInnerName();
}
private DexString computeName(DexType type) {
@@ -266,11 +266,11 @@
// of the outer class for the $ prefix.
DexType outerClass = getOutClassForType(type);
if (outerClass != null) {
- String innerClassSeparator =
- computeInnerClassSeparator(
- outerClass, type, getInnerNameForType(type), appView.options().isMinifying());
- assert innerClassSeparator != null;
- state = getStateForOuterClass(outerClass, innerClassSeparator);
+ String separator = computeInnerClassSeparator(outerClass, type, getInnerNameForType(type));
+ if (separator == null) {
+ separator = String.valueOf(INNER_CLASS_SEPARATOR);
+ }
+ state = getStateForOuterClass(outerClass, separator);
}
}
if (state == null) {
diff --git a/src/main/java/com/android/tools/r8/naming/MinifiedRenaming.java b/src/main/java/com/android/tools/r8/naming/MinifiedRenaming.java
index 41dd4dc..f14e456 100644
--- a/src/main/java/com/android/tools/r8/naming/MinifiedRenaming.java
+++ b/src/main/java/com/android/tools/r8/naming/MinifiedRenaming.java
@@ -64,7 +64,6 @@
}
DexType innerType = attribute.getInner();
String inner = DescriptorUtils.descriptorToInternalName(innerType.descriptor.toString());
- String innerName = attribute.getInnerName().toString();
// At this point we assume the input was of the form: <OuterType>$<index><InnerName>
// Find the mapped type and if it remains the same return that, otherwise split at
// either the input separator ($, $$, or anything that starts with $) or $ (that we recover
@@ -74,10 +73,12 @@
if (inner.equals(innerTypeMapped)) {
return attribute.getInnerName();
}
- String separator =
- DescriptorUtils.computeInnerClassSeparator(
- attribute.getOuter(), innerType, innerName, options.isMinifying());
- assert separator != null;
+ String separator = DescriptorUtils.computeInnerClassSeparator(
+ attribute.getOuter(), innerType, attribute.getInnerName());
+ if (separator == null) {
+ separator = String.valueOf(DescriptorUtils.INNER_CLASS_SEPARATOR);
+
+ }
int index = innerTypeMapped.lastIndexOf(separator);
if (index < 0) {
assert false : innerType + " -> " + innerTypeMapped;
diff --git a/src/main/java/com/android/tools/r8/utils/DescriptorUtils.java b/src/main/java/com/android/tools/r8/utils/DescriptorUtils.java
index d523610..dfb21da 100644
--- a/src/main/java/com/android/tools/r8/utils/DescriptorUtils.java
+++ b/src/main/java/com/android/tools/r8/utils/DescriptorUtils.java
@@ -8,6 +8,7 @@
import com.android.tools.r8.errors.CompilationError;
import com.android.tools.r8.errors.Unreachable;
+import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.naming.ClassNameMapper;
import com.google.common.collect.ImmutableMap;
@@ -309,19 +310,18 @@
}
public static String computeInnerClassSeparator(
- DexType outerClass, DexType innerClass, String innerName, boolean isMinifying) {
+ DexType outerClass, DexType innerClass, DexString innerName) {
+ if (innerName == null) {
+ return String.valueOf(INNER_CLASS_SEPARATOR);
+ }
return computeInnerClassSeparator(
- outerClass.getInternalName(), innerClass.getInternalName(), innerName, isMinifying);
+ outerClass.getInternalName(), innerClass.getInternalName(), innerName.toString());
}
public static String computeInnerClassSeparator(
- String outerDescriptor, String innerDescriptor, String innerName, boolean isMinifying) {
+ String outerDescriptor, String innerDescriptor, String innerName) {
// outer-internal<separator>inner-name == inner-internal
if (outerDescriptor.length() + innerName.length() > innerDescriptor.length()) {
- // But, if the minification is enabled, we can choose $ separator.
- if (isMinifying) {
- return String.valueOf(INNER_CLASS_SEPARATOR);
- }
return null;
}
String separator =
@@ -329,10 +329,6 @@
outerDescriptor.length(), innerDescriptor.length() - innerName.length());
// Any non-$ separator results in a runtime exception in getCanonicalName.
if (!separator.startsWith(String.valueOf(INNER_CLASS_SEPARATOR))) {
- // Again, if the minification is enabled, we can choose $ separator.
- if (isMinifying) {
- return String.valueOf(INNER_CLASS_SEPARATOR);
- }
return null;
}
return separator;
diff --git a/src/test/java/com/android/tools/r8/shaking/attributes/DataClassDumps.java b/src/test/java/com/android/tools/r8/shaking/attributes/DataClassDumps.java
new file mode 100644
index 0000000..8cd3fe1
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/attributes/DataClassDumps.java
@@ -0,0 +1,63 @@
+// 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.shaking.attributes;
+
+import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.Opcodes;
+
+class DataClassDumps implements Opcodes {
+
+ public static byte[] dump18 () {
+
+ ClassWriter classWriter = new ClassWriter(0);
+
+ classWriter.visit(
+ V1_8,
+ ACC_SUPER | ACC_SYNTHETIC,
+ "com/android/tools/r8/shaking/attributes/DataClass$1", null, "java/lang/Object", null);
+
+ classWriter.visitSource("MissingEnclosingMethodTest.java", null);
+
+ classWriter.visitOuterClass(
+ "com/android/tools/r8/shaking/attributes/DataClass", null, null);
+
+ classWriter.visitInnerClass(
+ // Inner
+ "com/android/tools/r8/shaking/attributes/DataClass$1",
+ // Outer, intentionally changed from null.
+ "com/android/tools/r8/shaking/attributes/DataClass",
+ // Inner-name
+ null,
+ ACC_STATIC | ACC_SYNTHETIC);
+
+ classWriter.visitEnd();
+
+ return classWriter.toByteArray();
+ }
+
+ public static byte[] dump16 () {
+
+ ClassWriter classWriter = new ClassWriter(0);
+
+ classWriter.visit(
+ V1_6,
+ ACC_SUPER | ACC_SYNTHETIC,
+ "com/android/tools/r8/shaking/attributes/DataClass$1", null, "java/lang/Object", null);
+
+ classWriter.visitSource("MissingEnclosingMethodTest.java", null);
+
+ classWriter.visitInnerClass(
+ // Inner
+ "com/android/tools/r8/shaking/attributes/DataClass$1",
+ // Outer
+ "com/android/tools/r8/shaking/attributes/DataClass",
+ // Inner-name
+ null,
+ ACC_STATIC | ACC_SYNTHETIC);
+
+ classWriter.visitEnd();
+
+ return classWriter.toByteArray();
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/attributes/MissingEnclosingMethodTest.java b/src/test/java/com/android/tools/r8/shaking/attributes/MissingEnclosingMethodTest.java
new file mode 100644
index 0000000..8c0157f
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/attributes/MissingEnclosingMethodTest.java
@@ -0,0 +1,125 @@
+// 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.shaking.attributes;
+
+import com.android.tools.r8.R8FullTestBuilder;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.errors.Unreachable;
+import com.android.tools.r8.utils.BooleanUtils;
+import com.android.tools.r8.utils.StringUtils;
+import java.io.IOException;
+import java.util.Collection;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+class DataClass {
+ public final int f1;
+ public final String f2;
+
+ private DataClass(Builder builder) {
+ this.f1 = builder.f1;
+ this.f2 = builder.f2;
+ }
+
+ @Override
+ public String toString() {
+ return "f1: " + f1 + ", f2: " + f2;
+ }
+
+ public static class Builder {
+ private int f1;
+ private String f2;
+
+ public Builder() {
+ }
+
+ public Builder setF1(int f1) {
+ this.f1 = f1;
+ return this;
+ }
+
+ public Builder setF2(String f2) {
+ this.f2 = f2;
+ return this;
+ }
+
+ public DataClass build() {
+ return new DataClass(this);
+ }
+ }
+}
+
+class DataClassUser {
+ public static void main(String... args) {
+ DataClass.Builder builder = new DataClass.Builder();
+ builder.setF1(8);
+ builder.setF2("8");
+ DataClass d = builder.build();
+ System.out.println(d);
+ }
+}
+
+@RunWith(Parameterized.class)
+public class MissingEnclosingMethodTest extends TestBase {
+ private static final String EXPECTED_OUTPUT = StringUtils.lines("f1: 8, f2: 8");
+ private final TestParameters parameters;
+ private final TestConfig config;
+ private final boolean enableMinification;
+
+ enum TestConfig {
+ CLASS,
+ DUMP_18,
+ DUMP_16;
+
+ public void addInnerClasses(R8FullTestBuilder builder) throws IOException {
+ switch (this) {
+ case CLASS:
+ builder.addInnerClasses(DataClass.class);
+ break;
+ case DUMP_18:
+ builder.addProgramClasses(DataClass.Builder.class);
+ builder.addProgramClassFileData(DataClassDumps.dump18());
+ break;
+ case DUMP_16:
+ builder.addProgramClasses(DataClass.Builder.class);
+ builder.addProgramClassFileData(DataClassDumps.dump16());
+ break;
+ default:
+ throw new Unreachable();
+ }
+ }
+ }
+
+ @Parameterized.Parameters(name = "{0} {1} minification: {2}")
+ public static Collection<Object[]> data() {
+ return buildParameters(
+ getTestParameters().withAllRuntimes().build(),
+ TestConfig.values(),
+ BooleanUtils.values());
+ }
+
+ public MissingEnclosingMethodTest(TestParameters parameters, TestConfig config, boolean enableMinification) {
+ this.parameters = parameters;
+ this.config = config;
+ this.enableMinification = enableMinification;
+ }
+
+ @Test
+ public void b131210377() throws Exception {
+ R8FullTestBuilder builder =
+ testForR8(parameters.getBackend())
+ .addProgramClasses(DataClass.class, DataClassUser.class);
+ config.addInnerClasses(builder);
+ builder
+ .addKeepMainRule(DataClassUser.class)
+ .addKeepRules("-keepattributes Signature,InnerClasses,EnclosingMethod")
+ .addOptionsModification(o -> o.forceProguardCompatibility = true)
+ .minification(enableMinification)
+ .setMinApi(parameters.getRuntime())
+ .run(parameters.getRuntime(), DataClassUser.class)
+ .assertSuccessWithOutput(EXPECTED_OUTPUT);
+ }
+}
