Version 1.3.26.
Merge: Implement workaround for Dalvik tracing JIT tracing past the end of instructions.
CL: https://r8-review.googlesource.com/c/r8/+/29242
R=christofferqa@google.com
Bug: 117907456
Change-Id: I73ef9b3263bf0f8d873736e2d9528a8f429780a4
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 5ee47e8..b7f1455 100644
--- a/src/main/java/com/android/tools/r8/Version.java
+++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@
// This field is accessed from release scripts using simple pattern matching.
// Therefore, changing this field could break our release scripts.
- public static final String LABEL = "1.3.25";
+ public static final String LABEL = "1.3.26";
private Version() {
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
index 3d83104..bca0395 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -329,7 +329,7 @@
public void setCode(IRCode ir, RegisterAllocator registerAllocator, InternalOptions options) {
checkIfObsolete();
final DexBuilder builder = new DexBuilder(ir, registerAllocator, options);
- setCode(builder.build(method.getArity()));
+ setCode(builder.build());
}
@Override
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/DexBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/DexBuilder.java
index 0084a74..1f7a684 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/DexBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/DexBuilder.java
@@ -32,6 +32,7 @@
import com.android.tools.r8.code.MoveWide16;
import com.android.tools.r8.code.MoveWideFrom16;
import com.android.tools.r8.code.Nop;
+import com.android.tools.r8.code.Throw;
import com.android.tools.r8.dex.Constants;
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.DebugLocalInfo;
@@ -110,6 +111,9 @@
// The string reference in the code with the highest index.
private DexString highestSortingReferencedString = null;
+ // Whether or not the generated code has a backwards branch.
+ private boolean hasBackwardsBranch = false;
+
BasicBlock nextBlock;
public DexBuilder(
@@ -142,7 +146,7 @@
* This is a two pass construction that will first compute concrete offsets and then construct
* the concrete instructions.
*/
- public DexCode build(int numberOfArguments) {
+ public DexCode build() {
int numberOfInstructions;
int offset;
@@ -202,6 +206,23 @@
debugEventBuilder.add(instructionStartOffset, instructionOffset, ir);
}
+ // Workaround dalvik tracing bug, where the dalvik tracing JIT can end up tracing
+ // past the end of the instruction stream if the instruction streams ends with a throw.
+ // We could have also changed the block order, however, moving the throwing block higher
+ // led to larger code in all experiments (multiple gmscore version and R8 run on itself).
+ if (options.canHaveTracingPastInstructionsStreamBug()
+ && dexInstructions.get(dexInstructions.size() - 1) instanceof Throw
+ && hasBackwardsBranch) {
+ Nop nop = new Nop();
+ dexInstructions.add(nop);
+ nop.setOffset(offset);
+ offset += nop.getSize();
+ Goto go = new Goto(-nop.getSize());
+ dexInstructions.add(go);
+ go.setOffset(offset);
+ offset += go.getSize();
+ }
+
// Compute switch payloads.
for (SwitchPayloadInfo switchPayloadInfo : switchPayloadInfos) {
// Align payloads at even addresses.
@@ -241,7 +262,8 @@
registerAllocator.registersUsed(),
inRegisterCount,
outRegisterCount,
- dexInstructions.toArray(new Instruction[dexInstructions.size()]), tryInfo.tries,
+ dexInstructions.toArray(new Instruction[dexInstructions.size()]),
+ tryInfo.tries,
tryInfo.handlers,
debugEventBuilder.build(),
highestSortingReferencedString);
@@ -962,6 +984,9 @@
int source = builder.getInfo(jump).getOffset();
Info targetInfo = builder.getTargetInfo(jump.getTarget());
int relativeOffset = targetInfo.getOffset() - source;
+ if (relativeOffset < 0) {
+ builder.hasBackwardsBranch = true;
+ }
// Emit a return if the target is a return and the size of the return is the computed
// size of this instruction.
Return ret = targetInfo.getIR().asReturn();
@@ -1047,6 +1072,11 @@
int target = builder.getInfo(branch.getTrueTarget().entry()).getOffset();
int relativeOffset = target - source;
int register1 = builder.allocatedRegister(branch.inValues().get(0), branch.getNumber());
+
+ if (relativeOffset < 0) {
+ builder.hasBackwardsBranch = true;
+ }
+
if (size == 3) {
assert branchesToSelf(builder);
Nop nop = new Nop();
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index e0e4227..332b0d3 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -734,4 +734,15 @@
public boolean canHaveExceptionTargetingLoopHeaderBug() {
return isGeneratingDex() && !debug && minApiLevel < AndroidApiLevel.M.getLevel();
}
+
+ // The Dalvik tracing JIT can trace past the end of the instruction stream and end up
+ // parsing non-code bytes as code (typically leading to a crash).
+ //
+ // In order to workaround this we insert "nop; goto -1;" after the throw when the instruction
+ // stream ends with a throw and there are backwards branches in the code.
+ //
+ // See b/117907456.
+ public boolean canHaveTracingPastInstructionsStreamBug() {
+ return minApiLevel < AndroidApiLevel.L.getLevel();
+ }
}
diff --git a/src/test/java/com/android/tools/r8/regress/b117907456/B117907456.java b/src/test/java/com/android/tools/r8/regress/b117907456/B117907456.java
new file mode 100644
index 0000000..0d83011
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/regress/b117907456/B117907456.java
@@ -0,0 +1,74 @@
+// Copyright (c) 2018 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.regress.b117907456;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.android.tools.r8.CompilationFailedException;
+import com.android.tools.r8.D8Command;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.code.Goto;
+import com.android.tools.r8.code.Instruction;
+import com.android.tools.r8.code.Nop;
+import com.android.tools.r8.code.Throw;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.AndroidApp;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import com.google.common.collect.ImmutableList;
+import java.io.IOException;
+import java.util.concurrent.ExecutionException;
+import org.junit.Test;
+
+class TestClass {
+ public static void f(boolean loop) {
+ while (loop) {
+ System.out.println("Noooooo!");
+ }
+ throw new RuntimeException();
+ }
+}
+
+public class B117907456 extends TestBase {
+ @Test
+ public void testNopDupInsertionForDalvikTracingBug()
+ throws IOException, CompilationFailedException, ExecutionException {
+ MethodSubject method = getMethodSubject(AndroidApiLevel.K);
+ Instruction[] instructions = method.getMethod().getCode().asDexCode().instructions;
+ Instruction lastInstruction = instructions[instructions.length - 1];
+ assert !(lastInstruction instanceof Throw);
+ assert lastInstruction instanceof Goto;
+ Instruction previous = instructions[instructions.length - 2];
+ assert previous instanceof Nop;
+ }
+
+ @Test
+ public void testNoNopDupInsertionForDalvikTracingBug()
+ throws IOException, CompilationFailedException, ExecutionException {
+ MethodSubject method = getMethodSubject(AndroidApiLevel.L);
+ Instruction[] instructions = method.getMethod().getCode().asDexCode().instructions;
+ Instruction lastInstruction = instructions[instructions.length - 1];
+ assert lastInstruction instanceof Throw;
+ }
+
+ private MethodSubject getMethodSubject(AndroidApiLevel level)
+ throws CompilationFailedException, IOException, ExecutionException {
+ AndroidApp app =
+ ToolHelper.runD8(
+ D8Command.builder()
+ .addClassProgramData(ToolHelper.getClassAsBytes(TestClass.class), Origin.unknown())
+ .setMinApiLevel(level.getLevel()));
+ CodeInspector inspector = new CodeInspector(app);
+ ClassSubject clazz = inspector.clazz(TestClass.class);
+ assertThat(clazz, isPresent());
+ MethodSubject method = clazz.method("void", "f", ImmutableList.of("boolean"));
+ assertThat(method, isPresent());
+ return method;
+ }
+}
