RFR(M): 8159335: Fix problems with stack overflow handling.
Lindenmaier, Goetz
goetz.lindenmaier at sap.com
Thu Jun 16 13:04:09 UTC 2016
Hi Martin,
thanks a lot for looking at these changes.
Best regards,
Goetz.
> -----Original Message-----
> From: Doerr, Martin
> Sent: Donnerstag, 16. Juni 2016 12:21
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Coleen Phillimore
> <coleen.phillimore at oracle.com>; hotspot-runtime-dev at openjdk.java.net
> Subject: RE: RFR(M): 8159335: Fix problems with stack overflow handling.
>
> Hi Götz,
>
> I have looked at the PPC64 and AIX part. Looks good.
> Thanks for contributing.
>
> Best regards,
> Martin
>
>
> -----Original Message-----
> From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-
> bounces at openjdk.java.net] On Behalf Of Lindenmaier, Goetz
> Sent: Donnerstag, 16. Juni 2016 10:57
> To: Coleen Phillimore <coleen.phillimore at oracle.com>; hotspot-runtime-
> dev at openjdk.java.net
> Subject: RE: RFR(M): 8159335: Fix problems with stack overflow handling.
>
> Hi Colleen,
>
> thanks for looking this closely at my change. I did the below
> edits. I'll upload a new webrev once you are done reviewing, ok?
>
> Best regards,
> Goetz
>
>
> diff -r 306e3d472b61
> src/cpu/x86/vm/templateInterpreterGenerator_x86.cpp
> --- a/src/cpu/x86/vm/templateInterpreterGenerator_x86.cpp Mon
> Jun 13 09:28:25 2016 +0200
> +++ b/src/cpu/x86/vm/templateInterpreterGenerator_x86.cpp Thu Jun
> 16 10:44:01 2016 +0200
> @@ -526,11 +526,11 @@
>
> // locals + overhead, in bytes
> __ mov(rax, rdx);
> - __ shlptr(rax, Interpreter::logStackElementSize); // 2 slots per parameter.
> + __ shlptr(rax, Interpreter::logStackElementSize); // Convert parameter
> count to bytes.
> __ addptr(rax, overhead_size);
>
> #ifdef ASSERT
> - Label limit_okay, stack_size_okay;
> + Label limit_okay;
> // Verify that thread stack overflow limit is non-zero.
> __ cmpptr(stack_limit, (int32_t)NULL_WORD);
> __ jcc(Assembler::notEqual, limit_okay);
> @@ -933,7 +933,7 @@
> __ load_unsigned_short(t, Address(t,
> ConstMethod::size_of_parameters_offset()));
>
> #ifndef _LP64
> - __ shlptr(t, Interpreter::logStackElementSize);
> + __ shlptr(t, Interpreter::logStackElementSize); // Convert parameter count
> to bytes.
> __ addptr(t, 2*wordSize); // allocate two more slots for JNIEnv and
> possible mirror
> __ subptr(rsp, t);
> __ andptr(rsp, -(StackAlignmentInBytes)); // gcc needs 16 byte aligned
> stacks to do XMM intrinsics
>
>
>
>
> > -----Original Message-----
> > From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-
> > bounces at openjdk.java.net] On Behalf Of Coleen Phillimore
> > Sent: Donnerstag, 16. Juni 2016 03:09
> > To: hotspot-runtime-dev at openjdk.java.net
> > Subject: Re: RFR(M): 8159335: Fix problems with stack overflow handling.
> >
> >
> > Also,
> >
> > http://cr.openjdk.java.net/~goetz/wr16/8159335-
> >
> stkOvrflw/webrev.01/src/cpu/x86/vm/templateInterpreterGenerator_x86.c
> > pp.udiff.html
> >
> > + Label limit_okay, stack_size_okay;
> >
> >
> > stack_size_okay is unused.
> >
> > Coleen
> >
> >
> > On 6/15/16 7:33 PM, Coleen Phillimore wrote:
> > >
> > > Hi,
> > > I've reviewed this in detail for x86 (less so for the others but the
> > > concept is good). It unifies the calculation for the interpreter
> > > checking for stack overflow.
> > >
> > > - __ shlptr(rax, Interpreter::logStackElementSize); // 2 slots per
> > > parameter.
> > > + __ shlptr(rax, Interpreter::logStackElementSize); // convert
> > > parameter count to bytes
> > >
> > > There are two of these comments "2 slots per parameter" which are
> > > leftover from TaggedStackInterpreter days. Can you make this change
> > > to both?
> > >
> > > I'm implementing this on one of our closed platforms and testing now.
> > >
> > > Thanks! It's nice to make progress on these buggy and inconsistent
> > > calculations.
> > >
> > > Coleen
> > >
> > >
> > > On 6/14/16 3:47 PM, Lindenmaier, Goetz wrote:
> > >> Hi Coleen,
> > >>
> > >> Thanks for offering to sponsor this!
> > >> My last edits are really minor, see that file linked below.
> > >>
> > >> Best regards,
> > >> Goetz
> > >>
> > >>> -----Original Message-----
> > >>> From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-
> > >>> bounces at openjdk.java.net] On Behalf Of Coleen Phillimore
> > >>> Sent: Tuesday, June 14, 2016 6:07 PM
> > >>> To: hotspot-runtime-dev at openjdk.java.net
> > >>> Subject: Re: RFR(M): 8159335: Fix problems with stack overflow
> > >>> handling.
> > >>>
> > >>>
> > >>> Hi, I haven't had a chance to look at this latest change but I will
> > >>> sponsor it.
> > >>>
> > >>> thanks,
> > >>> Coleen
> > >>>
> > >>> On 6/14/16 7:51 AM, Lindenmaier, Goetz wrote:
> > >>>> Hi,
> > >>>>
> > >>>> i found vm_default_page_size() on linux/aix is unused now,
> > >>>> and stack_page_size on aix was never used. I removed this
> > >>>> dead coding and updated the webrev in-place.
> > >>>>
> > >>>> Incremental, non-aix diff:
> > >>>> http://cr.openjdk.java.net/~goetz/wr16/8159335-
> > stkOvrflw/webrev.01/src/os/linux/vm/os_linux.hpp.udiff.html
> > >>>>
> > >>>>
> > >>>> Best regards,
> > >>>> Goetz.
> > >>>>
> > >>>>> -----Original Message-----
> > >>>>> From: Lindenmaier, Goetz
> > >>>>> Sent: Montag, 13. Juni 2016 13:33
> > >>>>> To: hotspot-runtime-dev at openjdk.java.net
> > >>>>> Subject: RFR(M): 8159335: Fix problems with stack overflow
> handling.
> > >>>>>
> > >>>>> Hi,
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>> During porting JEP 270 to ppc I detected a row of bugs in the stack
> > >>> overflow
> > >>>>> coding.
> > >>>>>
> > >>>>> This change fixes these.
> > >>>>>
> > >>>>> Please review these changes. I please need a sponsor.
> > >>>>>
> > >>>>> http://cr.openjdk.java.net/~goetz/wr16/8159335-
> > >>>>> stkOvrflw/webrev.01/index.html
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>> javaCalls.cpp:
> > >>>>>
> > >>>>> Windows calls map_stack_shadow_pages() after doing
> > >>>>> os::stack_shadow_pages_available().
> > >>>>>
> > >>>>> Both functions read the stack pointer, but this might result in
> > >>>>>
> > >>>>> different sp's. In consequence, map_stack_shadow_pages() can hit
> > >>>>>
> > >>>>> a guard page although the previous check succeeded.
> > >>>>>
> > >>>>> So I pass in the sp so we can assure we use the same in both. Also,
> > >>>>>
> > >>>>> I optimized the map_stack_shadow_pages() on windows, we saw
> bad
> > >>>>>
> > >>>>> assembly for it.
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>> os_<os>.cpp:
> > >>>>>
> > >>>>> fix bug & Streamline computation of min_stack_allowed.
> > >>>>>
> > >>>>> - In some OS'es it was forgotten to consider the reserved pages.
> > >>>>>
> > >>>>> - OS'es use different multiplicators (aix: 8K, bsd: system page size,
> > >>>>>
> > >>>>> linux: 8K, solaris: system page size (often 8K), win: system
> > >>>>> page size)
> > >>>>>
> > >>>>> --> Use 8K everywhere, but I noted it down with *4K as zone
> > >>>>>
> > >>>>> sizes are also given in 4K units.
> > >>>>>
> > >>>>> - min_stack_allowed should be page aligned, else the warning is
> > >>>>>
> > >>>>> misleading.
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>> templateTable_ppc_64.cpp:
> > >>>>>
> > >>>>> Fix bug on ppc: in monitorenter() the wrong stack bang mechanism
> > >>>>>
> > >>>>> was used.
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>> globals_<cpu>.hpp:
> > >>>>>
> > >>>>> Streamline ppc stack guard sizes. The current ppc sizes are not
> > >>>>>
> > >>>>> adapted to openJdk. There SocketOutputStream.socketWrite0()
> > >>>>>
> > >>>>> uses a large buffer, so shadow pages have to be increased.
> > >>>>>
> > >>>>> This makes no big difference as many ppc systems have 64K
> > >>>>>
> > >>>>> pages, and the zone sizes are adapted to page sizes.
> > >>>>>
> > >>>>> Because zone sizes are calculated based on 4K pages now,
> > >>>>>
> > >>>>> the shadow zone on sparc has to be increased, too.
> > >>>>>
> > >>>>> Sparc uses 8K pages per default (as far as I konw),
> > >>>>>
> > >>>>> which was the old multiplier (wrong since 8139864).
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>> zone sizes:
> > >>>>>
> > >>>>> red yellow res shadow
> > >>>>>
> > >>>>> aarch 1 2 0 4+5
> > >>>>>
> > >>>>> ppc 1 6 1 6+2 --> 1 / 2 / 1 / 20+2
> > >>>>>
> > >>>>> sparc 1 2 1 10+1 --> 1 / 2 / 1 / 20+2
> > >>>>>
> > >>>>> spac32 1 2 1 3+1 --> 1 / 2 / 1 / 6+2
> > >>>>>
> > >>>>> x86_64 1 2 1 20+2
> > >>>>>
> > >>>>> x86_32 1 2 1 4+5
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>> win
> > >>>>>
> > >>>>> x86_64 1 3 0 6+2
> > >>>>>
> > >>>>> x86_32 1 3 0 4+5
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>> templateInterpreterGeneratore_<cpu>.cpp:
> > >>>>>
> > >>>>> Simplify checking whether the new frame fits on the stack.
> > >>>>>
> > >>>>> Use JavaThread->_stack_overflow_limit on all cpus. This
> > >>>>>
> > >>>>> saves several instructions.
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>> I also have to increase the stack sizes of some tests. They fail
> > >>>>> because
> > >>>>>
> > >>>>> systems with 64K pages require five of them for the shadow/guard
> > >>>>> area.
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>> I ran this in our night build for a few days, so it's tested on
> > >>>>> linuxx86_64,
> > >>>>>
> > >>>>> Ppc aix, linux and linux-le, sparc and darwinintel. Among others,
> > >>>>> all
> > >>> hotspot
> > >>>>> Jtreg tests and a lot of jck tests have been executed.
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>> Best regards,
> > >>>>>
> > >>>>> Goetz.
> > >>>>>
> > >>>>>
> > >
More information about the hotspot-runtime-dev
mailing list