Revert "Remove plain stack trace parser"
This reverts commit cd791cb94e385b8c01d0166e83669d86b4e6cddc.
Change-Id: Ibc292a2f4046c7ffeeb11f162d4993d4fac91ff8
diff --git a/src/main/java/com/android/tools/r8/retrace/Retrace.java b/src/main/java/com/android/tools/r8/retrace/Retrace.java
index 63d0098..faf8616 100644
--- a/src/main/java/com/android/tools/r8/retrace/Retrace.java
+++ b/src/main/java/com/android/tools/r8/retrace/Retrace.java
@@ -11,6 +11,7 @@
import com.android.tools.r8.Keep;
import com.android.tools.r8.Version;
import com.android.tools.r8.retrace.RetraceCommand.Builder;
+import com.android.tools.r8.retrace.internal.PlainStackTraceLineParser;
import com.android.tools.r8.retrace.internal.RetraceAbortException;
import com.android.tools.r8.retrace.internal.RetracerImpl;
import com.android.tools.r8.retrace.internal.StackTraceRegularExpressionParser;
@@ -247,7 +248,9 @@
timing.begin("Report result");
StringRetrace stringRetrace =
new StringRetrace(
- new StackTraceRegularExpressionParser(options.getRegularExpression()),
+ options.getRegularExpression() == null
+ ? new PlainStackTraceLineParser()
+ : new StackTraceRegularExpressionParser(options.getRegularExpression()),
StackTraceElementProxyRetracer.createDefault(retracer),
diagnosticsHandler,
options.isVerbose());
diff --git a/src/main/java/com/android/tools/r8/retrace/RetraceCommand.java b/src/main/java/com/android/tools/r8/retrace/RetraceCommand.java
index 76d6b77..66819e7 100644
--- a/src/main/java/com/android/tools/r8/retrace/RetraceCommand.java
+++ b/src/main/java/com/android/tools/r8/retrace/RetraceCommand.java
@@ -25,14 +25,10 @@
List<String> stackTrace,
Consumer<List<String>> retracedStackTraceConsumer,
boolean isVerbose) {
+ options =
+ new RetraceOptions(regularExpression, diagnosticsHandler, proguardMapProducer, isVerbose);
this.stackTrace = stackTrace;
this.retracedStackTraceConsumer = retracedStackTraceConsumer;
- this.options =
- RetraceOptions.builder(diagnosticsHandler)
- .setRegularExpression(regularExpression)
- .setProguardMapProducer(proguardMapProducer)
- .setVerbose(isVerbose)
- .build();
assert this.stackTrace != null;
assert this.retracedStackTraceConsumer != null;
diff --git a/src/main/java/com/android/tools/r8/retrace/RetraceOptions.java b/src/main/java/com/android/tools/r8/retrace/RetraceOptions.java
index 47d8c74..4784f09 100644
--- a/src/main/java/com/android/tools/r8/retrace/RetraceOptions.java
+++ b/src/main/java/com/android/tools/r8/retrace/RetraceOptions.java
@@ -21,7 +21,7 @@
private final DiagnosticsHandler diagnosticsHandler;
private final ProguardMapProducer proguardMapProducer;
- private RetraceOptions(
+ RetraceOptions(
String regularExpression,
DiagnosticsHandler diagnosticsHandler,
ProguardMapProducer proguardMapProducer,
@@ -112,9 +112,6 @@
if (this.proguardMapProducer == null) {
throw new RuntimeException("ProguardMapSupplier not specified");
}
- if (this.regularExpression == null) {
- throw new RuntimeException("Regular expression not specified");
- }
return new RetraceOptions(
regularExpression, diagnosticsHandler, proguardMapProducer, isVerbose);
}
diff --git a/src/main/java/com/android/tools/r8/retrace/internal/PlainStackTraceLineParser.java b/src/main/java/com/android/tools/r8/retrace/internal/PlainStackTraceLineParser.java
new file mode 100644
index 0000000..ddaf675
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/retrace/internal/PlainStackTraceLineParser.java
@@ -0,0 +1,199 @@
+// 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.retrace.internal;
+
+import static com.android.tools.r8.retrace.internal.RetraceUtils.firstCharFromIndex;
+import static com.android.tools.r8.retrace.internal.RetraceUtils.firstNonWhiteSpaceCharacterFromIndex;
+
+import com.android.tools.r8.retrace.StackTraceLineParser;
+import com.android.tools.r8.retrace.internal.StackTraceElementStringProxy.ClassNameType;
+import com.android.tools.r8.retrace.internal.StackTraceElementStringProxy.StackTraceElementStringProxyBuilder;
+import com.android.tools.r8.utils.DescriptorUtils;
+
+public final class PlainStackTraceLineParser
+ implements StackTraceLineParser<String, StackTraceElementStringProxy> {
+
+ public PlainStackTraceLineParser() {}
+
+ @Override
+ public StackTraceElementStringProxy parse(String stackTraceLine) {
+ return parseLine(stackTraceLine);
+ }
+
+ /**
+ * Captures a stack trace line of the following formats:
+ *
+ * <ul>
+ * <li>com.android.r8.R8Exception
+ * <li>com.android.r8.R8Exception: Problem when compiling program
+ * <li>Caused by: com.android.r8.R8InnerException: You have to write the program first
+ * <li>com.android.r8.R8InnerException: You have to write the program first
+ * </ul>
+ *
+ * <p>This will also contains false positives, such as
+ *
+ * <pre>
+ * W( 8207) VFY: unable to resolve static method 11: Lprivateinterfacemethods/I$-CC;....
+ * </pre>
+ *
+ * <p>The only invalid chars for type-identifiers for a java type-name is ';', '[' and '/', so we
+ * cannot really disregard the above line.
+ *
+ * <p>Caused by and Suppressed seems to not change based on locale, so we use these as markers.
+ */
+ private static class ExceptionLine {
+
+ private static final String CAUSED_BY = "Caused by: ";
+ private static final String SUPPRESSED = "Suppressed: ";
+
+ private static StackTraceElementStringProxy tryParse(String line) {
+ if (line.isEmpty()) {
+ return null;
+ }
+ int firstNonWhiteSpaceChar = firstNonWhiteSpaceCharacterFromIndex(line, 0);
+ String description = "";
+ if (line.startsWith(CAUSED_BY, firstNonWhiteSpaceChar)) {
+ description = CAUSED_BY;
+ } else if (line.startsWith(SUPPRESSED, firstNonWhiteSpaceChar)) {
+ description = SUPPRESSED;
+ }
+ int exceptionStartIndex = firstNonWhiteSpaceChar + description.length();
+ int messageStartIndex = firstCharFromIndex(line, exceptionStartIndex, ':');
+ String className = line.substring(exceptionStartIndex, messageStartIndex);
+ if (!DescriptorUtils.isValidJavaType(className)) {
+ return null;
+ }
+ return StackTraceElementStringProxy.builder(line)
+ .registerClassName(exceptionStartIndex, messageStartIndex, ClassNameType.TYPENAME)
+ .build();
+ }
+ }
+
+ /**
+ * Captures a stack trace line on the following form
+ *
+ * <ul>
+ * <li>at dalvik.system.NativeStart.main(NativeStart.java:99)
+ * <li>at dalvik.system.NativeStart.main(:99)
+ * <li>at dalvik.system.NativeStart.main(Foo.java:)
+ * <li>at dalvik.system.NativeStart.main(Native Method)
+ * <li>at classloader/named_module@version/foo.bar.baz(:20)
+ * <li>at classloader//foo.bar.baz(:20)
+ * </ul>
+ *
+ * <p>Empirical evidence suggests that the "at" string is never localized.
+ */
+ private static class AtLine {
+
+ private static StackTraceElementStringProxy tryParse(String line) {
+ // Check that the line is indented with some amount of white space.
+ if (line.length() == 0 || !Character.isWhitespace(line.charAt(0))) {
+ return null;
+ }
+ // Find the first non-white space character and check that we have the sequence 'a', 't', ' '.
+ int firstNonWhiteSpace = firstNonWhiteSpaceCharacterFromIndex(line, 0);
+ if (firstNonWhiteSpace + 2 >= line.length()
+ || line.charAt(firstNonWhiteSpace) != 'a'
+ || line.charAt(firstNonWhiteSpace + 1) != 't'
+ || line.charAt(firstNonWhiteSpace + 2) != ' ') {
+ return null;
+ }
+ int classClassLoaderOrModuleStartIndex =
+ firstNonWhiteSpaceCharacterFromIndex(line, firstNonWhiteSpace + 2);
+ if (classClassLoaderOrModuleStartIndex >= line.length()
+ || classClassLoaderOrModuleStartIndex != firstNonWhiteSpace + 3) {
+ return null;
+ }
+ int parensStart = firstCharFromIndex(line, classClassLoaderOrModuleStartIndex, '(');
+ if (parensStart >= line.length()) {
+ return null;
+ }
+ int parensEnd = line.lastIndexOf(')');
+ if (parensEnd <= parensStart) {
+ return null;
+ }
+ if (firstNonWhiteSpaceCharacterFromIndex(line, parensEnd) == line.length()) {
+ return null;
+ }
+ int methodSeparator = line.lastIndexOf('.', parensStart);
+ if (methodSeparator <= classClassLoaderOrModuleStartIndex) {
+ return null;
+ }
+ int classStartIndex = classClassLoaderOrModuleStartIndex;
+ int classLoaderOrModuleEndIndex =
+ firstCharFromIndex(line, classClassLoaderOrModuleStartIndex, '/');
+ if (classLoaderOrModuleEndIndex < methodSeparator) {
+ int moduleEndIndex = firstCharFromIndex(line, classLoaderOrModuleEndIndex + 1, '/');
+ if (moduleEndIndex < methodSeparator) {
+ // The stack trace contains both a class loader and module
+ classStartIndex = moduleEndIndex + 1;
+ } else {
+ classStartIndex = classLoaderOrModuleEndIndex + 1;
+ }
+ }
+ StackTraceElementStringProxyBuilder builder =
+ StackTraceElementStringProxy.builder(line)
+ .registerClassName(classStartIndex, methodSeparator, ClassNameType.TYPENAME)
+ .registerMethodName(methodSeparator + 1, parensStart);
+ // Check if we have a filename and position.
+ int separatorIndex = line.lastIndexOf(':', parensEnd);
+ if (separatorIndex > -1 && separatorIndex < parensEnd) {
+ builder.registerSourceFile(parensStart + 1, separatorIndex);
+ builder.registerLineNumber(separatorIndex + 1, parensEnd);
+ } else {
+ builder.registerSourceFile(parensStart + 1, parensEnd);
+ }
+ return builder.build();
+ }
+ }
+
+ static class CircularReferenceLine {
+
+ private static final String CIRCULAR_REFERENCE = "[CIRCULAR REFERENCE:";
+
+ static StackTraceElementStringProxy tryParse(String line) {
+ // Check that the line is indented with some amount of white space.
+ if (line.length() == 0 || !Character.isWhitespace(line.charAt(0))) {
+ return null;
+ }
+ // Find the first non-white space character and check that we have the sequence
+ // '[CIRCULAR REFERENCE:Exception]'.
+ int firstNonWhiteSpace = firstNonWhiteSpaceCharacterFromIndex(line, 0);
+ if (!line.startsWith(CIRCULAR_REFERENCE, firstNonWhiteSpace)) {
+ return null;
+ }
+ int exceptionStartIndex = firstNonWhiteSpace + CIRCULAR_REFERENCE.length();
+ int lastBracketPosition = firstCharFromIndex(line, exceptionStartIndex, ']');
+ if (lastBracketPosition == line.length()) {
+ return null;
+ }
+ int onlyWhitespaceFromLastBracket =
+ firstNonWhiteSpaceCharacterFromIndex(line, lastBracketPosition + 1);
+ if (onlyWhitespaceFromLastBracket != line.length()) {
+ return null;
+ }
+ return StackTraceElementStringProxy.builder(line)
+ .registerClassName(exceptionStartIndex, lastBracketPosition, ClassNameType.TYPENAME)
+ .build();
+ }
+ }
+
+ private StackTraceElementStringProxy parseLine(String line) {
+ // Most lines are 'at lines' so attempt to parse it first.
+ StackTraceElementStringProxy parsedLine = AtLine.tryParse(line);
+ if (parsedLine != null) {
+ return parsedLine;
+ }
+ parsedLine = ExceptionLine.tryParse(line);
+ if (parsedLine != null) {
+ return parsedLine;
+ }
+ parsedLine = CircularReferenceLine.tryParse(line);
+ if (parsedLine != null) {
+ return parsedLine;
+ }
+ return StackTraceElementStringProxy.builder(line).build();
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/retrace/RetraceTests.java b/src/test/java/com/android/tools/r8/retrace/RetraceTests.java
index 220bea7..d7a13c8 100644
--- a/src/test/java/com/android/tools/r8/retrace/RetraceTests.java
+++ b/src/test/java/com/android/tools/r8/retrace/RetraceTests.java
@@ -4,10 +4,12 @@
package com.android.tools.r8.retrace;
+import static com.android.tools.r8.retrace.internal.StackTraceRegularExpressionParser.DEFAULT_REGULAR_EXPRESSION;
import static junit.framework.TestCase.assertEquals;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.fail;
+import static org.junit.Assume.assumeFalse;
import static org.junit.Assume.assumeTrue;
import com.android.tools.r8.TestBase;
@@ -59,6 +61,7 @@
import java.util.Collection;
import java.util.List;
import java.util.function.Consumer;
+import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -67,16 +70,19 @@
@RunWith(Parameterized.class)
public class RetraceTests extends TestBase {
- @Parameters(name = "{0}, external: {1}")
+ @Parameters(name = "{0}, use regular expression: {1}, external: {2}")
public static Collection<Object[]> data() {
- return buildParameters(getTestParameters().withCfRuntimes().build(), BooleanUtils.values());
+ return buildParameters(
+ getTestParameters().withCfRuntimes().build(), BooleanUtils.values(), BooleanUtils.values());
}
private final TestParameters testParameters;
+ private final boolean useRegExpParsing;
private final boolean external;
- public RetraceTests(TestParameters parameters, boolean external) {
+ public RetraceTests(TestParameters parameters, boolean useRegExpParsing, boolean external) {
this.testParameters = parameters;
+ this.useRegExpParsing = useRegExpParsing;
this.external = external;
}
@@ -148,7 +154,8 @@
List<ActualBotStackTraceBase> stackTraces =
ImmutableList.of(new ActualIdentityStackTrace(), new ActualRetraceBotStackTrace());
for (ActualBotStackTraceBase stackTrace : stackTraces) {
- runRetraceTest(stackTrace).assertNoWarnings();
+ runRetraceTest(stackTrace)
+ .assertWarningsCount(useRegExpParsing ? 0 : stackTrace.expectedWarnings());
}
}
@@ -186,7 +193,7 @@
public void testCircularReferenceStackTrace() throws Exception {
// Proguard retrace (and therefore the default regular expression) will not retrace circular
// reference exceptions.
- assumeTrue("b/178599214", false);
+ assumeFalse(useRegExpParsing);
runRetraceTest(new CircularReferenceStackTrace());
}
@@ -196,8 +203,9 @@
}
@Test
+ @Ignore("b/170293908")
public void testBootLoaderAndNamedModulesStackTrace() throws Exception {
- assumeTrue("b/170293908", false);
+ assumeFalse(useRegExpParsing);
runRetraceTest(new NamedModuleStackTrace());
}
@@ -279,6 +287,7 @@
private TestDiagnosticMessagesImpl runRetraceTest(
StackTraceForTest stackTraceForTest, boolean allowExperimentalMapping) throws Exception {
if (external) {
+ assumeTrue(useRegExpParsing);
assumeTrue(testParameters.isCfRuntime());
// The external dependency is built on top of R8Lib. If test.py is run with
// no r8lib, do not try and run the external R8 Retrace since it has not been built.
@@ -318,6 +327,7 @@
RetraceCommand.builder(diagnosticsHandler)
.setProguardMapProducer(ProguardMapProducer.fromString(stackTraceForTest.mapping()))
.setStackTrace(stackTraceForTest.obfuscatedStackTrace())
+ .setRegularExpression(useRegExpParsing ? DEFAULT_REGULAR_EXPRESSION : null)
.setRetracedStackTraceConsumer(
retraced ->
assertEquals(