RFR(M): 8141529: Fix handling of _JAVA_SR_SIGNUM

[Lindenmaier, Goetz]

Hi,

I'm not sure whether it's ideal to couple an internal data size to some
constant defined on the system.  But I'll change it.
To make sure nothing breaks, I will add guard
#if (64 < NSIG-1)
#error "Not all signals can be encoded in jvmsigs. Adapt its type!"
#endif
to jsig.c.

Best regards,
  Goetz.


> -----Original Message-----
> From: David Holmes [mailto:david.holmes at oracle.com]
> Sent: Donnerstag, 12. November 2015 09:29
> To: Lindenmaier, Goetz; hotspot-runtime-dev at openjdk.java.net;
> serviceability-dev
> Subject: Re: RFR(M): 8141529: Fix handling of _JAVA_SR_SIGNUM
> 
> Hi Goetz,
> 
> On 12/11/2015 6:22 PM, Lindenmaier, Goetz wrote:
> > Hi David,
> >
> > thanks for looking at this!
> >
> >> -----Original Message-----
> >> From: David Holmes [mailto:david.holmes at oracle.com]
> >> Sent: Donnerstag, 12. November 2015 08:35
> >> To: Lindenmaier, Goetz; hotspot-runtime-dev at openjdk.java.net;
> >> serviceability-dev
> >> Subject: Re: RFR(M): 8141529: Fix handling of _JAVA_SR_SIGNUM
> >>
> >> Hi Goetz,
> >>
> >> Adding in serviceability-dev
> >>
> >> On 9/11/2015 6:22 PM, Lindenmaier, Goetz wrote:
> >>> Hi,
> >>>
> >>> The environment variable _JAVA_SR_SIGNUM can be set to a signal
> >> number
> >>> do be used by the JVM's suspend/resume mechanism.
> >>>
> >>> If set, a signal handler is installed and the current signal handler is saved
> to
> >> an array.
> >>> On linux, this array had size MAXSIGNUM=32, and _JAVA_SR_SIGNUM
> was
> >> allowed
> >>> to range up to _NSIG=65. This could cause memory corruption.
> >>>
> >>> Further, in jsig.c, an unsinged int is used to set a bit for signals. This also
> >>> is too small, as only 32 signals can be supported.  Further, the signals are
> >> mapped
> >>> wrong to these bits.  '0' is not a valid signal, but '32' was.  1<<32 happens
> to
> >> map to
> >>> zero, so the signal could be stored, but this probably was not intended
> that
> >> way.
> >>>
> >>> This change increases MAXSIGNUM to 65 on linux, and to 64 on aix. It
> >> introduces
> >>> proper checking of the signal read from the env var, and issues a warning
> if
> >> it
> >>> does not use the signal set.  It adapts the data types in jisig.c properly.
> >>>
> >>> Please review this change.  I please need a sponsor.
> >>> http://cr.openjdk.java.net/~goetz/webrevs/8141529-NSIG/webrev.00
> >>
> >> This all sounds very good to me. (I must find out why Solaris is not
> >> involved here :) ).
> > The mechanism is not implemented there.  Why, I don't know.
> >
> >> On Linux you didn't add the bounds check to os::Linux::set_our_sigflags.
> > It came with 8140482 http://hg.openjdk.java.net/jdk9/hs-
> rt/hotspot/rev/cd86b5699825
> 
> That change isn't present in your new code:
> 
> void os::Linux::set_our_sigflags(int sig, int flags) {
>    assert(sig > 0 && sig < MAXSIGNUM, "vm signal out of expected range");
>    sigflags[sig] = flags;
> }
> 
> does it need to be re-based?
> 
> >> I'm also wondering about documenting where we are determining the
> >> maximum from? Is it simply _NSIG on some/all distributions? And I see
> >> _NSIG is supposed to be the biggest signal number + one. Also linux
> >> defines NSIG = _NSIG so which should we be using?
> > I checked a row of linux distributions and Versions and always found
> NSIG=65.
> > Also, as we compile NSIG into the code, we can not react on a difference
> > between systems. So I guess that's fine.
> 
> So why do we need MAXSIGNUM instead of just using NSIG ?
> 
> > As NSIG = _NSIG, I don't really care which we use.  I chose _NSIG because
> > it was used before.  On the other platforms I only found NSIG in the header
> > files.
> 
> I'd switch to NSIG as Dmitry suggested as it is the public value in
> signal.h.
> 
> Thanks,
> David
> 
> > Best regards,
> >    Goetz
> >
> >
> >>
> >> Thanks,
> >> David
> >>
> >>> Best regards,
> >>>     Goetz.
> >>>