Version 1.3.48.
Merge: Workaround another mediatek Lollipop VM bug.
CL: https://r8-review.googlesource.com/c/r8/+/32261
R=christofferqa@google.com, zerny@google.com
Bug: 117907456, 119895393
Change-Id: I6f4e55204eb74ada52eeac27367a061fa80e6ac4
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 62b346a..8fb488f 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.47";
+ public static final String LABEL = "1.3.48";
private Version() {
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexType.java b/src/main/java/com/android/tools/r8/graph/DexType.java
index fe63f46..5251aa2 100644
--- a/src/main/java/com/android/tools/r8/graph/DexType.java
+++ b/src/main/java/com/android/tools/r8/graph/DexType.java
@@ -363,6 +363,14 @@
return descriptor.content[0] == 'Z';
}
+ public boolean isLongType() {
+ return descriptor.content[0] == 'J';
+ }
+
+ public boolean isDoubleType() {
+ return descriptor.content[0] == 'D';
+ }
+
public boolean isIntType() {
return descriptor.content[0] == 'I';
}
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 8927476..e1bfb24 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
@@ -3,6 +3,8 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.ir.conversion;
+import com.android.tools.r8.code.Const4;
+import com.android.tools.r8.code.ConstWide16;
import com.android.tools.r8.code.FillArrayData;
import com.android.tools.r8.code.FillArrayDataPayload;
import com.android.tools.r8.code.Format31t;
@@ -33,6 +35,8 @@
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.ReturnVoid;
+import com.android.tools.r8.code.ReturnWide;
import com.android.tools.r8.code.Throw;
import com.android.tools.r8.dex.Constants;
import com.android.tools.r8.errors.Unreachable;
@@ -212,14 +216,22 @@
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();
+ List<Instruction> instructions = new ArrayList<>();
+ DexType returnType = ir.method.method.proto.returnType;
+ if (returnType.isVoidType()) {
+ instructions.add(new ReturnVoid());
+ } else if (returnType.isDoubleType() || returnType.isLongType()) {
+ instructions.add(new ConstWide16(0, 0));
+ instructions.add(new ReturnWide(0));
+ } else {
+ instructions.add(new Const4(0, 0));
+ instructions.add(new com.android.tools.r8.code.Return(0));
+ }
+ for (Instruction instruction : instructions) {
+ instruction.setOffset(offset);
+ offset += instruction.getSize();
+ dexInstructions.add(instruction);
+ }
}
// Compute switch payloads.
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 1299091..5e23686 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -743,12 +743,15 @@
}
// 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).
+ // parsing non-code bytes as code (typically leading to a crash). See b/117907456.
//
- // 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.
+ // In order to workaround this we insert a return of the right type after the throw
+ // when the instruction stream ends with a throw and there are backwards branches
+ // in the code.
//
- // See b/117907456.
+ // We used to insert a empty loop at the end, however, mediatek has an optimizer
+ // on lollipop devices that cannot deal with an unreachable infinite loop, so we
+ // couldn't do that. See b/119895393.
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
index 0d83011..98ac685 100644
--- a/src/test/java/com/android/tools/r8/regress/b117907456/B117907456.java
+++ b/src/test/java/com/android/tools/r8/regress/b117907456/B117907456.java
@@ -6,14 +6,17 @@
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
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.Return;
+import com.android.tools.r8.code.ReturnVoid;
+import com.android.tools.r8.code.ReturnWide;
import com.android.tools.r8.code.Throw;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.utils.AndroidApiLevel;
@@ -36,16 +39,21 @@
}
public class B117907456 extends TestBase {
+
+ private boolean isReturn(Instruction lastInstruction) {
+ return lastInstruction instanceof ReturnVoid
+ || lastInstruction instanceof Return
+ || lastInstruction instanceof ReturnWide;
+ }
+
@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;
+ assertFalse(lastInstruction instanceof Throw);
+ assertTrue(isReturn(lastInstruction));
}
@Test
@@ -54,7 +62,7 @@
MethodSubject method = getMethodSubject(AndroidApiLevel.L);
Instruction[] instructions = method.getMethod().getCode().asDexCode().instructions;
Instruction lastInstruction = instructions[instructions.length - 1];
- assert lastInstruction instanceof Throw;
+ assertTrue(lastInstruction instanceof Throw);
}
private MethodSubject getMethodSubject(AndroidApiLevel level)
