RFR(S): 8156992: [ppc] Implement template interpreter stack overflow checks as on x86/sparc.

[Lindenmaier, Goetz]

Hi Dan, 

Thanks for spotting this! 

Sorry and best regards,
  Goetz.

> -----Original Message-----
> From: Daniel D. Daugherty [mailto:daniel.daugherty at oracle.com]
> Sent: Thursday, May 19, 2016 7:19 PM
> To: dean.long at oracle.com; Lindenmaier, Goetz
> <goetz.lindenmaier at sap.com>; hotspot-runtime-dev at openjdk.java.net
> Subject: Re: RFR(S): 8156992: [ppc] Implement template interpreter stack
> overflow checks as on x86/sparc.
> 
> The correct bug ID for this email thread is:
> 
> JDK-8156922 [ppc] Implement template interpreter stack overflow checks
> as on x86/sparc
> https://bugs.openjdk.java.net/browse/JDK-8156922
> 
> when this fix reaches the commit phase, please make sure
> that 8156922 is used instead of 8156992.
> 
> Dan
> 
> 
> On 5/19/16 11:13 AM, dean.long at oracle.com wrote:
> > Hi Goetz.  Thanks for the explanation.  So using the shadow size in
> > MAX2 is so that the
> > stack bang that comes next does not "overshoot" the red zone and stomp
> > on memory
> > that might be mapped for other purposes?  That makes sense if
> > bang_stack_shadow_pages()
> > was only banging one page, but when called from
> > generate_normal_entry(), "native_call" is
> > set to false, so we bang from 1 to n_shadow_pages.  If I'm now
> > understanding the code
> > correctly, there may be an opportunity for a performance improvement
> > here, by banging
> > just 1 page instead of n_shadow_pages.
> >
> > I reread the description in 8156922, and I'm having troubling
> > following the note that
> > starts with "The x86/sparc design can not handle frames bigger than
> > ...."  Is the that
> > "overshoot" problem I described above?
> >
> >
> > thanks,
> >
> > dl
> >
> >
> > On 5/19/16 6:38 AM, Lindenmaier, Goetz wrote:
> >>
> >> Hi,
> >>
> >> reading the mail again I think I understand your question better now:
> >>
> >> I think this is because we first want to push the frame of the called
> >> method
> >>
> >> and then do the real stack bang.
> >>
> >> But at least it must be checked that this frame fits on the stack
> >> below the yellow zone.
> >>
> >> Obviously the person who implemented this also wanted to make sure
> >> that a
> >>
> >> stack bang on top of this frame does not exceed stack_end(),
> >> therefore the MAX2.
> >>
> >> If it does not fit, a stack overflow error is thrown without going
> >> through the
> >>
> >> signal handler - only the space of the shadow zone is used.
> >>
> >> If the frame fits, the real stack bang is executed. The frames below
> >> are all fine, thus
> >>
> >> the signal handler and code on top can walk the stack.
> >>
> >> Best regards,
> >>
> >> Goetz.
> >>
> >> *From:*dean.long at oracle.com [mailto:dean.long at oracle.com]
> >> *Sent:* Thursday, May 19, 2016 2:15 AM
> >> *To:* Lindenmaier, Goetz <goetz.lindenmaier at sap.com>;
> >> hotspot-runtime-dev at openjdk.java.net
> >> *Subject:* Re: RFR(S): 8156992: [ppc] Implement template interpreter
> >> stack overflow checks as on x86/sparc.
> >>
> >> By changing the value of the _stack_overflow_limit field, does the
> >> following x86/sparc comment
> >> still make sense?
> >>
> >> // TODO: rather than touching all pages, check against
> >> stack_overflow_limit and bang yellow page to
> >> // generate exception
> >>
> >> Also, I have to admit that I don't understand why we use different
> >> values for stack checks/bang in different places, especially the
> >> using MAX2 instead of +.  Can anyone explain this?  The + makes sense
> >> to me, but the MAX2 does not.
> >>
> >> dl
> >>
> >> On 5/18/16 3:02 AM, Lindenmaier, Goetz wrote:
> >>
> >>     Hi,
> >>
> >>     When porting the template interpreter, we implemented a different
> >> approach to
> >>
> >>     Stack overflow handling.  See also the detailed description in
> >> the Jira bug.
> >>
> >>     This change implements the stack overflow check as on x86/sparc.
> >>
> >>     It requires simple shared changes, but only to code only used on
> >> ppc.
> >>
> >>     The changes should not affect the other platforms.
> >>
> >>     Please review this change. I please need a sponsor.
> >>
> >> http://cr.openjdk.java.net/~goetz/wr16/8156922-
> ppcStackFix/webrev.01/
> >> <http://cr.openjdk.java.net/%7Egoetz/wr16/8156922-
> ppcStackFix/webrev.01/>
> >>
> >>     Best regards,
> >>
> >>        Goetz.
> >>
> >
> >