JDK-6311046 : -Xcheck:jni should support checking of GetPrimitiveArrayCritical
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 6
  • Priority: P3
  • Status: Closed
  • Resolution: Fixed
  • OS: solaris_9
  • CPU: sparc
  • Submitted: 2005-08-15
  • Updated: 2016-06-30
  • Resolved: 2014-06-11
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 8 JDK 9
8u40Fixed 9 b22Fixed
Related Reports
Relates :  
JDK-6300721 - Creating Animated GIF images repeatedly with different image frames crashes the VM
Relates :  
JDK-8160614 - Some of the checked JNI improvements are excessively verbose
Relates :  
JDK-6532325 - VM crashes after setting file associations in Eclipse 3.2
Relates :  
JDK-8046662 - Check JNI ReleaseStringChars / ReleaseStringUTFChars verify_guards test inverted
Description
While debugging 6300721 I discovered that user code was walking off the end on a Java array being accessed using GetPrimitiveArrayCritical.  Our implementation returns a pointer to the interior of the array so walking off the end stomps the java heap.  It's permissible to return a copy and I think with -Xcheck:jni that's what we should do.  We should return a chunk of memory with a guard region at each end and when ReleasePrimitiveArrayCritical is called the guard regions should be checked to see they are overwritten.
We should also do this for Release/Get*ArrayElements.  It would have been useful in diagnosing 6532325 as a JNI problem and not a JVM problem.
Comments
Fix is already backported to 8. Whether it goes back to 6 and 7 is a matter for sustaining.
30-10-2014
Don't usually back-port enhancements...
30-10-2014
Does the fix need to be backported to JDK 8, 7, 6?
30-10-2014
EVALUATION While fixing this, we should also add a check to see if a weakReference was passed in, this from CR: 6345487, please see comments for this.
10-11-2006
EVALUATION This is an extremely useful debugging feature, and must be implemented in dolphin. Tbanks to the submitter the code sketch is in the suggested fix field.
10-11-2006
SUGGESTED FIX Here's sample code implementing this fix. It needs some cleanup but illustrates the fix. JNI_ENTRY(void*, jni_GetPrimitiveArrayCritical(JNIEnv *env, jarray array, jboolean *isCopy)) JNIWrapper("GetPrimitiveArrayCritical"); HS_DTRACE_PROBE3(hotspot_jni, GetPrimitiveArrayCritical_entry, env, array, isCopy); GC_locker::lock_critical(thread); if (isCopy != NULL) { *isCopy = JNI_TRUE; } oop a = JNIHandles::resolve_non_null(array); assert(a->is_array(), "just checking"); BasicType type; if (a->is_objArray()) { type = T_OBJECT; } else { type = typeArrayKlass::cast(a->klass())->type(); } void* ret = arrayOop(a)->base(type); int size = a->size() * HeapWordSize - 8; int* value = (int*)malloc(size + 8 * BytesPerWord); value[0] = 0xabababab; value[1] = 0xabababab; value[2] = 0xabababab; value[3] = 0xabababab; value[size / 4 + 4 + 0] = 0xabababab; value[size / 4 + 4 + 1] = 0xabababab; value[size / 4 + 4 + 2] = 0xabababab; value[size / 4 + 4 + 3] = 0xabababab; memcpy(value + 4, ret, size); assert(value[0] == 0xabababab, "must be"); assert(value[1] == 0xabababab, "must be"); assert(value[2] == 0xabababab, "must be"); assert(value[3] == 0xabababab, "must be"); assert(value[size / 4 + 4 + 0] == 0xabababab, "must be"); assert(value[size / 4 + 4 + 1] == 0xabababab, "must be"); assert(value[size / 4 + 4 + 2] == 0xabababab, "must be"); assert(value[size / 4 + 4 + 3] == 0xabababab, "must be"); HS_DTRACE_PROBE1(hotspot_jni, GetPrimitiveArrayCritical_return, ret); return value + 4; JNI_END HS_DTRACE_PROBE_DECL4(hotspot_jni, ReleasePrimitiveArrayCritical_entry, JNIEnv*, jarray, void*, jint); HS_DTRACE_PROBE_DECL(hotspot_jni, ReleasePrimitiveArrayCritical_return); JNI_ENTRY(void, jni_ReleasePrimitiveArrayCritical(JNIEnv *env, jarray array, void *carray, jint mode)) JNIWrapper("ReleasePrimitiveArrayCritical"); HS_DTRACE_PROBE4(hotspot_jni, ReleasePrimitiveArrayCritical_entry, env, array, carray, mode); oop a = JNIHandles::resolve_non_null(array); assert(a->is_array(), "just checking"); BasicType type; if (a->is_objArray()) { type = T_OBJECT; } else { type = typeArrayKlass::cast(a->klass())->type(); } void* ret = arrayOop(a)->base(type); int size = a->size() * HeapWordSize - 8; int* value = ((int*)carray) - 4; assert(value[0] == 0xabababab, "must be"); assert(value[1] == 0xabababab, "must be"); assert(value[2] == 0xabababab, "must be"); assert(value[3] == 0xabababab, "must be"); assert(value[size / 4 + 4 + 0] == 0xabababab, "must be"); assert(value[size / 4 + 4 + 1] == 0xabababab, "must be"); assert(value[size / 4 + 4 + 2] == 0xabababab, "must be"); assert(value[size / 4 + 4 + 3] == 0xabababab, "must be"); memcpy(ret, value + 4, size); free(value); // The array, carray and mode arguments are ignored GC_locker::unlock_critical(thread); HS_DTRACE_PROBE(hotspot_jni, ReleasePrimitiveArrayCritical_return); JNI_END
15-08-2005