Fail early if malformed inner names are found.
Bug: 120597515
Change-Id: I2969308985e3ca9b2caf5c7f19eafc57e8da45b4
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 4aa5b7c..78a0ba9 100644
--- a/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
+++ b/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
@@ -12,6 +12,7 @@
import com.android.tools.r8.ProgramResource.Kind;
import com.android.tools.r8.dex.Constants;
import com.android.tools.r8.errors.CompilationError;
+import com.android.tools.r8.errors.InternalCompilerError;
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.DexValue.DexValueAnnotation;
import com.android.tools.r8.graph.DexValue.DexValueArray;
@@ -30,6 +31,7 @@
import com.android.tools.r8.jar.CfApplicationWriter;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.shaking.ProguardKeepAttributes;
+import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.FieldSignatureEquivalence;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.MethodSignatureEquivalence;
@@ -209,6 +211,14 @@
@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);
+ if (separator == null) {
+ throw new InternalCompilerError("Malformed class name: " + name);
+ }
+ }
innerClasses.add(
new InnerClassAttribute(
access,
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 40c0236..21c7903 100644
--- a/src/main/java/com/android/tools/r8/utils/DescriptorUtils.java
+++ b/src/main/java/com/android/tools/r8/utils/DescriptorUtils.java
@@ -311,18 +311,26 @@
public static String computeInnerClassSeparator(
DexType outerClass, DexType innerClass, String innerName, InternalOptions options) {
- String outerInternalRaw = outerClass.getInternalName();
- String innerInternalRaw = innerClass.getInternalName();
+ return computeInnerClassSeparator(
+ outerClass.getInternalName(), innerClass.getInternalName(), innerName, options);
+ }
+
+ public static String computeInnerClassSeparator(
+ String outerDescriptor, String innerDescriptor, String innerName, InternalOptions options) {
// outer-internal<separator>inner-name == inner-internal
- if (outerInternalRaw.length() + innerName.length() > innerInternalRaw.length()) {
+ if (outerDescriptor.length() + innerName.length() > innerDescriptor.length()) {
+ // But, if the minification is enabled, we can choose $ separator.
+ if (options.isMinifying()) {
+ return String.valueOf(INNER_CLASS_SEPARATOR);
+ }
return null;
}
String separator =
- innerInternalRaw.substring(
- outerInternalRaw.length(), innerInternalRaw.length() - innerName.length());
+ innerDescriptor.substring(
+ outerDescriptor.length(), innerDescriptor.length() - innerName.length());
// Any non-$ separator results in a runtime exception in getCanonicalName.
if (!separator.startsWith(String.valueOf(INNER_CLASS_SEPARATOR))) {
- // But, if the minification is enabled, we can choose $ separator.
+ // Again, if the minification is enabled, we can choose $ separator.
if (options.isMinifying()) {
return String.valueOf(INNER_CLASS_SEPARATOR);
}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/reflection/InnerClassNameTestRunner.java b/src/test/java/com/android/tools/r8/ir/optimize/reflection/InnerClassNameTestRunner.java
index 7d44d4b..4ac1d9a 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/reflection/InnerClassNameTestRunner.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/reflection/InnerClassNameTestRunner.java
@@ -4,6 +4,8 @@
package com.android.tools.r8.ir.optimize.reflection;
import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import com.android.tools.r8.CompilationFailedException;
@@ -14,6 +16,7 @@
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestRuntime.CfVm;
import com.android.tools.r8.ToolHelper.DexVm;
+import com.android.tools.r8.errors.InternalCompilerError;
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.utils.BooleanUtils;
import com.android.tools.r8.utils.DescriptorUtils;
@@ -171,9 +174,15 @@
.minification(minify)
.setMinApi(parameters.getRuntime())
.compile();
- } catch (CompilationFailedException e) {
- assertTrue("b/120597515", config == TestNamingConfig.NON_NESTED_INNER);
- assertTrue("b/120597515", minify);
+ } catch (InternalCompilerError e) {
+ assertThat(e.getMessage(), containsString("Malformed class name"));
+ assertThat(e.getMessage(), containsString(config.getInnerTypeRaw()));
+ assertTrue(
+ "b/120597515",
+ config == TestNamingConfig.EMTPY_SEPARATOR
+ || config == TestNamingConfig.UNDERBAR_SEPARATOR
+ || config == TestNamingConfig.NON_NESTED_INNER);
+ assertFalse("b/120597515", minify);
return;
}
CodeInspector inspector = r8CompileResult.inspector();
@@ -205,32 +214,9 @@
break;
case EMTPY_SEPARATOR:
case UNDERBAR_SEPARATOR:
- if (!minify
- && parameters.isCfRuntime()
- && parameters.getRuntime().asCf().getVm().lessThanOrEqual(CfVm.JDK8)) {
- // Any non-$ separator results in a runtime exception in getCanonicalName.
- r8RunResult.assertFailureWithErrorThatMatches(containsString("Malformed class name"));
- } else {
- assert minify
- || parameters.isDexRuntime()
- || !parameters.getRuntime().asCf().getVm().lessThanOrEqual(CfVm.JDK8);
- r8RunResult.assertSuccessWithOutput(getExpectedMinified(inspector));
- }
- break;
case NON_NESTED_INNER:
- if (minify) {
- // TODO(b/120597515): better fail early while reading class file.
- throw new Unreachable("Expect to see compilation error: " + config);
- }
- if (parameters.isCfRuntime()
- && parameters.getRuntime().asCf().getVm().lessThanOrEqual(CfVm.JDK8)) {
- // Any non-$ separator results in a runtime exception in getCanonicalName.
- r8RunResult.assertFailureWithErrorThatMatches(containsString("Malformed class name"));
- } else {
- assert parameters.isDexRuntime()
- || !parameters.getRuntime().asCf().getVm().lessThanOrEqual(CfVm.JDK8);
- r8RunResult.assertSuccessWithOutput(getExpectedMinified(inspector));
- }
+ assertTrue("Expect to see compilation error", minify);
+ r8RunResult.assertSuccessWithOutput(getExpectedMinified(inspector));
break;
default:
throw new Unreachable("Unexpected test configuration: " + config);
