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 (file)
--- a/compiler/optimizing/builder.cc
+++ b/compiler/optimizing/builder.cc
@@ -378,15 +378,15 @@ bool HGraphBuilder::ComputeBranchTargets(const uint16_t* code_ptr,
       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 (file)
--- a/compiler/optimizing/nodes.cc
+++ b/compiler/optimizing/nodes.cc
@@ -1035,9 +1035,8 @@ HBasicBlock* HBasicBlock::SplitAfter(HInstruction* cursor) {
 
 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 (file)
index 0000000..e69de29

diff --git a/test/517-checker-builder-fallthrough/info.txt b/test/517-checker-builder-fallthrough/info.txt
new file mode 100644 (file)
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 (file)
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 (file)
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");
+    }
+  }
+}