RFR(M): 8141529: Fix handling of _JAVA_SR_SIGNUM (original) (raw)
David Holmes david.holmes at oracle.com
Thu Nov 12 08:29:09 UTC 2015
- Previous message: RFR(M): 8141529: Fix handling of _JAVA_SR_SIGNUM
- Next message: RFR(M): 8141529: Fix handling of _JAVA_SR_SIGNUM
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
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::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.
- Previous message: RFR(M): 8141529: Fix handling of _JAVA_SR_SIGNUM
- Next message: RFR(M): 8141529: Fix handling of _JAVA_SR_SIGNUM
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]