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.hol...@oracle.com] > Sent: Donnerstag, 12. November 2015 09:29 > To: Lindenmaier, Goetz; hotspot-runtime-...@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.hol...@oracle.com] > >> Sent: Donnerstag, 12. November 2015 08:35 > >> To: Lindenmaier, Goetz; hotspot-runtime-...@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. > >>>
