[compiler-rt] r192686 - tsan: implement internal syscall-based versions of sigaction/sigprocmask

Alexey Samsonov samsonov at google.com
Tue Oct 15 05:21:53 PDT 2013


On Tue, Oct 15, 2013 at 4:16 PM, Dmitry Vyukov <dvyukov at google.com> wrote:

> On Tue, Oct 15, 2013 at 4:13 PM, Alexey Samsonov <samsonov at google.com>
> wrote:
> >
> > On Tue, Oct 15, 2013 at 4:09 PM, Dmitry Vyukov <dvyukov at google.com>
> wrote:
> >>
> >> the following lives in sanitizer_linux.h:
> >>
> >> uptr internal_getdents(fd_t fd, struct linux_dirent *dirp, unsigned int
> >> count);
> >> uptr internal_prctl(int option, uptr arg2, uptr arg3, uptr arg4, uptr
> >> arg5);
> >> uptr internal_sigaltstack(const struct sigaltstack* ss,
> >>                           struct sigaltstack* oss);
> >>
> >> sigaction naturally fits here
> >
> >
> > I disagree. I find it more convenient to have all re-definitions of POSIX
> > structures in one place. Also,
> > sanitizer_platform_limits_posix.cc has some useful compiler checks
> verifying
> > that our re-definitions of
> > POSIX structs are actually correct.
>
>
> Do you mean only struct definititions?
>
Yeah, I've meant kernel_sigset_t and kernel_sigaction_t struct definitions.


> What should I move to sanitizer_platform_limits_posix.cc?
>
See CHECK_SIZE_AND_OFFSET macro in that file - can we apply them to our
re-definitions of
kernel_sigset_t and kernel_sigaction_t?

