[PATCH] get rid of TCB_SIGTRAPPED

Denys Vlasenko dvlasenk at redhat.com
Wed Jan 18 14:56:38 UTC 2012


On attempts to block or set SIGTRAP handler,
for example, using sigaction syscall, we generate
an additional SIGSTOP:

                 if ((long)sa.SA_HANDLER == (long)SIG_ERR)
                         tprints("{SIG_ERR, ");
                 else if ((long)sa.SA_HANDLER == (long)SIG_DFL)
                         tprints("{SIG_DFL, ");
                 else if ((long)sa.SA_HANDLER == (long)SIG_IGN) {
                         if (tcp->u_arg[0] == SIGTRAP) {
                                 tcp->flags |= TCB_SIGTRAPPED;
                                 kill(tcp->pid, SIGSTOP);
                         }
                         tprints("{SIG_IGN, ");
                 }
                 else {
                         if (tcp->u_arg[0] == SIGTRAP) {
                                 tcp->flags |= TCB_SIGTRAPPED;
                                 kill(tcp->pid, SIGSTOP);
                         }
                         tprintf("{%#lx, ", (long) sa.SA_HANDLER);
                 }

In strace, it looks this way:

     Entering in sys_sigaction(SIGTRAP):
wait4(-1, [{WIFSTOPPED(s) && WSTOPSIG(s) == 133}], __WALL, NULL) = 5848
ptrace(PTRACE_GETREGS, 5848, 0, 0x808d3e0) = 0
ptrace(PTRACE_PEEKDATA, 5848, 0xbf996274, [0x804825b]) = 0
ptrace(PTRACE_PEEKDATA, 5848, 0xbf996278, [0]) = 0
ptrace(PTRACE_PEEKDATA, 5848, 0xbf99627c, [0x4000000]) = 0
ptrace(PTRACE_PEEKDATA, 5848, 0xbf996280, [0x8048253]) = 0
     Aha, it's the 'else' branch (in the above C code), send SIGSTOP:
kill(5848, SIGSTOP)                     = 0
     print the log:
write(3, "sigaction(SIGTRAP, {0x804825b, [], SA_RESTORER, 0x8048253}, ", 60) = 60
     go go go:
ptrace(PTRACE_SYSCALL, 5848, 0x1, SIG_0) = 0
--- {si_signo=SIGCHLD, si_code=CLD_TRAPPED, si_pid=5848, si_status=SIGTRAP, si_utime=0, si_stime=0} (Child exited) ---
wait4(-1, [{WIFSTOPPED(s) && WSTOPSIG(s) == 133}], __WALL, NULL) = 5848
     exiting sigaction - print result:
ptrace(PTRACE_GETREGS, 5848, 0, 0x808d3e0) = 0
write(3, "NULL, 0x8048253) = 0\n", 21)  = 21
     continue:
ptrace(PTRACE_SYSCALL, 5848, 0x1, SIG_0) = 0
     oops, got our STOP:
--- {si_signo=SIGCHLD, si_code=CLD_TRAPPED, si_pid=5848, si_status=SIGSTOP, si_utime=0, si_stime=0} (Child exited) ---
wait4(-1, [{WIFSTOPPED(s) && WSTOPSIG(s) == SIGSTOP}], __WALL, NULL) = 5848
     ignore it:
ptrace(PTRACE_SYSCALL, 5848, 0x1, SIG_0) = 0
--- {si_signo=SIGCHLD, si_code=CLD_TRAPPED, si_pid=5848, si_status=SIGTRAP, si_utime=0, si_stime=0} (Child exited) ---
     entering into next syscall, getpid:
wait4(-1, [{WIFSTOPPED(s) && WSTOPSIG(s) == 133}], __WALL, NULL) = 5848
ptrace(PTRACE_GETREGS, 5848, 0, 0x808d3e0) = 0
write(3, "getpid(", 7)                  = 7
...

Attached patch gets rid of this SIGSTOP sending/ignoring.
It appears to work just fine:

