RFR(M): 8159335: Fix problems with stack overflow handling.
Doerr, Martin
martin.doerr at sap.com
Thu Jun 16 10:21:26 UTC 2016
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