Add removeFrame for inlineframes throwing NPE without check
Bug: 198267337
Change-Id: I88770dd31e9f3f062d0ae60c1e0807d71265ec24
diff --git a/src/main/java/com/android/tools/r8/ir/code/Position.java b/src/main/java/com/android/tools/r8/ir/code/Position.java
index 259c18b..cb2ff68 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Position.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Position.java
@@ -17,14 +17,15 @@
// A no-position marker. Not having a position means the position is implicitly defined by the
// context, e.g., the marker does not materialize anything concrete.
- private static final Position NO_POSITION = new Position(-1, null, null, null, false);
+ private static final Position NO_POSITION = new Position(-1, null, null, null, false, false);
// A synthetic marker position that should never materialize.
// This is used specifically to mark exceptional exit blocks from synchronized methods in release.
- private static final Position NO_POSITION_SYNTHETIC = new Position(-1, null, null, null, true);
+ private static final Position NO_POSITION_SYNTHETIC =
+ new Position(-1, null, null, null, true, false);
// Fake position to use for representing an actual position in testing code.
- private static final Position TESTING_POSITION = new Position(0, null, null, null, true);
+ private static final Position TESTING_POSITION = new Position(0, null, null, null, true, false);
public final int line;
public final DexString file;
@@ -37,6 +38,7 @@
public final DexMethod method;
public final Position callerPosition;
+ public final boolean removeInnerFrameIfThrowingNpe;
private static void specify(StructuralSpecification<Position, ?> spec) {
spec.withInt(p -> p.line)
@@ -47,19 +49,25 @@
}
private Position(
- int line, DexString file, DexMethod method, Position callerPosition, boolean synthetic) {
+ int line,
+ DexString file,
+ DexMethod method,
+ Position callerPosition,
+ boolean synthetic,
+ boolean removeInnerFrameIfThrowingNpe) {
this.line = line;
this.file = file;
this.synthetic = synthetic;
this.method = method;
this.callerPosition = callerPosition;
+ this.removeInnerFrameIfThrowingNpe = removeInnerFrameIfThrowingNpe;
assert callerPosition == null || callerPosition.method != null;
}
public static Position synthetic(int line, DexMethod method, Position callerPosition) {
assert line >= 0;
assert method != null;
- return new Position(line, null, method, callerPosition, true);
+ return new Position(line, null, method, callerPosition, true, false);
}
public static Position none() {
@@ -80,7 +88,7 @@
// it as the caller of the inlined Positions.
public static Position noneWithMethod(DexMethod method, Position callerPosition) {
assert method != null;
- return new Position(-1, null, method, callerPosition, false);
+ return new Position(-1, null, method, callerPosition, false, false);
}
public static Position getPositionForInlining(
@@ -151,12 +159,8 @@
@Override
public int hashCode() {
- int result = line;
- result = 31 * result + Objects.hashCode(file);
- result = 31 * result + (synthetic ? 1 : 0);
- result = 31 * result + Objects.hashCode(method);
- result = 31 * result + Objects.hashCode(callerPosition);
- return result;
+ return Objects.hash(
+ line, file, synthetic, method, callerPosition, removeInnerFrameIfThrowingNpe);
}
private String toString(boolean forceMethod) {
@@ -196,7 +200,8 @@
.setFile(file)
.setMethod(method)
.setCallerPosition(callerPosition)
- .setSynthetic(synthetic);
+ .setSynthetic(synthetic)
+ .setRemoveInnerFramesIfThrowingNpe(removeInnerFrameIfThrowingNpe);
}
public static class Builder {
@@ -206,6 +211,7 @@
public boolean synthetic;
public DexMethod method;
public Position callerPosition;
+ public boolean removeInnerFrameIfThrowingNpe;
public Builder setLine(int line) {
this.line = line;
@@ -232,10 +238,16 @@
return this;
}
+ public Builder setRemoveInnerFramesIfThrowingNpe(boolean removeInnerFramesIfThrowingNpe) {
+ this.removeInnerFrameIfThrowingNpe = removeInnerFramesIfThrowingNpe;
+ return this;
+ }
+
public Position build() {
assert line >= 0;
assert method != null;
- return new Position(line, file, method, callerPosition, synthetic);
+ return new Position(
+ line, file, method, callerPosition, synthetic, removeInnerFrameIfThrowingNpe);
}
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/Value.java b/src/main/java/com/android/tools/r8/ir/code/Value.java
index 7b97131..27713d8 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Value.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Value.java
@@ -476,6 +476,10 @@
return false;
}
+ public boolean isMaybeNull() {
+ return !isNeverNull();
+ }
+
public boolean usedInMonitorOperation() {
for (Instruction instruction : uniqueUsers()) {
if (instruction.isMonitor()) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
index 2f61964..abf7239 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
@@ -642,7 +642,12 @@
InternalOptions options = appView.options();
// Build the IR for a yet not processed method, and perform minimal IR processing.
- IRCode code = inliningIRProvider.getInliningIR(invoke, target);
+ boolean removeInnerFramesIfThrowingNpe =
+ invoke.isInvokeMethodWithReceiver()
+ && invoke.asInvokeMethodWithReceiver().getReceiver().isMaybeNull()
+ && !shouldSynthesizeNullCheckForReceiver;
+ IRCode code =
+ inliningIRProvider.getInliningIR(invoke, target, removeInnerFramesIfThrowingNpe);
// Insert a init class instruction if this is needed to preserve class initialization
// semantics.
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerCostAnalysis.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerCostAnalysis.java
index 33fa6fe..262f8be 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerCostAnalysis.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerCostAnalysis.java
@@ -72,7 +72,7 @@
// Not a direct inlinee.
continue;
}
- IRCode inliningIR = inliningIRProvider.getAndCacheInliningIR(invoke, inlinee);
+ IRCode inliningIR = inliningIRProvider.getAndCacheInliningIR(invoke, inlinee, false);
int increment =
inlinee.getDefinition().getCode().estimatedSizeForInlining()
- estimateNumberOfNonMaterializingInstructions(invoke, inliningIR);
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java b/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java
index f93dc2a..a9c52e4 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java
@@ -527,7 +527,7 @@
method, triggersClassInitializationBeforeSideEffect(code));
} else {
// Identifies if the method preserves null check of the receiver after inlining.
- final Value receiver = code.getThis();
+ Value receiver = code.getThis();
feedback.markCheckNullReceiverBeforeAnySideEffect(
method, receiver.isUsed() && checksNullBeforeSideEffect(code, receiver));
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/inliner/InliningIRProvider.java b/src/main/java/com/android/tools/r8/ir/optimize/inliner/InliningIRProvider.java
index f348ce7..1387311 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/inliner/InliningIRProvider.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/inliner/InliningIRProvider.java
@@ -32,19 +32,24 @@
this.methodProcessor = methodProcessor;
}
- public IRCode getInliningIR(InvokeMethod invoke, ProgramMethod method) {
+ public IRCode getInliningIR(
+ InvokeMethod invoke, ProgramMethod method, boolean removeInnerFramesIfNpe) {
IRCode cached = cache.remove(invoke);
if (cached != null) {
return cached;
}
Position position = Position.getPositionForInlining(appView, invoke, context);
+ if (removeInnerFramesIfNpe) {
+ position = position.builderWithCopy().setRemoveInnerFramesIfThrowingNpe(true).build();
+ }
Origin origin = method.getOrigin();
return method.buildInliningIR(
context, appView, valueNumberGenerator, position, origin, methodProcessor);
}
- public IRCode getAndCacheInliningIR(InvokeMethod invoke, ProgramMethod method) {
- IRCode inliningIR = getInliningIR(invoke, method);
+ public IRCode getAndCacheInliningIR(
+ InvokeMethod invoke, ProgramMethod method, boolean removeInnerFrameIfThrowingNpe) {
+ IRCode inliningIR = getInliningIR(invoke, method, removeInnerFrameIfThrowingNpe);
cacheInliningIR(invoke, inliningIR);
return inliningIR;
}
diff --git a/src/main/java/com/android/tools/r8/naming/mappinginformation/RewriteFrameMappingInformation.java b/src/main/java/com/android/tools/r8/naming/mappinginformation/RewriteFrameMappingInformation.java
index d9d8c4b..b27fe19 100644
--- a/src/main/java/com/android/tools/r8/naming/mappinginformation/RewriteFrameMappingInformation.java
+++ b/src/main/java/com/android/tools/r8/naming/mappinginformation/RewriteFrameMappingInformation.java
@@ -9,6 +9,8 @@
import com.android.tools.r8.errors.CompilationError;
import com.android.tools.r8.errors.Unimplemented;
import com.android.tools.r8.naming.MapVersion;
+import com.android.tools.r8.references.ClassReference;
+import com.android.tools.r8.references.Reference;
import com.android.tools.r8.retrace.internal.RetraceStackTraceContextImpl;
import com.android.tools.r8.retrace.internal.RetraceStackTraceCurrentEvaluationInformation;
import com.android.tools.r8.utils.DescriptorUtils;
@@ -18,6 +20,7 @@
import com.google.gson.JsonObject;
import com.google.gson.JsonPrimitive;
import it.unimi.dsi.fastutil.ints.Int2IntMap;
+import java.util.ArrayList;
import java.util.List;
import java.util.function.Consumer;
@@ -102,9 +105,28 @@
return this;
}
- public static RewriteFrameMappingInformation create(
- List<Condition> conditions, List<RewriteAction> actions) {
- return new RewriteFrameMappingInformation(conditions, actions);
+ public static RewriteFrameMappingInformation.Builder builder() {
+ return new Builder();
+ }
+
+ public static class Builder {
+
+ private final List<Condition> conditions = new ArrayList<>();
+ private final List<RewriteAction> actions = new ArrayList<>();
+
+ public Builder addCondition(Condition condition) {
+ this.conditions.add(condition);
+ return this;
+ }
+
+ public Builder addRewriteAction(RewriteAction action) {
+ this.actions.add(action);
+ return this;
+ }
+
+ public RewriteFrameMappingInformation build() {
+ return new RewriteFrameMappingInformation(conditions, actions);
+ }
}
public abstract static class Condition {
@@ -140,15 +162,15 @@
static final String FUNCTION_NAME = "throws";
- private final String descriptor;
+ private final ClassReference classReference;
- private ThrowsCondition(String descriptor) {
- this.descriptor = descriptor;
+ private ThrowsCondition(ClassReference classReference) {
+ this.classReference = classReference;
}
@Override
protected JsonPrimitive serialize() {
- return new JsonPrimitive(FUNCTION_NAME + "(" + asThrowsCondition().getDescriptor() + ")");
+ return new JsonPrimitive(FUNCTION_NAME + "(" + classReference.getDescriptor() + ")");
}
@Override
@@ -156,10 +178,6 @@
return true;
}
- public String getDescriptor() {
- return descriptor;
- }
-
@Override
public ThrowsCondition asThrowsCondition() {
return this;
@@ -167,15 +185,18 @@
@Override
public boolean evaluate(RetraceStackTraceContextImpl context) {
- return context.getThrownException() != null
- && context.getThrownException().getDescriptor().equals(descriptor);
+ return classReference.equals(context.getThrownException());
}
public static ThrowsCondition deserialize(String conditionString) {
if (!DescriptorUtils.isClassDescriptor(conditionString)) {
throw new CompilationError("Unexpected throws-descriptor: " + conditionString);
}
- return new ThrowsCondition(conditionString);
+ return new ThrowsCondition(Reference.classFromDescriptor(conditionString));
+ }
+
+ public static ThrowsCondition create(ClassReference classReference) {
+ return new ThrowsCondition(classReference);
}
}
@@ -244,7 +265,7 @@
builder.incrementRemoveInnerFramesCount(numberOfFrames);
}
- static RemoveInnerFramesAction create(int numberOfFrames) {
+ public static RemoveInnerFramesAction create(int numberOfFrames) {
return new RemoveInnerFramesAction(numberOfFrames);
}
diff --git a/src/main/java/com/android/tools/r8/utils/LineNumberOptimizer.java b/src/main/java/com/android/tools/r8/utils/LineNumberOptimizer.java
index 37b3626..2a4db34 100644
--- a/src/main/java/com/android/tools/r8/utils/LineNumberOptimizer.java
+++ b/src/main/java/com/android/tools/r8/utils/LineNumberOptimizer.java
@@ -53,6 +53,10 @@
import com.android.tools.r8.naming.mappinginformation.CompilerSynthesizedMappingInformation;
import com.android.tools.r8.naming.mappinginformation.FileNameInformation;
import com.android.tools.r8.naming.mappinginformation.MappingInformation;
+import com.android.tools.r8.naming.mappinginformation.RewriteFrameMappingInformation;
+import com.android.tools.r8.naming.mappinginformation.RewriteFrameMappingInformation.RemoveInnerFramesAction;
+import com.android.tools.r8.naming.mappinginformation.RewriteFrameMappingInformation.ThrowsCondition;
+import com.android.tools.r8.references.Reference;
import com.android.tools.r8.retrace.internal.RetraceUtils;
import com.android.tools.r8.shaking.KeepInfoCollection;
import com.android.tools.r8.utils.InternalOptions.LineNumberOptimization;
@@ -481,19 +485,16 @@
}
}
Range originalRange = new Range(firstPosition.originalLine, lastPosition.originalLine);
-
Range obfuscatedRange;
if (method.getCode().isDexCode()
&& method.getCode().asDexCode().getDebugInfo()
== DexDebugInfoForSingleLineMethod.getInstance()) {
assert firstPosition.originalLine == lastPosition.originalLine;
- method.getCode().asDexCode().setDebugInfo(null);
obfuscatedRange = new Range(0, MAX_LINE_NUMBER);
} else {
obfuscatedRange =
new Range(firstPosition.obfuscatedLine, lastPosition.obfuscatedLine);
}
-
ClassNaming.Builder classNamingBuilder = onDemandClassNamingBuilder.get();
MappedRange lastMappedRange =
classNamingBuilder.addMappedRange(
@@ -502,13 +503,26 @@
originalRange,
obfuscatedName);
Position caller = firstPosition.caller;
+ int inlineFramesCount = 0;
while (caller != null) {
+ inlineFramesCount += 1;
lastMappedRange =
classNamingBuilder.addMappedRange(
obfuscatedRange,
getOriginalMethodSignature.apply(caller.method),
new Range(Math.max(caller.line, 0)), // Prevent against "no-position".
obfuscatedName);
+ if (caller.removeInnerFrameIfThrowingNpe) {
+ lastMappedRange.addMappingInformation(
+ RewriteFrameMappingInformation.builder()
+ .addCondition(
+ ThrowsCondition.create(
+ Reference.classFromDescriptor(
+ appView.options().dexItemFactory().npeDescriptor.toString())))
+ .addRewriteAction(RemoveInnerFramesAction.create(inlineFramesCount))
+ .build(),
+ Unreachable::raise);
+ }
caller = caller.callerPosition;
}
for (MappingInformation info : methodMappingInfo) {
diff --git a/src/test/java/com/android/tools/r8/naming/retrace/DesugarLambdaRetraceTest.java b/src/test/java/com/android/tools/r8/naming/retrace/DesugarLambdaRetraceTest.java
index 7a4914a..59bc15d 100644
--- a/src/test/java/com/android/tools/r8/naming/retrace/DesugarLambdaRetraceTest.java
+++ b/src/test/java/com/android/tools/r8/naming/retrace/DesugarLambdaRetraceTest.java
@@ -58,21 +58,21 @@
private void checkIsSame(StackTrace actualStackTrace, StackTrace retracedStackTrace) {
// Even when SourceFile is present retrace replaces the file name in the stack trace.
assertThat(retracedStackTrace, isSame(expectedStackTrace));
- assertEquals(expectedActualStackTraceHeight(), actualStackTrace.size());
+ assertEquals(expectedActualStackTraceHeight(), actualStackTrace.getStackTraceLines().size());
}
private void checkIsSameExceptForFileName(
StackTrace actualStackTrace, StackTrace retracedStackTrace) {
// Even when SourceFile is present retrace replaces the file name in the stack trace.
assertThat(retracedStackTrace, isSameExceptForFileName(expectedStackTrace));
- assertEquals(expectedActualStackTraceHeight(), actualStackTrace.size());
+ assertEquals(expectedActualStackTraceHeight(), actualStackTrace.getStackTraceLines().size());
}
private void checkIsSameExceptForFileNameAndLineNumber(
StackTrace actualStackTrace, StackTrace retracedStackTrace) {
// Even when SourceFile is present retrace replaces the file name in the stack trace.
assertThat(retracedStackTrace, isSameExceptForFileNameAndLineNumber(expectedStackTrace));
- assertEquals(expectedActualStackTraceHeight(), actualStackTrace.size());
+ assertEquals(expectedActualStackTraceHeight(), actualStackTrace.getStackTraceLines().size());
}
@Test
diff --git a/src/test/java/com/android/tools/r8/naming/retrace/InliningRetraceTest.java b/src/test/java/com/android/tools/r8/naming/retrace/InliningRetraceTest.java
index be514ec..c3a07cd 100644
--- a/src/test/java/com/android/tools/r8/naming/retrace/InliningRetraceTest.java
+++ b/src/test/java/com/android/tools/r8/naming/retrace/InliningRetraceTest.java
@@ -52,7 +52,8 @@
ImmutableList.of("-keepattributes SourceFile,LineNumberTable"),
(StackTrace actualStackTrace, StackTrace retracedStackTrace) -> {
assertThat(retracedStackTrace, isSame(expectedStackTrace));
- assertEquals(expectedActualStackTraceHeight(), actualStackTrace.size());
+ assertEquals(
+ expectedActualStackTraceHeight(), actualStackTrace.getStackTraceLines().size());
});
}
@@ -64,7 +65,8 @@
ImmutableList.of("-keepattributes LineNumberTable"),
(StackTrace actualStackTrace, StackTrace retracedStackTrace) -> {
assertThat(retracedStackTrace, isSameExceptForFileName(expectedStackTrace));
- assertEquals(expectedActualStackTraceHeight(), actualStackTrace.size());
+ assertEquals(
+ expectedActualStackTraceHeight(), actualStackTrace.getStackTraceLines().size());
});
}
@@ -76,7 +78,8 @@
ImmutableList.of(),
(StackTrace actualStackTrace, StackTrace retracedStackTrace) -> {
assertThat(retracedStackTrace, isSameExceptForFileNameAndLineNumber(expectedStackTrace));
- assertEquals(expectedActualStackTraceHeight(), actualStackTrace.size());
+ assertEquals(
+ expectedActualStackTraceHeight(), actualStackTrace.getStackTraceLines().size());
});
}
diff --git a/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineeWithNoSuchMethodErrorTest.java b/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineeWithNoSuchMethodErrorTest.java
new file mode 100644
index 0000000..1c2f06d
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineeWithNoSuchMethodErrorTest.java
@@ -0,0 +1,130 @@
+// Copyright (c) 2021, 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.naming.retrace;
+
+import static com.android.tools.r8.naming.retrace.StackTrace.isSame;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.R8FullTestBuilder;
+import com.android.tools.r8.R8TestRunResult;
+import com.android.tools.r8.SingleTestRunResult;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestBuilder;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.utils.BooleanUtils;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import java.util.List;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class RetraceInlineeWithNoSuchMethodErrorTest extends TestBase {
+
+ @Parameter() public TestParameters parameters;
+
+ @Parameter(1)
+ public boolean throwReceiverNpe;
+
+ @Parameters(name = "{0}, throwReceiverNpe: {1}")
+ public static List<Object[]> data() {
+ return buildParameters(
+ getTestParameters().withAllRuntimesAndApiLevels().build(), BooleanUtils.values());
+ }
+
+ public StackTrace expectedStackTrace;
+
+ @Before
+ public void setup() throws Exception {
+ // Get the expected stack trace by running on the JVM.
+ TestBuilder<? extends SingleTestRunResult<?>, ?> testBuilder =
+ testForRuntime(parameters)
+ .addProgramClasses(Caller.class)
+ .addProgramClassFileData(getFoo());
+ SingleTestRunResult<?> runResult;
+ if (throwReceiverNpe) {
+ runResult =
+ testBuilder
+ .run(parameters.getRuntime(), Caller.class, "foo")
+ .assertFailureWithErrorThatThrows(NullPointerException.class);
+ } else {
+ runResult =
+ testBuilder
+ .run(parameters.getRuntime(), Caller.class)
+ .assertFailureWithErrorThatThrows(NoSuchMethodError.class);
+ }
+ if (parameters.isCfRuntime()) {
+ expectedStackTrace = runResult.map(StackTrace::extractFromJvm);
+ } else {
+ expectedStackTrace =
+ StackTrace.extractFromArt(runResult.getStdErr(), parameters.asDexRuntime().getVm());
+ }
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ R8FullTestBuilder r8FullTestBuilder =
+ testForR8(parameters.getBackend())
+ .addProgramClasses(Caller.class)
+ .addProgramClassFileData(getFoo())
+ .addKeepMainRule(Caller.class)
+ .addKeepAttributeLineNumberTable()
+ .addKeepAttributeSourceFile()
+ .setMinApi(parameters.getApiLevel())
+ .enableInliningAnnotations()
+ .enableExperimentalMapFileVersion();
+ R8TestRunResult runResult;
+ if (throwReceiverNpe) {
+ runResult =
+ r8FullTestBuilder
+ .run(parameters.getRuntime(), Caller.class, "foo")
+ .assertFailureWithErrorThatThrows(NullPointerException.class);
+ } else {
+ runResult =
+ r8FullTestBuilder
+ .run(parameters.getRuntime(), Caller.class)
+ .assertFailureWithErrorThatThrows(NoSuchMethodError.class);
+ }
+ runResult.inspectStackTrace(
+ (stackTrace, codeInspector) -> {
+ assertThat(stackTrace, isSame(expectedStackTrace));
+ ClassSubject fooClass = codeInspector.clazz(Foo.class);
+ assertThat(fooClass, isPresent());
+ // We are not inlining this because resolution fails
+ assertThat(fooClass.uniqueMethodWithName("inlinable"), isPresent());
+ });
+ }
+
+ private static byte[] getFoo() throws Exception {
+ return transformer(Foo.class).removeMethodsWithName("foo").transform();
+ }
+
+ public static class Foo {
+
+ /* Removed on input */
+ public Object foo() {
+ throw new RuntimeException("Will be removed");
+ }
+
+ Object inlinable() {
+ return foo();
+ }
+ }
+
+ static class Caller {
+ @NeverInline
+ static void caller(Foo f) {
+ f.inlinable();
+ }
+
+ public static void main(String[] args) {
+ caller(args.length == 0 ? new Foo() : null);
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineeWithNullCheck.java b/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineeWithNullCheck.java
deleted file mode 100644
index 548025b..0000000
--- a/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineeWithNullCheck.java
+++ /dev/null
@@ -1,86 +0,0 @@
-// Copyright (c) 2021, 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.naming.retrace;
-
-import static com.android.tools.r8.naming.retrace.StackTrace.isSame;
-import static org.hamcrest.CoreMatchers.not;
-import static org.hamcrest.MatcherAssert.assertThat;
-
-import com.android.tools.r8.NeverInline;
-import com.android.tools.r8.TestBase;
-import com.android.tools.r8.TestParameters;
-import com.android.tools.r8.TestRuntime.CfRuntime;
-import java.util.List;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-import org.junit.runners.Parameterized.Parameter;
-import org.junit.runners.Parameterized.Parameters;
-
-@RunWith(Parameterized.class)
-public class RetraceInlineeWithNullCheck extends TestBase {
-
- @Parameter() public TestParameters parameters;
-
- @Parameters(name = "{0}")
- public static List<Object[]> data() {
- return buildParameters(
- getTestParameters().withAllRuntimes().withAllApiLevelsAlsoForCf().build());
- }
-
- public StackTrace expectedStackTrace;
-
- @Before
- public void setup() throws Exception {
- // Get the expected stack trace by running on the JVM.
- expectedStackTrace =
- testForJvm()
- .addTestClasspath()
- .run(CfRuntime.getSystemRuntime(), Caller.class)
- .assertFailure()
- .map(StackTrace::extractFromJvm);
- }
-
- @Test
- public void testR8() throws Exception {
- testForR8(parameters.getBackend())
- .addInnerClasses(getClass())
- .addKeepMainRule(Caller.class)
- .addKeepAttributeLineNumberTable()
- .addKeepAttributeSourceFile()
- .setMinApi(parameters.getApiLevel())
- .enableInliningAnnotations()
- .run(parameters.getRuntime(), Caller.class)
- .assertFailureWithErrorThatThrows(NullPointerException.class)
- // TODO(b/197936862): The two should be the same
- .inspectStackTrace(
- (stackTrace, codeInspector) -> {
- assertThat(stackTrace, not(isSame(expectedStackTrace)));
- });
- }
-
- static class Foo {
- @NeverInline
- Object notInlinable() {
- System.out.println("Hello, world!");
- throw new RuntimeException("Foo");
- }
-
- Object inlinable() {
- return notInlinable();
- }
- }
-
- static class Caller {
- @NeverInline
- static void caller(Foo f) {
- f.inlinable();
- }
-
- public static void main(String[] args) {
- caller(System.currentTimeMillis() < 0 ? new Foo() : null);
- }
- }
-}
diff --git a/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineeWithNullCheckSequence.java b/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineeWithNullCheckSequenceTest.java
similarity index 75%
rename from src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineeWithNullCheckSequence.java
rename to src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineeWithNullCheckSequenceTest.java
index f7f7b72..7c0f956 100644
--- a/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineeWithNullCheckSequence.java
+++ b/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineeWithNullCheckSequenceTest.java
@@ -4,13 +4,12 @@
package com.android.tools.r8.naming.retrace;
import static com.android.tools.r8.naming.retrace.StackTrace.isSame;
-import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.SingleTestRunResult;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
-import com.android.tools.r8.TestRuntime.CfRuntime;
import java.util.List;
import org.junit.Before;
import org.junit.Test;
@@ -20,7 +19,7 @@
import org.junit.runners.Parameterized.Parameters;
@RunWith(Parameterized.class)
-public class RetraceInlineeWithNullCheckSequence extends TestBase {
+public class RetraceInlineeWithNullCheckSequenceTest extends TestBase {
@Parameter() public TestParameters parameters;
@@ -35,12 +34,17 @@
@Before
public void setup() throws Exception {
// Get the expected stack trace by running on the JVM.
- expectedStackTrace =
- testForJvm()
- .addTestClasspath()
- .run(CfRuntime.getSystemRuntime(), Caller.class)
- .assertFailure()
- .map(StackTrace::extractFromJvm);
+ SingleTestRunResult<?> runResult =
+ testForRuntime(parameters)
+ .addProgramClasses(Caller.class, Foo.class)
+ .run(parameters.getRuntime(), Caller.class)
+ .assertFailureWithErrorThatThrows(NullPointerException.class);
+ if (parameters.isCfRuntime()) {
+ expectedStackTrace = runResult.map(StackTrace::extractFromJvm);
+ } else {
+ expectedStackTrace =
+ StackTrace.extractFromArt(runResult.getStdErr(), parameters.asDexRuntime().getVm());
+ }
}
@Test
@@ -52,16 +56,17 @@
.addKeepAttributeSourceFile()
.setMinApi(parameters.getApiLevel())
.enableInliningAnnotations()
+ .enableExperimentalMapFileVersion()
.run(parameters.getRuntime(), Caller.class)
.assertFailureWithErrorThatThrows(NullPointerException.class)
.inspectStackTrace(
(stackTrace, codeInspector) -> {
- // TODO(b/197936862): The two stacktraces should be the same
- assertThat(stackTrace, not(isSame(expectedStackTrace)));
+ assertThat(stackTrace, isSame(expectedStackTrace));
});
}
static class Foo {
+
@NeverInline
void notInlinable() {
System.out.println("Hello, world!");
diff --git a/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineeWithNullCheckTest.java b/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineeWithNullCheckTest.java
new file mode 100644
index 0000000..81c2200
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineeWithNullCheckTest.java
@@ -0,0 +1,109 @@
+// Copyright (c) 2021, 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.naming.retrace;
+
+import static com.android.tools.r8.naming.retrace.StackTrace.isSame;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.R8FullTestBuilder;
+import com.android.tools.r8.R8TestRunResult;
+import com.android.tools.r8.SingleTestRunResult;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestBuilder;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.utils.BooleanUtils;
+import java.util.List;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class RetraceInlineeWithNullCheckTest extends TestBase {
+
+ @Parameter(0)
+ public TestParameters parameters;
+
+ @Parameter(1)
+ public boolean throwReceiverNpe;
+
+ @Parameters(name = "{0}, throwReceiverNpe: {1}")
+ public static List<Object[]> data() {
+ return buildParameters(
+ getTestParameters().withAllRuntimes().withAllApiLevelsAlsoForCf().build(),
+ BooleanUtils.values());
+ }
+
+ public StackTrace expectedStackTrace;
+
+ @Before
+ public void setup() throws Exception {
+ // Get the expected stack trace by running on the JVM.
+ TestBuilder<? extends SingleTestRunResult<?>, ?> testBuilder =
+ testForRuntime(parameters).addProgramClasses(Caller.class, Foo.class);
+ SingleTestRunResult<?> runResult;
+ if (throwReceiverNpe) {
+ runResult = testBuilder.run(parameters.getRuntime(), Caller.class, "foo");
+ } else {
+ runResult = testBuilder.run(parameters.getRuntime(), Caller.class);
+ }
+ if (parameters.isCfRuntime()) {
+ expectedStackTrace = runResult.map(StackTrace::extractFromJvm);
+ } else {
+ expectedStackTrace =
+ StackTrace.extractFromArt(runResult.getStdErr(), parameters.asDexRuntime().getVm());
+ }
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ R8FullTestBuilder r8FullTestBuilder =
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepMainRule(Caller.class)
+ .addKeepAttributeLineNumberTable()
+ .addKeepAttributeSourceFile()
+ .setMinApi(parameters.getApiLevel())
+ .enableInliningAnnotations()
+ .enableExperimentalMapFileVersion();
+ R8TestRunResult runResult;
+ if (throwReceiverNpe) {
+ runResult = r8FullTestBuilder.run(parameters.getRuntime(), Caller.class, "foo");
+ } else {
+ runResult = r8FullTestBuilder.run(parameters.getRuntime(), Caller.class);
+ }
+ runResult
+ .assertFailureWithErrorThatThrows(NullPointerException.class)
+ .inspectStackTrace(
+ (stackTrace, codeInspector) -> {
+ assertThat(stackTrace, isSame(expectedStackTrace));
+ });
+ }
+
+ static class Foo {
+ @NeverInline
+ Object notInlinable() {
+ System.out.println("Hello, world!");
+ throw new NullPointerException("Foo");
+ }
+
+ Object inlinable() {
+ return notInlinable();
+ }
+ }
+
+ static class Caller {
+ @NeverInline
+ static void caller(Foo f) {
+ f.inlinable();
+ }
+
+ public static void main(String[] args) {
+ caller(args.length == 0 ? new Foo() : null);
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/naming/retrace/StackTrace.java b/src/test/java/com/android/tools/r8/naming/retrace/StackTrace.java
index 8dde075..c4d6dad 100644
--- a/src/test/java/com/android/tools/r8/naming/retrace/StackTrace.java
+++ b/src/test/java/com/android/tools/r8/naming/retrace/StackTrace.java
@@ -14,11 +14,13 @@
import com.android.tools.r8.retrace.ProguardMapProducer;
import com.android.tools.r8.retrace.RetraceCommand;
import com.android.tools.r8.retrace.RetraceHelper;
+import com.android.tools.r8.utils.Box;
import com.android.tools.r8.utils.StringUtils;
import com.google.common.base.Equivalence;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
+import java.util.Optional;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.function.Predicate;
@@ -35,6 +37,7 @@
public static class Builder {
+ private String exceptionLine;
private List<StackTraceLine> stackTraceLines = new ArrayList<>();
private Builder() {}
@@ -80,8 +83,14 @@
return this;
}
+ public Builder setExceptionLine(String exceptionLine) {
+ this.exceptionLine = exceptionLine;
+ return this;
+ }
+
public StackTrace build() {
return new StackTrace(
+ exceptionLine,
stackTraceLines,
StringUtils.join(
"\n",
@@ -224,16 +233,19 @@
}
}
+ private final String exceptionLine;
private final List<StackTraceLine> stackTraceLines;
private final String originalStderr;
- private StackTrace(List<StackTraceLine> stackTraceLines, String originalStderr) {
+ private StackTrace(
+ String exceptionLine, List<StackTraceLine> stackTraceLines, String originalStderr) {
+ this.exceptionLine = exceptionLine;
this.stackTraceLines = stackTraceLines;
this.originalStderr = originalStderr;
}
public int size() {
- return stackTraceLines.size();
+ return stackTraceLines.size() + 1;
}
public StackTraceLine get(int index) {
@@ -272,6 +284,7 @@
// \tat com.android.tools.r8.naming.retrace.Main.a(:150)
// \tat com.android.tools.r8.naming.retrace.Main.a(:156)
// \tat com.android.tools.r8.naming.retrace.Main.main(:162)
+ Optional<String> exceptionLine = Optional.empty();
for (int i = 0; i < stderrLines.size(); i++) {
String line = stderrLines.get(i);
// Find all lines starting with "\tat" except "dalvik.system.NativeStart.main" frame
@@ -279,23 +292,27 @@
if (line.startsWith(TAB_AT_PREFIX)
&& !(vm.isOlderThanOrEqual(DexVm.ART_4_4_4_HOST)
&& line.contains("dalvik.system.NativeStart.main"))) {
+ if (!exceptionLine.isPresent() && i > 0) {
+ exceptionLine = Optional.of(stderrLines.get(i - 1));
+ }
stackTraceLines.add(StackTraceLine.parse(stderrLines.get(i)));
}
}
- return new StackTrace(stackTraceLines, stderr);
+ return new StackTrace(exceptionLine.orElse(""), stackTraceLines, stderr);
}
private static List<StackTraceLine> internalConvert(Stream<String> lines) {
return lines.map(StackTraceLine::parse).collect(Collectors.toList());
}
- private static List<StackTraceLine> internalExtractFromJvm(String stderr) {
- return internalConvert(
- StringUtils.splitLines(stderr).stream().filter(s -> s.startsWith(TAB_AT_PREFIX)));
+ private static List<StackTraceLine> internalExtractFromJvm(List<String> strings) {
+ return internalConvert(strings.stream().filter(s -> s.startsWith(TAB_AT_PREFIX)));
}
public static StackTrace extractFromJvm(String stderr) {
- return new StackTrace(internalExtractFromJvm(stderr), stderr);
+ List<String> strings = StringUtils.splitLines(stderr);
+ return new StackTrace(
+ strings.isEmpty() ? "" : strings.get(0), internalExtractFromJvm(strings), stderr);
}
public static StackTrace extractFromJvm(SingleTestRunResult result) {
@@ -312,27 +329,27 @@
}
public StackTrace retrace(String map, boolean allowExperimentalMapping) {
- class Box {
- List<String> result;
- }
- Box box = new Box();
+ Box<List<String>> box = new Box<>();
+ List<String> stackTrace =
+ stackTraceLines.stream().map(line -> line.originalLine).collect(Collectors.toList());
+ stackTrace.add(0, exceptionLine);
RetraceHelper.runForTesting(
RetraceCommand.builder()
.setProguardMapProducer(ProguardMapProducer.fromString(map))
- .setStackTrace(
- stackTraceLines.stream()
- .map(line -> line.originalLine)
- .collect(Collectors.toList()))
- .setRetracedStackTraceConsumer(retraced -> box.result = retraced)
+ .setStackTrace(stackTrace)
+ .setRetracedStackTraceConsumer(box::set)
.build(),
allowExperimentalMapping);
// Keep the original stderr in the retraced stacktrace.
- return new StackTrace(internalConvert(box.result.stream()), originalStderr);
+ return new StackTrace(
+ box.get().get(0), internalConvert(box.get().stream().skip(1)), originalStderr);
}
public StackTrace filter(Predicate<StackTraceLine> filter) {
return new StackTrace(
- stackTraceLines.stream().filter(filter).collect(Collectors.toList()), originalStderr);
+ exceptionLine,
+ stackTraceLines.stream().filter(filter).collect(Collectors.toList()),
+ originalStderr);
}
@Override
diff --git a/src/test/java/com/android/tools/r8/naming/retrace/StackTraceTest.java b/src/test/java/com/android/tools/r8/naming/retrace/StackTraceTest.java
index 16d31bb..f866398 100644
--- a/src/test/java/com/android/tools/r8/naming/retrace/StackTraceTest.java
+++ b/src/test/java/com/android/tools/r8/naming/retrace/StackTraceTest.java
@@ -51,7 +51,8 @@
}
private void checkOneLine(StackTrace stackTrace) {
- assertEquals(1, stackTrace.size());
+ assertEquals(2, stackTrace.size());
+ assertEquals(1, stackTrace.getStackTraceLines().size());
StackTraceLine stackTraceLine = stackTrace.get(0);
assertEquals("Test", stackTraceLine.className);
assertEquals("main", stackTraceLine.methodName);
diff --git a/src/test/java/com/android/tools/r8/naming/retrace/VerticalClassMergingRetraceTest.java b/src/test/java/com/android/tools/r8/naming/retrace/VerticalClassMergingRetraceTest.java
index 0482945..fc1e432 100644
--- a/src/test/java/com/android/tools/r8/naming/retrace/VerticalClassMergingRetraceTest.java
+++ b/src/test/java/com/android/tools/r8/naming/retrace/VerticalClassMergingRetraceTest.java
@@ -86,7 +86,8 @@
? retracedStackTrace
: retracedStackTrace.filter(this::filterSynthesizedMethodWhenLineNumberAvailable);
assertThat(reprocessedStackTrace, isSameExceptForFileName(expectedStackTrace));
- assertEquals(expectedActualStackTraceHeight(), actualStackTrace.size());
+ assertEquals(
+ expectedActualStackTraceHeight(), actualStackTrace.getStackTraceLines().size());
});
}
@@ -102,7 +103,8 @@
? retracedStackTrace
: retracedStackTrace.filter(this::filterSynthesizedMethodWhenLineNumberAvailable);
assertThat(reprocessedStackTrace, isSameExceptForFileName(expectedStackTrace));
- assertEquals(expectedActualStackTraceHeight(), actualStackTrace.size());
+ assertEquals(
+ expectedActualStackTraceHeight(), actualStackTrace.getStackTraceLines().size());
});
}
@@ -144,7 +146,8 @@
: retracedStackTrace.filter(this::filterSynthesizedBridgeMethod);
assertThat(
reprocessedStackTrace, isSameExceptForFileNameAndLineNumber(expectedStackTrace));
- assertEquals(expectedActualStackTraceHeight(), actualStackTrace.size());
+ assertEquals(
+ expectedActualStackTraceHeight(), actualStackTrace.getStackTraceLines().size());
});
}
}