wait4(-1, [{WIFSTOPPED(s) && WSTOPSIG(s) == SIGTRAP}], __WALL, NULL) = 5967
ptrace(PTRACE_GETREGS, 5967, 0, 0x808c2c0) = 0
ptrace(PTRACE_PEEKDATA, 5967, 0xbfe764b4, [0x804825b]) = 0
ptrace(PTRACE_PEEKDATA, 5967, 0xbfe764b8, [0]) = 0
ptrace(PTRACE_PEEKDATA, 5967, 0xbfe764bc, [0x4000000]) = 0
ptrace(PTRACE_PEEKDATA, 5967, 0xbfe764c0, [0x8048253]) = 0
write(3, "sigaction(SIGTRAP, {0x804825b, [], SA_RESTORER, 0x8048253}, ", 60) = 60
ptrace(PTRACE_SYSCALL, 5967, 0x1, SIG_0) = 0
--- {si_signo=SIGCHLD, si_code=CLD_TRAPPED, si_pid=5967, si_status=SIGTRAP, si_utime=0, si_stime=0} (Child exited) ---
wait4(-1, [{WIFSTOPPED(s) && WSTOPSIG(s) == SIGTRAP}], __WALL, NULL) = 5967
ptrace(PTRACE_GETREGS, 5967, 0, 0x808c2c0) = 0
write(3, "NULL, 0x8048253) = 0\n", 21)  = 21
ptrace(PTRACE_SYSCALL, 5967, 0x1, SIG_0) = 0
--- {si_signo=SIGCHLD, si_code=CLD_TRAPPED, si_pid=5967, si_status=SIGTRAP, si_utime=0, si_stime=0} (Child exited) ---
wait4(-1, [{WIFSTOPPED(s) && WSTOPSIG(s) == SIGTRAP}], __WALL, NULL) = 5967
ptrace(PTRACE_GETREGS, 5967, 0, 0x808c2c0) = 0
write(3, "getpid(", 7)                  = 7
...

It also works if I force strace to not use PTRACE_O_TRACESYSGOOD,
which means strace stops will be marked with SIGTRAP,
not (SIGTRAP | 0x80) - I wondered maybe that's when it's needed:

wait4(-1, [{WIFSTOPPED(s) && WSTOPSIG(s) == SIGTRAP}], __WALL, NULL) = 6002
ptrace(PTRACE_GETREGS, 6002, 0, 0x808c2a0) = 0
ptrace(PTRACE_PEEKDATA, 6002, 0xbfa63254, [0x804825b]) = 0
ptrace(PTRACE_PEEKDATA, 6002, 0xbfa63258, [0]) = 0
ptrace(PTRACE_PEEKDATA, 6002, 0xbfa6325c, [0x4000000]) = 0
ptrace(PTRACE_PEEKDATA, 6002, 0xbfa63260, [0x8048253]) = 0
write(3, "sigaction(SIGTRAP, {0x804825b, [], SA_RESTORER, 0x8048253}, ", 60) = 60
ptrace(PTRACE_SYSCALL, 6002, 0x1, SIG_0) = 0
--- {si_signo=SIGCHLD, si_code=CLD_TRAPPED, si_pid=6002, si_status=SIGTRAP, si_utime=0, si_stime=0} (Child exited) ---
wait4(-1, [{WIFSTOPPED(s) && WSTOPSIG(s) == SIGTRAP}], __WALL, NULL) = 6002
ptrace(PTRACE_GETREGS, 6002, 0, 0x808c2a0) = 0
write(3, "NULL, 0x8048253) = 0\n", 21)  = 21
ptrace(PTRACE_SYSCALL, 6002, 0x1, SIG_0) = 0
--- {si_signo=SIGCHLD, si_code=CLD_TRAPPED, si_pid=6002, si_status=SIGTRAP, si_utime=0, si_stime=0} (Child exited) ---
wait4(-1, [{WIFSTOPPED(s) && WSTOPSIG(s) == SIGTRAP}], __WALL, NULL) = 6002
ptrace(PTRACE_GETREGS, 6002, 0, 0x808c2a0) = 0
write(3, "getpid(", 7)                  = 7
...

As you see, the same thing. No hangs alluded to by
                                /*
                                 * Trapped attempt to block SIGTRAP
                                 * Hope we are back in control now.
                                 */
comment which used to be there.


So, why we even have TCB_SIGTRAPPED? No one knows. It predates
version control: this code was present in the initial commit,
in 1999.

Moreover, TCB_SIGTRAPPED is not used in sys_rt_sigaction,
only in obsolete sys_signal, sys_sigaction, sys_sigsetmask,
and in some dead non-Linux code.

I think whatever bug it was fixing is gone long ago -
at least as long as sys_rt_sigaction is used by glibc.
Again, since glibc (and uclibc) uses sys_rt_sigaction
and sys_sigprocmask, modified code paths are not used
by most programs anyway.

Test program I used for generating logs shown above
is below. Use:

strace -oLOG0 -s99 ./strace -o LOG -s99 ./test

-- 
vda

