8164086: Checked JNI pending exception check should be cleared when returning to Java frame

Summary: Transitions to Java clear the pending pointer
Reviewed-by: dholmes, neliasso, coleenp

hotspot/make/test/JtregNative.gmk

@@ -44,6 +44,7 @@ BUILD_HOTSPOT_JTREG_NATIVE_SRC := \
     $(HOTSPOT_TOPDIR)/test/native_sanity \
     $(HOTSPOT_TOPDIR)/test/runtime/jni/8025979 \
     $(HOTSPOT_TOPDIR)/test/runtime/jni/8033445 \
+    $(HOTSPOT_TOPDIR)/test/runtime/jni/checked \
     $(HOTSPOT_TOPDIR)/test/runtime/jni/ToStringInInterfaceTest \
     $(HOTSPOT_TOPDIR)/test/runtime/modules/getModuleJNI \
     $(HOTSPOT_TOPDIR)/test/runtime/SameObject \

hotspot/src/cpu/aarch64/vm/sharedRuntime_aarch64.cpp

@@ -2041,6 +2041,11 @@ nmethod* SharedRuntime::generate_native_wrapper(MacroAssembler* masm,
       __ verify_oop(r0);
   }
 
+  if (CheckJNICalls) {
+    // clear_pending_jni_exception_check
+    __ str(zr, Address(rthread, JavaThread::pending_jni_exception_check_fn_offset()));
+  }
+
   if (!is_critical_native) {
     // reset handle block
     __ ldr(r2, Address(rthread, JavaThread::active_handles_offset()));

hotspot/src/cpu/aarch64/vm/templateInterpreterGenerator_aarch64.cpp

@@ -1355,6 +1355,11 @@ address TemplateInterpreterGenerator::generate_native_entry(bool synchronized) {
   // reset_last_Java_frame
   __ reset_last_Java_frame(true);
 
+  if (CheckJNICalls) {
+    // clear_pending_jni_exception_check
+    __ str(zr, Address(rthread, JavaThread::pending_jni_exception_check_fn_offset()));
+  }
+
   // reset handle block
   __ ldr(t, Address(rthread, JavaThread::active_handles_offset()));
   __ str(zr, Address(t, JNIHandleBlock::top_offset_in_bytes()));

hotspot/src/cpu/sparc/vm/sharedRuntime_sparc.cpp

@@ -2765,6 +2765,11 @@ nmethod* SharedRuntime::generate_native_wrapper(MacroAssembler* masm,
       __ verify_oop(I0);
   }
 
+  if (CheckJNICalls) {
+    // clear_pending_jni_exception_check
+    __ st_ptr(G0, G2_thread, JavaThread::pending_jni_exception_check_fn_offset());
+  }
+
   if (!is_critical_native) {
     // reset handle block
     __ ld_ptr(G2_thread, in_bytes(JavaThread::active_handles_offset()), L5);

hotspot/src/cpu/sparc/vm/templateInterpreterGenerator_sparc.cpp

@@ -1487,6 +1487,11 @@ address TemplateInterpreterGenerator::generate_native_entry(bool synchronized) {
   __ set(_thread_in_Java, G3_scratch);
   __ st(G3_scratch, thread_state);
 
+  if (CheckJNICalls) {
+    // clear_pending_jni_exception_check
+    __ st_ptr(G0, G2_thread, JavaThread::pending_jni_exception_check_fn_offset());
+  }
+
   // reset handle block
   __ ld_ptr(G2_thread, JavaThread::active_handles_offset(), G3_scratch);
   __ st(G0, G3_scratch, JNIHandleBlock::top_offset_in_bytes());

hotspot/src/cpu/x86/vm/sharedRuntime_x86_32.cpp

@@ -2236,6 +2236,11 @@ nmethod* SharedRuntime::generate_native_wrapper(MacroAssembler* masm,
       __ verify_oop(rax);
   }
 
+  if (CheckJNICalls) {
+    // clear_pending_jni_exception_check
+    __ movptr(Address(thread, JavaThread::pending_jni_exception_check_fn_offset()), NULL_WORD);
+  }
+
   if (!is_critical_native) {
     // reset handle block
     __ movptr(rcx, Address(thread, JavaThread::active_handles_offset()));

hotspot/src/cpu/x86/vm/sharedRuntime_x86_64.cpp

@@ -2589,6 +2589,11 @@ nmethod* SharedRuntime::generate_native_wrapper(MacroAssembler* masm,
       __ verify_oop(rax);
   }
 
+  if (CheckJNICalls) {
+    // clear_pending_jni_exception_check
+    __ movptr(Address(r15_thread, JavaThread::pending_jni_exception_check_fn_offset()), NULL_WORD);
+  }
+
   if (!is_critical_native) {
     // reset handle block
     __ movptr(rcx, Address(r15_thread, JavaThread::active_handles_offset()));

hotspot/src/cpu/x86/vm/templateInterpreterGenerator_x86.cpp

@@ -1169,6 +1169,11 @@ address TemplateInterpreterGenerator::generate_native_entry(bool synchronized) {
   // reset_last_Java_frame
   __ reset_last_Java_frame(thread, true);
 
+  if (CheckJNICalls) {
+    // clear_pending_jni_exception_check
+    __ movptr(Address(thread, JavaThread::pending_jni_exception_check_fn_offset()), NULL_WORD);
+  }
+
   // reset handle block
   __ movptr(t, Address(thread, JavaThread::active_handles_offset()));
   __ movl(Address(t, JNIHandleBlock::top_offset_in_bytes()), (int32_t)NULL_WORD);

hotspot/src/share/vm/prims/whitebox.cpp

@@ -894,6 +894,7 @@ static bool GetVMFlag(JavaThread* thread, JNIEnv* env, jstring name, T* value, F
   }
   ThreadToNativeFromVM ttnfv(thread);   // can't be in VM when we call JNI
   const char* flag_name = env->GetStringUTFChars(name, NULL);
+  CHECK_JNI_EXCEPTION_(env, false);
   Flag::Error result = (*TAt)(flag_name, value, true, true);
   env->ReleaseStringUTFChars(name, flag_name);
   return (result == Flag::SUCCESS);
@@ -906,6 +907,7 @@ static bool SetVMFlag(JavaThread* thread, JNIEnv* env, jstring name, T* value, F
   }
   ThreadToNativeFromVM ttnfv(thread);   // can't be in VM when we call JNI
   const char* flag_name = env->GetStringUTFChars(name, NULL);
+  CHECK_JNI_EXCEPTION_(env, false);
   Flag::Error result = (*TAtPut)(flag_name, value, Flag::INTERNAL);
   env->ReleaseStringUTFChars(name, flag_name);
   return (result == Flag::SUCCESS);
@@ -944,6 +946,7 @@ static jobject doubleBox(JavaThread* thread, JNIEnv* env, jdouble value) {
 static Flag* getVMFlag(JavaThread* thread, JNIEnv* env, jstring name) {
   ThreadToNativeFromVM ttnfv(thread);   // can't be in VM when we call JNI
   const char* flag_name = env->GetStringUTFChars(name, NULL);
+  CHECK_JNI_EXCEPTION_(env, NULL);
   Flag* result = Flag::find_flag(flag_name, strlen(flag_name), true, true);
   env->ReleaseStringUTFChars(name, flag_name);
   return result;
@@ -1084,7 +1087,14 @@ WB_END
 
 WB_ENTRY(void, WB_SetStringVMFlag(JNIEnv* env, jobject o, jstring name, jstring value))
   ThreadToNativeFromVM ttnfv(thread);   // can't be in VM when we call JNI
-  const char* ccstrValue = (value == NULL) ? NULL : env->GetStringUTFChars(value, NULL);
+  const char* ccstrValue;
+  if (value == NULL) {
+    ccstrValue = NULL;
+  }
+  else {
+    ccstrValue = env->GetStringUTFChars(value, NULL);
+    CHECK_JNI_EXCEPTION(env);
+  }
   ccstr ccstrResult = ccstrValue;
   bool needFree;
   {
@@ -1308,6 +1318,7 @@ WB_ENTRY(jobjectArray, WB_GetCodeHeapEntries(JNIEnv* env, jobject o, jint blob_t
   jclass clazz = env->FindClass(vmSymbols::java_lang_Object()->as_C_string());
   CHECK_JNI_EXCEPTION_(env, NULL);
   result = env->NewObjectArray(blobs.length(), clazz, NULL);
+  CHECK_JNI_EXCEPTION_(env, NULL);
   if (result == NULL) {
     return result;
   }
@@ -1317,6 +1328,7 @@ WB_ENTRY(jobjectArray, WB_GetCodeHeapEntries(JNIEnv* env, jobject o, jint blob_t
     jobjectArray obj = codeBlob2objectArray(thread, env, *it);
     CHECK_JNI_EXCEPTION_(env, NULL);
     env->SetObjectArrayElement(result, i, obj);
+    CHECK_JNI_EXCEPTION_(env, NULL);
     ++i;
   }
   return result;
@@ -1523,6 +1535,7 @@ static bool GetMethodOption(JavaThread* thread, JNIEnv* env, jobject method, jst
   // can't be in VM when we call JNI
   ThreadToNativeFromVM ttnfv(thread);
   const char* flag_name = env->GetStringUTFChars(name, NULL);
+  CHECK_JNI_EXCEPTION_(env, false);
   bool result =  CompilerOracle::has_option_value(mh, flag_name, *value);
   env->ReleaseStringUTFChars(name, flag_name);
   return result;
@@ -1678,6 +1691,7 @@ WB_ENTRY(jint, WB_AddCompilerDirective(JNIEnv* env, jobject o, jstring compDirec
   // can't be in VM when we call JNI
   ThreadToNativeFromVM ttnfv(thread);
   const char* dir = env->GetStringUTFChars(compDirect, NULL);
+  CHECK_JNI_EXCEPTION_(env, 0);
   int ret;
   {
     ThreadInVMfromNative ttvfn(thread); // back to VM

hotspot/src/share/vm/prims/whitebox.hpp

@@ -33,9 +33,22 @@
 #include "oops/symbol.hpp"
 #include "runtime/interfaceSupport.hpp"
 
+// Unconditionally clear pedantic pending JNI checks
+class ClearPendingJniExcCheck : public StackObj {
+private:
+  JavaThread* _thread;
+public:
+  ClearPendingJniExcCheck(JNIEnv* env) : _thread(JavaThread::thread_from_jni_environment(env)) {}
+  ~ClearPendingJniExcCheck() {
+    _thread->clear_pending_jni_exception_check();
+  }
+};
+
 // Entry macro to transition from JNI to VM state.
 
-#define WB_ENTRY(result_type, header) JNI_ENTRY(result_type, header)
+#define WB_ENTRY(result_type, header) JNI_ENTRY(result_type, header) \
+  ClearPendingJniExcCheck _clearCheck(env);
+
 #define WB_END JNI_END
 #define WB_METHOD_DECLARE(result_type) extern "C" result_type JNICALL
 

hotspot/src/share/vm/runtime/interfaceSupport.hpp

@@ -280,6 +280,7 @@ class ThreadToNativeFromVM : public ThreadStateTransition {
 
   ~ThreadToNativeFromVM() {
     trans_from_native(_thread_in_vm);
+    assert(!_thread->is_pending_jni_exception_check(), "Pending JNI Exception Check");
     // We don't need to clear_walkable because it will happen automagically when we return to java
   }
 };

hotspot/src/share/vm/runtime/thread.hpp

@@ -1542,6 +1542,9 @@ class JavaThread: public Thread {
   static ByteSize jmp_ring_offset()              { return byte_offset_of(JavaThread, _jmp_ring); }
 #endif // PRODUCT
   static ByteSize jni_environment_offset()       { return byte_offset_of(JavaThread, _jni_environment); }
+  static ByteSize pending_jni_exception_check_fn_offset() {
+    return byte_offset_of(JavaThread, _pending_jni_exception_check_fn);
+  }
   static ByteSize last_Java_sp_offset() {
     return byte_offset_of(JavaThread, _anchor) + JavaFrameAnchor::last_Java_sp_offset();
   }
@@ -1615,7 +1618,11 @@ class JavaThread: public Thread {
     assert(_jni_active_critical >= 0, "JNI critical nesting problem?");
   }
 
-  // Checked JNI, is the programmer required to check for exceptions, specify which function name
+  // Checked JNI: is the programmer required to check for exceptions, if so specify
+  // which function name. Returning to a Java frame should implicitly clear the
+  // pending check, this is done for Native->Java transitions (i.e. user JNI code).
+  // VM->Java transistions are not cleared, it is expected that JNI code enclosed
+  // within ThreadToNativeFromVM makes proper exception checks (i.e. VM internal).
   bool is_pending_jni_exception_check() const { return _pending_jni_exception_check_fn != NULL; }
   void clear_pending_jni_exception_check() { _pending_jni_exception_check_fn = NULL; }
   const char* get_pending_jni_exception_check() const { return _pending_jni_exception_check_fn; }

hotspot/test/runtime/jni/checked/TestCheckedJniExceptionCheck.java

+/*
+ * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/* @test
+ * @bug 8164086
+ * @summary regression tests for 8164086, verify correct warning from checked JNI
+ * @library /test/lib
+ * @modules java.management
+ * @run main/native TestCheckedJniExceptionCheck launch
+ */
+
+import java.util.List;
+import jdk.test.lib.process.ProcessTools;
+import jdk.test.lib.process.OutputAnalyzer;
+
+public class TestCheckedJniExceptionCheck {
+
+    static {
+        System.loadLibrary("TestCheckedJniExceptionCheck");
+    }
+
+    int callableMethodInvokeCount = 0;
+
+    static final String TEST_START           = "TEST STARTED";
+    static final String EXPECT_WARNING_START = "EXPECT_WARNING_START";
+    static final String EXPECT_WARNING_END   = "EXPECT_WARNING_END";
+
+    static final String JNI_CHECK_EXCEPTION = "WARNING in native method: JNI call made without checking exceptions when required to from";
+
+    static void printExpectWarningStart(int count) {
+        System.out.println(EXPECT_WARNING_START + " " + count);
+    }
+
+    static void printExpectWarningEnd() {
+        System.out.println(EXPECT_WARNING_END);
+    }
+
+    public TestCheckedJniExceptionCheck() {
+        initMethodIds("callableMethod", "()V",
+                      "callableNestedMethod", "(IZ)V");
+        System.out.println(TEST_START);
+    }
+
+    public void test() {
+        testSingleCallNoCheck();
+        testSingleCallCheck();
+        testSingleCallNoCheckMultipleTimes();
+
+        testMultipleCallsNoCheck();
+        testMultipleCallsCheck();
+
+        testNestedSingleCallsNoCheck();
+        testNestedSingleCallsCheck();
+        testNestedMultipleCallsNoCheck();
+        testNestedMultipleCallsCheck();
+    }
+
+    public void testSingleCallNoCheck() {
+        System.out.println("testSingleCallNoCheck start");
+        callJavaFromNative(1, false);
+        System.out.println("testSingleCallNoCheck end");
+    }
+
+    public void testSingleCallCheck() {
+        System.out.println("testSingleCallCheck start");
+        callJavaFromNative(1, true);
+        System.out.println("testSingleCallCheck end");
+    }
+
+    public void testSingleCallNoCheckMultipleTimes() {
+        System.out.println("testSingleCallNoCheckMultipleTimes start");
+        callJavaFromNative(1, false);
+        callJavaFromNative(1, false);
+        System.out.println("testSingleCallNoCheckMultipleTimes end");
+    }
+
+    public void testMultipleCallsNoCheck() {
+        System.out.println("testMultipleCallsNoCheck start");
+        printExpectWarningStart(1);
+        callJavaFromNative(2, false);
+        printExpectWarningEnd();
+        System.out.println("testMultipleCallsNoCheck end");
+    }
+
+    public void testMultipleCallsCheck() {
+        System.out.println("testMultipleCallsCheck start");
+        callJavaFromNative(2, true);
+        System.out.println("testMultipleCallsCheck end");
+    }
+
+    public void testNestedSingleCallsNoCheck() {
+        System.out.println("testNestedSingleCallsNoCheck start");
+        callNestedJavaFromNative(1, false);
+        System.out.println("testNestedSingleCallsNoCheck end");
+    }
+
+    public void testNestedSingleCallsCheck() {
+        System.out.println("testNestedSingleCallsCheck start");
+        callNestedJavaFromNative(1, true);
+        System.out.println("testNestedSingleCallsCheck end");
+    }
+
+    public void testNestedMultipleCallsNoCheck() {
+        System.out.println("testNestedMultipleCallsNoCheck start");
+        printExpectWarningStart(3);
+        callNestedJavaFromNative(2, false);
+        printExpectWarningEnd();
+        System.out.println("testNestedMultipleCallsNoCheck end");
+    }
+
+    public void testNestedMultipleCallsCheck() {
+        System.out.println("testNestedMultipleCallsCheck start");
+        callNestedJavaFromNative(2, true);
+        System.out.println("testNestedMultipleCallsCheck end");
+    }
+
+    public void callableMethod() {
+        callableMethodInvokeCount++;
+    }
+
+    public void callableNestedMethod(int nofCalls, boolean withExceptionChecks) {
+        callJavaFromNative(nofCalls, withExceptionChecks);
+    }
+
+    public native void callJavaFromNative(int nofCalls, boolean withExceptionChecks);
+
+    public native void callNestedJavaFromNative(int nofCalls, boolean withExceptionChecks);
+
+    private native void initMethodIds(String callableMethodName,
+                                      String callableMethodSig,
+                                      String callableNestedMethodName,
+                                      String callableNestedMethodSig);
+
+
+    // Check warnings appear where they should, with start/end statements in output...
+    static void checkOuputForCorrectWarnings(OutputAnalyzer oa) throws RuntimeException {
+        List<String> lines = oa.asLines();
+        int expectedWarnings = 0;
+        int warningCount = 0;
+        int lineNo = 0;
+        boolean testStartLine = false;
+        for (String line : lines) {
+            lineNo++;
+            if (!testStartLine) { // Skip any warning before the actual test itself
+                testStartLine = line.startsWith(TEST_START);
+                continue;
+            }
+            if (line.startsWith(JNI_CHECK_EXCEPTION)) {
+                if (expectedWarnings == 0) {
+                    oa.reportDiagnosticSummary();
+                    throw new RuntimeException("Unexpected warning at line " + lineNo);
+                }
+                warningCount++;
+                if (warningCount > expectedWarnings) {
+                    oa.reportDiagnosticSummary();
+                    throw new RuntimeException("Unexpected warning at line " + lineNo);
+                }
+            }
+            else if (line.startsWith(EXPECT_WARNING_START)) {
+                String countStr = line.substring(EXPECT_WARNING_START.length() + 1);
+                expectedWarnings = Integer.parseInt(countStr);
+            }
+            else if (line.startsWith(EXPECT_WARNING_END)) {
+                if (warningCount != expectedWarnings) {
+                    oa.reportDiagnosticSummary();
+                    throw new RuntimeException("Missing warning at line " + lineNo);
+                }
+                warningCount = 0;
+                expectedWarnings = 0;
+            }
+        }
+        /*
+        System.out.println("Output looks good...");
+        oa.reportDiagnosticSummary();
+        */
+    }
+
+    public static void main(String[] args) throws Throwable {
+        if (args == null || args.length == 0) {
+            new TestCheckedJniExceptionCheck().test();
+            return;
+        }
+
+        // launch and check output
+        checkOuputForCorrectWarnings(ProcessTools.executeTestJvm("-Xcheck:jni",
+                                                                  "TestCheckedJniExceptionCheck"));
+    }
+
+}

hotspot/test/runtime/jni/checked/libTestCheckedJniExceptionCheck.c

+/*
+ * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+#include <jni.h>
+
+static jmethodID _callable_method_id;
+static jmethodID _callable_nested_method_id;
+
+static void check_exceptions(JNIEnv *env) {
+  if ((*env)->ExceptionCheck(env)) {
+    (*env)->ExceptionDescribe(env);
+    (*env)->FatalError(env, "Unexpected Exception");
+  }
+}
+
+static jmethodID get_method_id(JNIEnv *env, jclass clz, jstring jname, jstring jsig) {
+  jmethodID mid;
+  const char *name, *sig;
+
+  name = (*env)->GetStringUTFChars(env, jname, NULL);
+  check_exceptions(env);
+
+  sig  = (*env)->GetStringUTFChars(env, jsig, NULL);
+  check_exceptions(env);
+
+  mid  = (*env)->GetMethodID(env, clz, name, sig);
+  check_exceptions(env);
+
+  (*env)->ReleaseStringUTFChars(env, jname, name);
+  (*env)->ReleaseStringUTFChars(env, jsig, sig);
+  return mid;
+}
+
+JNIEXPORT void JNICALL
+Java_TestCheckedJniExceptionCheck_initMethodIds(JNIEnv *env,
+                                                jobject obj,
+                                                jstring callable_method_name,
+                                                jstring callable_method_sig,
+                                                jstring callable_nested_method_name,
+                                                jstring callable_nested_method_sig) {
+  jclass clz = (*env)->GetObjectClass(env, obj);
+
+  _callable_method_id = get_method_id(env, clz,
+                                      callable_method_name,
+                                      callable_method_sig);
+
+  _callable_nested_method_id = get_method_id(env, clz,
+                                             callable_nested_method_name,
+                                             callable_nested_method_sig);
+}
+
+JNIEXPORT void JNICALL
+Java_TestCheckedJniExceptionCheck_callJavaFromNative(JNIEnv *env,
+                                                     jobject obj,
+                                                     jint nofCalls,
+                                                     jboolean checkExceptions) {
+  int i;
+  for (i = 0; i < nofCalls; i++) {
+    (*env)->CallVoidMethod(env, obj, _callable_method_id);
+    if (checkExceptions == JNI_TRUE) {
+      check_exceptions(env);
+    }
+  }
+}
+
+JNIEXPORT void JNICALL
+Java_TestCheckedJniExceptionCheck_callNestedJavaFromNative(JNIEnv *env,
+                                                           jobject obj,
+                                                           jint nofCalls,
+                                                           jboolean checkExceptions) {
+  int i;
+  for (i = 0; i < nofCalls; i++) {
+    (*env)->CallVoidMethod(env, obj, _callable_nested_method_id, nofCalls, checkExceptions);
+    if (checkExceptions == JNI_TRUE) {
+      check_exceptions(env);
+    }
+  }
+}