[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Re: RFR(S) 8140665: SIGSEGV when a primitive type's class is used as the host class in a call to Def
From:       harold seigel <harold.seigel () oracle ! com>
Date:       2016-03-22 20:20:48
Message-ID: 56F1A920.6090508 () oracle ! com
[Download RAW message or body]

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


[prev in list] [next in list] [prev in thread] [next in thread]