public inbox for glibc-cvs@sourceware.org
help / color / mirror / Atom feed
* [glibc] Fix __libc_signal_block_all on sparc64
@ 2019-12-19 13:59 Adhemerval Zanella
  0 siblings, 0 replies; only message in thread
From: Adhemerval Zanella @ 2019-12-19 13:59 UTC (permalink / raw)
  To: glibc-cvs

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=1b132d55e2d3a4eb421c0f77f63b67b5022c22e3

commit 1b132d55e2d3a4eb421c0f77f63b67b5022c22e3
Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Date:   Mon Dec 9 15:50:29 2019 -0300

    Fix __libc_signal_block_all on sparc64
    
    The posix_spawn on sparc issues invalid sigprocmask calls:
    
      rt_sigprocmask(0xffe5e15c /* SIG_??? */, ~[], 0xffe5e1dc, 8) = -1 EINVAL (Invalid argument)
    
    Which make support/tst-support_capture_subprocess fails with random
    output (due the child signal being wrongly captured by the parent).
    
    Tracking the culprit it seems to be a wrong code generation in the
    INTERNAL_SYSCALL due the automatic sigset_t used on
    __libc_signal_block_all:
    
      return INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_BLOCK, &SIGALL_SET,
                              set, _NSIG / 8);
    
    Where SIGALL_SET is defined as:
    
      ((__sigset_t) { .__val = {[0 ...  _SIGSET_NWORDS-1 ] =  -1 } })
    
    Building the expanded __libc_signal_block_all on sparc64 with recent
    compiler (gcc 8.3.1 and 9.1.1):
    
      #include <signal>
    
      int
      _libc_signal_block_all (sigset_t *set)
      {
        INTERNAL_SYSCALL_DECL (err);
        return INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_BLOCK, &SIGALL_SET,
    			     set, _NSIG / 8);
      }
    
    The first argument (SIG_BLOCK) is not correctly set on 'o0' register:
    
      __libc_signal_block_all:
    	save    %sp, -304, %sp
    	add     %fp, 1919, %o0
    	mov     128, %o2
    	sethi   %hi(.LC0), %o1
    	call    memcpy, 0
    	 or     %o1, %lo(.LC0), %o1
    	add     %fp, 1919, %o1
    	mov     %i0, %o2
    	mov     8, %o3
    	mov     103, %g1
    	ta      0x6d;
    	bcc,pt  %xcc, 1f
    	mov     0, %g1
    	sub     %g0, %o0, %o0
    	mov     1, %g1
         1:	sra     %o0, 0, %i0
    	return  %i7+8
    	 nop
    
    Where if SIGALL_SET is defined a const object, gcc correctly sets the
    expected kernel argument in correct register:
    
            sethi   %hi(.LC0), %o1
            call    memcpy, 0
             or     %o1, %lo(.LC0), %o1
       ->   mov     1, %o0
    	add     %fp, 1919, %o1
    
    Another possible fix is use a static const object.  Although there
    should not be a difference between a const compound literal and a static
    const object, the gcc C99 status page [1] has a note stating that this
    optimization is not implemented:
    
      "const-qualified compound literals could share storage with each
       other and with string literals, but currently don't.".
    
    This patch fixes it by moving both sigset_t that represent the
    signal sets to static const data object.  It generates slight better
    code where the object reference is used directly instead of a stack
    allocation plus the content materialization.
    
    Checked on x86_64-linux-gnu, i686-linux-gnu, and sparc64-linux-gnu.
    
    [1] https://gcc.gnu.org/c99status.html

Diff:
---
 sysdeps/unix/sysv/linux/internal-signals.h | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/internal-signals.h b/sysdeps/unix/sysv/linux/internal-signals.h
index 01d8bf0..a496c71 100644
--- a/sysdeps/unix/sysv/linux/internal-signals.h
+++ b/sysdeps/unix/sysv/linux/internal-signals.h
@@ -22,6 +22,7 @@
 #include <signal.h>
 #include <sigsetops.h>
 #include <stdbool.h>
+#include <limits.h>
 #include <sysdep.h>
 
 /* The signal used for asynchronous cancelation.  */
@@ -53,15 +54,16 @@ __clear_internal_signals (sigset_t *set)
   __sigdelset (set, SIGSETXID);
 }
 
-#define SIGALL_SET \
-  ((__sigset_t) { .__val = {[0 ...  _SIGSET_NWORDS-1 ] =  -1 } })
+static const sigset_t sigall_set = {
+   .__val = {[0 ...  _SIGSET_NWORDS-1 ] =  -1 }
+};
 
 /* Block all signals, including internal glibc ones.  */
 static inline int
 __libc_signal_block_all (sigset_t *set)
 {
   INTERNAL_SYSCALL_DECL (err);
-  return INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_BLOCK, &SIGALL_SET,
+  return INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_BLOCK, &sigall_set,
 			   set, _NSIG / 8);
 }
 
@@ -69,11 +71,11 @@ __libc_signal_block_all (sigset_t *set)
 static inline int
 __libc_signal_block_app (sigset_t *set)
 {
-  sigset_t allset = SIGALL_SET;
+  sigset_t allset = sigall_set;
   __clear_internal_signals (&allset);
   INTERNAL_SYSCALL_DECL (err);
-  return INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_BLOCK, &allset, set,
-			   _NSIG / 8);
+  return INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_BLOCK, &allset,
+			   set, _NSIG / 8);
 }
 
 /* Restore current process signal mask.  */


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2019-12-19 13:59 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-19 13:59 [glibc] Fix __libc_signal_block_all on sparc64 Adhemerval Zanella

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).