ART: Ignore result for exception-case JNIEndWithReference

The value may not contain a valid jobject, so ignore and use null
directly.

Refactor a bit to have one common function for both synchronized
and non-synchronized case.

Add a test to the JNI compiler tests.

Bug: 18135031
Change-Id: If2f004a112f36f4ff68172a946dec67ce561ae4d

3 files changed, 31 insertions, 20 deletions

diff --git a/compiler/jni/jni_compiler_test.cc b/compiler/jni/jni_compiler_test.cc
index 70bfb81..4186891 100644
--- a/compiler/jni/jni_compiler_test.cc
+++ b/compiler/jni/jni_compiler_test.cc
@@ -164,6 +164,7 @@ class JniCompilerTest : public CommonCompilerTest {
   void CheckParameterAlignImpl();
   void MaxParamNumberImpl();
   void WithoutImplementationImpl();
+  void WithoutImplementationRefReturnImpl();
   void StackArgsIntsFirstImpl();
   void StackArgsFloatsFirstImpl();
   void StackArgsMixedImpl();
@@ -1494,6 +1495,20 @@ void JniCompilerTest::WithoutImplementationImpl() {
 
 JNI_TEST(WithoutImplementation)
 
+void JniCompilerTest::WithoutImplementationRefReturnImpl() {
+  // This will lead to error messages in the log.
+  ScopedLogSeverity sls(LogSeverity::FATAL);
+
+  SetUpForTest(false, "withoutImplementationRefReturn", "()Ljava/lang/Object;", nullptr);
+
+  env_->CallObjectMethod(jobj_, jmethod_);
+
+  EXPECT_TRUE(Thread::Current()->IsExceptionPending());
+  EXPECT_TRUE(env_->ExceptionCheck() == JNI_TRUE);
+}
+
+JNI_TEST(WithoutImplementationRefReturn)
+
 void Java_MyClassNatives_stackArgsIntsFirst(JNIEnv*, jclass, jint i1, jint i2, jint i3,
                                             jint i4, jint i5, jint i6, jint i7, jint i8, jint i9,
                                             jint i10, jfloat f1, jfloat f2, jfloat f3, jfloat f4,
diff --git a/runtime/entrypoints/quick/quick_jni_entrypoints.cc b/runtime/entrypoints/quick/quick_jni_entrypoints.cc
index c1276b5..e478d2a 100644
--- a/runtime/entrypoints/quick/quick_jni_entrypoints.cc
+++ b/runtime/entrypoints/quick/quick_jni_entrypoints.cc
@@ -68,7 +68,6 @@ extern void JniMethodEnd(uint32_t saved_local_ref_cookie, Thread* self) {
   PopLocalReferences(saved_local_ref_cookie, self);
 }
 
-
 extern void JniMethodEndSynchronized(uint32_t saved_local_ref_cookie, jobject locked,
                                      Thread* self) {
   GoToRunnable(self);
@@ -76,38 +75,34 @@ extern void JniMethodEndSynchronized(uint32_t saved_local_ref_cookie, jobject lo
   PopLocalReferences(saved_local_ref_cookie, self);
 }
 
-extern mirror::Object* JniMethodEndWithReference(jobject result, uint32_t saved_local_ref_cookie,
-                                                 Thread* self) {
-  GoToRunnable(self);
-  mirror::Object* o = self->DecodeJObject(result);  // Must decode before pop.
+// Common result handling for EndWithReference.
+static mirror::Object* JniMethodEndWithReferenceHandleResult(jobject result,
+                                                             uint32_t saved_local_ref_cookie,
+                                                             Thread* self)
+    NO_THREAD_SAFETY_ANALYSIS {
+  // Must decode before pop. The 'result' may not be valid in case of an exception, though.
+  mirror::Object* o = self->IsExceptionPending() ? nullptr : self->DecodeJObject(result);
   PopLocalReferences(saved_local_ref_cookie, self);
   // Process result.
   if (UNLIKELY(self->GetJniEnv()->check_jni)) {
-    if (self->IsExceptionPending()) {
-      return NULL;
-    }
     CheckReferenceResult(o, self);
   }
   VerifyObject(o);
   return o;
 }
 
+extern mirror::Object* JniMethodEndWithReference(jobject result, uint32_t saved_local_ref_cookie,
+                                                 Thread* self) {
+  GoToRunnable(self);
+  return JniMethodEndWithReferenceHandleResult(result, saved_local_ref_cookie, self);
+}
+
 extern mirror::Object* JniMethodEndWithReferenceSynchronized(jobject result,
                                                              uint32_t saved_local_ref_cookie,
                                                              jobject locked, Thread* self) {
   GoToRunnable(self);
-  UnlockJniSynchronizedMethod(locked, self);  // Must decode before pop.
-  mirror::Object* o = self->DecodeJObject(result);
-  PopLocalReferences(saved_local_ref_cookie, self);
-  // Process result.
-  if (UNLIKELY(self->GetJniEnv()->check_jni)) {
-    if (self->IsExceptionPending()) {
-      return NULL;
-    }
-    CheckReferenceResult(o, self);
-  }
-  VerifyObject(o);
-  return o;
+  UnlockJniSynchronizedMethod(locked, self);
+  return JniMethodEndWithReferenceHandleResult(result, saved_local_ref_cookie, self);
 }
 
 }  // namespace art
diff --git a/test/MyClassNatives/MyClassNatives.java b/test/MyClassNatives/MyClassNatives.java
index fab153b..8b4a9a4 100644
--- a/test/MyClassNatives/MyClassNatives.java
+++ b/test/MyClassNatives/MyClassNatives.java
@@ -80,6 +80,7 @@ class MyClassNatives {
         Object o248, Object o249, Object o250, Object o251, Object o252, Object o253);
 
     native void withoutImplementation();
+    native Object withoutImplementationRefReturn();
 
     native static void stackArgsIntsFirst(int i1, int i2, int i3, int i4, int i5, int i6, int i7,
         int i8, int i9, int i10, float f1, float f2, float f3, float f4, float f5, float f6,