RFR(M): 8141529: Fix handling of _JAVA_SR_SIGNUM

[David Holmes]

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 _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.
>>>