summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Nicolas Geoffray <ngeoffray@google.com> 2020-05-14 14:51:11 +0100
committer Nicolas Geoffray <ngeoffray@google.com> 2020-05-14 19:15:01 +0100
commit33c091eaaa0febedc93cff820def75b122fde867 (patch)
treecbbe7369f8206af3180a9530bcd1729042cdd544
parent5d2311a349f208f056b33da8fc9c950aad1a7ffe (diff)
Code sinking can move around LoadString that can throw.
The test accidentally used a string part of the boot image, which means we know the instruction won't throw. However, a change in the boot classpath meant the string "a" was not part of the boot image anymore, and the test started failing. The CL now handles the case the LoadString might throw, and treat it like NewInstance/NewArray. Test: 672-checker-throw-method, 673-checker-throw-vmethod Bug: 156559242 Change-Id: If9df2ed2c7c39c56254970172e315ec5113db64e
-rw-r--r--compiler/optimizing/code_sinking.cc4
-rw-r--r--test/672-checker-throw-method/src/Main.java30
-rw-r--r--test/673-checker-throw-vmethod/src/Main.java16
3 files changed, 34 insertions, 16 deletions
diff --git a/compiler/optimizing/code_sinking.cc b/compiler/optimizing/code_sinking.cc
index f406983fc2..f946e5031a 100644
--- a/compiler/optimizing/code_sinking.cc
+++ b/compiler/optimizing/code_sinking.cc
@@ -58,8 +58,8 @@ static bool IsInterestingInstruction(HInstruction* instruction) {
}
}
- // Check allocations first, as they can throw, but it is safe to move them.
- if (instruction->IsNewInstance() || instruction->IsNewArray()) {
+ // Check allocations and strings first, as they can throw, but it is safe to move them.
+ if (instruction->IsNewInstance() || instruction->IsNewArray() || instruction->IsLoadString()) {
return true;
}
diff --git a/test/672-checker-throw-method/src/Main.java b/test/672-checker-throw-method/src/Main.java
index 360b52c79d..c2344b23e8 100644
--- a/test/672-checker-throw-method/src/Main.java
+++ b/test/672-checker-throw-method/src/Main.java
@@ -70,7 +70,9 @@ public class Main {
/// CHECK: Throw
/// CHECK: end_block
static public void doit1(int[] a) {
- String par = "a";
+ // Being in the boot image means we know the load string cannot throw. Create one that is
+ // unlikely to be there to ensure we handle that case.
+ String par = "stringUnlikelyToBeInBootImage";
if (a == null)
throw new Error("you are null: " + par);
for (int i = 0; i < a.length; i++) {
@@ -98,7 +100,9 @@ public class Main {
/// CHECK: InvokeStaticOrDirect [<<Str>>] method_name:Main.doThrow
/// CHECK: end_block
static public void doit2(int[] a) {
- String par = "a";
+ // Being in the boot image means we know the load string cannot throw. Create one that is
+ // unlikely to be there to ensure we handle that case.
+ String par = "stringUnlikelyToBeInBootImage";
if (a == null)
doThrow(par);
for (int i = 0; i < a.length; i++) {
@@ -128,7 +132,9 @@ public class Main {
/// CHECK: Throw
/// CHECK: end_block
static public void doit3(int[] a) {
- String par = "a";
+ // Being in the boot image means we know the load string cannot throw. Create one that is
+ // unlikely to be there to ensure we handle that case.
+ String par = "stringUnlikelyToBeInBootImage";
checkNotNullDirect(a, par);
for (int i = 0; i < a.length; i++) {
a[i] = 3;
@@ -155,7 +161,9 @@ public class Main {
/// CHECK: InvokeStaticOrDirect [<<Str>>] method_name:Main.doThrow
/// CHECK: end_block
static public void doit4(int[] a) {
- String par = "a";
+ // Being in the boot image means we know the load string cannot throw. Create one that is
+ // unlikely to be there to ensure we handle that case.
+ String par = "stringUnlikelyToBeInBootImage";
checkNotNullSplit(a, par); // resembles Kotlin runtime lib
// (test is lined, doThrow is not)
for (int i = 0; i < a.length; i++) {
@@ -166,7 +174,9 @@ public class Main {
// Ensures Phi values are merged properly.
static public int doit5(int[] a) {
int t = 100;
- String par = "a";
+ // Being in the boot image means we know the load string cannot throw. Create one that is
+ // unlikely to be there to ensure we handle that case.
+ String par = "stringUnlikelyToBeInBootImage";
if (a == null) {
doThrow(par);
} else {
@@ -205,7 +215,7 @@ public class Main {
/// CHECK-START: int Main.deleteNullCheck(int[]) dead_code_elimination$after_inlining (after)
/// CHECK-NOT: NullCheck
static public int deleteNullCheck(int[] a) {
- checkNotNullSplit(a, "a");
+ checkNotNullSplit(a, "stringUnlikelyToBeInBootImage");
return a[0];
}
@@ -215,7 +225,7 @@ public class Main {
/// CHECK-START: int Main.deleteNullCheckAlt(int[]) dead_code_elimination$after_inlining (after)
/// CHECK-NOT: NullCheck
static public int deleteNullCheckAlt(int[] a) {
- checkNotNullSplitAlt(a, "a");
+ checkNotNullSplitAlt(a, "stringUnlikeltyToBeInBootImage");
return a[0];
}
@@ -227,9 +237,9 @@ public class Main {
/// CHECK-START: int Main.deleteNullChecks3(int[], int[], int[]) dead_code_elimination$after_inlining (after)
/// CHECK-NOT: NullCheck
static public int deleteNullChecks3(int[] a, int[] b, int[] c) {
- checkNotNullSplit(a, "a");
- checkNotNullSplit(b, "b");
- checkNotNullSplit(c, "c");
+ checkNotNullSplit(a, "stringUnlikelytoBeInBootImage1");
+ checkNotNullSplit(b, "stringUnlikelytoBeInBootImage2");
+ checkNotNullSplit(c, "stringUnlikelytoBeInBootImage3");
return a[0] + b[0] + c[0];
}
diff --git a/test/673-checker-throw-vmethod/src/Main.java b/test/673-checker-throw-vmethod/src/Main.java
index 206dfaf330..f62cfc85a7 100644
--- a/test/673-checker-throw-vmethod/src/Main.java
+++ b/test/673-checker-throw-vmethod/src/Main.java
@@ -64,7 +64,9 @@ public class Main {
/// CHECK: Throw
/// CHECK: end_block
public void doit1(int[] a) {
- String par = "a";
+ // Being in the boot image means we know the load string cannot throw. Create one that is
+ // unlikely to be there to ensure we handle that case.
+ String par = "stringUnlikelyToBeInBootImage";
if (a == null)
throw new Error("you are null: " + par);
for (int i = 0; i < a.length; i++) {
@@ -92,7 +94,9 @@ public class Main {
/// CHECK: InvokeVirtual [{{l\d+}},<<Str>>] method_name:Main.doThrow
/// CHECK: end_block
public void doit2(int[] a) {
- String par = "a";
+ // Being in the boot image means we know the load string cannot throw. Create one that is
+ // unlikely to be there to ensure we handle that case.
+ String par = "stringUnlikelyToBeInBootImage";
if (a == null)
doThrow(par);
for (int i = 0; i < a.length; i++) {
@@ -122,7 +126,9 @@ public class Main {
/// CHECK: Throw
/// CHECK: end_block
public void doit3(int[] a) {
- String par = "a";
+ // Being in the boot image means we know the load string cannot throw. Create one that is
+ // unlikely to be there to ensure we handle that case.
+ String par = "stringUnlikelyToBeInBootImage";
checkNotNullDirect(a, par);
for (int i = 0; i < a.length; i++) {
a[i] = 3;
@@ -149,7 +155,9 @@ public class Main {
/// CHECK: InvokeVirtual [{{l\d+}},<<Str>>] method_name:Main.doThrow
/// CHECK: end_block
public void doit4(int[] a) {
- String par = "a";
+ // Being in the boot image means we know the load string cannot throw. Create one that is
+ // unlikely to be there to ensure we handle that case.
+ String par = "stringUnlikelyToBeInBootImage";
checkNotNullSplit(a, par);
for (int i = 0; i < a.length; i++) {
a[i] = 4;