changeset 48808:2b0b7f222800

8195690: JNI GetObjectRefType doesn't handle NULL Summary: Properly handle NULL, add some non-NULL preconditions. Reviewed-by: dholmes, mdoerr
author kbarrett
date Tue, 23 Jan 2018 14:27:10 -0500
parents fd8ccb37fce9
children a81c930a8838
files src/hotspot/share/gc/shared/oopStorage.cpp src/hotspot/share/gc/shared/oopStorage.hpp src/hotspot/share/prims/jni.cpp src/hotspot/share/prims/jniCheck.cpp src/hotspot/share/runtime/jniHandles.cpp src/hotspot/share/runtime/jniHandles.hpp src/hotspot/share/runtime/os.cpp
diffstat 7 files changed, 26 insertions(+), 6 deletions(-) [+]
line wrap: on
line diff
--- a/src/hotspot/share/gc/shared/oopStorage.cpp	Fri Jan 19 17:01:34 2018 +0100
+++ b/src/hotspot/share/gc/shared/oopStorage.cpp	Tue Jan 23 14:27:10 2018 -0500
@@ -467,6 +467,7 @@
 void OopStorage::release(const oop* const* ptrs, size_t size) {
   size_t i = 0;
   while (i < size) {
+    check_release_entry(ptrs[i]);
     Block* block = find_block_or_null(ptrs[i]);
     check_release(block, ptrs[i]);
     log_info(oopstorage, ref)("%s: released " PTR_FORMAT, name(), p2i(ptrs[i]));
--- a/src/hotspot/share/gc/shared/oopStorage.hpp	Fri Jan 19 17:01:34 2018 +0100
+++ b/src/hotspot/share/gc/shared/oopStorage.hpp	Tue Jan 23 14:27:10 2018 -0500
@@ -103,6 +103,7 @@
   };
 
   // Locks _allocate_mutex.
+  // precondition: ptr != NULL.
   EntryStatus allocation_status(const oop* ptr) const;
 
   // Allocates and returns a new entry.  Returns NULL if memory allocation
--- a/src/hotspot/share/prims/jni.cpp	Fri Jan 19 17:01:34 2018 +0100
+++ b/src/hotspot/share/prims/jni.cpp	Tue Jan 23 14:27:10 2018 -0500
@@ -862,7 +862,10 @@
 
   HOTSPOT_JNI_GETOBJECTREFTYPE_ENTRY(env, obj);
 
-  jobjectRefType ret = JNIHandles::handle_type(thread, obj);
+  jobjectRefType ret = JNIInvalidRefType;
+  if (obj != NULL) {
+    ret = JNIHandles::handle_type(thread, obj);
+  }
 
   HOTSPOT_JNI_GETOBJECTREFTYPE_RETURN((void *) ret);
   return ret;
--- a/src/hotspot/share/prims/jniCheck.cpp	Fri Jan 19 17:01:34 2018 +0100
+++ b/src/hotspot/share/prims/jniCheck.cpp	Tue Jan 23 14:27:10 2018 -0500
@@ -435,7 +435,7 @@
 }
 
 oop jniCheck::validate_handle(JavaThread* thr, jobject obj) {
-  if (JNIHandles::handle_type(thr, obj) != JNIInvalidRefType) {
+  if ((obj != NULL) && (JNIHandles::handle_type(thr, obj) != JNIInvalidRefType)) {
     ASSERT_OOPS_ALLOWED;
     return JNIHandles::resolve_external_guard(obj);
   }
--- a/src/hotspot/share/runtime/jniHandles.cpp	Fri Jan 19 17:01:34 2018 +0100
+++ b/src/hotspot/share/runtime/jniHandles.cpp	Tue Jan 23 14:27:10 2018 -0500
@@ -136,6 +136,7 @@
 }
 
 oop JNIHandles::resolve_jweak(jweak handle) {
+  assert(handle != NULL, "precondition");
   assert(is_jweak(handle), "precondition");
   oop result = jweak_ref(handle);
 #if INCLUDE_ALL_GCS
@@ -147,6 +148,7 @@
 }
 
 bool JNIHandles::is_global_weak_cleared(jweak handle) {
+  assert(handle != NULL, "precondition");
   assert(is_jweak(handle), "not a weak handle");
   return jweak_ref(handle) == NULL;
 }
@@ -200,6 +202,7 @@
 
 
 jobjectRefType JNIHandles::handle_type(Thread* thread, jobject handle) {
+  assert(handle != NULL, "precondition");
   jobjectRefType result = JNIInvalidRefType;
   if (is_jweak(handle)) {
     if (is_storage_handle(_weak_global_handles, &jweak_ref(handle))) {
@@ -232,6 +235,7 @@
 
 
 bool JNIHandles::is_local_handle(Thread* thread, jobject handle) {
+  assert(handle != NULL, "precondition");
   JNIHandleBlock* block = thread->active_handles();
 
   // Look back past possible native calls to jni_PushLocalFrame.
@@ -249,22 +253,25 @@
 // We easily can't isolate any particular stack frame the handle might
 // come from, so we'll check the whole stack.
 
-bool JNIHandles::is_frame_handle(JavaThread* thr, jobject obj) {
+bool JNIHandles::is_frame_handle(JavaThread* thr, jobject handle) {
+  assert(handle != NULL, "precondition");
   // If there is no java frame, then this must be top level code, such
   // as the java command executable, in which case, this type of handle
   // is not permitted.
   return (thr->has_last_Java_frame() &&
-         (void*)obj < (void*)thr->stack_base() &&
-         (void*)obj >= (void*)thr->last_Java_sp());
+         (void*)handle < (void*)thr->stack_base() &&
+         (void*)handle >= (void*)thr->last_Java_sp());
 }
 
 
 bool JNIHandles::is_global_handle(jobject handle) {
+  assert(handle != NULL, "precondition");
   return !is_jweak(handle) && is_storage_handle(_global_handles, &jobject_ref(handle));
 }
 
 
 bool JNIHandles::is_weak_global_handle(jobject handle) {
+  assert(handle != NULL, "precondition");
   return is_jweak(handle) && is_storage_handle(_weak_global_handles, &jweak_ref(handle));
 }
 
@@ -603,6 +610,7 @@
 }
 
 bool JNIHandleBlock::any_contains(jobject handle) {
+  assert(handle != NULL, "precondition");
   for (JNIHandleBlock* current = _block_list; current != NULL; current = current->_block_list_link) {
     if (current->contains(handle)) {
       return true;
--- a/src/hotspot/share/runtime/jniHandles.hpp	Fri Jan 19 17:01:34 2018 +0100
+++ b/src/hotspot/share/runtime/jniHandles.hpp	Tue Jan 23 14:27:10 2018 -0500
@@ -94,8 +94,9 @@
   static void print_on(outputStream* st);
   static void print()           { print_on(tty); }
   static void verify();
+  // The category predicates all require handle != NULL.
   static bool is_local_handle(Thread* thread, jobject handle);
-  static bool is_frame_handle(JavaThread* thr, jobject obj);
+  static bool is_frame_handle(JavaThread* thread, jobject handle);
   static bool is_global_handle(jobject handle);
   static bool is_weak_global_handle(jobject handle);
   static size_t global_handle_memory_usage();
@@ -106,6 +107,7 @@
   static bool is_local_handle(jobject handle);
 #endif
 
+  // precondition: handle != NULL.
   static jobjectRefType handle_type(Thread* thread, jobject handle);
 
   // Garbage collection support(global handles only, local handles are traversed from thread)
--- a/src/hotspot/share/runtime/os.cpp	Fri Jan 19 17:01:34 2018 +0100
+++ b/src/hotspot/share/runtime/os.cpp	Tue Jan 23 14:27:10 2018 -0500
@@ -987,6 +987,11 @@
 // The verbose parameter is only set by the debug code in one case
 void os::print_location(outputStream* st, intptr_t x, bool verbose) {
   address addr = (address)x;
+  // Handle NULL first, so later checks don't need to protect against it.
+  if (addr == NULL) {
+    st->print_cr("0x0 is NULL");
+    return;
+  }
   CodeBlob* b = CodeCache::find_blob_unsafe(addr);
   if (b != NULL) {
     if (b->is_buffer_blob()) {