From 2a1b7ac77446d12befceb9946dc418d15119bc05 Mon Sep 17 00:00:00 2001
From: Aart Bik <ajcbik@google.com>
Date: Wed, 29 Jun 2016 14:54:26 -0700
Subject: [PATCH] Fix divergence between interpreter and compiler.

Rationale:
Access checks on a method invocation should be performed
prior to checking null pointer for non-static method calls.

Test: 600-verifier-fails

BUG=29068831

Change-Id: I21874d96ff83e7e6c6cf7c239e65ce1b515b9729
---
 runtime/entrypoints/entrypoint_utils-inl.h | 31 ++++++++++++++++--------------
 test/600-verifier-fails/expected.txt       |  1 +
 test/600-verifier-fails/src/Main.java      |  3 +--
 3 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/runtime/entrypoints/entrypoint_utils-inl.h b/runtime/entrypoints/entrypoint_utils-inl.h
index fc6257302..1b55d2f33 100644
--- a/runtime/entrypoints/entrypoint_utils-inl.h
+++ b/runtime/entrypoints/entrypoint_utils-inl.h
@@ -448,23 +448,10 @@ inline ArtMethod* FindMethodFromCode(uint32_t method_idx, mirror::Object** this_
                      : ClassLinker::kNoICCECheckForCache;
     resolved_method = class_linker->ResolveMethod<resolve_mode>(self, method_idx, referrer, type);
   }
+  // Resolution and access check.
   if (UNLIKELY(resolved_method == nullptr)) {
     DCHECK(self->IsExceptionPending());  // Throw exception and unwind.
     return nullptr;  // Failure.
-  } else if (UNLIKELY(*this_object == nullptr && type != kStatic)) {
-    if (UNLIKELY(resolved_method->GetDeclaringClass()->IsStringClass() &&
-                 resolved_method->IsConstructor())) {
-      // Hack for String init:
-      //
-      // We assume that the input of String.<init> in verified code is always
-      // an unitialized reference. If it is a null constant, it must have been
-      // optimized out by the compiler. Do not throw NullPointerException.
-    } else {
-      // Maintain interpreter-like semantics where NullPointerException is thrown
-      // after potential NoSuchMethodError from class linker.
-      ThrowNullPointerExceptionForMethodAccess(method_idx, type);
-      return nullptr;  // Failure.
-    }
   } else if (access_check) {
     mirror::Class* methods_class = resolved_method->GetDeclaringClass();
     bool can_access_resolved_method =
@@ -482,6 +469,22 @@ inline ArtMethod* FindMethodFromCode(uint32_t method_idx, mirror::Object** this_
       return nullptr;  // Failure.
     }
   }
+  // Next, null pointer check.
+  if (UNLIKELY(*this_object == nullptr && type != kStatic)) {
+    if (UNLIKELY(resolved_method->GetDeclaringClass()->IsStringClass() &&
+                 resolved_method->IsConstructor())) {
+      // Hack for String init:
+      //
+      // We assume that the input of String.<init> in verified code is always
+      // an unitialized reference. If it is a null constant, it must have been
+      // optimized out by the compiler. Do not throw NullPointerException.
+    } else {
+      // Maintain interpreter-like semantics where NullPointerException is thrown
+      // after potential NoSuchMethodError from class linker.
+      ThrowNullPointerExceptionForMethodAccess(method_idx, type);
+      return nullptr;  // Failure.
+    }
+  }
   switch (type) {
     case kStatic:
     case kDirect:
diff --git a/test/600-verifier-fails/expected.txt b/test/600-verifier-fails/expected.txt
index 8399969a2..eaa0c933c 100644
--- a/test/600-verifier-fails/expected.txt
+++ b/test/600-verifier-fails/expected.txt
@@ -2,3 +2,4 @@ passed A
 passed B
 passed C
 passed D
+passed E
diff --git a/test/600-verifier-fails/src/Main.java b/test/600-verifier-fails/src/Main.java
index 64c3d5c16..fa25d58e4 100644
--- a/test/600-verifier-fails/src/Main.java
+++ b/test/600-verifier-fails/src/Main.java
@@ -38,7 +38,6 @@ public class Main {
     test("B");
     test("C");
     test("D");
-    // TODO: enable again
-    // test("E");
+    test("E");
   }
 }
-- 
2.11.0