RFR(M): 8141529: Fix handling of _JAVA_SR_SIGNUM (original) (raw)

Lindenmaier, Goetz goetz.lindenmaier at sap.com
Thu Nov 12 10:15:00 UTC 2015

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 JAVASRSIGNUM

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 JAVASRSIGNUM >> >> Hi Goetz, >> >> Adding in serviceability-dev >> >> On 9/11/2015 6:22 PM, Lindenmaier, Goetz wrote: >>> Hi, >>> >>> The environment variable JAVASRSIGNUM 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 JAVASRSIGNUM 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::setoursigflags. > 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::setoursigflags(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. >>>

More information about the hotspot-runtime-dev mailing list