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

[Lindenmaier, Goetz]

Hi Coleen, 

thanks for looking at this and going through the long Mail thread!
I also read your other comments. 
Altogether I understand the following:

- This change is ok for now to get ppc to the same implementation 
   as the other platforms.
- stack_overflow_limit should be used on the other platforms, too.
   I'll make a follow up change for this. It's basically an optimization of 
   the checking and does not change behavior, so it should be fine 
   to do in jdk9.  I also found some other small optimizations of the 
   overflow checking in our code I'd like to include in that change.
- A compare is needed in the template interpreter because frames
  can be bigger than the protected zones.  This is not the case for
  compiled frames, where the compiler tailors the banging code
  for the frame size.
- The compare + banging in the template interpreter should be
   done before pushing the frame, and both should go through
   the signal handler to have similar behavior as in the compilers.
   This is a change that should go to jdk10.

While going through all our stack banging modifications I stumbled 
over another issue.  SocketOutputStream.socketWrite0()
(jdk/src/java.base/unix/native/libnet/SocketOutputStream.c)
pushes a 65K buffer on the stack. I think we saw failing jck tests because
this reaches past all the stack protection zones.  We first increased
the ShadowZone, which lead to overhead in banging. We then
optimized banging but last reduced the size of that buffer to 8K.
Have you ever seen problems with that?

Best regards,
  Goetz.







> -----Original Message-----
> From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-
> bounces at openjdk.java.net] On Behalf Of Coleen Phillimore
> Sent: Wednesday, June 01, 2016 2:43 AM
> To: hotspot-runtime-dev at openjdk.java.net
> Subject: Re: [ping] RFR(S): 8156992: [ppc] Implement template interpreter
> stack overflow checks as on x86/sparc.
> 
> 
> 
> On 5/30/16 7:23 PM, David Holmes wrote:
> > On 30/05/2016 5:28 PM, Lindenmaier, Goetz wrote:
> >> Hi David,
> >>
> >>> -----Original Message-----
> >>> From: David Holmes [mailto:david.holmes at oracle.com]
> >>> Sent: Samstag, 28. Mai 2016 00:10
> >>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-
> runtime-
> >>> dev at openjdk.java.net; Frederic Parain <frederic.parain at oracle.com>
> >>> Subject: Re: [ping] RFR(S): 8156992: [ppc] Implement template
> >>> interpreter
> >>> stack overflow checks as on x86/sparc.
> >>>
> >>> On 28/05/2016 1:35 AM, Lindenmaier, Goetz wrote:
> >>>> Hi David,
> >>>>
> >>>> well, yes, Dean had similar questions.
> >>>> X86/sparc load stack_base and stack_size, compute stack_end and
> then
> >>> add the
> >>>> Max(shadow, quard). All in assembly in the template interpreter
> >>>> prolog.
> >>> On ppc,
> >>>> we load it from the precomputed field stackoverflow_limit.  The
> >>>> value in
> >>> that field
> >>>> differed from what x86/sparc computed, which is the error I want to
> >>>> fix
> >>> here.
> >>>
> >>> That all sounds a bit broken. :(
> >> Broken as far as my change is concerned, or broken wrt. to the existing
> >> implementation?
> >
> > The existing implementation.
> >
> >>>> I adapted the drawing to the change of that field.
> >>> I still don't see how the drawing matches the code given the MAX usage.
> >> If guard_zones > shadow zone, stack_overflow_limit points to
> >> stack_reserved_zone_base().
> >> If guard_zones < shadow zone, stack_overflow_limit points to
> >> somewhere in the shadow zone.
> >
> > Ah! I was using frames view on the webrev and the "somewhere in here"
> > comment was off the page.
> 
> I'm having trouble with the same picture too.
> 
> The generate_stack_overflow_check() should be checking that the frame
> fits under the stack_guard_zone or stack_reserved_zone_base. Or what
> used to be yellow+red zones.
> 
> Why is it
> 
>    // Use the bigger size for banging.
>    const int max_bang_size =
> (int)MAX2(JavaThread::stack_shadow_zone_size(),
> JavaThread::stack_guard_zone_size());
> 
> 
> Which would put the pointer for _stack_overflow_limit in the "somewhere
> in here" place.  But I don't get it.
> 
> This comment and variable name is wrong in the x86 code, I think, but
> this isn't your problem.
> 
> The code looks good, but we might need a comment.   And we should file
> an RFE to use Thread::_stack_overflow_limit for the other platforms.
> 
> Coleen
> >
> > Thanks,
> > David
> >
> >> Best regards,
> >>   Goetz.
> >>
> >>> I can't convince myself that any of this stuff is correct. Given
> >>> Frederic was working in this area most recently I wonder if he can
> >>> comment (but I know he may not be around at the moment).
> >>>
> >>> Thanks,
> >>> David
> >>>
> >>>> Best regards,
> >>>>   Goetz
> >>>>
> >>>>
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: David Holmes [mailto:david.holmes at oracle.com]
> >>>>> Sent: Donnerstag, 26. Mai 2016 00:36
> >>>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-
> runtime-
> >>>>> dev at openjdk.java.net
> >>>>> Subject: Re: [ping] RFR(S): 8156992: [ppc] Implement template
> >>>>> interpreter
> >>>>> stack overflow checks as on x86/sparc.
> >>>>>
> >>>>> Hi Goetz,
> >>>>>
> >>>>> I know you were having discussions with Dean in relation to this
> >>>>> but I
> >>>>> really don't understand the change to the diagram in thread.hpp
> >>>>> nor the
> >>>>> change to the definition of stackoverflow_limit. To me the limit
> >>>>> has to
> >>>>> be the sum of the guard zone and shadow zone. With your MAX
> change
> >>> the
> >>>>> diagram doesn't even make sense AFAICS.
> >>>>>
> >>>>> ??
> >>>>>
> >>>>> David
> >>>>>
> >>>>> On 25/05/2016 6:07 PM, Lindenmaier, Goetz wrote:
> >>>>>> Hi,
> >>>>>>
> >>>>>> could someone else have a look at this change?  I need a reviewer,
> >>> please.
> >>>>>> I also please need a sponsor.
> >>>>>>
> >>>>>> Best regards,
> >>>>>>   Goetz.
> >>>>>>
> >>>>>>> -----Original Message-----
> >>>>>>> From: Lindenmaier, Goetz
> >>>>>>> Sent: Mittwoch, 18. Mai 2016 12:02
> >>>>>>> To: hotspot-runtime-dev at openjdk.java.net
> >>>>>>> Subject: RFR(S): 8156992: [ppc] Implement template interpreter
> >>>>>>> stack
> >>>>>>> overflow checks as on x86/sparc.
> >>>>>>>
> >>>>>>> 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/
> >>>>>>>
> >>>>>>> Best regards,
> >>>>>>>   Goetz.
> >>>>>>