Version 1.4.87
Cherry pick: Do a workaround for dominator tree calculation crash on mediatek 4.4.2/4.4.4
CL: https://r8-review.googlesource.com/c/r8/+/36818
Bug: 117907456, 119895393, 121355317, 128926846
Change-Id: I6112322b27d9c9a89b4bb53e17d755dbe0806aea
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 99999c6..4cf066b 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.4.86";
+ public static final String LABEL = "1.4.87";
private Version() {
}
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 d989d07..262b3ac 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,7 +32,6 @@
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.Throw;
import com.android.tools.r8.dex.Constants;
import com.android.tools.r8.errors.Unreachable;
@@ -238,19 +237,45 @@
// 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.
+ // See: b/117907456
// 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) {
- // Generating a throw with the right type makes some Art constant propagation
- // implementations crash. Therefore, since this is dead code anyway, we do not
- // generate a return of the right type. Instead we just generate an unreachable
- // return-void. See b/121355317.
- Instruction returnVoid = new ReturnVoid();
- returnVoid.setOffset(offset);
- offset += returnVoid.getSize();
- dexInstructions.add(returnVoid);
+ // This is the last in a series of different workarounds tried out.
+ // Having an empty non reachable loop make some mediatek vms crash: b/119895393
+ // Having a (unreachable) return null/return-void (type correct) trips up the constant
+ // propagation in some vms: b/121355317
+ // Having always a (unreachable) return-void causes Mediatek 4.4.2/4.4.4 to crash trying
+ // to get a dominator for the unreachable code: b/128926846
+ // Current implementation generates code that jumps over the throw, and then back to the throw
+ // again.
+ // throw v10
+ // becomes:
+ // goto +2
+ // throw v10
+ // goto -1
+ // That way we have no unreachable code, and we never end in a throw. The tracer will still
+ // trace to the throw, but after moving to the second goto it will trace back again and see
+ // an instruction it has already seen.
+ Instruction throwInstruction = dexInstructions.get(dexInstructions.size() - 1);
+ offset = throwInstruction.getOffset();
+
+ // Generate the new forward and backward gotos, update offsets.
+ Instruction forward = new Goto(throwInstruction.getSize() + Goto.SIZE);
+ Instruction backward = new Goto(-throwInstruction.getSize());
+ forward.setOffset(offset);
+ offset += forward.getSize();
+ throwInstruction.setOffset(offset);
+ offset += throwInstruction.getSize();
+ backward.setOffset(offset);
+ offset += backward.getSize();
+ // Replace the throw in the instruction stream with goto(forward), throw, goto(backwards)
+ dexInstructions.remove(dexInstructions.size() - 1);
+ dexInstructions.add(forward);
+ dexInstructions.add(throwInstruction);
+ dexInstructions.add(backward);
}
// 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 bf78270..112fa5f 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -864,13 +864,14 @@
// 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). See b/117907456.
//
- // 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.
- //
+ // In order to workaround this we insert a goto past the throw, and another goto after the throw
+ // jumping back to the throw.
+
// 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.2
+ // couldn't do that. See b/119895393.
+ // We also could not insert any dead code (e.g. a return) because that would make mediatek
+ // dominator calculations on 7.0.0 crash. See b/128926846.
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 2639fe8..a4bb99a 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
@@ -11,10 +11,8 @@
import com.android.tools.r8.CompilationFailedException;
import com.android.tools.r8.TestBase;
+import com.android.tools.r8.code.Goto;
import com.android.tools.r8.code.Instruction;
-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.utils.AndroidApiLevel;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
@@ -36,10 +34,8 @@
public class B117907456 extends TestBase {
- private boolean isReturn(Instruction lastInstruction) {
- return lastInstruction instanceof ReturnVoid
- || lastInstruction instanceof Return
- || lastInstruction instanceof ReturnWide;
+ private boolean isGoto(Instruction lastInstruction) {
+ return lastInstruction instanceof Goto;
}
@Test
@@ -49,7 +45,7 @@
Instruction[] instructions = method.getMethod().getCode().asDexCode().instructions;
Instruction lastInstruction = instructions[instructions.length - 1];
assertFalse(lastInstruction instanceof Throw);
- assertTrue(isReturn(lastInstruction));
+ assertTrue(isGoto(lastInstruction));
}
@Test
