6739363: Xcheck jni doesn't check native function arguments
Summary: Fix adds support for verifying arguments with -Xcheck:jni.
Reviewed-by: coleenp
--- a/hotspot/src/os/windows/vm/os_windows.cpp Fri Oct 31 10:34:20 2008 -0700
+++ b/hotspot/src/os/windows/vm/os_windows.cpp Thu Dec 04 17:29:56 2008 -0800
@@ -2217,15 +2217,10 @@
// 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 @@
// 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);
--- a/hotspot/src/share/vm/includeDB_core Fri Oct 31 10:34:20 2008 -0700
+++ b/hotspot/src/share/vm/includeDB_core Thu Dec 04 17:29:56 2008 -0800
@@ -2303,6 +2303,7 @@
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
--- a/hotspot/src/share/vm/includeDB_features Fri Oct 31 10:34:20 2008 -0700
+++ b/hotspot/src/share/vm/includeDB_features Thu Dec 04 17:29:56 2008 -0800
@@ -115,6 +115,8 @@
heapInspection.cpp os.hpp
heapInspection.cpp resourceArea.hpp
+javaCalls.cpp jniCheck.hpp
+
jniCheck.cpp fieldDescriptor.hpp
jniCheck.cpp handles.hpp
jniCheck.cpp instanceKlass.hpp
--- a/hotspot/src/share/vm/prims/jniCheck.cpp Fri Oct 31 10:34:20 2008 -0700
+++ b/hotspot/src/share/vm/prims/jniCheck.cpp Thu Dec 04 17:29:56 2008 -0800
@@ -112,18 +112,6 @@
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);
--- a/hotspot/src/share/vm/prims/jniCheck.hpp Fri Oct 31 10:34:20 2008 -0700
+++ b/hotspot/src/share/vm/prims/jniCheck.hpp Thu Dec 04 17:29:56 2008 -0800
@@ -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
//
--- a/hotspot/src/share/vm/runtime/javaCalls.cpp Fri Oct 31 10:34:20 2008 -0700
+++ b/hotspot/src/share/vm/runtime/javaCalls.cpp Thu Dec 04 17:29:56 2008 -0800
@@ -309,8 +309,12 @@
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 @@
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 @@
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 @@
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 @@
// 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
--- a/hotspot/src/share/vm/runtime/javaCalls.hpp Fri Oct 31 10:34:20 2008 -0700
+++ b/hotspot/src/share/vm/runtime/javaCalls.hpp Thu Dec 04 17:29:56 2008 -0800
@@ -150,7 +150,7 @@
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
--- a/hotspot/src/share/vm/runtime/sharedRuntime.cpp Fri Oct 31 10:34:20 2008 -0700
+++ b/hotspot/src/share/vm/runtime/sharedRuntime.cpp Thu Dec 04 17:29:56 2008 -0800
@@ -556,7 +556,10 @@
// 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 @@
}
} 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 @@
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();
}