From ad61517890168ff6ed19063cc8032a9c033d135b Mon Sep 17 00:00:00 2001
From: Andreas Gampe <agampe@google.com>
Date: Fri, 4 Apr 2014 16:20:13 -0700
Subject: [PATCH] Fix wrong handling of Generic JNI not finding native method.

Code did not properly call JNIMethodEnd, such that locks etc
where not correctly handled.

Add a test case to jni_compiler_test.

Change-Id: If2d5c628517d65a56dd6bb5c4cabdff77c7664a1
---
 compiler/jni/jni_compiler_test.cc                  | 10 ++++
 .../quick/quick_trampoline_entrypoints.cc          | 57 +++++++++++++---------
 test/MyClassNatives/MyClassNatives.java            |  1 +
 3 files changed, 46 insertions(+), 22 deletions(-)

diff --git a/compiler/jni/jni_compiler_test.cc b/compiler/jni/jni_compiler_test.cc
index 3204282e2..a0797f314 100644
--- a/compiler/jni/jni_compiler_test.cc
+++ b/compiler/jni/jni_compiler_test.cc
@@ -1268,4 +1268,14 @@ TEST_F(JniCompilerTest, MaxParamNumber) {
   env_->CallNonvirtualVoidMethodA(jobj_, jklass_, jmethod_, args);
 }
 
+TEST_F(JniCompilerTest, WithoutImplementation) {
+  TEST_DISABLED_FOR_PORTABLE();
+  SetUpForTest(false, "withoutImplementation", "()V", nullptr);
+
+  env_->CallVoidMethod(jobj_, jmethod_);
+
+  EXPECT_TRUE(Thread::Current()->IsExceptionPending());
+  EXPECT_TRUE(env_->ExceptionCheck() == JNI_TRUE);
+}
+
 }  // namespace art
diff --git a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
index 963c3d156..fcbcac233 100644
--- a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
+++ b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
@@ -1496,6 +1496,22 @@ void BuildGenericJniFrameVisitor::FinalizeSirt(Thread* self) {
 
 extern "C" void* artFindNativeMethod();
 
+uint64_t artQuickGenericJniEndJNIRef(Thread* self, uint32_t cookie, jobject l, jobject lock) {
+  if (lock != nullptr) {
+    return reinterpret_cast<uint64_t>(JniMethodEndWithReferenceSynchronized(l, cookie, lock, self));
+  } else {
+    return reinterpret_cast<uint64_t>(JniMethodEndWithReference(l, cookie, self));
+  }
+}
+
+void artQuickGenericJniEndJNINonRef(Thread* self, uint32_t cookie, jobject lock) {
+  if (lock != nullptr) {
+    JniMethodEndSynchronized(cookie, lock, self);
+  } else {
+    JniMethodEnd(cookie, self);
+  }
+}
+
 /*
  * Initializes an alloca region assumed to be directly below sp for a native call:
  * Create a Sirt and call stack and fill a mini stack with values to be pushed to registers.
@@ -1555,6 +1571,15 @@ extern "C" ssize_t artQuickGenericJniTrampoline(Thread* self, mirror::ArtMethod*
 
     if (nativeCode == nullptr) {
       DCHECK(self->IsExceptionPending());    // There should be an exception pending now.
+
+      // End JNI, as the assembly will move to deliver the exception.
+      jobject lock = called->IsSynchronized() ? visitor.GetFirstSirtEntry() : nullptr;
+      if (mh.GetShorty()[0] == 'L') {
+        artQuickGenericJniEndJNIRef(self, cookie, nullptr, lock);
+      } else {
+        artQuickGenericJniEndJNINonRef(self, cookie, lock);
+      }
+
       return -1;
     }
     // Note that the native code pointer will be automatically set by artFindNativeMethod().
@@ -1580,33 +1605,21 @@ extern "C" uint64_t artQuickGenericJniEndTrampoline(Thread* self, mirror::ArtMet
   mirror::ArtMethod* called = *sp;
   uint32_t cookie = *(sp32 - 1);
 
+  jobject lock = nullptr;
+  if (called->IsSynchronized()) {
+    StackIndirectReferenceTable* table =
+        reinterpret_cast<StackIndirectReferenceTable*>(
+            reinterpret_cast<uint8_t*>(sp) + kPointerSize);
+    lock = reinterpret_cast<jobject>(table->GetStackReference(0));
+  }
+
   MethodHelper mh(called);
   char return_shorty_char = mh.GetShorty()[0];
 
   if (return_shorty_char == 'L') {
-    // the only special ending call
-    if (called->IsSynchronized()) {
-      StackIndirectReferenceTable* table =
-          reinterpret_cast<StackIndirectReferenceTable*>(
-              reinterpret_cast<uint8_t*>(sp) + kPointerSize);
-      jobject tmp = reinterpret_cast<jobject>(table->GetStackReference(0));
-
-      return reinterpret_cast<uint64_t>(JniMethodEndWithReferenceSynchronized(result.l, cookie, tmp,
-                                                                              self));
-    } else {
-      return reinterpret_cast<uint64_t>(JniMethodEndWithReference(result.l, cookie, self));
-    }
+    return artQuickGenericJniEndJNIRef(self, cookie, result.l, lock);
   } else {
-    if (called->IsSynchronized()) {
-      StackIndirectReferenceTable* table =
-          reinterpret_cast<StackIndirectReferenceTable*>(
-              reinterpret_cast<uint8_t*>(sp) + kPointerSize);
-      jobject tmp = reinterpret_cast<jobject>(table->GetStackReference(0));
-
-      JniMethodEndSynchronized(cookie, tmp, self);
-    } else {
-      JniMethodEnd(cookie, self);
-    }
+    artQuickGenericJniEndJNINonRef(self, cookie, lock);
 
     switch (return_shorty_char) {
       case 'F':  // Fall-through.
diff --git a/test/MyClassNatives/MyClassNatives.java b/test/MyClassNatives/MyClassNatives.java
index 6e7a42693..09f178378 100644
--- a/test/MyClassNatives/MyClassNatives.java
+++ b/test/MyClassNatives/MyClassNatives.java
@@ -79,4 +79,5 @@ class MyClassNatives {
         Object o240, Object o241, Object o242, Object o243, Object o244, Object o245, Object o246, Object o247,
         Object o248, Object o249, Object o250, Object o251, Object o252, Object o253);
 
+    native void withoutImplementation();
 }
-- 
2.11.0