Bug 338974 - glibc 2.20 (and kernel) changed size of struct sigaction sa_flags field on s390
Summary: glibc 2.20 (and kernel) changed size of struct sigaction sa_flags field on s390
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: memcheck (show other bugs)
Version: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-09-10 11:55 UTC by Mark Wielaard
Modified: 2014-09-10 12:08 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Wielaard 2014-09-10 11:55:01 UTC
To track POSIX glibc and the kernel agreed to change the type of the sa_flags field in struct sigaction to an int. There is now a int __glibc_reserved0 padding field that can be passed undefined. See https://sourceware.org/ml/libc-alpha/2014-09/msg00161.html

This causes memcheck to complain about code like the following from nptl-init.c:

   struct sigaction sa;
   sa.sa_sigaction = sigcancel_handler;
   sa.sa_flags = SA_SIGINFO;
   __sigemptyset (&sa.sa_mask);

   (void) __libc_sigaction (SIGCANCEL, &sa, NULL);

valgrind will now warn:

  Syscall param rt_sigaction(act->sa_flags) points to uninitialised byte(s)
     at 0x42EECC8: __libc_sigaction (sigaction.c:42)
     by 0x42E2253: __pthread_initialize_minimal (nptl-init.c:381)
     by 0x42E15EF: ??? (in /usr/lib64/libpthread-2.19.90.so)

Because the '__glibc_reserved0' part of sa_flags as the kernel will see
them is indeed undefined. Code like this is called often (if only to initialize nptl).

So valgrind should adjust its view of struct sigaction accordingly:

diff --git a/include/vki/vki-s390x-linux.h b/include/vki/vki-s390x-linux.h
index 902bbf5..85c449d 100644
--- a/include/vki/vki-s390x-linux.h
+++ b/include/vki/vki-s390x-linux.h
@@ -258,7 +258,13 @@ struct vki_old_sigaction {
 struct vki_sigaction {
         // [[See comment about extra 'k' above]]
         __vki_sighandler_t ksa_handler;
-        unsigned long sa_flags;
+        // Yes, the reserved field is really glibc specific. The kernel
+        // doesn't have it and uses an unsigned long for sa_flags.
+        // The glibc and the kernel agreed this is fine and the
+        // __glibc_reserved0 field can be undefined.
+        // See https://sourceware.org/ml/libc-alpha/2014-09/msg00161.html
+        int __glibc_reserved0;
+        int sa_flags;
         void (*sa_restorer)(void);
         vki_sigset_t sa_mask;               /* mask last for extensibility */
 };


Reproducible: Always
Comment 1 Mark Wielaard 2014-09-10 12:08:40 UTC
Pushed as valgrind svn r14508.