Hi David, Dmitry,
I've come up with a new webrev:
http://cr.openjdk.java.net/~goetz/webrevs/8141529-NSIG/webrev.01/
I hit on some more issues:
- As proposed, I replaced MAXSIGNUM by NSIG
- On AIX, NSIG=255. Therefore storing bits in a word does not work.
I'm now using bitset functionality from signal.h as it's done in other
places.
sigset_t is >> NSIG on linux, so it's no good idea to use it there.
- In the os files I found another bit vector that now is too small: sigs.
I adapted that, too. Removed the dead declaration of this on solaris.
Best regards,
Goetz.
> -----Original Message-----
> From: Dmitry Samersoff [mailto:dmitry.samers...@oracle.com]
> Sent: Donnerstag, 12. November 2015 10:05
> To: Lindenmaier, Goetz; David Holmes; hotspot-runtime-
> d...@openjdk.java.net; serviceability-dev
> Subject: Re: RFR(M): 8141529: Fix handling of _JAVA_SR_SIGNUM
>
> Goetz,
>
> *BSD including OS X also defines NSIG (just checked) and if my memory is
> not bogus, AIX defines it too.
>
> So you may consider to use NSIG on all platform.
>
> -Dmitry
>
> On 2015-11-12 11:36, Lindenmaier, Goetz wrote:
> > OK I'll change it to NSIG. That's used in other places in os_linux, too.
> > So it's really more consistent.
> >
> > Best regards,
> > Goetz
> >
> >> -----Original Message-----
> >> From: Dmitry Samersoff [mailto:dmitry.samers...@oracle.com]
> >> Sent: Donnerstag, 12. November 2015 09:22
> >> To: David Holmes; Lindenmaier, Goetz; hotspot-runtime-
> >> d...@openjdk.java.net; serviceability-dev
> >> Subject: Re: RFR(M): 8141529: Fix handling of _JAVA_SR_SIGNUM
> >>
> >> David,
> >>
> >> I think it's better to use NSIG (without underscore) defined in signal.h
> >>
> >> -Dmitry
> >>
> >>
> >> On 2015-11-12 10:35, David Holmes wrote:
> >>> 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 :) ).
> >>>
> >>> On Linux you didn't add the bounds check to os::Linux::set_our_sigflags.
> >>>
> >>> 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?
> >>>
> >>> Thanks,
> >>> David
> >>>
> >>>> Best regards,
> >>>> Goetz.
> >>>>
> >>
> >>
> >> --
> >> Dmitry Samersoff
> >> Oracle Java development team, Saint Petersburg, Russia
> >> * I would love to change the world, but they won't give me the sources.
>
>
> --
> Dmitry Samersoff
> Oracle Java development team, Saint Petersburg, Russia
> * I would love to change the world, but they won't give me the sources.
