Fix incompatible class change error for JIT stress mode

There was a problem with miranda methods, when we would dequicken to
one of these, it wouldn't resolve as virtual during the method
lowering resolve. The solution is to try resolving as interface if we
fail to resolve as virtual.

Fixed a bug in dequickening where unreachable register lines with
quick invokes would cause CHECK failuers. In this case we punt to the
interpreter (test 435-try-*).

Added test regression test. Example failure:
java.lang.IncompatibleClassChangeError: The method
'void Main$TheInterface.m()' was expected to be of type virtual but
instead was found to be of type interface (declaration of
'java.lang.reflect.ArtMethod' appears in
out/host/linux-x86/framework/core-libart-hostdex.jar)
  at Main.DoStuff(Main.java:37)
  at Main.main(Main.java:44)

Bug: 17950037

Change-Id: I39c32cc8849bf02032a4f61a7ce57462b7fcac75

11 files changed, 123 insertions, 43 deletions

diff --git a/compiler/dex/dex_to_dex_compiler.cc b/compiler/dex/dex_to_dex_compiler.cc
index 7e916bee4a..fcefb6fbfc 100644
--- a/compiler/dex/dex_to_dex_compiler.cc
+++ b/compiler/dex/dex_to_dex_compiler.cc
@@ -252,10 +252,8 @@ void DexCompiler::CompileInstanceFieldAccess(Instruction* inst,
   }
 }
 