#include <ctype.h>
#include <dirent.h>
#include <errno.h>
#include <fcntl.h>
#include <inttypes.h>
#include <netdb.h>
#include <setjmp.h>
#include <signal.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <stdarg.h>
#include <stddef.h>
#include <string.h>
#include <syscall.h>
#include <sys/poll.h>
#include <sys/ioctl.h>
#include <sys/mman.h>
#include <sys/socket.h>
#include <sys/stat.h>
#include <sys/time.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <termios.h>
#include <time.h>
#include <sys/param.h>

/* This is the sigaction structure from the Linux 2.1.20 kernel.  */
struct old_kernel_sigaction {
         __sighandler_t k_sa_handler;
         unsigned long sa_mask;
         unsigned long sa_flags;
         void (*sa_restorer)(void);
};

#define SA_RESTORER     0x04000000

extern void restoreZ(void) __asm__ ("__restoreZ");

int sigactionZ(int sig, const struct sigaction *act, struct sigaction *oact)
{
	int result;
	struct old_kernel_sigaction kact, koact;

	if (act) {
		kact.k_sa_handler = act->sa_handler;
		kact.sa_mask = act->sa_mask.__val[0];
		kact.sa_flags = act->sa_flags | SA_RESTORER;
		kact.sa_restorer = &restoreZ;
	}
	__asm__ __volatile__ (
		"	pushl	%%ebx\n"
		"	movl	%3, %%ebx\n"
		"	int	$0x80\n"
		"	popl	%%ebx\n"
		: "=a" (result), "=m" (koact)
		: "0" (__NR_sigaction), "r" (sig), "m" (kact),
		  "c" (act ? &kact : NULL),
		  "d" (oact ? &koact : NULL));
	if (result < 0) {
		__set_errno(-result);
		return -1;
	}
	if (oact) {
		oact->sa_handler = koact.k_sa_handler;
		oact->sa_mask.__val[0] = koact.sa_mask;
		oact->sa_flags = koact.sa_flags;
		oact->sa_restorer = koact.sa_restorer;
	}
	return result;
}
#define RESTORE(name, syscall) RESTORE2(name, syscall)

#define RESTORE2(name, syscall) \
__asm__ (						\
	".text\n"					\
	"__" #name ":\n"				\
	"	popl	%eax\n"				\
	"	movl	$" #syscall ", %eax\n"		\
	"	int	$0x80\n"			\
);

RESTORE(restoreZ, __NR_sigreturn)

static void handler(int sig)
{
	printf("Got sig:%d\n", sig);
}

int main(int argc, char **argv)
{
	struct sigaction sa;

	memset(&sa, 0, sizeof(sa));
	sa.sa_handler = handler;

	///signal(SIGTRAP, handler);
	sigactionZ(SIGTRAP, &sa, NULL);

	raise(SIGTRAP); // 1

	sigset_t set;
	sigemptyset(&set);
	sigaddset(&set, SIGTRAP);
	sigprocmask(SIG_BLOCK, &set, NULL);
	raise(SIGTRAP);
	// trap should not happen

	sigprocmask(SIG_UNBLOCK, &set, NULL);
	// 2 trap should happen here

	///signal(SIGTRAP, SIG_IGN);
	sa.sa_handler = SIG_IGN;
	sigactionZ(SIGTRAP, &sa, NULL);
	raise(SIGTRAP);
	// trap should not happen

	///signal(SIGTRAP, handler);
	sa.sa_handler = handler;
	sigactionZ(SIGTRAP, &sa, NULL);
	raise(SIGTRAP); // 3

	return 0;
}





