ART: Stop creating a fallthrough block for Goto
Optimizing's Builder used to create a basic block after a Goto under
the assumption that control flow can fall through.
Bug: 19084197
Change-Id: Id85f31df98a4177466750d3cd0bc8bb74782ca2d
diff --git a/compiler/optimizing/builder.cc b/compiler/optimizing/builder.cc
index c497526..48ec497 100644
--- a/compiler/optimizing/builder.cc
+++ b/compiler/optimizing/builder.cc
@@ -378,15 +378,15 @@
dex_pc += instruction.SizeInCodeUnits();
code_ptr += instruction.SizeInCodeUnits();
- if (code_ptr >= code_end) {
- if (instruction.CanFlowThrough()) {
+ if (instruction.CanFlowThrough()) {
+ if (code_ptr >= code_end) {
// In the normal case we should never hit this but someone can artificially forge a dex
// file to fall-through out the method code. In this case we bail out compilation.
return false;
+ } else if (FindBlockStartingAt(dex_pc) == nullptr) {
+ block = new (arena_) HBasicBlock(graph_, dex_pc);
+ branch_targets_.Put(dex_pc, block);
}
- } else if (FindBlockStartingAt(dex_pc) == nullptr) {
- block = new (arena_) HBasicBlock(graph_, dex_pc);
- branch_targets_.Put(dex_pc, block);
}
} else if (instruction.IsSwitch()) {
SwitchTable table(instruction, dex_pc, instruction.Opcode() == Instruction::SPARSE_SWITCH);
diff --git a/compiler/optimizing/nodes.cc b/compiler/optimizing/nodes.cc
index 01eb2d7..feb9afe 100644
--- a/compiler/optimizing/nodes.cc
+++ b/compiler/optimizing/nodes.cc
@@ -1035,9 +1035,8 @@
bool HBasicBlock::IsSingleGoto() const {
HLoopInformation* loop_info = GetLoopInformation();
- // TODO: Remove the null check b/19084197.
- return GetFirstInstruction() != nullptr
- && GetPhis().IsEmpty()
+ DCHECK(EndsWithControlFlowInstruction());
+ return GetPhis().IsEmpty()
&& GetFirstInstruction() == GetLastInstruction()
&& GetLastInstruction()->IsGoto()
// Back edges generate the suspend check.
diff --git a/test/517-checker-builder-fallthrough/expected.txt b/test/517-checker-builder-fallthrough/expected.txt
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/test/517-checker-builder-fallthrough/expected.txt
diff --git a/test/517-checker-builder-fallthrough/info.txt b/test/517-checker-builder-fallthrough/info.txt
new file mode 100644
index 0000000..5789b56
--- /dev/null
+++ b/test/517-checker-builder-fallthrough/info.txt
@@ -0,0 +1,2 @@
+Regression test for optimizing's builder which created a block under
+the assumption that Goto can fall through.
diff --git a/test/517-checker-builder-fallthrough/smali/TestCase.smali b/test/517-checker-builder-fallthrough/smali/TestCase.smali
new file mode 100644
index 0000000..bc9502b
--- /dev/null
+++ b/test/517-checker-builder-fallthrough/smali/TestCase.smali
@@ -0,0 +1,67 @@
+# Copyright (C) 2015 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+.class public LTestCase;
+
+.super Ljava/lang/Object;
+
+.field public static value:Z
+
+## CHECK-START: int TestCase.testCase(int) builder (after)
+
+## CHECK: name "B0"
+## CHECK: <<Const0:i\d+>> IntConstant 0
+
+## CHECK: name "B1"
+## CHECK: successors "B5" "B2"
+## CHECK: StoreLocal [v0,<<Const0>>]
+## CHECK: If
+
+## CHECK: name "B2"
+## CHECK: successors "B4"
+## CHECK: Goto
+
+## CHECK: name "B3"
+## CHECK: Return
+
+## CHECK: name "B4"
+## CHECK: successors "B3"
+## CHECK: Goto
+
+## CHECK: name "B5"
+## CHECK: successors "B3"
+## CHECK: Goto
+
+.method public static testCase(I)I
+ .registers 2
+
+ const/4 v0, 0
+ packed-switch v0, :switch_data
+ goto :default
+
+ :switch_data
+ .packed-switch 0x0
+ :case
+ .end packed-switch
+
+ :return
+ return v1
+
+ :default
+ goto :return
+
+ :case
+ goto :return
+
+.end method
diff --git a/test/517-checker-builder-fallthrough/src/Main.java b/test/517-checker-builder-fallthrough/src/Main.java
new file mode 100644
index 0000000..23d94e6
--- /dev/null
+++ b/test/517-checker-builder-fallthrough/src/Main.java
@@ -0,0 +1,32 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import java.lang.reflect.*;
+
+public class Main {
+
+ public static int runTest(int input) throws Exception {
+ Class<?> c = Class.forName("TestCase");
+ Method m = c.getMethod("testCase", new Class[] { int.class });
+ return (Integer) m.invoke(null, input);
+ }
+
+ public static void main(String[] args) throws Exception {
+ if (runTest(42) != 42) {
+ throw new Error("Expected 42");
+ }
+ }
+}