Bailout from compilation if an invoke is malformed.

Because the verifier does not check trivially dead instructions,
the compilers must prepare for bogus instructions. This change
fixes the case the arguments for an invoke do not match the
formal parameters.

bug:21865459

(cherry picked from commit 2e33525bd4eb892246b4c244c6d4ebf6c6d07501)

Change-Id: I392f86eafefde28263fe35a31f17b398ff8dfc24

6 files changed, 141 insertions, 10 deletions

diff --git a/compiler/optimizing/builder.cc b/compiler/optimizing/builder.cc
index c2a9b5ccf..a53c488e3 100644
--- a/compiler/optimizing/builder.cc
+++ b/compiler/optimizing/builder.cc
@@ -605,7 +605,12 @@ bool HGraphBuilder::BuildInvoke(const Instruction& instruction,
   const char* descriptor = dex_file_->StringDataByIdx(proto_id.shorty_idx_);
   Primitive::Type return_type = Primitive::GetType(descriptor[0]);
   bool is_instance_call = invoke_type != kStatic;
-  size_t number_of_arguments = strlen(descriptor) - (is_instance_call ? 0 : 1);
+  // Remove the return type from the 'proto'.
+  size_t number_of_arguments = strlen(descriptor) - 1;
+  if (is_instance_call) {
+    // One extra argument for 'this'.
+    ++number_of_arguments;
+  }
 
   MethodReference target_method(dex_file_, method_idx);
   uintptr_t direct_code;
@@ -616,7 +621,8 @@ bool HGraphBuilder::BuildInvoke(const Instruction& instruction,
   if (!compiler_driver_->ComputeInvokeInfo(dex_compilation_unit_, dex_pc, true, true,
                                            &optimized_invoke_type, &target_method, &table_index,
                                            &direct_code, &direct_method)) {
-    VLOG(compiler) << "Did not compile " << PrettyMethod(method_idx, *dex_file_)
+    VLOG(compiler) << "Did not compile "
+                   << PrettyMethod(dex_compilation_unit_->GetDexMethodIndex(), *dex_file_)
                    << " because a method call could not be resolved";
     MaybeRecordStat(MethodCompilationStat::kNotCompiledUnresolvedMethod);
     return false;
@@ -738,19 +744,29 @@ bool HGraphBuilder::BuildInvoke(const Instruction& instruction,
     start_index = 1;
   }
 
-  uint32_t descriptor_index = 1;
+  uint32_t descriptor_index = 1;  // Skip the return type.
   uint32_t argument_index = start_index;
   if (is_string_init) {
     start_index = 1;
   }
-  for (size_t i = start_index; i < number_of_vreg_arguments; i++, argument_index++) {
+  for (size_t i = start_index;
+       // Make sure we don't go over the expected arguments or over the number of
+       // dex registers given. If the instruction was seen as dead by the verifier,
+       // it hasn't been properly checked.
+       (i < number_of_vreg_arguments) && (argument_index < number_of_arguments);
+       i++, argument_index++) {
     Primitive::Type type = Primitive::GetType(descriptor[descriptor_index++]);
     bool is_wide = (type == Primitive::kPrimLong) || (type == Primitive::kPrimDouble);
-    if (!is_range && is_wide && args[i] + 1 != args[i + 1]) {
-      LOG(WARNING) << "Non sequential register pair in " << dex_compilation_unit_->GetSymbol()
-                   << " at " << dex_pc;
-      // We do not implement non sequential register pair.
-      MaybeRecordStat(MethodCompilationStat::kNotCompiledNonSequentialRegPair);
+    if (!is_range
+        && is_wide
+        && ((i + 1 == number_of_vreg_arguments) || (args[i] + 1 != args[i + 1]))) {
+      // Longs and doubles should be in pairs, that is, sequential registers. The verifier should
+      // reject any class where this is violated. However, the verifier only does these checks
+      // on non trivially dead instructions, so we just bailout the compilation.
+      VLOG(compiler) << "Did not compile "
+                     << PrettyMethod(dex_compilation_unit_->GetDexMethodIndex(), *dex_file_)
+                     << " because of non-sequential dex register pair in wide argument";
+      MaybeRecordStat(MethodCompilationStat::kNotCompiledMalformedOpcode);
       return false;
     }
     HInstruction* arg = LoadLocal(is_range ? register_index + i : args[i], type);
@@ -759,7 +775,14 @@ bool HGraphBuilder::BuildInvoke(const Instruction& instruction,
       i++;
     }
   }
-  DCHECK_EQ(argument_index, number_of_arguments);
+
+  if (argument_index != number_of_arguments) {
+    VLOG(compiler) << "Did not compile "
+                   << PrettyMethod(dex_compilation_unit_->GetDexMethodIndex(), *dex_file_)
+                   << " because of wrong number of arguments in invoke instruction";
+    MaybeRecordStat(MethodCompilationStat::kNotCompiledMalformedOpcode);
+    return false;
+  }
 
   if (clinit_check_requirement == HInvokeStaticOrDirect::ClinitCheckRequirement::kExplicit) {
     // Add the class initialization check as last input of `invoke`.
diff --git a/compiler/optimizing/optimizing_compiler_stats.h b/compiler/optimizing/optimizing_compiler_stats.h
index b6b1bb1ca..6d340e226 100644
--- a/compiler/optimizing/optimizing_compiler_stats.h
+++ b/compiler/optimizing/optimizing_compiler_stats.h
@@ -37,6 +37,7 @@ enum MethodCompilationStat {
   kNotCompiledClassNotVerified,
   kNotCompiledHugeMethod,
   kNotCompiledLargeMethodNoBranches,
+  kNotCompiledMalformedOpcode,
   kNotCompiledNoCodegen,
   kNotCompiledNonSequentialRegPair,
   kNotCompiledPathological,
@@ -105,6 +106,7 @@ class OptimizingCompilerStats {
       case kNotCompiledClassNotVerified : return "kNotCompiledClassNotVerified";
       case kNotCompiledHugeMethod : return "kNotCompiledHugeMethod";
       case kNotCompiledLargeMethodNoBranches : return "kNotCompiledLargeMethodNoBranches";
+      case kNotCompiledMalformedOpcode : return "kNotCompiledMalformedOpcode";
       case kNotCompiledNoCodegen : return "kNotCompiledNoCodegen";
       case kNotCompiledNonSequentialRegPair : return "kNotCompiledNonSequentialRegPair";
       case kNotCompiledPathological : return "kNotCompiledPathological";
diff --git a/test/503-dead-instructions/expected.txt b/test/503-dead-instructions/expected.txt
new file mode 100644
index 000000000..ccaf6f8f0
--- /dev/null
+++ b/test/503-dead-instructions/expected.txt
@@ -0,0 +1 @@
+Enter
diff --git a/test/503-dead-instructions/info.txt b/test/503-dead-instructions/info.txt
new file mode 100644
index 000000000..7e3f1aba3
--- /dev/null
+++ b/test/503-dead-instructions/info.txt
@@ -0,0 +1,2 @@
+Regression test for the building phase of the optimizing
+compiler. See comment in smali file.
diff --git a/test/503-dead-instructions/smali/DeadInstructions.smali b/test/503-dead-instructions/smali/DeadInstructions.smali
new file mode 100644
index 000000000..9f6c5653f
--- /dev/null
+++ b/test/503-dead-instructions/smali/DeadInstructions.smali
@@ -0,0 +1,63 @@
+# 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 LDeadInstructions;
+
+.super Ljava/lang/Object;
+
+.method public static method1()V
+   .registers 2
+   return-void
+   # Create a label and a branch to that label to trick the
+   # optimizing compiler into thinking the invoke is live.
+   :start
+   const/4 v0, 0
+   const/4 v1, 0
+   # Provide more arguments than we should. Because this is dead
+   # code, the verifier will not check the argument count. So
+   # the compilers must do the same.
+   invoke-static {v0, v1}, LDeadInstructions;->method1()V
+   goto :start
+.end method
+
+.method public static method2(J)V
+   .registers 3
+   return-void
+   :start
+   const/4 v0, 0
+   const/4 v1, 0
+   const/4 v2, 0
+   # Give a non-sequential pair for the long argument.
+   invoke-static {v0, v2}, LDeadInstructions;->method2(J)V
+   goto :start
+.end method
+
+.method public static method3()V
+   .registers 1
+   return-void
+   :start
+   const/4 v0, 0
+   # Give one half of a pair.
+   invoke-static {v0}, LDeadInstructions;->method2(J)V
+   goto :start
+.end method
+
+.method public static method4()V
+   .registers 2
+   return-void
+   :start
+   # Provide less arguments than we should.
+   invoke-static {}, LDeadInstructions;->method3(J)V
+   goto :start
+.end method
diff --git a/test/503-dead-instructions/src/Main.java b/test/503-dead-instructions/src/Main.java
new file mode 100644
index 000000000..6249dc79f
--- /dev/null
+++ b/test/503-dead-instructions/src/Main.java
@@ -0,0 +1,40 @@
+/*
+ * 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 {
+  public static void main(String[] args) throws Exception {
+    // Workaround for b/18051191.
+    System.out.println("Enter");
+    Class<?> c = Class.forName("DeadInstructions");
+    Method m = c.getMethod("method1");
+    Object[] arguments1 = { };
+    m.invoke(null, arguments1);
+
+    Object[] arguments2 = { (long)4 };
+    m = c.getMethod("method2", long.class);
+    m.invoke(null, arguments2);
+
+    Object[] arguments3 = { };
+    m = c.getMethod("method3");
+    m.invoke(null, arguments3);
+
+    Object[] arguments4 = { };
+    m = c.getMethod("method4");
+    m.invoke(null, arguments4);
+  }
+}