diff -d -urpN strace.7/defs.h strace.8/defs.h
--- strace.7/defs.h	2012-01-18 15:18:36.384963666 +0100
+++ strace.8/defs.h	2012-01-16 13:09:14.313265676 +0100
@@ -477,7 +477,6 @@ struct tcb {
  #define TCB_INSYSCALL	00010
  #define TCB_ATTACHED	00020	/* Process is not our own child */
  #define TCB_BPTSET	00100	/* "Breakpoint" set after fork(2) */
-#define TCB_SIGTRAPPED	00200	/* Process wanted to block SIGTRAP */
  #define TCB_REPRINT	01000	/* We should reprint this syscall on exit */
  #define TCB_FILTERED	02000	/* This system call has been filtered out */
  #ifdef LINUX
diff -d -urpN strace.7/signal.c strace.8/signal.c
--- strace.7/signal.c	2012-01-18 15:19:26.334601560 +0100
+++ strace.8/signal.c	2012-01-18 15:16:33.872902944 +0100
@@ -842,24 +842,12 @@ sys_sigvec(struct tcb *tcp)
  			tprints("{SIG_DFL}");
  			break;
  		case (int) SIG_IGN:
-			if (tcp->u_arg[0] == SIGTRAP) {
-				tcp->flags |= TCB_SIGTRAPPED;
-				kill(tcp->pid, SIGSTOP);
-			}
  			tprints("{SIG_IGN}");
  			break;
  		case (int) SIG_HOLD:
-			if (tcp->u_arg[0] == SIGTRAP) {
-				tcp->flags |= TCB_SIGTRAPPED;
-				kill(tcp->pid, SIGSTOP);
-			}
-			tprints("SIG_HOLD");
+			tprints("{SIG_HOLD}");
  			break;
  		default:
-			if (tcp->u_arg[0] == SIGTRAP) {
-				tcp->flags |= TCB_SIGTRAPPED;
-				kill(tcp->pid, SIGSTOP);
-			}
  			tprintf("{%#lx, ", (unsigned long) sv.sv_handler);
  			printsigmask(&sv.sv_mask, 0);
  			tprints(", ");
@@ -923,14 +911,6 @@ sys_sigsetmask(struct tcb *tcp)
  		sigset_t sigm;
  		long_to_sigset(tcp->u_arg[0], &sigm);
  		printsigmask(&sigm, 0);
-#ifndef USE_PROCFS
-		if ((tcp->u_arg[0] & sigmask(SIGTRAP))) {
-			/* Mark attempt to block SIGTRAP */
-			tcp->flags |= TCB_SIGTRAPPED;
-			/* Send unblockable signal */
-			kill(tcp->pid, SIGSTOP);
-		}
-#endif /* !USE_PROCFS */
  	}
  	else if (!syserror(tcp)) {
  		sigset_t sigm;
@@ -1005,24 +985,10 @@ sys_sigaction(struct tcb *tcp)
  			tprints("{SIG_ERR, ");
  		else if ((long)sa.SA_HANDLER == (long)SIG_DFL)
  			tprints("{SIG_DFL, ");
-		else if ((long)sa.SA_HANDLER == (long)SIG_IGN) {
-#ifndef USE_PROCFS
-			if (tcp->u_arg[0] == SIGTRAP) {
-				tcp->flags |= TCB_SIGTRAPPED;
-				kill(tcp->pid, SIGSTOP);
-			}
-#endif /* !USE_PROCFS */
+		else if ((long)sa.SA_HANDLER == (long)SIG_IGN)
  			tprints("{SIG_IGN, ");
-		}
-		else {
-#ifndef USE_PROCFS
-			if (tcp->u_arg[0] == SIGTRAP) {
-				tcp->flags |= TCB_SIGTRAPPED;
-				kill(tcp->pid, SIGSTOP);
-			}
-#endif /* !USE_PROCFS */
+		else
  			tprintf("{%#lx, ", (long) sa.SA_HANDLER);
-		}
  #ifndef LINUX
  		printsigmask(&sa.sa_mask, 0);
  #else
@@ -1060,21 +1026,9 @@ sys_signal(struct tcb *tcp)
  			tprints("SIG_DFL");
  			break;
  		case (long) SIG_IGN:
-#ifndef USE_PROCFS
-			if (tcp->u_arg[0] == SIGTRAP) {
-				tcp->flags |= TCB_SIGTRAPPED;
-				kill(tcp->pid, SIGSTOP);
-			}
-#endif /* !USE_PROCFS */
  			tprints("SIG_IGN");
  			break;
  		default:
-#ifndef USE_PROCFS
-			if (tcp->u_arg[0] == SIGTRAP) {
-				tcp->flags |= TCB_SIGTRAPPED;
-				kill(tcp->pid, SIGSTOP);
-			}
-#endif /* !USE_PROCFS */
  			tprintf("%#lx", tcp->u_arg[1]);
  		}
  		return 0;
diff -d -urpN strace.7/strace.c strace.8/strace.c
--- strace.7/strace.c	2012-01-18 15:18:36.386963635 +0100
+++ strace.8/strace.c	2012-01-16 13:09:04.314285771 +0100
@@ -2540,15 +2540,6 @@ trace()
  		}

  		if (sig != syscall_trap_sig) {
-			if (sig == SIGSTOP &&
-					(tcp->flags & TCB_SIGTRAPPED)) {
-				/*
-				 * Trapped attempt to block SIGTRAP
-				 * Hope we are back in control now.
-				 */
-				tcp->flags &= ~(TCB_INSYSCALL | TCB_SIGTRAPPED);
-				goto restart_tracee_with_sig_0;
-			}
  			if (cflag != CFLAG_ONLY_STATS
  			    && (qual_flags[sig] & QUAL_SIGNAL)) {
  				siginfo_t si;




More information about the Strace-devel mailing list