[10/12] Add another test for setcontext

  On Wed, Jul 25, 2018 at 9:21 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 07/21/2018 04:20 PM, H.J. Lu wrote:
>>
>> +  /* check sigmask in old context of swapcontext-call  */
>> +  if (sigismember (&oldctx.uc_sigmask, SIGUSR2) != 1)
>> +    {
>> +      puts ("FAIL: SIGUSR2 is not blocked in oldctx.uc_sigmask.");
>> +      exit (1);
>> +    }
>
>
> This breaks on ia64 because uc_sigmask does not have the correct type there:
>
> tst-setcontext4.c: In function ‘do_test’:
> tst-setcontext4.c:202:20: error: passing argument 1 of ‘sigismember’ from
> incompatible pointer type [-Werror=incompatible-pointer-types]
>    if (sigismember (&oldctx.uc_sigmask, SIGUSR2) != 1)
> In file included from ../include/signal.h:2,
>                  from tst-setcontext4.c:23:
> ../signal/signal.h:208:41: note: expected ‘const sigset_t *’ {aka ‘const
> struct <anonymous> *’} but argument is of type ‘long unsigned int *’
>  extern int sigismember (const sigset_t *__set, int __signo)
>                          ~~~~~~~~~~~~~~~~^~~~~
>
> I don't have a quick solution for this.  How important is this part of the
> test?
>

I am testing this patch.
  

On 07/25/2018 12:31 PM, H.J. Lu wrote:
> From 70951c51d03623c10485380fcccd3a384dff20d6 Mon Sep 17 00:00:00 2001
> From: "H.J. Lu" <hjl.tools@gmail.com>
> Date: Wed, 25 Jul 2018 09:30:50 -0700
> Subject: [PATCH] ia64: Work around incorrect type of IA64 uc_sigmask
> 
> 	* sysdeps/unix/sysv/linux/ia64/tst-setcontext4.c: New file.

Please file a bug for ia64 about this issue, then add the comment below.

Pre-accepted for 2.28 / master.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> ---
>  .../unix/sysv/linux/ia64/tst-setcontext4.c    | 24 +++++++++++++++++++
>  1 file changed, 24 insertions(+)
>  create mode 100644 sysdeps/unix/sysv/linux/ia64/tst-setcontext4.c
> 
> diff --git a/sysdeps/unix/sysv/linux/ia64/tst-setcontext4.c b/sysdeps/unix/sysv/linux/ia64/tst-setcontext4.c
> new file mode 100644
> index 0000000000..58807e78cb
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/ia64/tst-setcontext4.c
> @@ -0,0 +1,24 @@
> +/* Work around incorrect type of IA64 uc_sigmask.
> +   Copyright (C) 2018 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <signal.h>
> +

Add comment:

/* The uc_sigmask on IA64 has the wrong type and this needs fixing,
   but until that change is evaluated, we fix this here with a cast.
   See bug XXX.  */

> +#undef sigismember
> +#define sigismember(set, signo) sigismember ((const sigset_t *) (set), (signo))
> +
> +#include <stdlib/tst-setcontext4.c>
> -- 2.17.1
  

* Carlos O'Donell:

> /* The uc_sigmask on IA64 has the wrong type and this needs fixing,
>    but until that change is evaluated, we fix this here with a cast.
>    See bug XXX.  */

Is this really a bug?  After this long, I would consider it just a
quirk of the ia64 API/ABI and move on …
  

On Wed, 25 Jul 2018, Carlos O'Donell wrote:

> On 07/25/2018 12:31 PM, H.J. Lu wrote:
> > From 70951c51d03623c10485380fcccd3a384dff20d6 Mon Sep 17 00:00:00 2001
> > From: "H.J. Lu" <hjl.tools@gmail.com>
> > Date: Wed, 25 Jul 2018 09:30:50 -0700
> > Subject: [PATCH] ia64: Work around incorrect type of IA64 uc_sigmask
> > 
> > 	* sysdeps/unix/sysv/linux/ia64/tst-setcontext4.c: New file.
> 
> Please file a bug for ia64 about this issue, then add the comment below.

