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

[harold seigel]

Thanks Karen!

Harold

On 3/22/2016 4:20 PM, Karen Kinnear wrote:
> Agree with Coleen on both points.
>
> thanks,
> Karen
>
>> On Mar 22, 2016, at 11:59 AM, harold seigel <harold.seigel at oracle.com> wrote:
>>
>> 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