6739363: Xcheck jni doesn't check native function arguments

Summary: Fix adds support for verifying arguments with -Xcheck:jni.
Reviewed-by: coleenp

src/os/windows/vm/os_windows.cpp

@@ -2217,15 +2217,10 @@ LONG WINAPI topLevelExceptionFilter(struct _EXCEPTION_POINTERS* exceptionInfo) {
                 // We only expect null pointers in the stubs (vtable)
                 // the rest are checked explicitly now.
                 //
-                CodeBlob* cb = CodeCache::find_blob(pc);
-                if (cb != NULL) {
-                  if (VtableStubs::stub_containing(pc) != NULL) {
-                    if (((uintptr_t)addr) < os::vm_page_size() ) {
-                      // an access to the first page of VM--assume it is a null pointer
-                      return Handle_Exception(exceptionInfo,
-                        SharedRuntime::continuation_for_implicit_exception(thread, pc, SharedRuntime::IMPLICIT_NULL));
-                    }
-                  }
+                if (((uintptr_t)addr) < os::vm_page_size() ) {
+                  // an access to the first page of VM--assume it is a null pointer
+                  address stub = SharedRuntime::continuation_for_implicit_exception(thread, pc, SharedRuntime::IMPLICIT_NULL);
+                  if (stub != NULL) return Handle_Exception(exceptionInfo, stub);
                 }
               }
             } // in_java
