RFR(XS): 8232921: assert(is_object_aligned(result)) failed: address not aligned

[Lindenmaier, Goetz]

Hi Coleen, 

ok, i'll use 01.
I'll push it right away.

Thanks for reviewing!

@Tom, thanks for analyzing this in first place!

Best regards,
  Goetz.

> -----Original Message-----
> From: coleen.phillimore at oracle.com <coleen.phillimore at oracle.com>
> Sent: Donnerstag, 31. Oktober 2019 16:43
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-runtime-
> dev at openjdk.java.net
> Subject: Re: RFR(XS): 8232921: assert(is_object_aligned(result)) failed: address
> not aligned
> 
> 
> 
> On 10/31/19 9:20 AM, Lindenmaier, Goetz wrote:
> > Hi Coleen,
> >
> >>> java_lang_Boolean::get_TRUE is not working during the initialization
> >>> because the offsets are not yet set:
> java_lang_Boolean::_static_TRUE_offset
> >>> is zero.
> >> I don't understand this sentence.  The patch 02 looks like it
> >> initializes it at the right place.  It doesn't work?
> > Yes, patch 02 works. I just wanted to reason why I'm not using
> > get_TRUE()... just ignore ...
> >
> > Another possibility, much more local, and using Boolean::TRUE, is this:
> > http://cr.openjdk.java.net/~goetz/wr19/8232921-fix_hidden_frames/03/
> > It uses _method as a fallback during initialization. Actually, this should never
> > happen with the current VM. It does not throw exceptions with hidden
> frames
> > during initialization. I only ran into this during testing when I forced all
> > exceptions to call get_TRUE.
> > I still think 01 is the most simple :).
> 
> I definitely don't like 03.  I think you should go with 01.  The
> comments are good.
> Coleen
> 
> >
> >> On second thought, I think I agree with you.  01 looks better, and if
> >> you can add the comment in BacktraceBuilder, it would be fine.
> > 01 with the comment added:
> > http://cr.openjdk.java.net/~goetz/wr19/8232921-fix_hidden_frames/01a/
> >
> > Best regards,
> >    Goetz.
> >
> >
> >> thanks,
> >> Coleen
> >>
> >>> webrev 01 passed our testing tonight.
> >>>
> >>> Best regards,
> >>>     Goetz.
> >>>
> >>>> I don't think there's any downside to
> >>>> putting the methods array there, but it's really mind blowing.
> >>>>
> >>>> Sorry I didn't notice this in the review.  I should have looked more
> >>>> closely when I saw the PauseNoSafepointVerifier.
> >>>>
> >>>>      // GC unsafe class for assigning backtrace elements.  Allocation is
> >>>> only allowed
> >>>>      // in expand().
> >>>>      NoSafepointVerifier _nsv;
> >>>>
> >>>> You could add a comment like this.
> >>>>
> >>>> Coleen
> >>>>
> >>>> On 10/29/19 10:32 AM, Lindenmaier, Goetz wrote:
> >>>>> Hi,
> >>>>>
> >>>>> Originally, I wanted to model indicating that frames
> >>>>> have been dropped nicely in Java using
> >>>>> a meaningful java object. Exceptions in hidden frames
> >>>>> should be rare, so allocating only in that case should have
> >>>>> been cheap. Unfortunately, I can not allocate here.
> >>>>> Also, Boolean::TRUE is not necessarily initialized properly.
> >>>>>
> >>>>> Actually, putting the modeling aside, I only need any legal
> >>>>> oop for this field. It is only tested for null later on.
> >>>>> So the most simple (and cheap) fix is just reusing
> >>>>> one of the oops in the array at hand.
> >>>>>
> >>>>> So please review this fix.
> >>>>> http://cr.openjdk.java.net/~goetz/wr19/8232921-
> fix_hidden_frames/01/
> >>>>>
> >>>>> I'll run it through our testing tonight.
> >>>>>
> >>>>> Best regards,
> >>>>>      Goetz.