[Retrace] Account for invalid and incorrect signatures
Bug: b/267413327
Change-Id: I5f13f1cd5b20627c42d88c6a9a3fb6456cf62d0e
diff --git a/src/main/java/com/android/tools/r8/naming/ProguardMapReader.java b/src/main/java/com/android/tools/r8/naming/ProguardMapReader.java
index c1dfb81..749f1ea 100644
--- a/src/main/java/com/android/tools/r8/naming/ProguardMapReader.java
+++ b/src/main/java/com/android/tools/r8/naming/ProguardMapReader.java
@@ -389,11 +389,26 @@
MappingInformationDiagnostics.notAllowedCombination(
info, conflictingInfo, lineNo)));
if (info.isResidualSignatureMappingInformation()) {
+ ResidualSignatureMappingInformation mappingInfo =
+ info.asResidualSignatureMappingInformation();
+ if (!mappingInfo.isValid()) {
+ diagnosticsHandler.warning(
+ MappingInformationDiagnostics.invalidResidualSignature(
+ line.trim(), lineNo));
+ return;
+ }
Signature residualSignature =
getResidualSignatureFromMappingInformation(
info.asResidualSignatureMappingInformation(), currentRenamedNameFinal);
currentResidualSignature.set(residualSignature);
if (currentRange != null) {
+ if (!mappingInfo.isResidualMethodSignatureMappingInformation()) {
+ diagnosticsHandler.warning(
+ MappingInformationDiagnostics.invalidResidualSignatureType(
+ info.serialize(), lineNo));
+ currentResidualSignature.clear();
+ return;
+ }
currentRange.setResidualSignatureInternal(
residualSignature.asMethodSignature());
}
@@ -575,7 +590,14 @@
private Signature getResidualSignatureForMemberNaming(
Box<Signature> residualSignature, Signature originalSignature, String renamedName) {
if (residualSignature.isSet()) {
- return residualSignature.get();
+ if (residualSignature.get().getClass() != originalSignature.getClass()) {
+ diagnosticsHandler.warning(
+ MappingInformationDiagnostics.invalidResidualSignatureType(
+ residualSignature.get().toString(), lineNo));
+
+ } else {
+ return residualSignature.get();
+ }
}
Signature signature = originalSignature.asRenamed(renamedName);
signature = signatureCache.computeIfAbsent(signature, Function.identity());
diff --git a/src/main/java/com/android/tools/r8/naming/mappinginformation/MappingInformationDiagnostics.java b/src/main/java/com/android/tools/r8/naming/mappinginformation/MappingInformationDiagnostics.java
index 1c4242a..cc0da51 100644
--- a/src/main/java/com/android/tools/r8/naming/mappinginformation/MappingInformationDiagnostics.java
+++ b/src/main/java/com/android/tools/r8/naming/mappinginformation/MappingInformationDiagnostics.java
@@ -99,4 +99,21 @@
"The mapping '" + one + "' is not allowed in combination with '" + other + "'",
new TextPosition(1, lineNumber, TextPosition.UNKNOWN_COLUMN));
}
+
+ public static MappingInformationDiagnostics invalidResidualSignature(
+ String info, int lineNumber) {
+ return new MappingInformationDiagnostics(
+ "The residual signature mapping '" + info + "' is invalid'",
+ new TextPosition(1, lineNumber, TextPosition.UNKNOWN_COLUMN));
+ }
+
+ public static MappingInformationDiagnostics invalidResidualSignatureType(
+ String info, int lineNumber) {
+ return new MappingInformationDiagnostics(
+ "The residual signature mapping '"
+ + info
+ + "' is not of the same type as the "
+ + "member it describes.'",
+ new TextPosition(1, lineNumber, TextPosition.UNKNOWN_COLUMN));
+ }
}
diff --git a/src/main/java/com/android/tools/r8/naming/mappinginformation/ResidualSignatureMappingInformation.java b/src/main/java/com/android/tools/r8/naming/mappinginformation/ResidualSignatureMappingInformation.java
index 05bbad1..db604e3 100644
--- a/src/main/java/com/android/tools/r8/naming/mappinginformation/ResidualSignatureMappingInformation.java
+++ b/src/main/java/com/android/tools/r8/naming/mappinginformation/ResidualSignatureMappingInformation.java
@@ -63,6 +63,8 @@
}
}
+ public abstract boolean isValid();
+
@Override
public boolean isResidualSignatureMappingInformation() {
return true;
@@ -76,6 +78,9 @@
public static class ResidualMethodSignatureMappingInformation
extends ResidualSignatureMappingInformation {
+ private static final ResidualMethodSignatureMappingInformation INVALID_METHOD_SIGNATURE =
+ new ResidualMethodSignatureMappingInformation(new String[0], "LINVALID;");
+
private final String returnType;
private final String[] parameters;
@@ -93,20 +98,27 @@
parameters, method.getReturnType().toDescriptorString());
}
- public static ResidualFieldSignatureMappingInformation fromDexField(DexField residualField) {
- return new ResidualFieldSignatureMappingInformation(
- residualField.getType().toDescriptorString());
- }
-
@Override
protected String serializeInternal() {
return StringUtils.join("", Arrays.asList(parameters), BraceType.PARENS) + returnType;
}
+ @Override
+ public boolean isValid() {
+ return this != INVALID_METHOD_SIGNATURE;
+ }
+
public static ResidualMethodSignatureMappingInformation deserialize(String signature) {
- return new ResidualMethodSignatureMappingInformation(
- DescriptorUtils.getArgumentTypeDescriptors(signature),
- DescriptorUtils.getReturnTypeDescriptor(signature));
+ String[] argumentTypeDescriptors = DescriptorUtils.getArgumentTypeDescriptors(signature);
+ String returnTypeDescriptor = DescriptorUtils.getReturnTypeDescriptor(signature);
+ boolean isValid = DescriptorUtils.isDescriptor(returnTypeDescriptor);
+ for (String argumentTypeDescriptor : argumentTypeDescriptors) {
+ isValid &= DescriptorUtils.isDescriptor(argumentTypeDescriptor);
+ }
+ return isValid
+ ? new ResidualMethodSignatureMappingInformation(
+ argumentTypeDescriptors, returnTypeDescriptor)
+ : INVALID_METHOD_SIGNATURE;
}
public String getReturnType() {
@@ -148,6 +160,9 @@
public static class ResidualFieldSignatureMappingInformation
extends ResidualSignatureMappingInformation {
+ private static final ResidualFieldSignatureMappingInformation INVALID_FIELD_SIGNATURE =
+ new ResidualFieldSignatureMappingInformation("LINVALID;");
+
private final String type;
private ResidualFieldSignatureMappingInformation(String type) {
@@ -168,8 +183,15 @@
return type;
}
+ @Override
+ public boolean isValid() {
+ return this != INVALID_FIELD_SIGNATURE;
+ }
+
public static ResidualFieldSignatureMappingInformation deserialize(String signature) {
- return new ResidualFieldSignatureMappingInformation(signature);
+ return DescriptorUtils.isDescriptor(signature)
+ ? new ResidualFieldSignatureMappingInformation(signature)
+ : INVALID_FIELD_SIGNATURE;
}
@Override
diff --git a/src/main/java/com/android/tools/r8/utils/positions/MappedPositionToClassNameMapperBuilder.java b/src/main/java/com/android/tools/r8/utils/positions/MappedPositionToClassNameMapperBuilder.java
index b3834cf..7f67d3d 100644
--- a/src/main/java/com/android/tools/r8/utils/positions/MappedPositionToClassNameMapperBuilder.java
+++ b/src/main/java/com/android/tools/r8/utils/positions/MappedPositionToClassNameMapperBuilder.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.utils.positions;
+import static com.android.tools.r8.naming.mappinginformation.ResidualSignatureMappingInformation.ResidualFieldSignatureMappingInformation.fromDexField;
import static com.android.tools.r8.utils.positions.PositionUtils.mustHaveResidualDebugInfo;
import com.android.tools.r8.errors.Unreachable;
@@ -184,9 +185,7 @@
MemberNaming memberNaming = new MemberNaming(originalSignature, residualSignature);
if (ResidualSignatureMappingInformation.isSupported(mapFileVersion)
&& !originalSignature.type.equals(residualSignature.type)) {
- memberNaming.addMappingInformation(
- ResidualMethodSignatureMappingInformation.fromDexField(residualField),
- Unreachable::raise);
+ memberNaming.addMappingInformation(fromDexField(residualField), Unreachable::raise);
}
if (dexEncodedField.isD8R8Synthesized()) {
memberNaming.addMappingInformation(
diff --git a/src/test/java/com/android/tools/r8/retrace/RetraceInvalidResidualSignatureTest.java b/src/test/java/com/android/tools/r8/retrace/RetraceInvalidResidualSignatureTest.java
index 5be9fc2..7663ee1 100644
--- a/src/test/java/com/android/tools/r8/retrace/RetraceInvalidResidualSignatureTest.java
+++ b/src/test/java/com/android/tools/r8/retrace/RetraceInvalidResidualSignatureTest.java
@@ -4,12 +4,14 @@
package com.android.tools.r8.retrace;
-import static org.junit.Assert.assertThrows;
+import static org.hamcrest.CoreMatchers.containsString;
-import com.android.tools.r8.DiagnosticsHandler;
+import com.android.tools.r8.DiagnosticsMatcher;
import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestDiagnosticMessagesImpl;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.naming.mappinginformation.MappingInformationDiagnostics;
import com.android.tools.r8.references.ClassReference;
import com.android.tools.r8.references.Reference;
import java.util.Collections;
@@ -44,22 +46,44 @@
+ " # { id:'com.android.tools.r8.residualsignature',signature:'()Ljava/lang/Object;' }\n"
+ " int method1() -> a\n"
+ " # { id:'com.android.tools.r8.residualsignature',signature:'I' }\n"
- + " int method2() -> b\n"
+ + " 1:2:int method2():3:4 -> a\n"
+ + " # { id:'com.android.tools.r8.residualsignature',signature:'Z' }\n"
+ + " int method3() -> b\n"
+ " # { id:'com.android.tools.r8.residualsignature',signature:'VOLAPYK' }\n";
@Test
public void testInvalidResidualMapping() {
- // TODO(b/267413327): Fail more gracefully than just throwing.
- assertThrows(
- AssertionError.class,
- () ->
- Retracer.createDefault(
- ProguardMapProducer.fromString(mapping), new DiagnosticsHandler() {})
- .retraceMethod(
- Reference.method(
- someClassRenamed,
- "method1",
- Collections.emptyList(),
- Reference.primitiveFromDescriptor("I"))));
+ TestDiagnosticMessagesImpl diagnosticsHandler = new TestDiagnosticMessagesImpl();
+ Retracer.createDefault(ProguardMapProducer.fromString(mapping), diagnosticsHandler)
+ .retraceMethod(
+ Reference.method(
+ someClassRenamed,
+ "method1",
+ Collections.emptyList(),
+ Reference.primitiveFromDescriptor("I")));
+ diagnosticsHandler.assertOnlyWarnings();
+ diagnosticsHandler.assertAllWarningsMatch(
+ DiagnosticsMatcher.diagnosticType(MappingInformationDiagnostics.class));
+ diagnosticsHandler.assertWarningsMatch(
+ DiagnosticsMatcher.diagnosticMessage(containsString(getMessage("java.lang.Object a()"))),
+ DiagnosticsMatcher.diagnosticMessage(
+ containsString(
+ getMessage(
+ "{\"id\":\"com.android.tools.r8.residualsignature\",\"signature\":\"I\"}"))),
+ DiagnosticsMatcher.diagnosticMessage(
+ containsString(
+ getMessage(
+ "{\"id\":\"com.android.tools.r8.residualsignature\",\"signature\":\"Z\"}"))),
+ DiagnosticsMatcher.diagnosticMessage(
+ containsString(
+ "The residual signature mapping '# {"
+ + " id:'com.android.tools.r8.residualsignature',signature:'VOLAPYK' }' is"
+ + " invalid")));
+ }
+
+ private String getMessage(String variable) {
+ return "The residual signature mapping '"
+ + variable
+ + "' is not of the same type as the member it describes.";
}
}
