[PATCH] 8170639: [Linux] jsig is limited to a maximum of 64 signals

[David Holmes]

On 18/12/2018 9:57 am, Ao Qi wrote:
> Hi,
> 
> On Tue, Dec 18, 2018 at 4:32 AM David Holmes <david.holmes at oracle.com> wrote:
>>
>> On 17/12/2018 9:48 pm, Ao Qi wrote:
>>> Hi David Holmes,
>>>
>>> Thanks for your reply. It makes sense. Is it possible to create
>>> another bug ID to fix the problem?
>>
>> I'd prefer to see this fixed properly than point fixed on linux for an
>> unsupported platform.
> 
> I thought the problem is "[Linux] jsig is limited to a maximum of 64
> signals" and the patch was intended to fixed this problem. It just
> didn't use "A better solution", but I think it is better than the
> current situation.

Better for you but I would prefer to see an overall better solution. It 
really shouldn't be very difficult. Compare the AIX version with your 
improved Linux version, reconcile any differences, and make it a shared 
posix version, then confirm it works on OS X and Solaris.

> Is zero/mips (or all platforms having more than 64 signals) a
> unsupported platform?

Yes. We make allowances for Zero as it can support a vast range of 
platforms for which no native OpenJDK port exists (in the case of MIPS 
that port seems inactive), but in general we don't go out of our way to 
make changes in the main code that only benefit such platforms.

I'm subtly trying to coerce you into making the change I'd like to see 
here. ;-) I can assist with build/test on other POSIX platforms.

Cheers,
David

> Cheers,
> Ao Qi
>>
>> Thanks,
>> David
>>
>>> Cheers,
>>> Ao Qi
>>>
>>> On Mon, Dec 17, 2018 at 7:30 PM David Holmes <david.holmes at oracle.com> wrote:
>>>>
>>>> Thanks for reminding me about the rest of the discussion. The point of
>>>> the RFE I filed was to examine if we could get a consolidated POSIX
>>>> version using sigset_t. I'd rather see that than a point fix for Linux.
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>> On 17/12/2018 6:50 pm, Ao Qi wrote:
>>>>> Hi David Holmes,
>>>>>
>>>>> I am not sure what is the specific issue of "NSIG not being defined".
>>>>> Do you mean the issue mentioned in "but I found that NSIG was missing
>>>>> from signal.h on some architectures, mips being among them"[1]? If
>>>>> yes, James Cowgill answered this in [2]. I check it on MIPS, NSIG is
>>>>> defined in signal.h and it is 128.
>>>>>
>>>>> Cheers,
>>>>> Ao Qi
>>>>>
>>>>> [1] http://mail.openjdk.java.net/pipermail/hotspot-dev/2016-November/025401.html
>>>>> [2] http://mail.openjdk.java.net/pipermail/hotspot-dev/2016-December/025416.html
>>>>>
>>>>> On Mon, Dec 17, 2018 at 8:02 AM David Holmes <david.holmes at oracle.com> wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Was the issue of NSIG not being defined resolved?
>>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>>
>>>>>> On 14/12/2018 10:23 pm, Ao Qi wrote:
>>>>>>> Hi all,
>>>>>>>
>>>>>>> Zero does not build on Linux MIPS (actually all Linux of NSIG > 64)
>>>>>>> since OpenJDK 9, because NISG is 128 on MIPS and can not be encoded in
>>>>>>> uint64_t sigs. James Cowgill from Debian tried to push a patch fixing
>>>>>>> this[1] but it seems he failed. I would like to try again to request
>>>>>>> fixing this in http://hg.openjdk.java.net/jdk/jdk. This was fixed in
>>>>>>> AIX implementation.
>>>>>>>
>>>>>>> After applying this patch, zero can be built on MIPS (Debian testing).
>>>>>>> I've run the jtreg tests [2] on x86 Linux.
>>>>>>>
>>>>>>> [1] http://mail.openjdk.java.net/pipermail/hotspot-dev/2016-November/025399.html
>>>>>>> [2] https://download.java.net/openjdk/testresults/12/docs/howtoruntests.html
>>>>>>>
>>>>>>> Cheers,
>>>>>>> Ao Qi
>>>>>>>
>>>>>>> $ hg export -r 52869
>>>>>>> # HG changeset patch
>>>>>>> # User aoqi
>>>>>>> # Date 1544089853 0
>>>>>>> #      Thu Dec 06 09:50:53 2018 +0000
>>>>>>> # Node ID 3eea22b79dc3a4bf26616bde1acb587e3f56e6fe
>>>>>>> # Parent  b4982a22926b4ddf1a7b1f770e4d42ce8c1dd575
>>>>>>> 8170639: [Linux] jsig is limited to a maximum of 64 signals
>>>>>>>
>>>>>>> diff -r b4982a22926b -r 3eea22b79dc3 src/hotspot/os/linux/os_linux.cpp
>>>>>>> --- a/src/hotspot/os/linux/os_linux.cpp Thu Dec 06 11:54:39 2018 +0530
>>>>>>> +++ b/src/hotspot/os/linux/os_linux.cpp Thu Dec 06 09:50:53 2018 +0000
>>>>>>> @@ -4434,10 +4434,7 @@
>>>>>>>
>>>>>>>      // For signal-chaining
>>>>>>>      struct sigaction sigact[NSIG];
>>>>>>> -uint64_t sigs = 0;
>>>>>>> -#if (64 < NSIG-1)
>>>>>>> -#error "Not all signals can be encoded in sigs. Adapt its type!"
>>>>>>> -#endif
>>>>>>> +sigset_t sigs;
>>>>>>>      bool os::Linux::libjsig_is_loaded = false;
>>>>>>>      typedef struct sigaction *(*get_signal_t)(int);
>>>>>>>      get_signal_t os::Linux::get_signal_action = NULL;
>>>>>>> @@ -4516,7 +4513,7 @@
>>>>>>>      }
>>>>>>>
>>>>>>>      struct sigaction* os::Linux::get_preinstalled_handler(int sig) {
>>>>>>> -  if ((((uint64_t)1 << (sig-1)) & sigs) != 0) {
>>>>>>> +  if (sigismember(&sigs, sig)) {
>>>>>>>          return &sigact[sig];
>>>>>>>        }
>>>>>>>        return NULL;
>>>>>>> @@ -4525,7 +4522,7 @@
>>>>>>>      void os::Linux::save_preinstalled_handler(int sig, struct sigaction& oldAct) {
>>>>>>>        assert(sig > 0 && sig < NSIG, "vm signal out of expected range");
>>>>>>>        sigact[sig] = oldAct;
>>>>>>> -  sigs |= (uint64_t)1 << (sig-1);
>>>>>>> +  sigaddset(&sigs, sig);
>>>>>>>      }
>>>>>>>
>>>>>>>      // for diagnostic
>>>>>>> @@ -4616,6 +4613,7 @@
>>>>>>>            (*begin_signal_setting)();
>>>>>>>          }
>>>>>>>
>>>>>>> +    ::sigemptyset(&sigs);
>>>>>>>          set_signal_handler(SIGSEGV, true);
>>>>>>>          set_signal_handler(SIGPIPE, true);
>>>>>>>          set_signal_handler(SIGBUS, true);
>>>>>>>