Merge "Quick: Fix crash on fall-through out of method code." into mnc-dev

6 files changed, 118 insertions, 5 deletions

diff --git a/compiler/dex/mir_graph.cc b/compiler/dex/mir_graph.cc
index b5c42f1..9e3fbbc 100644
--- a/compiler/dex/mir_graph.cc
+++ b/compiler/dex/mir_graph.cc
@@ -291,8 +291,12 @@ BasicBlock* MIRGraph::SplitBlock(DexOffset code_offset,
 BasicBlock* MIRGraph::FindBlock(DexOffset code_offset, bool create,
                                 BasicBlock** immed_pred_block_p,
                                 ScopedArenaVector<uint16_t>* dex_pc_to_block_map) {
-  if (code_offset >= current_code_item_->insns_size_in_code_units_) {
-    return nullptr;
+  if (UNLIKELY(code_offset >= current_code_item_->insns_size_in_code_units_)) {
+    // There can be a fall-through out of the method code. We shall record such a block
+    // here (assuming create==true) and check that it's dead at the end of InlineMethod().
+    // Though we're only aware of the cases where code_offset is exactly the same as
+    // insns_size_in_code_units_, treat greater code_offset the same just in case.
+    code_offset = current_code_item_->insns_size_in_code_units_;
   }
 
   int block_id = (*dex_pc_to_block_map)[code_offset];
@@ -483,6 +487,7 @@ BasicBlock* MIRGraph::ProcessCanBranch(BasicBlock* cur_block, MIR* insn, DexOffs
   BasicBlock* taken_block = FindBlock(target, /* create */ true,
                                       /* immed_pred_block_p */ &cur_block,
                                       dex_pc_to_block_map);
+  DCHECK(taken_block != nullptr);
   cur_block->taken = taken_block->id;
   taken_block->predecessors.push_back(cur_block->id);
 
@@ -494,6 +499,7 @@ BasicBlock* MIRGraph::ProcessCanBranch(BasicBlock* cur_block, MIR* insn, DexOffs
                                              /* immed_pred_block_p */
                                              &cur_block,
                                              dex_pc_to_block_map);
+    DCHECK(fallthrough_block != nullptr);
     cur_block->fall_through = fallthrough_block->id;
     fallthrough_block->predecessors.push_back(cur_block->id);
   } else if (code_ptr < code_end) {
@@ -508,7 +514,8 @@ BasicBlock* MIRGraph::ProcessCanSwitch(BasicBlock* cur_block, MIR* insn, DexOffs
                                        ScopedArenaVector<uint16_t>* dex_pc_to_block_map) {
   UNUSED(flags);
   const uint16_t* switch_data =
-      reinterpret_cast<const uint16_t*>(GetCurrentInsns() + cur_offset + insn->dalvikInsn.vB);
+      reinterpret_cast<const uint16_t*>(GetCurrentInsns() + cur_offset +
+          static_cast<int32_t>(insn->dalvikInsn.vB));
   int size;
   const int* keyTable;
   const int* target_table;
@@ -561,6 +568,7 @@ BasicBlock* MIRGraph::ProcessCanSwitch(BasicBlock* cur_block, MIR* insn, DexOffs
     BasicBlock* case_block = FindBlock(cur_offset + target_table[i],  /* create */ true,
                                        /* immed_pred_block_p */ &cur_block,
                                        dex_pc_to_block_map);
+    DCHECK(case_block != nullptr);
     SuccessorBlockInfo* successor_block_info =
         static_cast<SuccessorBlockInfo*>(arena_->Alloc(sizeof(SuccessorBlockInfo),
                                                        kArenaAllocSuccessor));
@@ -576,6 +584,7 @@ BasicBlock* MIRGraph::ProcessCanSwitch(BasicBlock* cur_block, MIR* insn, DexOffs
   BasicBlock* fallthrough_block = FindBlock(cur_offset +  width, /* create */ true,
                                             /* immed_pred_block_p */ nullptr,
                                             dex_pc_to_block_map);
+  DCHECK(fallthrough_block != nullptr);
   cur_block->fall_through = fallthrough_block->id;
   fallthrough_block->predecessors.push_back(cur_block->id);
   return cur_block;
@@ -709,8 +718,8 @@ void MIRGraph::InlineMethod(const DexFile::CodeItem* code_item, uint32_t access_
   // FindBlock lookup cache.
   ScopedArenaAllocator allocator(&cu_->arena_stack);
   ScopedArenaVector<uint16_t> dex_pc_to_block_map(allocator.Adapter());
-  dex_pc_to_block_map.resize(dex_pc_to_block_map.size() +
-                             current_code_item_->insns_size_in_code_units_);
+  dex_pc_to_block_map.resize(current_code_item_->insns_size_in_code_units_ +
+                             1 /* Fall-through on last insn; dead or punt to interpreter. */);
 
   // TODO: replace with explicit resize routine.  Using automatic extension side effect for now.
   try_block_addr_->SetBit(current_code_item_->insns_size_in_code_units_);
@@ -876,6 +885,20 @@ void MIRGraph::InlineMethod(const DexFile::CodeItem* code_item, uint32_t access_
   if (cu_->verbose) {
     DumpMIRGraph();
   }
+
+  // Check if there's been a fall-through out of the method code.
+  BasicBlockId out_bb_id = dex_pc_to_block_map[current_code_item_->insns_size_in_code_units_];
+  if (UNLIKELY(out_bb_id != NullBasicBlockId)) {
+    // Eagerly calculate DFS order to determine if the block is dead.
+    DCHECK(!DfsOrdersUpToDate());
+    ComputeDFSOrders();
+    BasicBlock* out_bb = GetBasicBlock(out_bb_id);
+    DCHECK(out_bb != nullptr);
+    if (out_bb->block_type != kDead) {
+      LOG(WARNING) << "Live fall-through out of method in " << PrettyMethod(method_idx, dex_file);
+      SetPuntToInterpreter(true);
+    }
+  }
 }
 
 void MIRGraph::ShowOpcodeStats() {
diff --git a/test/472-unreachable-if-regression/expected.txt b/test/472-unreachable-if-regression/expected.txt
new file mode 100644
index 0000000..9fc8bea
--- /dev/null
+++ b/test/472-unreachable-if-regression/expected.txt
@@ -0,0 +1,3 @@
+Test started.
+Successfully called UnreachableIf().
+Successfully called UnreachablePackedSwitch().
diff --git a/test/472-unreachable-if-regression/info.txt b/test/472-unreachable-if-regression/info.txt
new file mode 100644
index 0000000..d8b5a45
--- /dev/null
+++ b/test/472-unreachable-if-regression/info.txt
@@ -0,0 +1,3 @@
+Regression test for crashes during compilation of methods which end
+with an if-cc or switch, i.e. there's a fall-through out of method code.
+Also tests a packed-switch with negative offset to its data.
diff --git a/test/472-unreachable-if-regression/smali/Test.smali b/test/472-unreachable-if-regression/smali/Test.smali
new file mode 100644
index 0000000..c7107d1
--- /dev/null
+++ b/test/472-unreachable-if-regression/smali/Test.smali
@@ -0,0 +1,46 @@
+#
+# 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 LTest;
+
+.super Ljava/lang/Object;
+
+.method public static UnreachableIf()V
+    .registers 1
+    return-void
+    : unreachable
+    not-int v0, v0
+    if-lt v0, v0, :unreachable
+    # fall-through out of code item
+.end method
+
+.method public static UnreachablePackedSwitch()V
+    .registers 1
+    return-void
+    : unreachable
+    goto :pswitch_2
+    :pswitch_data
+    .packed-switch 1
+        :pswitch_1
+        :pswitch_2
+        :pswitch_1
+        :pswitch_2
+    .end packed-switch
+    :pswitch_1
+    not-int v0, v0
+    :pswitch_2
+    packed-switch v0, :pswitch_data
+    # fall-through out of code item
+.end method
diff --git a/test/472-unreachable-if-regression/src/Main.java b/test/472-unreachable-if-regression/src/Main.java
new file mode 100644
index 0000000..c9f9511
--- /dev/null
+++ b/test/472-unreachable-if-regression/src/Main.java
@@ -0,0 +1,37 @@
+/*
+ * 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.Method;
+
+public class Main {
+
+  // Workaround for b/18051191.
+  class InnerClass {}
+
+  public static void main(String args[]) throws Exception {
+    System.out.println("Test started.");
+    Class<?> c = Class.forName("Test");
+
+    Method unreachableIf = c.getMethod("UnreachableIf", (Class[]) null);
+    unreachableIf.invoke(null, (Object[]) null);
+    System.out.println("Successfully called UnreachableIf().");
+
+    Method unreachablePackedSwitch = c.getMethod("UnreachablePackedSwitch", (Class[]) null);
+    unreachablePackedSwitch.invoke(null, (Object[]) null);
+    System.out.println("Successfully called UnreachablePackedSwitch().");
+  }
+
+}
diff --git a/test/Android.run-test.mk b/test/Android.run-test.mk
index 93340fb..3804fb7 100644
--- a/test/Android.run-test.mk
+++ b/test/Android.run-test.mk
@@ -386,6 +386,7 @@ TEST_ART_BROKEN_OPTIMIZING_ARM64_RUN_TESTS :=
 # Known broken tests for the optimizing compiler.
 TEST_ART_BROKEN_OPTIMIZING_RUN_TESTS :=
 TEST_ART_BROKEN_OPTIMIZING_RUN_TESTS += 099-vmdebug # b/18098594
+TEST_ART_BROKEN_OPTIMIZING_RUN_TESTS += 472-unreachable-if-regression # b/19988134
 TEST_ART_BROKEN_OPTIMIZING_RUN_TESTS += 802-deoptimization # b/18547544
 
 ifneq (,$(filter optimizing,$(COMPILER_TYPES)))