RFR(M): 8141529: Fix handling of _JAVA_SR_SIGNUM

[David Holmes]

Hi Goetz,

On 13/11/2015 5:24 AM, Lindenmaier, Goetz wrote:
> Hi David, Dmitry,
>
> I've come up with a new webrev:
> http://cr.openjdk.java.net/~goetz/webrevs/8141529-NSIG/webrev.01/
>
> I hit on some more issues:

That's what happens when you pull on a loose thread :)

>   - As proposed, I replaced MAXSIGNUM by NSIG
>   - On AIX, NSIG=255.  Therefore storing bits in a word does not work.
>      I'm now using bitset functionality from signal.h as it's done in other places.

Okay that's nice and neat.

>     sigset_t is >> NSIG on linux, so it's no good idea to use it there.

That's unfortunate. I had a momentary vision of a single POSIX based 
shared implementation. The size may not be such a big deal - and we 
already use a number of sigset_t variables on linux. Future cleanup perhaps?

>   - In the os files I found another bit vector that now is too small: sigs.
>     I adapted that, too.  Removed the dead declaration of this on solaris.

Okay.

This all seems okay to me.

Thanks,
David

> Best regards,
>    Goetz.
>
>> -----Original Message-----
>> From: Dmitry Samersoff [mailto:dmitry.samersoff at oracle.com]
>> Sent: Donnerstag, 12. November 2015 10:05
>> To: Lindenmaier, Goetz; David Holmes; hotspot-runtime-
>> dev at openjdk.java.net; serviceability-dev
>> Subject: Re: RFR(M): 8141529: Fix handling of _JAVA_SR_SIGNUM
>>
>> Goetz,
>>
>> *BSD including OS X also defines NSIG (just checked) and if my memory is
>> not bogus, AIX defines it too.
>>
>> So you may consider to use NSIG on all platform.
>>
>> -Dmitry
>>
>> On 2015-11-12 11:36, Lindenmaier, Goetz wrote:
>>> OK I'll change it to NSIG.  That's used in other places in os_linux, too.
>>> So it's really more consistent.
>>>
>>> Best regards,
>>>    Goetz
>>>
>>>> -----Original Message-----
>>>> From: Dmitry Samersoff [mailto:dmitry.samersoff at oracle.com]
>>>> Sent: Donnerstag, 12. November 2015 09:22
>>>> To: David Holmes; Lindenmaier, Goetz; hotspot-runtime-
>>>> dev at openjdk.java.net; serviceability-dev
>>>> Subject: Re: RFR(M): 8141529: Fix handling of _JAVA_SR_SIGNUM
>>>>
>>>> David,
>>>>
>>>> I think it's better to use NSIG (without underscore) defined in signal.h
>>>>
>>>> -Dmitry
>>>>
>>>>
>>>> On 2015-11-12 10:35, David Holmes wrote:
>>>>> 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 :) ).
>>>>>
>>>>> On Linux you didn't add the bounds check to os::Linux::set_our_sigflags.
>>>>>
>>>>> 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?
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>> Best regards,
>>>>>>     Goetz.
>>>>>>
>>>>
>>>>
>>>> --
>>>> Dmitry Samersoff
>>>> Oracle Java development team, Saint Petersburg, Russia
>>>> * I would love to change the world, but they won't give me the sources.
>>
>>
>> --
>> Dmitry Samersoff
>> Oracle Java development team, Saint Petersburg, Russia
>> * I would love to change the world, but they won't give me the sources.