It's bug 21634 (filed for XFAILing the relevant conform/ test assertions, 
not because of any expectation of being fixable while staying 
ABI-compatible).
  

On 07/25/2018 04:42 PM, Florian Weimer wrote:
> * Carlos O'Donell:
> 
>> /* The uc_sigmask on IA64 has the wrong type and this needs fixing,
>>    but until that change is evaluated, we fix this here with a cast.
>>    See bug XXX.  */
> 
> Is this really a bug?  After this long, I would consider it just a
> quirk of the ia64 API/ABI and move on …

You can't use sigismember(); I would consider that a bug?

c.
  

* Carlos O'Donell:

> On 07/25/2018 04:42 PM, Florian Weimer wrote:
>> * Carlos O'Donell:
>> 
>>> /* The uc_sigmask on IA64 has the wrong type and this needs fixing,
>>>    but until that change is evaluated, we fix this here with a cast.
>>>    See bug XXX.  */
>> 
>> Is this really a bug?  After this long, I would consider it just a
>> quirk of the ia64 API/ABI and move on …
>
> You can't use sigismember(); I would consider that a bug?

Well, if the ABI says the member has the type it has, and that's not
compatible with sigismember, then that's how things are.  Surely this
isn't the only such example.  What about the return type of the
tsearch function?
  

On 07/25/2018 05:21 PM, Florian Weimer wrote:
> * Carlos O'Donell:
> 
>> On 07/25/2018 04:42 PM, Florian Weimer wrote:
>>> * Carlos O'Donell:
>>>
>>>> /* The uc_sigmask on IA64 has the wrong type and this needs fixing,
>>>>    but until that change is evaluated, we fix this here with a cast.
>>>>    See bug XXX.  */
>>>
>>> Is this really a bug?  After this long, I would consider it just a
>>> quirk of the ia64 API/ABI and move on …
>>
>> You can't use sigismember(); I would consider that a bug?
> 
> Well, if the ABI says the member has the type it has, and that's not
> compatible with sigismember, then that's how things are.  Surely this
> isn't the only such example.  What about the return type of the
> tsearch function?

What about tsearch? It returns a void* and so you do have to cast that
to something. Are you saying this is a similar issue? The void* creates
an expectation of a cast, which I think we don't have here.

c.
  

* Carlos O'Donell:

> On 07/25/2018 05:21 PM, Florian Weimer wrote:
>> * Carlos O'Donell:
>> 
>>> On 07/25/2018 04:42 PM, Florian Weimer wrote:
>>>> * Carlos O'Donell:
>>>>
>>>>> /* The uc_sigmask on IA64 has the wrong type and this needs fixing,
>>>>>    but until that change is evaluated, we fix this here with a cast.
>>>>>    See bug XXX.  */
>>>>
>>>> Is this really a bug?  After this long, I would consider it just a
>>>> quirk of the ia64 API/ABI and move on …
>>>
>>> You can't use sigismember(); I would consider that a bug?
>> 
>> Well, if the ABI says the member has the type it has, and that's not
>> compatible with sigismember, then that's how things are.  Surely this
>> isn't the only such example.  What about the return type of the
>> tsearch function?
>
> What about tsearch? It returns a void* and so you do have to cast that
> to something. Are you saying this is a similar issue? The void* creates
> an expectation of a cast, which I think we don't have here.

The return type is a pointer to a const void *.  You are not supposed
to cast the returned pointer, but that const void * (after checking
that the return value is not NULL).
  

On Jul 25 2018, Florian Weimer <fw@deneb.enyo.de> wrote:

> * Carlos O'Donell:
>
>> /* The uc_sigmask on IA64 has the wrong type and this needs fixing,
>>    but until that change is evaluated, we fix this here with a cast.
>>    See bug XXX.  */
>
> Is this really a bug?  After this long, I would consider it just a
> quirk of the ia64 API/ABI and move on …

It isn't POSIX compatible (when POSIX still supported it).

Andreas.
  

