Review request (S): 6539281 -Xcheck:jni should validate char* argument to ReleaseStringUTFChars

[Staffan Larsen]

Dmitry,

1) Yes, I agree. But for ReleaseStringUTFChars a similar check would require a UTF conversion which I'd like to avoid. And I prefer to have similar code in both the UTF and non-UTF case. It has been suggested that the similiar code should be abstracted into some helper method, but I can't figure a way that doesn't make the code more complex and harder to read.

2) Any ideas?

3) Yes, "chars" should definitely be NULL-checked.

Thanks,
/Staffan

On 2 dec 2010, at 18.12, Dmitry Samersoff wrote:

> Staffan,
> 
> 1. Logically string argument of GetStringChars and ReleaseStringChars have to be the same.
>  So:
>    checked_jni_ReleaseStringChars:
>       chars_to_check = GetStringChars(env,str,isCopy);
>       memcmp(chars,chars_to_check, len > 10 ? 10 : len);
> 
> could be a better approach.
> 
> BUT:
> 
> 2.
> As far as I know GetStringChars do alloc/memcpy inside it
> Could we avoid extra copying?
> 
> 3.
> Code below:
> jint *tagLocation = ((jint*) chars) - 1;
> 
> Could lead to cryptic crash e.g. if we pass 0 as a char (common case) to this code we will have a crash on read from 0xFFFFFFFF rather than much more clean crash on zero-access. So either gurantee chars != 0 have to be there or tag should be placed at the end of chars, after terminating zero.
> 
> -Dmitry
> 
> 
> 
> On 2010-12-02 17:18, Staffan Larsen wrote:
>> http://cr.openjdk.java.net/~sla/6539281/webrev.00/
>> 
>> Validate that ReleaseStringUTFChars/ReleaseStringChars is called with
>> something allocated by GetStringUTChars/GetStringChars when running with
>> -Xcheck:jni. This is accomplished by adding a well-known tag in the
>> memory immediately before the pointer that is returned to the user. This
>> tag is verified in ReleaseStringUTFChars.
>> 
>> Thanks,
>> 
>> /Staffan
>> 
> 
> 
> -- 
> Dmitry Samersoff
> J2SE Sustaining team, SPB04
> * Give Rabbit time and he'll always get the answer ...