pselect.c: Pass a pointer to SYSCALL_CANCEL [BZ #26606]

Message ID 20200912024533.3957431-1-hjl.tools@gmail.com
State Committed
Headers
Series pselect.c: Pass a pointer to SYSCALL_CANCEL [BZ #26606] |

Commit Message

H.J. Lu Sept. 12, 2020, 2:45 a.m. UTC
  commit a92f4e6299fe0e3cb6f77e79de00817aece501ce
Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Date:   Mon Jul 6 13:27:12 2020 -0300

    linux: Add time64 pselect support

changed pselect.c to

     r = SYSCALL_CANCEL (pselect6_time64, nfds, readfds, writefds, exceptfds,
			  timeout,
			  ((__syscall_ulong_t[]){ (uintptr_t) sigmask,
						  __NSIG_BYTES }));

which doesn't work with x32's ARGIFY and data passed to syscall isn't
initialized with sigmask and __NSIG_BYTES.  Change to

     __syscall_ulong_t data[2] =
	{
	  (uintptr_t) sigmask, __NSIG_BYTES
	};
      r = SYSCALL_CANCEL (pselect6_time64, nfds, readfds, writefds, exceptfds,
			  timeout, data);

fixes the issue.
---
 sysdeps/unix/sysv/linux/pselect.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
  

Comments

H.J. Lu Sept. 12, 2020, 2:54 a.m. UTC | #1
On Fri, Sep 11, 2020 at 7:45 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> commit a92f4e6299fe0e3cb6f77e79de00817aece501ce
> Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> Date:   Mon Jul 6 13:27:12 2020 -0300
>
>     linux: Add time64 pselect support
>
> changed pselect.c to
>
>      r = SYSCALL_CANCEL (pselect6_time64, nfds, readfds, writefds, exceptfds,
>                           timeout,
>                           ((__syscall_ulong_t[]){ (uintptr_t) sigmask,
>                                                   __NSIG_BYTES }));
>
> which doesn't work with x32's ARGIFY and data passed to syscall isn't
> initialized with sigmask and __NSIG_BYTES.  Change to
>
>      __syscall_ulong_t data[2] =
>         {
>           (uintptr_t) sigmask, __NSIG_BYTES
>         };
>       r = SYSCALL_CANCEL (pselect6_time64, nfds, readfds, writefds, exceptfds,
>                           timeout, data);
>
> fixes the issue.
> ---
>  sysdeps/unix/sysv/linux/pselect.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/sysdeps/unix/sysv/linux/pselect.c b/sysdeps/unix/sysv/linux/pselect.c
> index ed36121023..aa5835ed0f 100644
> --- a/sysdeps/unix/sysv/linux/pselect.c
> +++ b/sysdeps/unix/sysv/linux/pselect.c
> @@ -44,10 +44,12 @@ __pselect64 (int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
>    int r;
>    if (supports_time64 ())
>      {
> +      __syscall_ulong_t data[2] =
> +       {
> +         (uintptr_t) sigmask, __NSIG_BYTES
> +       };
>        r = SYSCALL_CANCEL (pselect6_time64, nfds, readfds, writefds, exceptfds,
> -                         timeout,
> -                         ((__syscall_ulong_t[]){ (uintptr_t) sigmask,
> -                                                 __NSIG_BYTES }));
> +                         timeout, data);
>        if (r == 0 || errno != ENOSYS)
>         return r;
>

x86-64 is also missing sigmask and __NSIG_BYTES.  I suspect that many
targets are broken.
  
H.J. Lu Sept. 14, 2020, 12:48 p.m. UTC | #2
On Fri, Sep 11, 2020 at 7:45 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> commit a92f4e6299fe0e3cb6f77e79de00817aece501ce
> Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> Date:   Mon Jul 6 13:27:12 2020 -0300
>
>     linux: Add time64 pselect support
>
> changed pselect.c to
>
>      r = SYSCALL_CANCEL (pselect6_time64, nfds, readfds, writefds, exceptfds,
>                           timeout,
>                           ((__syscall_ulong_t[]){ (uintptr_t) sigmask,
>                                                   __NSIG_BYTES }));
>
> which doesn't work with x32's ARGIFY and data passed to syscall isn't
> initialized with sigmask and __NSIG_BYTES.  Change to
>
>      __syscall_ulong_t data[2] =
>         {
>           (uintptr_t) sigmask, __NSIG_BYTES
>         };
>       r = SYSCALL_CANCEL (pselect6_time64, nfds, readfds, writefds, exceptfds,
>                           timeout, data);
>
> fixes the issue.

Here is a patch to add x86-64 pselect.c.  If there are no objections,
I will check it in tomorrow.
  
Adhemerval Zanella Netto Sept. 14, 2020, 7:25 p.m. UTC | #3
> Il giorno 14 set 2020, alle ore 09:48, H.J. Lu <HJl.tools@gmail.com> ha scritto:
> 
> On Fri, Sep 11, 2020 at 7:45 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>> 
>> commit a92f4e6299fe0e3cb6f77e79de00817aece501ce
>> Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>> Date:   Mon Jul 6 13:27:12 2020 -0300
>> 
>>    linux: Add time64 pselect support
>> 
>> changed pselect.c to
>> 
>>     r = SYSCALL_CANCEL (pselect6_time64, nfds, readfds, writefds, exceptfds,
>>                          timeout,
>>                          ((__syscall_ulong_t[]){ (uintptr_t) sigmask,
>>                                                  __NSIG_BYTES }));
>> 
>> which doesn't work with x32's ARGIFY and data passed to syscall isn't
>> initialized with sigmask and __NSIG_BYTES.  Change to
>> 
>>     __syscall_ulong_t data[2] =
>>        {
>>          (uintptr_t) sigmask, __NSIG_BYTES
>>        };
>>      r = SYSCALL_CANCEL (pselect6_time64, nfds, readfds, writefds, exceptfds,
>>                          timeout, data);
>> 
>> fixes the issue.
> 
> Here is a patch to add x86-64 pselect.c.  If there are no objections,
> I will check it in tomorrow.
> 

Sign, I forgot the x32 pointer kABI requirement where the INLINE_* macro might not produce the correct code. 

Instead of pushing a x86_64 version, could you patch the generic version instead to use the same strategy (and maybe add a comment why this construct is required)? 

In long term I have some patches to revamp the syscall macros to use proper inline functions, it should avoid this kind of regressions.

> -- 
> H.J.
> <0001-x86-64-Pass-a-pointer-to-SYSCALL_CANCEL-in-pselect.c.patch>
  
H.J. Lu Sept. 14, 2020, 8:42 p.m. UTC | #4
On Mon, Sep 14, 2020 at 12:25 PM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> > Il giorno 14 set 2020, alle ore 09:48, H.J. Lu <HJl.tools@gmail.com> ha scritto:
> >
> > On Fri, Sep 11, 2020 at 7:45 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >>
> >> commit a92f4e6299fe0e3cb6f77e79de00817aece501ce
> >> Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> >> Date:   Mon Jul 6 13:27:12 2020 -0300
> >>
> >>    linux: Add time64 pselect support
> >>
> >> changed pselect.c to
> >>
> >>     r = SYSCALL_CANCEL (pselect6_time64, nfds, readfds, writefds, exceptfds,
> >>                          timeout,
> >>                          ((__syscall_ulong_t[]){ (uintptr_t) sigmask,
> >>                                                  __NSIG_BYTES }));
> >>
> >> which doesn't work with x32's ARGIFY and data passed to syscall isn't
> >> initialized with sigmask and __NSIG_BYTES.  Change to
> >>
> >>     __syscall_ulong_t data[2] =
> >>        {
> >>          (uintptr_t) sigmask, __NSIG_BYTES
> >>        };
> >>      r = SYSCALL_CANCEL (pselect6_time64, nfds, readfds, writefds, exceptfds,
> >>                          timeout, data);
> >>
> >> fixes the issue.
> >
> > Here is a patch to add x86-64 pselect.c.  If there are no objections,
> > I will check it in tomorrow.
> >
>
> Sign, I forgot the x32 pointer kABI requirement where the INLINE_* macro might not produce the correct code.
>
> Instead of pushing a x86_64 version, could you patch the generic version instead to use the same strategy (and maybe add a comment why this construct is required)?

Here is the updated generic patch.

> In long term I have some patches to revamp the syscall macros to use proper inline functions, it should avoid this kind of regressions.
>
  
Andreas Schwab Sept. 15, 2020, 12:01 p.m. UTC | #5
On Sep 11 2020, H.J. Lu via Libc-alpha wrote:

> commit a92f4e6299fe0e3cb6f77e79de00817aece501ce
> Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> Date:   Mon Jul 6 13:27:12 2020 -0300
>
>     linux: Add time64 pselect support
>
> changed pselect.c to
>
>      r = SYSCALL_CANCEL (pselect6_time64, nfds, readfds, writefds, exceptfds,
> 			  timeout,
> 			  ((__syscall_ulong_t[]){ (uintptr_t) sigmask,
> 						  __NSIG_BYTES }));

Does it work to take the address of the compound literal?

Andreas.
  

Patch

diff --git a/sysdeps/unix/sysv/linux/pselect.c b/sysdeps/unix/sysv/linux/pselect.c
index ed36121023..aa5835ed0f 100644
--- a/sysdeps/unix/sysv/linux/pselect.c
+++ b/sysdeps/unix/sysv/linux/pselect.c
@@ -44,10 +44,12 @@  __pselect64 (int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
   int r;
   if (supports_time64 ())
     {
+      __syscall_ulong_t data[2] =
+	{
+	  (uintptr_t) sigmask, __NSIG_BYTES
+	};
       r = SYSCALL_CANCEL (pselect6_time64, nfds, readfds, writefds, exceptfds,
-			  timeout,
-			  ((__syscall_ulong_t[]){ (uintptr_t) sigmask,
-						  __NSIG_BYTES }));
+			  timeout, data);
       if (r == 0 || errno != ENOSYS)
 	return r;