On 25/07/2018 17:45, Joseph Myers wrote:
> On Wed, 25 Jul 2018, Carlos O'Donell wrote:
> 
>> On 07/25/2018 12:31 PM, H.J. Lu wrote:
>>> From 70951c51d03623c10485380fcccd3a384dff20d6 Mon Sep 17 00:00:00 2001
>>> From: "H.J. Lu" <hjl.tools@gmail.com>
>>> Date: Wed, 25 Jul 2018 09:30:50 -0700
>>> Subject: [PATCH] ia64: Work around incorrect type of IA64 uc_sigmask
>>>
>>> 	* sysdeps/unix/sysv/linux/ia64/tst-setcontext4.c: New file.
>>
>> Please file a bug for ia64 about this issue, then add the comment below.
> 
> It's bug 21634 (filed for XFAILing the relevant conform/ test assertions, 
> not because of any expectation of being fixable while staying 
> ABI-compatible).
> 

What would require to fix this issue for ia64? The kernel interface already
seems to use the expected sigset_t type for sc_mask and I think it would
be feasible to use the *context functions as is with only adjusting the
size of rt_sigprocmask to be dependent of _NSIG.
  

On 07/26/2018 08:31 AM, Adhemerval Zanella wrote:
> 
> 
> On 25/07/2018 17:45, Joseph Myers wrote:
>> On Wed, 25 Jul 2018, Carlos O'Donell wrote:
>>
>>> On 07/25/2018 12:31 PM, H.J. Lu wrote:
>>>> From 70951c51d03623c10485380fcccd3a384dff20d6 Mon Sep 17 00:00:00 2001
>>>> From: "H.J. Lu" <hjl.tools@gmail.com>
>>>> Date: Wed, 25 Jul 2018 09:30:50 -0700
>>>> Subject: [PATCH] ia64: Work around incorrect type of IA64 uc_sigmask
>>>>
>>>> 	* sysdeps/unix/sysv/linux/ia64/tst-setcontext4.c: New file.
>>>
>>> Please file a bug for ia64 about this issue, then add the comment below.
>>
>> It's bug 21634 (filed for XFAILing the relevant conform/ test assertions, 
>> not because of any expectation of being fixable while staying 
>> ABI-compatible).
>>
> 
> What would require to fix this issue for ia64? The kernel interface already
> seems to use the expected sigset_t type for sc_mask and I think it would
> be feasible to use the *context functions as is with only adjusting the
> size of rt_sigprocmask to be dependent of _NSIG.

That all sounds reasonable :-)

c.
  

On Thu, 26 Jul 2018, Adhemerval Zanella wrote:

> > It's bug 21634 (filed for XFAILing the relevant conform/ test assertions, 
> > not because of any expectation of being fixable while staying 
> > ABI-compatible).
> 
> What would require to fix this issue for ia64? The kernel interface already
> seems to use the expected sigset_t type for sc_mask and I think it would
> be feasible to use the *context functions as is with only adjusting the
> size of rt_sigprocmask to be dependent of _NSIG.

glibc sigset_t is much larger than kernel sigset_t.
  

On 26/07/2018 12:04, Joseph Myers wrote:
> On Thu, 26 Jul 2018, Adhemerval Zanella wrote:
> 
>>> It's bug 21634 (filed for XFAILing the relevant conform/ test assertions, 
>>> not because of any expectation of being fixable while staying 
>>> ABI-compatible).
>>
>> What would require to fix this issue for ia64? The kernel interface already
>> seems to use the expected sigset_t type for sc_mask and I think it would
>> be feasible to use the *context functions as is with only adjusting the
>> size of rt_sigprocmask to be dependent of _NSIG.
> 
> glibc sigset_t is much larger than kernel sigset_t.
> 

Sigh, indeed 'new' kernel sigset_t has the same size as previous one,
it has changed only to use a struct. I still think it would be feasible
to change uc_sigmask to glibc sigset_t, the *context functions is
already using the expected sizes for __NR_sigprocmask. Do you see 
anything that need further adjustment?
  

On Fri, 27 Jul 2018, Adhemerval Zanella wrote:

> Sigh, indeed 'new' kernel sigset_t has the same size as previous one,
> it has changed only to use a struct. I still think it would be feasible
> to change uc_sigmask to glibc sigset_t, the *context functions is
> already using the expected sizes for __NR_sigprocmask. Do you see 
> anything that need further adjustment?

Well, there are all the usual risks of any structure size change.