-void DexCompiler::CompileInvokeVirtual(Instruction* inst,
-                                uint32_t dex_pc,
-                                Instruction::Code new_opcode,
-                                bool is_range) {
+void DexCompiler::CompileInvokeVirtual(Instruction* inst, uint32_t dex_pc,
+                                       Instruction::Code new_opcode, bool is_range) {
   if (!kEnableQuickening || !PerformOptimizations()) {
     return;
   }
diff --git a/compiler/dex/mir_method_info.cc b/compiler/dex/mir_method_info.cc
index 3d3d9790c3..34fb1bf0e0 100644
--- a/compiler/dex/mir_method_info.cc
+++ b/compiler/dex/mir_method_info.cc
@@ -58,8 +58,9 @@ void MirMethodLoweringInfo::Resolve(CompilerDriver* compiler_driver,
   auto current_dex_cache(hs.NewHandle<mirror::DexCache>(nullptr));
   // Even if the referrer class is unresolved (i.e. we're compiling a method without class
   // definition) we still want to resolve methods and record all available info.
+  Runtime* const runtime = Runtime::Current();
   const DexFile* const dex_file = mUnit->GetDexFile();
-  const bool use_jit = Runtime::Current()->UseJit();
+  const bool use_jit = runtime->UseJit();
   const VerifiedMethod* const verified_method = mUnit->GetVerifiedMethod();
 
   for (auto it = method_infos, end = method_infos + count; it != end; ++it) {
@@ -80,7 +81,7 @@ void MirMethodLoweringInfo::Resolve(CompilerDriver* compiler_driver,
       it->target_method_idx_ = it->MethodIndex();
       current_dex_cache.Assign(dex_cache.Get());
       resolved_method = compiler_driver->ResolveMethod(soa, dex_cache, class_loader, mUnit,
-                                                       it->MethodIndex(), invoke_type);
+                                                       it->target_method_idx_, invoke_type, true);
     } else {
       // The method index is actually the dex PC in this case.
       // Calculate the proper dex file and target method idx.
@@ -89,8 +90,7 @@ void MirMethodLoweringInfo::Resolve(CompilerDriver* compiler_driver,
       // Don't devirt if we are in a different dex file since we can't have direct invokes in
       // another dex file unless we always put a direct / patch pointer.
       devirt_target = nullptr;
-      current_dex_cache.Assign(
-          Runtime::Current()->GetClassLinker()->FindDexCache(*it->target_dex_file_));
+      current_dex_cache.Assign(runtime->GetClassLinker()->FindDexCache(*it->target_dex_file_));
       CHECK(current_dex_cache.Get() != nullptr);
       DexCompilationUnit cu(
           mUnit->GetCompilationUnit(), mUnit->GetClassLoader(), mUnit->GetClassLinker(),
@@ -99,6 +99,14 @@ void MirMethodLoweringInfo::Resolve(CompilerDriver* compiler_driver,
           nullptr /* verified_method not used */);
       resolved_method = compiler_driver->ResolveMethod(soa, current_dex_cache, class_loader, &cu,
                                                        it->target_method_idx_, invoke_type, false);
+      if (resolved_method == nullptr) {
+        // If the method is null then it should be a miranda method, in this case try
+        // re-loading it, this time as an interface method. The actual miranda method is in the
+        // vtable, but it will resolve to an interface method.
+        resolved_method = compiler_driver->ResolveMethod(
+            soa, current_dex_cache, class_loader, &cu, it->target_method_idx_, kInterface, false);
+        CHECK(resolved_method != nullptr);
+      }
       if (resolved_method != nullptr) {
         // Since this was a dequickened virtual, it is guaranteed to be resolved. However, it may be
         // resolved to an interface method. If this is the case then change the invoke type to
@@ -123,10 +131,9 @@ void MirMethodLoweringInfo::Resolve(CompilerDriver* compiler_driver,
       it->vtable_idx_ =
           compiler_driver->GetResolvedMethodVTableIndex(resolved_method, invoke_type);
     }
-
     MethodReference target_method(it->target_dex_file_, it->target_method_idx_);
     int fast_path_flags = compiler_driver->IsFastInvoke(
-        soa, dex_cache, class_loader, mUnit, referrer_class.Get(), resolved_method,
+        soa, current_dex_cache, class_loader, mUnit, referrer_class.Get(), resolved_method,
         &invoke_type, &target_method, devirt_target, &it->direct_code_, &it->direct_method_);
     const bool is_referrers_class = referrer_class.Get() == resolved_method->GetDeclaringClass();
     const bool is_class_initialized =
diff --git a/compiler/dex/verified_method.cc b/compiler/dex/verified_method.cc
index 42d66be60d..5b90ba92e0 100644
--- a/compiler/dex/verified_method.cc
+++ b/compiler/dex/verified_method.cc
@@ -55,8 +55,8 @@ const VerifiedMethod* VerifiedMethod::Create(verifier::MethodVerifier* method_ve
     }
 
     // Only need dequicken info for JIT so far.
-    if (Runtime::Current()->UseJit()) {
-      verified_method->GenerateDequickenMap(method_verifier);
+    if (Runtime::Current()->UseJit() && !verified_method->GenerateDequickenMap(method_verifier)) {
+      return nullptr;
     }
   }
 
@@ -194,9 +194,9 @@ void VerifiedMethod::ComputeGcMapSizes(verifier::MethodVerifier* method_verifier
   *log2_max_gc_pc = i;
 }
 
-void VerifiedMethod::GenerateDequickenMap(verifier::MethodVerifier* method_verifier) {
+bool VerifiedMethod::GenerateDequickenMap(verifier::MethodVerifier* method_verifier) {
   if (method_verifier->HasFailures()) {
-    return;
+    return false;
   }
   const DexFile::CodeItem* code_item = method_verifier->CodeItem();
   const uint16_t* insns = code_item->insns_;
@@ -209,8 +209,11 @@ void VerifiedMethod::GenerateDequickenMap(verifier::MethodVerifier* method_verif
       uint32_t dex_pc = inst->GetDexPc(insns);
       verifier::RegisterLine* line = method_verifier->GetRegLine(dex_pc);
       mirror::ArtMethod* method =
-          method_verifier->GetQuickInvokedMethod(inst, line, is_range_quick);
-      CHECK(method != nullptr);
+          method_verifier->GetQuickInvokedMethod(inst, line, is_range_quick, true);
+      if (method == nullptr) {
+        // It can be null if the line wasn't verified since it was unreachable.
+        return false;
+      }
       // The verifier must know what the type of the object was or else we would have gotten a
       // failure. Put the dex method index in the dequicken map since we need this to get number of
       // arguments in the compiler.
@@ -220,7 +223,10 @@ void VerifiedMethod::GenerateDequickenMap(verifier::MethodVerifier* method_verif
       uint32_t dex_pc = inst->GetDexPc(insns);
       verifier::RegisterLine* line = method_verifier->GetRegLine(dex_pc);
       mirror::ArtField* field = method_verifier->GetQuickFieldAccess(inst, line);
-      CHECK(field != nullptr);
+      if (field == nullptr) {
+        // It can be null if the line wasn't verified since it was unreachable.
+        return false;
+      }
       // The verifier must know what the type of the field was or else we would have gotten a
       // failure. Put the dex field index in the dequicken map since we need this for lowering
       // in the compiler.
@@ -228,6 +234,7 @@ void VerifiedMethod::GenerateDequickenMap(verifier::MethodVerifier* method_verif
       dequicken_map_.Put(dex_pc, DexFileReference(field->GetDexFile(), field->GetDexFieldIndex()));
     }
   }
+  return true;
 }
 
 void VerifiedMethod::GenerateDevirtMap(verifier::MethodVerifier* method_verifier) {
diff --git a/compiler/dex/verified_method.h b/compiler/dex/verified_method.h
index 748bdcb71c..954cbf4b26 100644
--- a/compiler/dex/verified_method.h
+++ b/compiler/dex/verified_method.h
@@ -93,8 +93,8 @@ class VerifiedMethod {
   void GenerateDevirtMap(verifier::MethodVerifier* method_verifier)
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
-  // Generate dequickening map into dequicken_map_.
-  void GenerateDequickenMap(verifier::MethodVerifier* method_verifier)
+  // Generate dequickening map into dequicken_map_. Returns false if there is an error.
+  bool GenerateDequickenMap(verifier::MethodVerifier* method_verifier)
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
   // Generate safe case set into safe_cast_set_.
diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc
index 87a29ed2f2..a48735b081 100644
--- a/runtime/verifier/method_verifier.cc
+++ b/runtime/verifier/method_verifier.cc
@@ -513,7 +513,7 @@ mirror::ArtMethod* MethodVerifier::FindInvokedMethodAtDexPc(uint32_t dex_pc) {
   }
   const Instruction* inst = Instruction::At(code_item_->insns_ + dex_pc);
   const bool is_range = (inst->Opcode() == Instruction::INVOKE_VIRTUAL_RANGE_QUICK);
-  return GetQuickInvokedMethod(inst, register_line, is_range);
+  return GetQuickInvokedMethod(inst, register_line, is_range, false);
 }
 
 bool MethodVerifier::Verify() {
@@ -3431,10 +3431,14 @@ mirror::ArtMethod* MethodVerifier::VerifyInvocationArgs(const Instruction* inst,
 }
 
 mirror::ArtMethod* MethodVerifier::GetQuickInvokedMethod(const Instruction* inst,
-                                                         RegisterLine* reg_line, bool is_range) {
-  DCHECK(inst->Opcode() == Instruction::INVOKE_VIRTUAL_QUICK ||
-         inst->Opcode() == Instruction::INVOKE_VIRTUAL_RANGE_QUICK);
-  const RegType& actual_arg_type = reg_line->GetInvocationThis(this, inst, is_range);
+                                                         RegisterLine* reg_line, bool is_range,
+                                                         bool allow_failure) {
+  if (is_range) {
+    DCHECK_EQ(inst->Opcode(), Instruction::INVOKE_VIRTUAL_RANGE_QUICK);
+  } else {
+    DCHECK_EQ(inst->Opcode(), Instruction::INVOKE_VIRTUAL_QUICK);
+  }
+  const RegType& actual_arg_type = reg_line->GetInvocationThis(this, inst, is_range, allow_failure);
   if (!actual_arg_type.HasClass()) {
     VLOG(verifier) << "Failed to get mirror::Class* from '" << actual_arg_type << "'";
     return nullptr;
@@ -3445,29 +3449,29 @@ mirror::ArtMethod* MethodVerifier::GetQuickInvokedMethod(const Instruction* inst
     // Derive Object.class from Class.class.getSuperclass().
     mirror::Class* object_klass = klass->GetClass()->GetSuperClass();
     if (FailOrAbort(this, object_klass->IsObjectClass(),
-                    "Failed to find Object class in quickened invoke receiver",
-                    work_insn_idx_)) {
+                    "Failed to find Object class in quickened invoke receiver", work_insn_idx_)) {
       return nullptr;
     }
     dispatch_class = object_klass;
   } else {
     dispatch_class = klass;
   }
-  if (FailOrAbort(this, dispatch_class->HasVTable(),
-                  "Receiver class has no vtable for quickened invoke at ",
-                  work_insn_idx_)) {
+  if (!dispatch_class->HasVTable()) {
+    FailOrAbort(this, allow_failure, "Receiver class has no vtable for quickened invoke at ",
+                work_insn_idx_);
     return nullptr;
   }
   uint16_t vtable_index = is_range ? inst->VRegB_3rc() : inst->VRegB_35c();
-  if (FailOrAbort(this, static_cast<int32_t>(vtable_index) < dispatch_class->GetVTableLength(),
-                  "Receiver class has not enough vtable slots for quickened invoke at ",
-                  work_insn_idx_)) {
+  if (static_cast<int32_t>(vtable_index) >= dispatch_class->GetVTableLength()) {
+    FailOrAbort(this, allow_failure,
+                "Receiver class has not enough vtable slots for quickened invoke at ",
+                work_insn_idx_);
     return nullptr;
   }
   mirror::ArtMethod* res_method = dispatch_class->GetVTableEntry(vtable_index);
-  if (FailOrAbort(this, !self_->IsExceptionPending(),
-                  "Unexpected exception pending for quickened invoke at ",
-                  work_insn_idx_)) {
+  if (self_->IsExceptionPending()) {
+    FailOrAbort(this, allow_failure, "Unexpected exception pending for quickened invoke at ",
+                work_insn_idx_);
     return nullptr;
   }
   return res_method;
@@ -3478,8 +3482,7 @@ mirror::ArtMethod* MethodVerifier::VerifyInvokeVirtualQuickArgs(const Instructio
   DCHECK(Runtime::Current()->IsStarted() || verify_to_dump_)
       << PrettyMethod(dex_method_idx_, *dex_file_, true) << "@" << work_insn_idx_;
 
-  mirror::ArtMethod* res_method = GetQuickInvokedMethod(inst, work_line_.get(),
-                                                             is_range);
+  mirror::ArtMethod* res_method = GetQuickInvokedMethod(inst, work_line_.get(), is_range, false);
   if (res_method == nullptr) {
     Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "Cannot infer method from " << inst->Name();
     return nullptr;
diff --git a/runtime/verifier/method_verifier.h b/runtime/verifier/method_verifier.h
index bdd62596a6..d7c2071cbc 100644
--- a/runtime/verifier/method_verifier.h
+++ b/runtime/verifier/method_verifier.h
@@ -244,7 +244,7 @@ class MethodVerifier {
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
   // Returns the method of a quick invoke or nullptr if it cannot be found.
   mirror::ArtMethod* GetQuickInvokedMethod(const Instruction* inst, RegisterLine* reg_line,
-                                           bool is_range)
+                                           bool is_range, bool allow_failure)
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
   // Returns the access field of a quick field access (iget/iput-quick) or nullptr
   // if it cannot be found.
diff --git a/runtime/verifier/register_line.cc b/runtime/verifier/register_line.cc
index 3b098718db..ed588fcdac 100644
--- a/runtime/verifier/register_line.cc
+++ b/runtime/verifier/register_line.cc
@@ -81,18 +81,23 @@ bool RegisterLine::CheckConstructorReturn(MethodVerifier* verifier) const {
 }
 
 const RegType& RegisterLine::GetInvocationThis(MethodVerifier* verifier, const Instruction* inst,
-                                               bool is_range) {
+                                               bool is_range, bool allow_failure) {
   const size_t args_count = is_range ? inst->VRegA_3rc() : inst->VRegA_35c();
   if (args_count < 1) {
-    verifier->Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "invoke lacks 'this'";
+    if (!allow_failure) {
+      verifier->Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "invoke lacks 'this'";
+    }
     return verifier->GetRegTypeCache()->Conflict();
   }
   /* Get the element type of the array held in vsrc */
   const uint32_t this_reg = (is_range) ? inst->VRegC_3rc() : inst->VRegC_35c();
   const RegType& this_type = GetRegisterType(verifier, this_reg);
   if (!this_type.IsReferenceTypes()) {
-    verifier->Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "tried to get class from non-reference register v"
-                                                 << this_reg << " (type=" << this_type << ")";
+    if (!allow_failure) {
+      verifier->Fail(VERIFY_ERROR_BAD_CLASS_HARD)
+          << "tried to get class from non-reference register v" << this_reg
+          << " (type=" << this_type << ")";
+    }
     return verifier->GetRegTypeCache()->Conflict();
   }
   return this_type;
diff --git a/runtime/verifier/register_line.h b/runtime/verifier/register_line.h
index ca61a0b8f0..376dbf1fad 100644
--- a/runtime/verifier/register_line.h
+++ b/runtime/verifier/register_line.h
@@ -188,9 +188,11 @@ class RegisterLine {
    *
    * The argument count is in vA, and the first argument is in vC, for both "simple" and "range"
    * versions. We just need to make sure vA is >= 1 and then return vC.
+   * allow_failure will return Conflict() instead of causing a verification failure if there is an
+   * error.
    */
   const RegType& GetInvocationThis(MethodVerifier* verifier, const Instruction* inst,
-                                   bool is_range)
+                                   bool is_range, bool allow_failure = false)
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
   /*
diff --git a/test/135-MirandaDispatch/expected.txt b/test/135-MirandaDispatch/expected.txt
new file mode 100644
index 0000000000..134d8d0b47
--- /dev/null
+++ b/test/135-MirandaDispatch/expected.txt
@@ -0,0 +1 @@
+Finishing
diff --git a/test/135-MirandaDispatch/info.txt b/test/135-MirandaDispatch/info.txt
new file mode 100644
index 0000000000..22d2777b5c
--- /dev/null
+++ b/test/135-MirandaDispatch/info.txt
@@ -0,0 +1,6 @@
+Regression test for JIT related incompatible class changes caused by miranda methods.
+E.g.
+java.lang.IncompatibleClassChangeError: The method 'void Main$TheInterface.m()' was expected to be of type virtual but instead was found to be of type interface (declaration of 'java.lang.reflect.ArtMethod' appears in out/host/linux-x86/framework/core-libart-hostdex.jar)
+  at Main.DoStuff(Main.java:37)
+  at Main.main(Main.java:44)
+
diff --git a/test/135-MirandaDispatch/src/Main.java b/test/135-MirandaDispatch/src/Main.java
new file mode 100644
index 0000000000..bb005b0103
--- /dev/null
+++ b/test/135-MirandaDispatch/src/Main.java
@@ -0,0 +1,51 @@
+/*
+ * 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.
+ */
+
+public class Main {
+    // Enough to trigger JIT.
+    static final int loopIterations = 5000;
+    static int counter = 0;
+
+    static interface TheInterface {
+      public void m();
+    }
+
+    static abstract class AbstractClass implements TheInterface {
+    }
+
+    static class ConcreteClass extends AbstractClass {
+        public void m() {
+            ++counter;
+        }
+    }
+
+    static void doStuff(AbstractClass c) {
+        for (int i = 0; i < loopIterations; ++i) {
+            c.m();
+        }
+    }
+
+    public static void main(String[] args) throws Exception {
+        ConcreteClass o = new ConcreteClass();
+        for (int i = 0; i < loopIterations; ++i) {
+          doStuff(o);
+        }
+        if (counter != loopIterations * loopIterations) {
+          System.out.println("Expected " + loopIterations * loopIterations + " got " + counter);
+        }
+        System.out.println("Finishing");
+    }
+}