RFR (S): 8210242: vmTestbase/nsk/stress/jni/jnistress001.java crashes with EXCEPTION_ACCESS_VIOLATION on windows-x86

[David Holmes]

Thanks Harold!

David

On 25/10/2018 10:50 PM, Harold David Seigel wrote:
> +1
> 
> Thanks, Harold
> 
> 
> On 10/25/2018 7:00 AM, Lois Foltan wrote:
>> Looks good David.
>> Lois
>>
>> On 10/25/2018 2:03 AM, David Holmes wrote:
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8210242
>>> webrev: http://cr.openjdk.java.net/~dholmes/8210242/webrev/
>>>
>>> The fundamental bug here was that a character array was copied 
>>> without attempting to copy**/add a NUL terminating character so it 
>>> formed a valid C string (char*). The array was passed to printf %s 
>>> and by good fortune happened to have a NUL 99% of the time, 
>>> occasionally had a junk character or two that would at least print 
>>> okay on Linux etc but once in a blue moon would trigger an 
>>> EXCEPTION_ACCESS_VIOLATION on windows-x86.
>>>
>>> ** There's actually no guarantee the original array was itself 
>>> NUL-terminated (hotspot does do so but the JNI spec does not require 
>>> it) - but that's a cleanup for later - see below.
>>>
>>> The fix I chose for that was to use %.*s and pass the actual length. 
>>> The main reason I chose that was to reinforce that arrays need not be 
>>> NUL-terminated and we should get out of that mind set when working 
>>> with Java Strings and JNI. In doing that I factored an expression:
>>>
>>> javachars->size[index]
>>>
>>> (sometimes expressed as index-1 due to index being sneakily 
>>> incremented in the middle of the code), into a local variable elem_len.
>>>
>>> In addition, due to thinking it may be the cause of the problem, I 
>>> introduced a utility function c_malloc to check malloc does not 
>>> return NULL and to fail if it does. So all mallocs became c_malloc 
>>> calls.
>>>
>>> There is a lot of potential cleanup possible in this test and the 
>>> others in Testbase/nsk/stress/jni/ but I had neither the time nor the 
>>> inclination to clean up what is in places truly awful code. So I 
>>> filed a follow RFE for someone else to do that cleanup:
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8212961
>>>
>>> Testing:
>>>   - local testing showed corrupted names before the fix and no 
>>> corruption afterwards
>>>   - Windows specific testing showed 1 in 50 crash before and no 
>>> crashes in 210 after
>>>
>>> Thanks,
>>> David
>>
>