>
>
> >> On Tue, Oct 15, 2013 at 4:03 PM, Alexey Samsonov <samsonov at google.com>
> >> wrote:
> >> >
> >> > On Tue, Oct 15, 2013 at 3:31 PM, Dmitry Vyukov <dvyukov at google.com>
> >> > wrote:
> >> >>
> >> >> Author: dvyukov
> >> >> Date: Tue Oct 15 06:31:51 2013
> >> >> New Revision: 192686
> >> >>
> >> >> URL: http://llvm.org/viewvc/llvm-project?rev=192686&view=rev
> >> >> Log:
> >> >> tsan: implement internal syscall-based versions of
> >> >> sigaction/sigprocmask
> >> >> use them in stoptheworld
> >> >> fixes applications that intercept sigaction/sigprocmask
> >> >>
> >> >>
> >> >>
> >> >> Modified:
> >> >>
> >> >> compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_syscalls.inc
> >> >>     compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc
> >> >>     compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.h
> >> >>
> >> >>
> >> >>
> compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
> >> >>
> >> >> Modified:
> >> >> compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_syscalls.inc
> >> >> URL:
> >> >>
> >> >>
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_syscalls.inc?rev=192686&r1=192685&r2=192686&view=diff
> >> >>
> >> >>
> >> >>
> ==============================================================================
> >> >> ---
> >> >> compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_syscalls.inc
> >> >> (original)
> >> >> +++
> >> >> compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_syscalls.inc
> >> >> Tue Oct 15 06:31:51 2013
> >> >> @@ -37,6 +37,7 @@
> >> >>  #if SANITIZER_LINUX
> >> >>
> >> >>  #include "sanitizer_libc.h"
> >> >> +#include "sanitizer_linux.h"
> >> >>
> >> >>  #define PRE_SYSCALL(name)
> >> >> \
> >> >>    SANITIZER_INTERFACE_ATTRIBUTE void
> >> >> __sanitizer_syscall_pre_impl_##name
> >> >> @@ -103,10 +104,6 @@ struct sanitizer_kernel_sockaddr {
> >> >>    char sa_data[14];
> >> >>  };
> >> >>
> >> >> -// Real sigset size is always passed as a syscall argument.
> >> >> -// Declare it "void" to catch sizeof(kernel_sigset_t).
> >> >> -typedef void kernel_sigset_t;
> >> >> -
> >> >>  static void kernel_write_iovec(const __sanitizer_iovec *iovec,
> >> >>                          SIZE_T iovlen, SIZE_T maxlen) {
> >> >>    for (SIZE_T i = 0; i < iovlen && maxlen; ++i) {
> >> >>
> >> >> Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc
> >> >> URL:
> >> >>
> >> >>
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc?rev=192686&r1=192685&r2=192686&view=diff
> >> >>
> >> >>
> >> >>
> ==============================================================================
> >> >> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc
> >> >> (original)
> >> >> +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc Tue Oct
> >> >> 15
> >> >> 06:31:51 2013
> >> >> @@ -602,6 +602,31 @@ uptr internal_sigaltstack(const struct s
> >> >>    return internal_syscall(__NR_sigaltstack, ss, oss);
> >> >>  }
> >> >>
> >> >> +uptr internal_sigaction(int signum, const kernel_sigaction_t *act,
> >> >> +    struct kernel_sigaction_t *oldact) {
> >> >> +  return internal_syscall(__NR_rt_sigaction, signum, act, oldact,
> >> >> +      sizeof(kernel_sigset_t));
> >> >> +}
> >> >> +
> >> >> +uptr internal_sigprocmask(int how, kernel_sigset_t *set,
> >> >> +    kernel_sigset_t *oldset) {
> >> >> +  return internal_syscall(__NR_rt_sigprocmask, (uptr)how,
> >> >> &set->sig[0],
> >> >> +      &oldset->sig[0], sizeof(kernel_sigset_t));
> >> >> +}
> >> >> +
> >> >> +void internal_sigfillset(kernel_sigset_t *set) {
> >> >> +  internal_memset(set, 0xff, sizeof(*set));
> >> >> +}
> >> >> +
> >> >> +void internal_sigdelset(kernel_sigset_t *set, int signum) {
> >> >> +  signum -= 1;
> >> >> +  CHECK_GE(signum, 0);
> >> >> +  CHECK_LT(signum, sizeof(*set) * 8);
> >> >> +  const uptr idx = signum / (sizeof(set->sig[0]) * 8);
> >> >> +  const uptr bit = signum % (sizeof(set->sig[0]) * 8);
> >> >> +  set->sig[idx] &= ~(1 << bit);
> >> >> +}
> >> >> +
> >> >>  // ThreadLister implementation.
> >> >>  ThreadLister::ThreadLister(int pid)
> >> >>    : pid_(pid),
> >> >>
> >> >> Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.h
> >> >> URL:
> >> >>
> >> >>
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.h?rev=192686&r1=192685&r2=192686&view=diff
> >> >>
> >> >>
> >> >>
> ==============================================================================
> >> >> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.h
> (original)
> >> >> +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.h Tue Oct
> 15
> >> >> 06:31:51 2013
> >> >> @@ -20,17 +20,39 @@
> >> >>
> >> >>  struct link_map;  // Opaque type returned by dlopen().
> >> >>  struct sigaltstack;
> >> >> +typedef struct siginfo siginfo_t;
> >> >>
> >> >>  namespace __sanitizer {
> >> >>  // Dirent structure for getdents(). Note that this structure is
> >> >> different
> >> >> from
> >> >>  // the one in <dirent.h>, which is used by readdir().
> >> >>  struct linux_dirent;
> >> >>
> >> >> +struct kernel_sigset_t {
> >> >> +  u8 sig[FIRST_32_SECOND_64(16, 8)];
> >> >> +};
> >> >> +
> >> >> +struct kernel_sigaction_t {
> >> >> +  union {
> >> >> +    void (*sigaction)(int signo, siginfo_t *info, void *ctx);
> >> >> +    void (*handler)(int signo);
> >> >> +  };
> >> >> +  unsigned long sa_flags;
> >> >> +  void (*sa_restorer)(void);
> >> >> +  kernel_sigset_t sa_mask;
> >> >> +};
> >> >
> >> >
> >> > Hm... Consider moving this to sanitizer_platform_limits_posix.(h|cc).
> >> > I think that's where we collect definitions of OS-specific structures.
> >> >
> >> >>
> >> >> +
> >> >>  // Syscall wrappers.
> >> >>  uptr internal_getdents(fd_t fd, struct linux_dirent *dirp, unsigned
> >> >> int
> >> >> count);
> >> >>  uptr internal_prctl(int option, uptr arg2, uptr arg3, uptr arg4,
> uptr
> >> >> arg5);
> >> >>  uptr internal_sigaltstack(const struct sigaltstack* ss,
> >> >>                            struct sigaltstack* oss);
> >> >> +uptr internal_sigaction(int signum, const kernel_sigaction_t *act,
> >> >> +    kernel_sigaction_t *oldact);
> >> >> +uptr internal_sigprocmask(int how, kernel_sigset_t *set,
> >> >> +    kernel_sigset_t *oldset);
> >> >> +void internal_sigfillset(kernel_sigset_t *set);
> >> >> +void internal_sigdelset(kernel_sigset_t *set, int signum);
> >> >> +
> >> >>  #ifdef __x86_64__
> >> >>  uptr internal_clone(int (*fn)(void *), void *child_stack, int flags,
> >> >> void
> >> >> *arg,
> >> >>                      int *parent_tidptr, void *newtls, int
> >> >> *child_tidptr);
> >> >>
> >> >> Modified:
> >> >>
> >> >>
> compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
> >> >> URL:
> >> >>
> >> >>
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc?rev=192686&r1=192685&r2=192686&view=diff
> >> >>
> >> >>
> >> >>
> ==============================================================================
> >> >> ---
> >> >>
> >> >>
> compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
> >> >> (original)
> >> >> +++
> >> >>
> >> >>
> compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
> >> >> Tue Oct 15 06:31:51 2013
> >> >> @@ -252,12 +252,12 @@ static int TracerThread(void* argument)
> >> >>    // the mask we inherited from the caller thread.
> >> >>    for (uptr signal_index = 0; signal_index <
> >> >> ARRAY_SIZE(kUnblockedSignals);
> >> >>         signal_index++) {
> >> >> -    struct sigaction new_sigaction;
> >> >> +    kernel_sigaction_t new_sigaction;
> >> >>      internal_memset(&new_sigaction, 0, sizeof(new_sigaction));
> >> >> -    new_sigaction.sa_sigaction = TracerThreadSignalHandler;
> >> >> +    new_sigaction.sigaction = TracerThreadSignalHandler;
> >> >>      new_sigaction.sa_flags = SA_ONSTACK | SA_SIGINFO;
> >> >> -    sigfillset(&new_sigaction.sa_mask);
> >> >> -    sigaction(kUnblockedSignals[signal_index], &new_sigaction,
> NULL);
> >> >> +    internal_sigfillset(&new_sigaction.sa_mask);
> >> >> +    internal_sigaction(kUnblockedSignals[signal_index],
> >> >> &new_sigaction,
> >> >> NULL);
> >> >>    }
> >> >>
> >> >>    int exit_code = 0;
> >> >> @@ -307,9 +307,9 @@ NOINLINE static void WipeStack() {
> >> >>
> >> >>  // We have a limitation on the stack frame size, so some stuff had
> to
> >> >> be
> >> >> moved
> >> >>  // into globals.
> >> >> -static sigset_t blocked_sigset;
> >> >> -static sigset_t old_sigset;
> >> >> -static struct sigaction
> old_sigactions[ARRAY_SIZE(kUnblockedSignals)];
> >> >> +static kernel_sigset_t blocked_sigset;
> >> >> +static kernel_sigset_t old_sigset;
> >> >> +static kernel_sigaction_t
> >> >> old_sigactions[ARRAY_SIZE(kUnblockedSignals)];
> >> >>
> >> >>  class StopTheWorldScope {
> >> >>   public:
> >> >> @@ -323,21 +323,21 @@ class StopTheWorldScope {
> >> >>      // We cannot allow user-defined handlers to run while the
> >> >> ThreadSuspender
> >> >>      // thread is active, because they could conceivably call some
> libc
> >> >> functions
> >> >>      // which modify errno (which is shared between the two threads).
> >> >> -    sigfillset(&blocked_sigset);
> >> >> +    internal_sigfillset(&blocked_sigset);
> >> >>      for (uptr signal_index = 0; signal_index <
> >> >> ARRAY_SIZE(kUnblockedSignals);
> >> >>           signal_index++) {
> >> >>        // Remove the signal from the set of blocked signals.
> >> >> -      sigdelset(&blocked_sigset, kUnblockedSignals[signal_index]);
> >> >> +      internal_sigdelset(&blocked_sigset,
> >> >> kUnblockedSignals[signal_index]);
> >> >>        // Install the default handler.
> >> >> -      struct sigaction new_sigaction;
> >> >> +      kernel_sigaction_t new_sigaction;
> >> >>        internal_memset(&new_sigaction, 0, sizeof(new_sigaction));
> >> >> -      new_sigaction.sa_handler = SIG_DFL;
> >> >> -      sigfillset(&new_sigaction.sa_mask);
> >> >> -      sigaction(kUnblockedSignals[signal_index], &new_sigaction,
> >> >> +      new_sigaction.handler = SIG_DFL;
> >> >> +      internal_sigfillset(&new_sigaction.sa_mask);
> >> >> +      internal_sigaction(kUnblockedSignals[signal_index],
> >> >> &new_sigaction,
> >> >>                        &old_sigactions[signal_index]);
> >> >>      }
> >> >>      int sigprocmask_status =
> >> >> -        sigprocmask(SIG_BLOCK, &blocked_sigset, &old_sigset);
> >> >> +        internal_sigprocmask(SIG_BLOCK, &blocked_sigset,
> &old_sigset);
> >> >>      CHECK_EQ(sigprocmask_status, 0); // sigprocmask should never
> fail
> >> >>      // Make this process dumpable. Processes that are not dumpable
> >> >> cannot
> >> >> be
> >> >>      // attached to.
> >> >> @@ -355,10 +355,10 @@ class StopTheWorldScope {
> >> >>      // Restore the signal handlers.
> >> >>      for (uptr signal_index = 0; signal_index <
> >> >> ARRAY_SIZE(kUnblockedSignals);
> >> >>           signal_index++) {
> >> >> -      sigaction(kUnblockedSignals[signal_index],
> >> >> +      internal_sigaction(kUnblockedSignals[signal_index],
> >> >>                  &old_sigactions[signal_index], NULL);
> >> >>      }
> >> >> -    sigprocmask(SIG_SETMASK, &old_sigset, &old_sigset);
> >> >> +    internal_sigprocmask(SIG_SETMASK, &old_sigset, &old_sigset);
> >> >>    }
> >> >>
> >> >>   private:
> >> >>
> >> >>
> >> >> _______________________________________________
> >> >> llvm-commits mailing list
> >> >> llvm-commits at cs.uiuc.edu
> >> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >> >
> >> >
> >> >
> >> >
> >> > --
> >> > Alexey Samsonov, MSK
> >
> >
> >
> >
> > --
> > Alexey Samsonov, MSK
>



-- 
Alexey Samsonov, MSK
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131015/5ce29a70/attachment.html>


More information about the llvm-commits mailing list