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

[David Simms]

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.