RFR(XS): 8031427: AllocObject and Unsafe.allocateInstance segfault for primitive types

[Coleen Phillimore]

I see no problem with this change.  The spec says that we don't check 
for programming errors, but it doesn't say we must not check for 
programming errors.   We can decide case by case whether we want to add 
helpful checks that don't degrade performance, and this is one such 
check that would be useful.   I have reviewed it and will sponsor it for 
you.   It needs another reviewer.

Thanks,
Coleen


On 1/10/2014 2:59 AM, David Holmes wrote:
> Hi Gilles,
>
> On 9/01/2014 10:58 PM, Gilles Duboscq wrote:
>> Hello,
>>
>> As i wrote in a bug report [1]:
>> Currently, calling AllocObject or Unsafe.allocateInstance with a
>> primitive type (e.g. unsafe.allocateInstance(int.class)) leads to a
>> segmentation fault because Classes representing primitive types do not
>> have an associated Klass.
>>
>> While AllocObject or allocateInstance should in principle never be
>> called with a Class representing a primitive, it should probably not
>> crash in such a case since it throws sensible exceptions for many other
>> invalid arguments (interfaces, abstract types, arrays, j.l.Class).
>
> As per the bug report both Unsafe and JNI require the caller to pass 
> the correct parameters to a method and if they do not then crashes are 
> entirely possible. The intent is to avoid penalizing correct code with 
> unnecessary checks.
>
> The existing checks (check_valid_for_instantiation) arise simply from 
> the fact that the underlying allocation routine is used by other parts 
> of the VM that do require more extensive checks.
>
>> The proposed fix [2] just throws an InstantiationException if the Klass
>> is NULL.
>
> While this may seem quite innocuous I am concerned about setting a 
> precedent for "fixing" unsafe/JNI when used incorrectly.
>
> David
>
>> -Gilles
>>
>> [1] https://bugs.openjdk.java.net/browse/JDK-8031427
>> [2] http://lafo.ssw.uni-linz.ac.at/alloc_object.diff