RFR(S) 8140665: SIGSEGV when a primitive type's class is used as the host class in a call to DefineAnonymousClass call

[harold seigel]

Hi Coleen,

Thanks for the review.  I'll add the comment.

I don't know about future plans for accessing Unsafe.

Harold

On 3/22/2016 11:36 AM, Coleen Phillimore wrote:
>
> Hi Harold, This looks good but a single line comment that primitive 
> types have NULL Klass* fields in their java.lang.Class instances would 
> be helpful, especially if this is changed in the future.
>
> Since the hostClass is checked in the caller, my vote is to not add 
> another check in the vm.
>
> In the test, is there going to be another way to get to Unsafe in the 
> future, or present?
>
> Thanks,
> Coleen
>
>
> On 3/22/16 11:19 AM, harold seigel wrote:
>> Hi Karen,
>>
>> Thanks for looking at the changes.  The JDK caller of 
>> Unsafe_DefineAnonymousClass_impl() is in 
>> java.base/share/classes/jdk/internal/misc/Unsafe.java.  It checks for 
>> NULL args and throws NPE:
>>
>>     public Class<?> defineAnonymousClass(Class<?> hostClass, byte[] 
>> data, Object[] cpPatches) {
>>         if (hostClass == null || data == null) {
>>             throw new NullPointerException();
>>         }
>>         return defineAnonymousClass0(hostClass, data, cpPatches);
>>     }
>>
>> If somehow a null host_class was passed to 
>> Unsafe_DefineAnonymousClass_impl() then it would probably seg fault 
>> in product.  Perhaps that's okay because it is an Unsafe_... API.
>>
>> Thanks, Harold
>>
>> On 3/22/2016 10:57 AM, Karen Kinnear wrote:
>>> Harold,
>>>
>>> Thank you for fixing this. Looks good.
>>>
>>> One question -
>>>
>>> 1. If you were to pass in a null host_class in product - what happens?
>>> The way I read the resolve_non_null, it asserts in debug that you 
>>> did not pass in a null, which suggests to me that it assumes
>>> we have already made that check. I would expect and if (host_class 
>>> == NULL) || … 0> IllegalArgumentException ?
>>>
>>> thanks,
>>> Karen
>>>
>>>> On Mar 22, 2016, at 10:23 AM, harold seigel 
>>>> <harold.seigel at oracle.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> Please review this small change to fix bug 8140665.
>>>>
>>>> Instead of asserting or getting a SIGSEGV when trying to define an 
>>>> anonymous class when the host class is a primitive type, this fix 
>>>> throws an IllegalArgumentException.
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~hseigel/bug_8140665/
>>>>
>>>> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8140665
>>>>
>>>> The fix was tested with JCK Lang and VM java_lang tests, the UTE 
>>>> non-collated quick tests, and the hotspot, JDK vm, java/io, 
>>>> java/lang, java/util and JFR JTreg tests, and the test included in 
>>>> this RFR.
>>>>
>>>> Thanks, Harold
>>
>