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

[Christian Thalinger]

Looks good.  Reviewed.

On Jan 17, 2014, at 9:24 AM, Coleen Phillimore <coleen.phillimore at oracle.com> wrote:

> 
> 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
>