@@ -2241,9 +2236,8 @@ LONG WINAPI topLevelExceptionFilter(struct _EXCEPTION_POINTERS* exceptionInfo) {
             // Windows 98 reports faulting addresses incorrectly
             if (!MacroAssembler::needs_explicit_null_check((intptr_t)addr) ||
                 !os::win32::is_nt()) {
-
-              return Handle_Exception(exceptionInfo,
-                  SharedRuntime::continuation_for_implicit_exception(thread, pc, SharedRuntime::IMPLICIT_NULL));
+              address stub = SharedRuntime::continuation_for_implicit_exception(thread, pc, SharedRuntime::IMPLICIT_NULL);
+              if (stub != NULL) return Handle_Exception(exceptionInfo, stub);
             }
             report_error(t, exception_code, pc, exceptionInfo->ExceptionRecord,
                          exceptionInfo->ContextRecord);

src/share/vm/includeDB_core

@@ -2303,6 +2303,7 @@ javaCalls.cpp                           handles.inline.hpp
 javaCalls.cpp                           interfaceSupport.hpp
 javaCalls.cpp                           interpreter.hpp
 javaCalls.cpp                           javaCalls.hpp
+javaCalls.cpp                           jniCheck.hpp
 javaCalls.cpp                           linkResolver.hpp
 javaCalls.cpp                           mutexLocker.hpp
 javaCalls.cpp                           nmethod.hpp

src/share/vm/includeDB_features

@@ -115,6 +115,8 @@ heapInspection.cpp                      klassOop.hpp
 heapInspection.cpp                      os.hpp
 heapInspection.cpp                      resourceArea.hpp
 
+javaCalls.cpp                           jniCheck.hpp
+
 jniCheck.cpp                            fieldDescriptor.hpp
 jniCheck.cpp                            handles.hpp
 jniCheck.cpp                            instanceKlass.hpp

src/share/vm/prims/jniCheck.cpp

@@ -112,18 +112,6 @@ static const char * fatal_instance_field_mismatch = "Field type (instance) misma
 static const char * fatal_non_string = "JNI string operation received a non-string";
 
 
-
-// Report a JNI failure caught by -Xcheck:jni.  Perform a core dump.
-// Note: two variations -- one to be called when in VM state (e.g. when
-// within IN_VM macro), one to be called when in NATIVE state.
-
-// When in VM state:
-static void ReportJNIFatalError(JavaThread* thr, const char *msg) {
-  tty->print_cr("FATAL ERROR in native method: %s", msg);
-  thr->print_stack();
-  os::abort(true);
-}
-
 // When in VM state:
 static void ReportJNIWarning(JavaThread* thr, const char *msg) {
   tty->print_cr("WARNING in native method: %s", msg);

src/share/vm/prims/jniCheck.hpp

@@ -22,6 +22,19 @@
  *
  */
 
+extern "C" {
+  // Report a JNI failure caught by -Xcheck:jni.  Perform a core dump.
+  // Note: two variations -- one to be called when in VM state (e.g. when
+  // within IN_VM macro), one to be called when in NATIVE state.
+
+  // When in VM state:
+  static void ReportJNIFatalError(JavaThread* thr, const char *msg) {
+    tty->print_cr("FATAL ERROR in native method: %s", msg);
+    thr->print_stack();
+    os::abort(true);
+  }
+}
+
 //
 // Checked JNI routines that are useful for outside of checked JNI
 //

src/share/vm/runtime/javaCalls.cpp

@@ -309,8 +309,12 @@ void JavaCalls::call_helper(JavaValue* result, methodHandle* m, JavaCallArgument
 
   CHECK_UNHANDLED_OOPS_ONLY(thread->clear_unhandled_oops();)
 
-  // Make sure that the arguments have the right type
-  debug_only(args->verify(method, result->get_type(), thread));
+  // Verify the arguments
+
+  if (CheckJNICalls)  {
+    args->verify(method, result->get_type(), thread);
+  }
+  else debug_only(args->verify(method, result->get_type(), thread));
 
   // Ignore call if method is empty
   if (method->is_empty_method()) {
@@ -431,24 +435,26 @@ intptr_t* JavaCallArguments::parameters() {
   return TaggedStackInterpreter ? _parameters : _value;
 }
 
-//--------------------------------------------------------------------------------------
-// Non-Product code
-#ifndef PRODUCT
 
 class SignatureChekker : public SignatureIterator {
  private:
    bool *_is_oop;
    int   _pos;
    BasicType _return_type;
+   intptr_t*   _value;
+   Thread* _thread;
 
  public:
   bool _is_return;
 
-  SignatureChekker(symbolHandle signature, BasicType return_type, bool is_static, bool* is_oop) : SignatureIterator(signature) {
+  SignatureChekker(symbolHandle signature, BasicType return_type, bool is_static, bool* is_oop, intptr_t* value, Thread* thread) : SignatureIterator(signature) {
     _is_oop = is_oop;
     _is_return = false;
     _return_type = return_type;
     _pos = 0;
+    _value = value;
+    _thread = thread;
+
     if (!is_static) {
       check_value(true); // Receiver must be an oop
     }
@@ -489,6 +495,24 @@ class SignatureChekker : public SignatureIterator {
       check_return_type(t);
       return;
     }
+
+    // verify handle and the oop pointed to by handle
+    int p = _pos;
+    bool bad = false;
+    // If argument is oop
+    if (_is_oop[p]) {
+      intptr_t v = _value[p];
+      if (v != 0 ) {
+        size_t t = (size_t)v;
+        bad = (t < (size_t)os::vm_page_size() ) || !(*(oop*)v)->is_oop_or_null(true);
+        if (CheckJNICalls && bad) {
+          ReportJNIFatalError((JavaThread*)_thread, "Bad JNI oop argument");
+        }
+      }
+      // for the regular debug case.
+      assert(!bad, "Bad JNI oop argument");
+    }
+
     check_value(true);
   }
 
@@ -505,6 +529,7 @@ class SignatureChekker : public SignatureIterator {
   void do_array(int begin, int end)    { check_obj(T_OBJECT);        }
 };
 
+
 void JavaCallArguments::verify(methodHandle method, BasicType return_type,
   Thread *thread) {
   guarantee(method->size_of_parameters() == size_of_parameters(), "wrong no. of arguments pushed");
@@ -515,10 +540,9 @@ void JavaCallArguments::verify(methodHandle method, BasicType return_type,
   // Check that oop information is correct
   symbolHandle signature (thread,  method->signature());
 
-  SignatureChekker sc(signature, return_type, method->is_static(),_is_oop);
+  SignatureChekker sc(signature, return_type, method->is_static(),_is_oop, _value, thread);
   sc.iterate_parameters();
   sc.check_doing_return(true);
   sc.iterate_returntype();
 }
 
-#endif // PRODUCT

src/share/vm/runtime/javaCalls.hpp

@@ -150,7 +150,7 @@ class JavaCallArguments : public StackObj {
   int   size_of_parameters() const { return _size; }
 
   // Verify that pushed arguments fits a given method
-  void verify(methodHandle method, BasicType return_type, Thread *thread) PRODUCT_RETURN;
+  void verify(methodHandle method, BasicType return_type, Thread *thread);
 };
 
 // All calls to Java have to go via JavaCalls. Sets up the stack frame

src/share/vm/runtime/sharedRuntime.cpp

@@ -556,7 +556,10 @@ address SharedRuntime::continuation_for_implicit_exception(JavaThread* thread,
           // the caller was at a call site, it's safe to destroy all
           // caller-saved registers, as these entry points do.
           VtableStub* vt_stub = VtableStubs::stub_containing(pc);
-          guarantee(vt_stub != NULL, "unable to find SEGVing vtable stub");
+
+          // If vt_stub is NULL, then return NULL to signal handler to report the SEGV error.
+          if (vt_stub == NULL) return NULL;
+
           if (vt_stub->is_abstract_method_error(pc)) {
             assert(!vt_stub->is_vtable_stub(), "should never see AbstractMethodErrors from vtable-type VtableStubs");
             return StubRoutines::throw_AbstractMethodError_entry();
@@ -565,7 +568,9 @@ address SharedRuntime::continuation_for_implicit_exception(JavaThread* thread,
           }
         } else {
           CodeBlob* cb = CodeCache::find_blob(pc);
-          guarantee(cb != NULL, "exception happened outside interpreter, nmethods and vtable stubs (1)");
+
+          // If code blob is NULL, then return NULL to signal handler to report the SEGV error.
+          if (cb == NULL) return NULL;
 
           // Exception happened in CodeCache. Must be either:
           // 1. Inline-cache check in C2I handler blob,
@@ -574,7 +579,7 @@ address SharedRuntime::continuation_for_implicit_exception(JavaThread* thread,
 
           if (!cb->is_nmethod()) {
             guarantee(cb->is_adapter_blob(),
-                      "exception happened outside interpreter, nmethods and vtable stubs (2)");
+                      "exception happened outside interpreter, nmethods and vtable stubs (1)");
             // There is no handler here, so we will simply unwind.
             return StubRoutines::throw_NullPointerException_at_call_entry();
           }