RFR (S) JDK-6311046: -Xcheck:jni should support checking of GetPrimitiveArrayCritical

[David Simms]

Updated web-review:

http://cr.openjdk.java.net/~dsimms/6311046/rev4/

  * Renamed FencedMemory to GuardedMemory, inner class to "Guard"
  * Changed FENCE_SIZE from const to enum.
  * Fix code example in header doc.


On 6/05/2014 1:31 p.m., David Simms wrote:
>
> Whoops, but there is a problem with the example, the decl does take in 
> the size:
>
> static size_t get_total_size(size_t user_size)
>
> The example should read:
>
>   60 * <code>
>   61  *   Thing* alloc_thing() {
>   62  *    void* mem = 
> FencedMemory::get_total_size(alloc_fn(sizeof(thing)));
>   63  *    FencedMemory fenced(mem, sizeof(thing));
>   64  *    return (Thing*) fenced.get_user_ptr();
>   65  *   }
>   66  * </code>
>
> Will update, thanks !
>
>
> On 6/05/2014 1:18 p.m., David Simms wrote:
>>
>> Reply in-line...
>>
>>
>> On 6/05/2014 12:51 p.m., Florian Weimer wrote:
>>> On 05/05/2014 01:37 PM, David Simms wrote:
>>>> Gidday all:
>>>>
>>>> Bug/Enhancement: https://bugs.openjdk.java.net/browse/JDK-6311046
>>>>
>>>> Web review: http://cr.openjdk.java.net/~dsimms/6311046/rev2/
>>>>
>>>> Cleaned up the "hand rolled" memory bounds checking in
>>>> os::malloc/realloc/free and type checking in checked JNI (GetString*),
>>>> and unified into a single helper class "FencedMemory". Added some 
>>>> extra
>>>> checks to checked JNI (release mode).
>>>
>>> The name is a bit unfortunate because it's not related to memory 
>>> fences at all. :-)
>>>
>>
>> Can change the name (see earlier reply to D.Holmes).
>>
>>>   60 * <code>
>>>   61  *   Thing* alloc_thing() {
>>>   62  *    void* mem = alloc_fn(sizeof(thing) + 
>>> FencedMemory::get_total_size());
>>>   63  *    FencedMemory fenced(mem, sizeof(thing));
>>>   64  *    return (Thing*) fenced.get_user_ptr();
>>>   65  *   }
>>>   66  * </code>
>>>
>>> I don't like this interface because it pushes overflow checking into 
>>> client code.  There is get_total_size(size_t) which asserts on 
>>> wraparound, which seems marginally saner. Something that performs 
>>> the allocation internally would better, but this would seem to need 
>>> a callback.
>>>
>>
>> If you look at the use-cases, os::malloc and checked JNI, there are 
>> different methods of allocation. It was totally my intention to limit 
>> the functionality to bounds checking and allow users to define their 
>> own allocation and failure handling. In the spirit of "Tell, don't 
>> ask" design pattern, I intended the code to be unit-test friendly 
>> (e.g.: test_fenced_memory()).
>>
>>> Technicaly, FencedMemory::Fence::FENCE_SIZE needs a definition.  You 
>>> could avoid this by using an enum constant.
>>>
>> Noted, will update.
>