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

[Gilles Duboscq]

Hi all,

David, in general, I understand your concerns regarding the cost of a
check that will only be useful for incorrect usages.
For this precise case, I think the exception can be useful for
developers using JNI or Unsafe while the cost of the check is probably
quite small compared to the other checks already in place.

Also, the code produced by C2's intrinsic for Unsafe.allocateInstance
contains the same additional check.

-Gilles

On Fri, Jan 10, 2014 at 8:59 AM, David Holmes <david.holmes at oracle.com> 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