RFR(S): 8156922: [ppc] Implement template interpreter stack overflow checks as on x86/sparc.
Lindenmaier, Goetz
goetz.lindenmaier at sap.com
Fri May 20 07:28:51 UTC 2016
Hi Dean,
> OK, that helps, but also reinforces the idea that this code is more
> complicated that I thought.
Yes :)
> Let me ask a simpler question. What happens if this first pointer
> compare check simply uses
> stack_guard_zone_size() instead of MAX2(stack_shadow_zone_size(),
> stack_guard_zone_size())?
> It seems the worst that can happen is that the first check succeeds more
> often, so we detect the stack
> overflow with the stack bang and the signal handler instead.
I think the second check, the banging, might not touch all pages in
that case. But I think this could also happen if the frame >> shadow zone,
as it's pushed already.
I think a good implementation in the template interpreter would be
- do all the checking before pushing the frame
- first compare whether sp+newframe < stack_end + guardzone + shadowzone,
i.e., a bang would hit the yellow zone.
- if the compare fails, push a dummy frame, and touch the yellow page --> signal handler
- if the compare succeeds, bang the pages so they are committed (might only be needed
on windows.)
This would give a similar behavior as in the compiler. Also, it makes it more easy to
document and configure the different stack zones if it's handled the same everywhere.
But actually, I wondered about the implementation and had hoped to get an
answer to this on the mailing list. Also, this is an interesting issue, but not
topic of my change :)
Best regards,
Goetz.
> dl
>
> On 5/19/16 12:34 PM, Lindenmaier, Goetz wrote:
> > Hi Dean,
> >
> >> 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.
> > As I understand, we need to bang the space for the new frame on top of
> the old shadow
> > zone. The old (caller's) shadow zone is 'clean'. Not so if there are native
> frames below
> > us, they did not bang. So in that case we have to bang all pages
> (shadow+new frame). See also
> > AbstractAssembler::generate_stack_overflow_check().
> > But it's not handled consistently. It would be good if
> > AbstractAssembler::generate_stack_overflow_check()
> > (and only that) was called from the template interpreter.
> >
> >> 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.
> > One bang could always skip over the protected zones. If you use a
> compare, you could
> > get along with one compare.
> > But there is also the issue that the pages have to be commited, so
> > you need to touch all of them. Else you think there
> > is stack, and then you get out of memory or the like. That's not supposed
> to happen.
> >
> >> 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?
> > That's for the first check for only the frame. Assume
> yellow=red=reserved=shadow=1page=4K.
> > The MAX2 is the 12K, leaving just the shadow zone=4k in the first check.
> > Only if the frame is < 4K you will get the real overflow handling through
> signal handler
> > and utilizing the yellow zone / reserved zone after the stack bang. If the
> frame
> > is 5K you run into the special overflow at the beginning.
> >
> > Best regards,
> > Goetz.
> >
> >
> > 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/
> >
> >
> >
> > Best regards,
> >
> > Goetz.
> >
> >
> >
More information about the hotspot-runtime-dev
mailing list