ART: Check invoke-interface earlier in verifier

Invoke-interface should only be called on an interface method.
Move the check earlier, as otherwise we'll try to resolve and
potentially inject a method into the dex cache.

Also templatize ResolveMethod with a version always checking
the invoke type, and on a cache miss check whether type target
type is an interface when an interface invoke type was given.

Bug: 21869691
Change-Id: Ica27158f675b5aa223d9229248189612f4706832
diff --git a/compiler/driver/compiler_driver-inl.h b/compiler/driver/compiler_driver-inl.h
index 10841e6..0eb3e43 100644
--- a/compiler/driver/compiler_driver-inl.h
+++ b/compiler/driver/compiler_driver-inl.h
@@ -264,18 +264,16 @@
     Handle<mirror::ClassLoader> class_loader, const DexCompilationUnit* mUnit,
     uint32_t method_idx, InvokeType invoke_type, bool check_incompatible_class_change) {
   DCHECK_EQ(class_loader.Get(), soa.Decode<mirror::ClassLoader*>(mUnit->GetClassLoader()));
-  ArtMethod* resolved_method = mUnit->GetClassLinker()->ResolveMethod(
-      *dex_cache->GetDexFile(), method_idx, dex_cache, class_loader, nullptr, invoke_type);
-  DCHECK_EQ(resolved_method == nullptr, soa.Self()->IsExceptionPending());
+  ArtMethod* resolved_method =
+      check_incompatible_class_change
+          ? mUnit->GetClassLinker()->ResolveMethod<ClassLinker::kForceICCECheck>(
+              *dex_cache->GetDexFile(), method_idx, dex_cache, class_loader, nullptr, invoke_type)
+          : mUnit->GetClassLinker()->ResolveMethod<ClassLinker::kNoICCECheckForCache>(
+              *dex_cache->GetDexFile(), method_idx, dex_cache, class_loader, nullptr, invoke_type);
   if (UNLIKELY(resolved_method == nullptr)) {
+    DCHECK(soa.Self()->IsExceptionPending());
     // Clean up any exception left by type resolution.
     soa.Self()->ClearException();
-    return nullptr;
-  }
-  if (check_incompatible_class_change &&
-      UNLIKELY(resolved_method->CheckIncompatibleClassChange(invoke_type))) {
-    // Silently return null on incompatible class change.
-    return nullptr;
   }
   return resolved_method;
 }
