]> sourceware.org Git - glibc.git/commitdiff
sparc: Fix restartable syscalls (BZ 32173)
authorAdhemerval Zanella <adhemerval.zanella@linaro.org>
Fri, 13 Sep 2024 14:11:56 +0000 (11:11 -0300)
committerAdhemerval Zanella <adhemerval.zanella@linaro.org>
Wed, 16 Oct 2024 17:54:24 +0000 (14:54 -0300)
The commit 'sparc: Use Linux kABI for syscall return'
(86c5d2cf0ce046279baddc7faa27da71f1a89fde) did not take into account
a subtle sparc syscall kABI constraint.  For syscalls that might block
indefinitely, on an interrupt (like SIGCONT) the kernel will set the
instruction pointer to just before the syscall:

arch/sparc/kernel/signal_64.c
476 static void do_signal(struct pt_regs *regs, unsigned long orig_i0)
477 {
[...]
525                 if (restart_syscall) {
526                         switch (regs->u_regs[UREG_I0]) {
527                         case ERESTARTNOHAND:
528                         case ERESTARTSYS:
529                         case ERESTARTNOINTR:
530                                 /* replay the system call when we are done */
531                                 regs->u_regs[UREG_I0] = orig_i0;
532                                 regs->tpc -= 4;
533                                 regs->tnpc -= 4;
534                                 pt_regs_clear_syscall(regs);
535                                 fallthrough;
536                         case ERESTART_RESTARTBLOCK:
537                                 regs->u_regs[UREG_G1] = __NR_restart_syscall;
538                                 regs->tpc -= 4;
539                                 regs->tnpc -= 4;
540                                 pt_regs_clear_syscall(regs);
541                         }

However, on a SIGCONT it seems that 'g1' register is being clobbered after the
syscall returns.  Before 86c5d2cf0ce046279, the 'g1' was always placed jus
before the 'ta' instruction which then reloads the syscall number and restarts
the syscall.

On master, where 'g1' might be placed before 'ta':

  $ cat test.c
  #include <unistd.h>

  int main ()
  {
    pause ();
  }
  $ gcc test.c -o test
  $ strace -f ./t
  [...]
  ppoll(NULL, 0, NULL, NULL, 0

On another terminal

  $ kill -STOP 2262828

  $ strace -f ./t
  [...]
  --- SIGSTOP {si_signo=SIGSTOP, si_code=SI_USER, si_pid=2521813, si_uid=8289} ---
  --- stopped by SIGSTOP ---

And then

  $ kill -CONT 2262828

Results in:

  --- SIGCONT {si_signo=SIGCONT, si_code=SI_USER, si_pid=2521813, si_uid=8289} ---
  restart_syscall(<... resuming interrupted ppoll ...>) = -1 EINTR (Interrupted system call)

Where the expected behaviour would be:

  $ strace -f ./t
  [...]
  ppoll(NULL, 0, NULL, NULL, 0)           = ? ERESTARTNOHAND (To be restarted if no handler)
  --- SIGSTOP {si_signo=SIGSTOP, si_code=SI_USER, si_pid=2521813, si_uid=8289} ---
  --- stopped by SIGSTOP ---
  --- SIGCONT {si_signo=SIGCONT, si_code=SI_USER, si_pid=2521813, si_uid=8289} ---
  ppoll(NULL, 0, NULL, NULL, 0

Just moving the 'g1' setting near the syscall asm is not suffice,
the compiler might optimize it away (as I saw on cancellation.c by
trying this fix).  Instead, I have change the inline asm to put the
'g1' setup in ithe asm block.  This would require to change the asm
constraint for INTERNAL_SYSCALL_NCS, since the syscall number is not
constant.

Checked on sparc64-linux-gnu.

Reported-by: René Rebe <rene@exactcode.de>
Tested-by: Sam James <sam@gentoo.org>
Reviewed-by: Sam James <sam@gentoo.org>
sysdeps/unix/sysv/linux/Makefile
sysdeps/unix/sysv/linux/sparc/sparc32/syscall_cancel.S
sysdeps/unix/sysv/linux/sparc/sparc32/sysdep.h
sysdeps/unix/sysv/linux/sparc/sparc64/syscall_cancel.S
sysdeps/unix/sysv/linux/sparc/sparc64/sysdep.h
sysdeps/unix/sysv/linux/sparc/sysdep.h
sysdeps/unix/sysv/linux/tst-syscall-restart.c [new file with mode: 0644]

index 7df51a325cef5f65869e0e3b600ceb905c0147cc..527c7a5ae898acea8599ed9f32584ae55b0d8d0a 100644 (file)
@@ -230,6 +230,7 @@ tests += \
   tst-scm_rights \
   tst-sigtimedwait \
   tst-sync_file_range \
+  tst-syscall-restart \
   tst-sysconf-iov_max \
   tst-sysvmsg-linux \
   tst-sysvsem-linux \
index 0db93c77bf467e50b06db055d617ada54c5b8ddd..45c6ae7a8625b5df62574d9d65f1631c024a3ac7 100644 (file)
@@ -48,13 +48,13 @@ __syscall_cancel_arch_start:
        bne     2f
 #endif
        /* Issue a 6 argument syscall.  */
-        mov    %i1, %g1
-       mov     %i2, %o0
+        mov    %i2, %o0
        mov     %i3, %o1
        mov     %i4, %o2
        mov     %i5, %o3
        ld      [%fp+92], %o4
        ld      [%fp+96], %o5
+        mov    %i1, %g1
        ta      0x10
 
        .globl __syscall_cancel_arch_end
index d2d68f53122b8d2aa449d79936bb706ba1780f1e..c2ffbb5c8f78287652b26fd2e5f7b145da0e72f9 100644 (file)
@@ -107,6 +107,7 @@ ENTRY(name);                                        \
 #else  /* __ASSEMBLER__ */
 
 #define __SYSCALL_STRING                                               \
+       "mov    %[scn], %%g1;"                                          \
        "ta     0x10;"                                                  \
        "bcc    1f;"                                                    \
        " nop;"                                                         \
@@ -114,7 +115,7 @@ ENTRY(name);                                        \
        "1:"
 
 #define __SYSCALL_CLOBBERS                                             \
-       "f0", "f1", "f2", "f3", "f4", "f5", "f6", "f7",                 \
+       "g1", "f0", "f1", "f2", "f3", "f4", "f5", "f6", "f7",           \
        "f8", "f9", "f10", "f11", "f12", "f13", "f14", "f15",           \
        "f16", "f17", "f18", "f19", "f20", "f21", "f22", "f23",         \
        "f24", "f25", "f26", "f27", "f28", "f29", "f30", "f31",         \
index 21b0728d5a549047951383fa23dc512385970969..6c8d1330cb66e1e0b77bae85714963a7c388cccb 100644 (file)
@@ -46,13 +46,13 @@ __syscall_cancel_arch_start:
        andcc   %g2, TCB_CANCELED_BITMASK, %g0
        bne,pn  %xcc, 2f
        /* Issue a 6 argument syscall.  */
-        mov    %i1, %g1
-       mov     %i2, %o0
+        mov    %i2, %o0
        mov     %i3, %o1
        mov     %i4, %o2
        mov     %i5, %o3
        ldx     [%fp + STACK_BIAS + 176], %o4
        ldx     [%fp + STACK_BIAS + 184], %o5
+       mov     %i1, %g1
        ta      0x6d
 
        .global __syscall_cancel_arch_end
index 96047424e9745784a7bf8cecde7114cbe1b99f5f..5598fab08a38985c534123ba0b32c179019d4e9a 100644 (file)
@@ -106,6 +106,7 @@ ENTRY(name);                                        \
 #else  /* __ASSEMBLER__ */
 
 #define __SYSCALL_STRING                                               \
+       "mov    %[scn], %%g1;"                                          \
        "ta     0x6d;"                                                  \
        "bcc,pt %%xcc, 1f;"                                             \
        " nop;"                                                         \
@@ -113,7 +114,7 @@ ENTRY(name);                                        \
        "1:"
 
 #define __SYSCALL_CLOBBERS                                             \
-       "f0", "f1", "f2", "f3", "f4", "f5", "f6", "f7",                 \
+       "g1", "f0", "f1", "f2", "f3", "f4", "f5", "f6", "f7",           \
        "f8", "f9", "f10", "f11", "f12", "f13", "f14", "f15",           \
        "f16", "f17", "f18", "f19", "f20", "f21", "f22", "f23",         \
        "f24", "f25", "f26", "f27", "f28", "f29", "f30", "f31",         \
index dcabb57fe28e5717bf54315eee6c3173d32e4d62..c287740a8c5c0d7fadfe09a6c4b3bbb7ef9cf31f 100644 (file)
 
 #undef INTERNAL_SYSCALL_NCS
 #define INTERNAL_SYSCALL_NCS(name, nr, args...) \
-  internal_syscall##nr(__SYSCALL_STRING, name, args)
+  _internal_syscall##nr(__SYSCALL_STRING, "p", name, args)
 
-#define internal_syscall0(string,name,dummy...)                        \
+#define _internal_syscall0(string,nc,name,dummy...)    \
 ({                                                                     \
-       register long int __g1 __asm__ ("g1") = (name);                 \
        register long __o0 __asm__ ("o0");                              \
+       long int _name = (long int) (name);                             \
        __asm __volatile (string : "=r" (__o0) :                        \
-                         "r" (__g1) :                                  \
+                         [scn] nc (_name) :                            \
                          __SYSCALL_CLOBBERS);                          \
        __o0;                                                           \
 })
+#define internal_syscall0(string,name,args...)                         \
+  _internal_syscall0(string, "i", name, args)
 
-#define internal_syscall1(string,name,arg1)                            \
+#define _internal_syscall1(string,nc,name,arg1)                                \
 ({                                                                     \
        long int _arg1 = (long int) (arg1);                             \
-       register long int __g1 __asm__("g1") = (name);                  \
+       long int _name = (long int) (name);                             \
        register long int  __o0 __asm__ ("o0") = _arg1;                 \
-       __asm __volatile (string : "=r" (__o0) :                        \
-                         "r" (__g1), "0" (__o0) :                      \
+       __asm __volatile (string : "+r" (__o0) :                        \
+                         [scn] nc (_name) :                            \
                          __SYSCALL_CLOBBERS);                          \
        __o0;                                                           \
 })
+#define internal_syscall1(string,name,args...)                         \
+  _internal_syscall1(string, "i", name, args)
 
-#define internal_syscall2(string,name,arg1,arg2)                       \
+#define _internal_syscall2(string,nc,name,arg1,arg2)                   \
 ({                                                                     \
        long int _arg1 = (long int) (arg1);                             \
        long int _arg2 = (long int) (arg2);                             \
-       register long int __g1 __asm__("g1") = (name);                  \
+       long int _name = (long int) (name);                             \
        register long int __o0 __asm__ ("o0") = _arg1;                  \
        register long int __o1 __asm__ ("o1") = _arg2;                  \
-       __asm __volatile (string : "=r" (__o0) :                        \
-                         "r" (__g1), "0" (__o0), "r" (__o1) :          \
+       __asm __volatile (string : "+r" (__o0) :                        \
+                         [scn] nc (_name), "r" (__o1) :                \
                          __SYSCALL_CLOBBERS);                          \
        __o0;                                                           \
 })
+#define internal_syscall2(string,name,args...)                         \
+  _internal_syscall2(string, "i", name, args)
 
-#define internal_syscall3(string,name,arg1,arg2,arg3)                  \
+#define _internal_syscall3(string,nc,name,arg1,arg2,arg3)              \
 ({                                                                     \
        long int _arg1 = (long int) (arg1);                             \
        long int _arg2 = (long int) (arg2);                             \
        long int _arg3 = (long int) (arg3);                             \
-       register long int __g1 __asm__("g1") = (name);                  \
+       long int _name = (long int) (name);                             \
        register long int __o0 __asm__ ("o0") = _arg1;                  \
        register long int __o1 __asm__ ("o1") = _arg2;                  \
        register long int __o2 __asm__ ("o2") = _arg3;                  \
-       __asm __volatile (string : "=r" (__o0) :                        \
-                         "r" (__g1), "0" (__o0), "r" (__o1),           \
+       __asm __volatile (string : "+r" (__o0) :                        \
+                         [scn] nc (_name), "r" (__o1),                 \
                          "r" (__o2) :                                  \
                          __SYSCALL_CLOBBERS);                          \
        __o0;                                                           \
 })
+#define internal_syscall3(string,name,args...)                         \
+  _internal_syscall3(string, "i", name, args)
 
-#define internal_syscall4(string,name,arg1,arg2,arg3,arg4)             \
+#define _internal_syscall4(string,nc,name,arg1,arg2,arg3,arg4)         \
 ({                                                                     \
        long int _arg1 = (long int) (arg1);                             \
        long int _arg2 = (long int) (arg2);                             \
        long int _arg3 = (long int) (arg3);                             \
        long int _arg4 = (long int) (arg4);                             \
-       register long int __g1 __asm__("g1") = (name);                  \
+       long int _name = (long int) (name);                             \
        register long int __o0 __asm__ ("o0") = _arg1;                  \
        register long int __o1 __asm__ ("o1") = _arg2;                  \
        register long int __o2 __asm__ ("o2") = _arg3;                  \
        register long int __o3 __asm__ ("o3") = _arg4;                  \
-       __asm __volatile (string : "=r" (__o0) :                        \
-                         "r" (__g1), "0" (__o0), "r" (__o1),           \
+       __asm __volatile (string : "+r" (__o0) :                        \
+                         [scn] nc (_name), "r" (__o1),                 \
                          "r" (__o2), "r" (__o3) :                      \
                          __SYSCALL_CLOBBERS);                          \
        __o0;                                                           \
 })
+#define internal_syscall4(string,name,args...)                         \
+  _internal_syscall4(string, "i", name, args)
 
-#define internal_syscall5(string,name,arg1,arg2,arg3,arg4,arg5)                \
+#define _internal_syscall5(string,nc,name,arg1,arg2,arg3,arg4,arg5)    \
 ({                                                                     \
        long int _arg1 = (long int) (arg1);                             \
        long int _arg2 = (long int) (arg2);                             \
        long int _arg3 = (long int) (arg3);                             \
        long int _arg4 = (long int) (arg4);                             \
        long int _arg5 = (long int) (arg5);                             \
-       register long int __g1 __asm__("g1") = (name);                  \
+       long int _name = (long int) (name);                             \
        register long int __o0 __asm__ ("o0") = _arg1;                  \
        register long int __o1 __asm__ ("o1") = _arg2;                  \
        register long int __o2 __asm__ ("o2") = _arg3;                  \
        register long int __o3 __asm__ ("o3") = _arg4;                  \
        register long int __o4 __asm__ ("o4") = _arg5;                  \
-       __asm __volatile (string : "=r" (__o0) :                        \
-                         "r" (__g1), "0" (__o0), "r" (__o1),           \
+       __asm __volatile (string : "+r" (__o0) :                        \
+                         [scn] nc (_name), "r" (__o1),                 \
                          "r" (__o2), "r" (__o3), "r" (__o4) :          \
                          __SYSCALL_CLOBBERS);                          \
        __o0;                                                           \
 })
+#define internal_syscall5(string,name,args...)                         \
+  _internal_syscall5(string, "i", name, args)
 
-#define internal_syscall6(string,name,arg1,arg2,arg3,arg4,arg5,arg6)   \
+#define _internal_syscall6(string,nc,name,arg1,arg2,arg3,arg4,arg5,arg6)\
 ({                                                                     \
        long int _arg1 = (long int) (arg1);                             \
        long int _arg2 = (long int) (arg2);                             \
        long int _arg4 = (long int) (arg4);                             \
        long int _arg5 = (long int) (arg5);                             \
        long int _arg6 = (long int) (arg6);                             \
-       register long int __g1 __asm__("g1") = (name);                  \
+       long int _name = (long int) (name);                             \
        register long int __o0 __asm__ ("o0") = _arg1;                  \
        register long int __o1 __asm__ ("o1") = _arg2;                  \
        register long int __o2 __asm__ ("o2") = _arg3;                  \
        register long int __o3 __asm__ ("o3") = _arg4;                  \
        register long int __o4 __asm__ ("o4") = _arg5;                  \
        register long int __o5 __asm__ ("o5") = _arg6;                  \
-       __asm __volatile (string : "=r" (__o0) :                        \
-                         "r" (__g1), "0" (__o0), "r" (__o1),           \
+       __asm __volatile (string : "+r" (__o0) :                        \
+                         [scn] nc (_name), "r" (__o1),                 \
                          "r" (__o2), "r" (__o3), "r" (__o4),           \
                          "r" (__o5) :                                  \
                          __SYSCALL_CLOBBERS);                          \
        __o0;                                                           \
 })
+#define internal_syscall6(string,name,args...)                         \
+  _internal_syscall6(string, "i", name, args)
 
 #define INLINE_CLONE_SYSCALL(arg1,arg2,arg3,arg4,arg5)                 \
 ({                                                                     \
        long int _arg3 = (long int) (arg3);                             \
        long int _arg4 = (long int) (arg4);                             \
        long int _arg5 = (long int) (arg5);                             \
+       long int _name = __NR_clone;                                    \
        register long int __o0 __asm__ ("o0") = _arg1;                  \
        register long int __o1 __asm__ ("o1") = _arg2;                  \
        register long int __o2 __asm__ ("o2") = _arg3;                  \
        register long int __o3 __asm__ ("o3") = _arg4;                  \
        register long int __o4 __asm__ ("o4") = _arg5;                  \
-       register long int __g1 __asm__ ("g1") = __NR_clone;             \
        __asm __volatile (__SYSCALL_STRING :                            \
                          "=r" (__o0), "=r" (__o1) :                    \
-                         "r" (__g1), "0" (__o0), "1" (__o1),           \
+                         [scn] "i" (_name), "0" (__o0), "1" (__o1),    \
                          "r" (__o2), "r" (__o3), "r" (__o4) :          \
                          __SYSCALL_CLOBBERS);                          \
        if (__glibc_unlikely ((unsigned long int) (__o0) > -4096UL))    \
diff --git a/sysdeps/unix/sysv/linux/tst-syscall-restart.c b/sysdeps/unix/sysv/linux/tst-syscall-restart.c
new file mode 100644 (file)
index 0000000..84a8a41
--- /dev/null
@@ -0,0 +1,112 @@
+/* Test if a syscall is correctly restarted.
+   Copyright (C) 2024 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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <support/xsignal.h>
+#include <support/check.h>
+#include <support/process_state.h>
+#include <support/xunistd.h>
+#include <support/xthread.h>
+#include <sys/wait.h>
+
+static int
+check_pid (pid_t pid)
+{
+  /* Wait until the child has called pause and it blocking on kernel.  */
+  support_process_state_wait (pid, support_process_state_sleeping);
+
+  TEST_COMPARE (kill (pid, SIGSTOP), 0);
+
+  /* Adding process_state_tracing_stop ('t') allows the test to work under
+     trace programs such as ptrace.  */
+  support_process_state_wait (pid, support_process_state_stopped
+                                  | support_process_state_tracing_stop);
+
+  TEST_COMPARE (kill (pid, SIGCONT), 0);
+
+  enum support_process_state state
+    = support_process_state_wait (pid, support_process_state_sleeping
+                                      | support_process_state_zombie);
+
+  TEST_COMPARE (state, support_process_state_sleeping);
+
+  TEST_COMPARE (kill (pid, SIGTERM), 0);
+
+  siginfo_t info;
+  TEST_COMPARE (waitid (P_PID, pid, &info, WEXITED), 0);
+  TEST_COMPARE (info.si_signo, SIGCHLD);
+  TEST_COMPARE (info.si_code, CLD_KILLED);
+  TEST_COMPARE (info.si_status, SIGTERM);
+  TEST_COMPARE (info.si_pid, pid);
+
+  return 0;
+}
+
+static void *
+tf (void *)
+{
+  pause ();
+  return NULL;
+}
+
+static void
+child_mt (void)
+{
+  /* Let only the created thread to handle signals.  */
+  sigset_t set;
+  sigfillset (&set);
+  xpthread_sigmask (SIG_BLOCK, &set, NULL);
+
+  sigdelset (&set, SIGSTOP);
+  sigdelset (&set, SIGCONT);
+  sigdelset (&set, SIGTERM);
+
+  pthread_attr_t attr;
+  xpthread_attr_init (&attr);
+  TEST_COMPARE (pthread_attr_setsigmask_np (&attr, &set), 0);
+
+  xpthread_join (xpthread_create (&attr, tf, NULL));
+}
+
+static void
+do_test_syscall (bool multithread)
+{
+  pid_t pid = xfork ();
+  if (pid == 0)
+    {
+      if (multithread)
+       child_mt ();
+      else
+       pause ();
+      _exit (127);
+    }
+
+  check_pid (pid);
+}
+
+static int
+do_test (void)
+{
+  /* Check for both single and multi thread, since they use different syscall
+     mechanisms.  */
+  do_test_syscall (false);
+  do_test_syscall (true);
+
+  return 0;
+}
+
+#include <support/test-driver.c>
This page took 0.065242 seconds and 5 git commands to generate.