diff options
author | 2020-05-14 14:51:11 +0100 | |
---|---|---|
committer | 2020-05-14 19:15:01 +0100 | |
commit | 33c091eaaa0febedc93cff820def75b122fde867 (patch) | |
tree | cbbe7369f8206af3180a9530bcd1729042cdd544 | |
parent | 5d2311a349f208f056b33da8fc9c950aad1a7ffe (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.cc | 4 | ||||
-rw-r--r-- | test/672-checker-throw-method/src/Main.java | 30 | ||||
-rw-r--r-- | test/673-checker-throw-vmethod/src/Main.java | 16 |
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; |