@@ -361,7 +359,7 @@
     ArtMethod* called_method;
     ClassLinker* class_linker = mUnit->GetClassLinker();
     if (LIKELY(devirt_target->dex_file == mUnit->GetDexFile())) {
-      called_method = class_linker->ResolveMethod(
+      called_method = class_linker->ResolveMethod<ClassLinker::kNoICCECheckForCache>(
           *devirt_target->dex_file, devirt_target->dex_method_index, dex_cache, class_loader,
           nullptr, kVirtual);
     } else {
@@ -369,7 +367,7 @@
       auto target_dex_cache(hs.NewHandle(class_linker->RegisterDexFile(
           *devirt_target->dex_file,
           class_linker->GetOrCreateAllocatorForClassLoader(class_loader.Get()))));
-      called_method = class_linker->ResolveMethod(
+      called_method = class_linker->ResolveMethod<ClassLinker::kNoICCECheckForCache>(
           *devirt_target->dex_file, devirt_target->dex_method_index, target_dex_cache,
           class_loader, nullptr, kVirtual);
     }
diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc
index 9d3af16..a05105b 100644
--- a/compiler/driver/compiler_driver.cc
+++ b/compiler/driver/compiler_driver.cc
@@ -1902,7 +1902,7 @@
       }
       if (resolve_fields_and_methods) {
         while (it.HasNextDirectMethod()) {
-          ArtMethod* method = class_linker->ResolveMethod(
+          ArtMethod* method = class_linker->ResolveMethod<ClassLinker::kNoICCECheckForCache>(
               dex_file, it.GetMemberIndex(), dex_cache, class_loader, nullptr,
               it.GetMethodInvokeType(class_def));
           if (method == nullptr) {
@@ -1911,7 +1911,7 @@
           it.Next();
         }
         while (it.HasNextVirtualMethod()) {
-          ArtMethod* method = class_linker->ResolveMethod(
+          ArtMethod* method = class_linker->ResolveMethod<ClassLinker::kNoICCECheckForCache>(
               dex_file, it.GetMemberIndex(), dex_cache, class_loader, nullptr,
               it.GetMethodInvokeType(class_def));
           if (method == nullptr) {
diff --git a/compiler/oat_writer.cc b/compiler/oat_writer.cc
index a6a49f9..9015800 100644
--- a/compiler/oat_writer.cc
+++ b/compiler/oat_writer.cc
@@ -641,8 +641,12 @@
     StackHandleScope<1> hs(soa.Self());
     Handle<mirror::DexCache> dex_cache(hs.NewHandle(linker->FindDexCache(
         Thread::Current(), *dex_file_)));
-    ArtMethod* method = linker->ResolveMethod(
-        *dex_file_, it.GetMemberIndex(), dex_cache, NullHandle<mirror::ClassLoader>(), nullptr,
+    ArtMethod* method = linker->ResolveMethod<ClassLinker::kNoICCECheckForCache>(
+        *dex_file_,
+        it.GetMemberIndex(),
+        dex_cache,
+        NullHandle<mirror::ClassLoader>(),
+        nullptr,
         invoke_type);
     if (method == nullptr) {
       LOG(INTERNAL_FATAL) << "Unexpected failure to resolve a method: "
diff --git a/compiler/optimizing/builder.cc b/compiler/optimizing/builder.cc
index 8e75bdc..2bbf500 100644
--- a/compiler/optimizing/builder.cc
+++ b/compiler/optimizing/builder.cc
@@ -744,7 +744,7 @@
       soa.Decode<mirror::ClassLoader*>(dex_compilation_unit_->GetClassLoader())));
   Handle<mirror::Class> compiling_class(hs.NewHandle(GetCompilingClass()));
 
-  ArtMethod* resolved_method = class_linker->ResolveMethod(
+  ArtMethod* resolved_method = class_linker->ResolveMethod<ClassLinker::kForceICCECheck>(
       *dex_compilation_unit_->GetDexFile(),
       method_idx,
       dex_compilation_unit_->GetDexCache(),
diff --git a/compiler/optimizing/reference_type_propagation.cc b/compiler/optimizing/reference_type_propagation.cc
index dd34924..fea903d 100644
--- a/compiler/optimizing/reference_type_propagation.cc
+++ b/compiler/optimizing/reference_type_propagation.cc
@@ -469,7 +469,7 @@
       // but then we would need to pass it to RTPVisitor just for this debug check. Since
       // the method is from the String class, the null loader is good enough.
       Handle<mirror::ClassLoader> loader;
-      ArtMethod* method = cl->ResolveMethod(
+      ArtMethod* method = cl->ResolveMethod<ClassLinker::kNoICCECheckForCache>(
           invoke->GetDexFile(), invoke->GetDexMethodIndex(), dex_cache, loader, nullptr, kDirect);
       DCHECK(method != nullptr);
       mirror::Class* declaring_class = method->GetDeclaringClass();
diff --git a/runtime/class_linker-inl.h b/runtime/class_linker-inl.h
index 88a3996..a5d10b2 100644
--- a/runtime/class_linker-inl.h
+++ b/runtime/class_linker-inl.h
@@ -116,6 +116,7 @@
   return resolved_method;
 }
 
+template <ClassLinker::ResolveMode kResolveMode>
 inline ArtMethod* ClassLinker::ResolveMethod(Thread* self,
                                              uint32_t method_idx,
                                              ArtMethod* referrer,
@@ -127,12 +128,12 @@
     Handle<mirror::DexCache> h_dex_cache(hs.NewHandle(declaring_class->GetDexCache()));
     Handle<mirror::ClassLoader> h_class_loader(hs.NewHandle(declaring_class->GetClassLoader()));
     const DexFile* dex_file = h_dex_cache->GetDexFile();
-    resolved_method = ResolveMethod(*dex_file,
-                                    method_idx,
-                                    h_dex_cache,
-                                    h_class_loader,
-                                    referrer,
-                                    type);
+    resolved_method = ResolveMethod<kResolveMode>(*dex_file,
+                                                  method_idx,
+                                                  h_dex_cache,
+                                                  h_class_loader,
+                                                  referrer,
+                                                  type);
   }
   // Note: We cannot check here to see whether we added the method to the cache. It
   //       might be an erroneous class, which results in it being hidden from us.
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 2dd2a83..3f31fcd 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -6149,6 +6149,7 @@
   return resolved;
 }
 
+template <ClassLinker::ResolveMode kResolveMode>
 ArtMethod* ClassLinker::ResolveMethod(const DexFile& dex_file,
                                       uint32_t method_idx,
                                       Handle<mirror::DexCache> dex_cache,
@@ -6160,6 +6161,12 @@
   ArtMethod* resolved = dex_cache->GetResolvedMethod(method_idx, image_pointer_size_);
   if (resolved != nullptr && !resolved->IsRuntimeMethod()) {
     DCHECK(resolved->GetDeclaringClassUnchecked() != nullptr) << resolved->GetDexMethodIndex();
+    if (kResolveMode == ClassLinker::kForceICCECheck) {
+      if (resolved->CheckIncompatibleClassChange(type)) {
+        ThrowIncompatibleClassChangeError(type, resolved->GetInvokeType(), resolved, referrer);
+        return nullptr;
+      }
+    }
     return resolved;
   }
   // Fail, get the declaring class.
@@ -6178,8 +6185,36 @@
       DCHECK(resolved == nullptr || resolved->GetDeclaringClassUnchecked() != nullptr);
       break;
     case kInterface:
-      resolved = klass->FindInterfaceMethod(dex_cache.Get(), method_idx, image_pointer_size_);
-      DCHECK(resolved == nullptr || resolved->GetDeclaringClass()->IsInterface());
+      // We have to check whether the method id really belongs to an interface (dex static bytecode
+      // constraint A15). Otherwise you must not invoke-interface on it.
+      //
+      // This is not symmetric to A12-A14 (direct, static, virtual), as using FindInterfaceMethod
+      // assumes that the given type is an interface, and will check the interface table if the
+      // method isn't declared in the class. So it may find an interface method (usually by name
+      // in the handling below, but we do the constraint check early). In that case,
+      // CheckIncompatibleClassChange will succeed (as it is called on an interface method)
+      // unexpectedly.
+      // Example:
+      //    interface I {
+      //      foo()
+      //    }
+      //    class A implements I {
+      //      ...
+      //    }
+      //    class B extends A {
+      //      ...
+      //    }
+      //    invoke-interface B.foo
+      //      -> FindInterfaceMethod finds I.foo (interface method), not A.foo (miranda method)
+      if (UNLIKELY(!klass->IsInterface())) {
+        ThrowIncompatibleClassChangeError(klass,
+                                          "Found class %s, but interface was expected",
+                                          PrettyDescriptor(klass).c_str());
+        return nullptr;
+      } else {
+        resolved = klass->FindInterfaceMethod(dex_cache.Get(), method_idx, image_pointer_size_);
+        DCHECK(resolved == nullptr || resolved->GetDeclaringClass()->IsInterface());
+      }
       break;
     case kSuper:  // Fall-through.
     case kVirtual:
@@ -6781,4 +6816,20 @@
   }
 }
 
+// Instantiate ResolveMethod.
+template ArtMethod* ClassLinker::ResolveMethod<ClassLinker::kForceICCECheck>(
+    const DexFile& dex_file,
+    uint32_t method_idx,
+    Handle<mirror::DexCache> dex_cache,
+    Handle<mirror::ClassLoader> class_loader,
+    ArtMethod* referrer,
+    InvokeType type);
+template ArtMethod* ClassLinker::ResolveMethod<ClassLinker::kNoICCECheckForCache>(
+    const DexFile& dex_file,
+    uint32_t method_idx,
+    Handle<mirror::DexCache> dex_cache,
+    Handle<mirror::ClassLoader> class_loader,
+    ArtMethod* referrer,
+    InvokeType type);
+
 }  // namespace art
diff --git a/runtime/class_linker.h b/runtime/class_linker.h
index 29aac31..0d3bc1e 100644
--- a/runtime/class_linker.h
+++ b/runtime/class_linker.h
@@ -246,11 +246,19 @@
       SHARED_REQUIRES(Locks::mutator_lock_)
       REQUIRES(!dex_lock_, !Roles::uninterruptible_);
 
+  // Determine whether a dex cache result should be trusted, or an IncompatibleClassChangeError
+  // check should be performed even after a hit.
+  enum ResolveMode {  // private.
+    kNoICCECheckForCache,
+    kForceICCECheck
+  };
+
   // Resolve a method with a given ID from the DexFile, storing the
   // result in DexCache. The ClassLinker and ClassLoader are used as
   // in ResolveType. What is unique is the method type argument which
   // is used to determine if this method is a direct, static, or
   // virtual method.
+  template <ResolveMode kResolveMode>
   ArtMethod* ResolveMethod(const DexFile& dex_file,
                            uint32_t method_idx,
                            Handle<mirror::DexCache> dex_cache,
@@ -262,6 +270,7 @@
 
   ArtMethod* GetResolvedMethod(uint32_t method_idx, ArtMethod* referrer)
       SHARED_REQUIRES(Locks::mutator_lock_);
+  template <ResolveMode kResolveMode>
   ArtMethod* ResolveMethod(Thread* self, uint32_t method_idx, ArtMethod* referrer, InvokeType type)
       SHARED_REQUIRES(Locks::mutator_lock_)
       REQUIRES(!dex_lock_, !Roles::uninterruptible_);
diff --git a/runtime/entrypoints/entrypoint_utils-inl.h b/runtime/entrypoints/entrypoint_utils-inl.h
index dccb1da..ba2fb94 100644
--- a/runtime/entrypoints/entrypoint_utils-inl.h
+++ b/runtime/entrypoints/entrypoint_utils-inl.h
@@ -68,7 +68,7 @@
     class_loader.Assign(caller->GetClassLoader());
   }
 
-  return class_linker->ResolveMethod(
+  return class_linker->ResolveMethod<ClassLinker::kNoICCECheckForCache>(
       *outer_method->GetDexFile(), method_index, dex_cache, class_loader, nullptr, invoke_type);
 }
 
@@ -401,7 +401,10 @@
     mirror::Object* null_this = nullptr;
     HandleWrapper<mirror::Object> h_this(
         hs.NewHandleWrapper(type == kStatic ? &null_this : this_object));
-    resolved_method = class_linker->ResolveMethod(self, method_idx, referrer, type);
+    constexpr ClassLinker::ResolveMode resolve_mode =
+        access_check ? ClassLinker::kForceICCECheck
+                     : ClassLinker::kNoICCECheckForCache;
+    resolved_method = class_linker->ResolveMethod<resolve_mode>(self, method_idx, referrer, type);
   }
   if (UNLIKELY(resolved_method == nullptr)) {
     DCHECK(self->IsExceptionPending());  // Throw exception and unwind.
diff --git a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
index c41ee45..6361144 100644
--- a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
+++ b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
@@ -1012,7 +1012,8 @@
     HandleWrapper<mirror::Object> h_receiver(
         hs.NewHandleWrapper(virtual_or_interface ? &receiver : &dummy));
     DCHECK_EQ(caller->GetDexFile(), called_method.dex_file);
-    called = linker->ResolveMethod(self, called_method.dex_method_index, caller, invoke_type);
+    called = linker->ResolveMethod<ClassLinker::kForceICCECheck>(
+        self, called_method.dex_method_index, caller, invoke_type);
   }
   const void* code = nullptr;
   if (LIKELY(!self->IsExceptionPending())) {
diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc
index 364b8ce..cf27ff2 100644
--- a/runtime/verifier/method_verifier.cc
+++ b/runtime/verifier/method_verifier.cc
@@ -192,7 +192,7 @@
     }
     previous_method_idx = method_idx;
     InvokeType type = it->GetMethodInvokeType(*class_def);
-    ArtMethod* method = linker->ResolveMethod(
+    ArtMethod* method = linker->ResolveMethod<ClassLinker::kNoICCECheckForCache>(
         *dex_file, method_idx, dex_cache, class_loader, nullptr, type);
     if (method == nullptr) {
       DCHECK(self->IsExceptionPending());
@@ -3638,6 +3638,30 @@
   const RegType& referrer = GetDeclaringClass();
   auto* cl = Runtime::Current()->GetClassLinker();
   auto pointer_size = cl->GetImagePointerSize();
+
+  // Check that interface methods are static or match interface classes.
+  // We only allow statics if we don't have default methods enabled.
+  if (klass->IsInterface()) {
+    Runtime* runtime = Runtime::Current();
+    const bool default_methods_supported =
+        runtime == nullptr ||
+        runtime->AreExperimentalFlagsEnabled(ExperimentalFlags::kDefaultMethods);
+    if (method_type != METHOD_INTERFACE &&
+        (!default_methods_supported || method_type != METHOD_STATIC)) {
+      Fail(VERIFY_ERROR_CLASS_CHANGE)
+          << "non-interface method " << PrettyMethod(dex_method_idx, *dex_file_)
+          << " is in an interface class " << PrettyClass(klass);
+      return nullptr;
+    }
+  } else {
+    if (method_type == METHOD_INTERFACE) {
+      Fail(VERIFY_ERROR_CLASS_CHANGE)
+          << "interface method " << PrettyMethod(dex_method_idx, *dex_file_)
+          << " is in a non-interface class " << PrettyClass(klass);
+      return nullptr;
+    }
+  }
+
   ArtMethod* res_method = dex_cache_->GetResolvedMethod(dex_method_idx, pointer_size);
   if (res_method == nullptr) {
     const char* name = dex_file_->GetMethodName(method_id);
@@ -3692,23 +3716,6 @@
                                       << PrettyMethod(res_method);
     return nullptr;
   }
-  // Check that interface methods are static or match interface classes.
-  // We only allow statics if we don't have default methods enabled.
-  Runtime* runtime = Runtime::Current();
-  const bool default_methods_supported =
-      runtime == nullptr ||
-      runtime->AreExperimentalFlagsEnabled(ExperimentalFlags::kDefaultMethods);
-  if (klass->IsInterface() &&
-      method_type != METHOD_INTERFACE &&
-      (!default_methods_supported || method_type != METHOD_STATIC)) {
-    Fail(VERIFY_ERROR_CLASS_CHANGE) << "non-interface method " << PrettyMethod(res_method)
-                                    << " is in an interface class " << PrettyClass(klass);
-    return nullptr;
-  } else if (!klass->IsInterface() && method_type == METHOD_INTERFACE) {
-    Fail(VERIFY_ERROR_CLASS_CHANGE) << "interface method " << PrettyMethod(res_method)
-                                    << " is in a non-interface class " << PrettyClass(klass);
-    return nullptr;
-  }
   // See if the method type implied by the invoke instruction matches the access flags for the
   // target method.
   if ((method_type == METHOD_DIRECT && (!res_method->IsDirect() || res_method->IsStatic())) ||
diff --git a/test/800-smali/expected.txt b/test/800-smali/expected.txt
index a590cf1..ebefeea 100644
--- a/test/800-smali/expected.txt
+++ b/test/800-smali/expected.txt
@@ -47,4 +47,5 @@
 b/23502994 (if-eqz)
 b/23502994 (check-cast)
 b/25494456
+b/21869691
 Done!
diff --git a/test/800-smali/smali/b_21869691A.smali b/test/800-smali/smali/b_21869691A.smali
new file mode 100644
index 0000000..a7a6ef4
--- /dev/null
+++ b/test/800-smali/smali/b_21869691A.smali
@@ -0,0 +1,47 @@
+# Test that the verifier does not stash methods incorrectly because they are being invoked with
+# the wrong opcode.
+#
+# When using invoke-interface on a method id that is not from an interface class, we should throw
+# an IncompatibleClassChangeError. FindInterfaceMethod assumes that the given type is an interface,
+# so we can construct a class hierarchy that would have a surprising result:
+#
+#   interface I {
+#     void a();
+#   }
+#
+#   class B implements I {
+#      // miranda method for a, or a implemented.
+#   }
+#
+#   class C extends B {
+#   }
+#
+# Then calling invoke-interface C.a() will go wrong if there is no explicit check: a can't be found
+# in C, but in the interface table, so we will find an interface method and pass ICCE checks.
+#
+# If we do this before a correct invoke-virtual C.a(), we poison the dex cache with an incorrect
+# method. In this test, this is done in A (A < B, so processed first). The "real" call is in B.
+
+.class public LB21869691A;
+
+.super Ljava/lang/Object;
+
+.method public constructor <init>()V
+    .registers 1
+    invoke-direct {p0}, Ljava/lang/Object;-><init>()V
+    return-void
+.end method
+
+.method public run()V
+  .registers 3
+  new-instance v0, LB21869691C;
+  invoke-direct {v0}, LB21869691C;-><init>()V
+  invoke-virtual {v2, v0}, LB21869691A;->callinf(LB21869691C;)V
+  return-void
+.end method
+
+.method public callinf(LB21869691C;)V
+  .registers 2
+  invoke-interface {p1}, LB21869691C;->a()V
+  return-void
+.end method
diff --git a/test/800-smali/smali/b_21869691B.smali b/test/800-smali/smali/b_21869691B.smali
new file mode 100644
index 0000000..1172bdb
--- /dev/null
+++ b/test/800-smali/smali/b_21869691B.smali
@@ -0,0 +1,33 @@
+# Test that the verifier does not stash methods incorrectly because they are being invoked with
+# the wrong opcode. See b_21869691A.smali for explanation.
+
+.class public abstract LB21869691B;
+
+.super Ljava/lang/Object;
+.implements LB21869691I;
+
+.method protected constructor <init>()V
+    .registers 1
+    invoke-direct {p0}, Ljava/lang/Object;-><init>()V
+    return-void
+.end method
+
+# Have an implementation for the interface method.
+.method public a()V
+  .registers 1
+  return-void
+.end method
+
+# Call ourself with invoke-virtual.
+.method public callB()V
+  .registers 1
+  invoke-virtual {p0}, LB21869691B;->a()V
+  return-void
+.end method
+
+# Call C with invoke-virtual.
+.method public callB(LB21869691C;)V
+  .registers 2
+  invoke-virtual {p1}, LB21869691C;->a()V
+  return-void
+.end method
diff --git a/test/800-smali/smali/b_21869691C.smali b/test/800-smali/smali/b_21869691C.smali
new file mode 100644
index 0000000..4f89a04
--- /dev/null
+++ b/test/800-smali/smali/b_21869691C.smali
@@ -0,0 +1,12 @@
+# Test that the verifier does not stash methods incorrectly because they are being invoked with
+# the wrong opcode. See b_21869691A.smali for explanation.
+
+.class public LB21869691C;
+
+.super LB21869691B;
+
+.method public constructor <init>()V
+    .registers 1
+    invoke-direct {p0}, LB21869691B;-><init>()V
+    return-void
+.end method
diff --git a/test/800-smali/smali/b_21869691I.smali b/test/800-smali/smali/b_21869691I.smali
new file mode 100644
index 0000000..72a27dd
--- /dev/null
+++ b/test/800-smali/smali/b_21869691I.smali
@@ -0,0 +1,11 @@
+# Test that the verifier does not stash methods incorrectly because they are being invoked with
+# the wrong opcode.
+#
+# This is the interface class that has an "a" method.
+
+.class public abstract interface LB21869691I;
+
+.super Ljava/lang/Object;
+
+.method public abstract a()V
+.end method
diff --git a/test/800-smali/src/Main.java b/test/800-smali/src/Main.java
index 4844848..3b62a46 100644
--- a/test/800-smali/src/Main.java
+++ b/test/800-smali/src/Main.java
@@ -139,6 +139,8 @@
                 new Object[] { "abc" }, null, null));
         testCases.add(new TestCase("b/25494456", "B25494456", "run", null, new VerifyError(),
                 null));
+        testCases.add(new TestCase("b/21869691", "B21869691A", "run", null,
+                new IncompatibleClassChangeError(), null));
     }
 
     public void runTests() {
@@ -208,7 +210,7 @@
                                                         tc.expectedException.getClass().getName() +
                                                         ", but got " + exc.getClass(), exc);
             } else {
-              // Expected exception, do nothing.
+                // Expected exception, do nothing.
             }
         } finally {
             if (errorReturn != null) {