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.

Reply via email to