Re: truss

On Mon, Sep 19, 2011 at 04:03:42PM +0000, Anton Yuzhaninov wrote:
> On Mon, 19 Sep 2011 15:00:31 +0000 (UTC), Anton Yuzhaninov wrote:
> AY> On Mon, 19 Sep 2011 15:58:02 +0300, Mikolaj Golub wrote:
>  AY>>> ktrace -i for truss sleep 5
>  AY>>> http://dl.dropbox.com/u/8798217/tmp/truss_ktrace2.txt
> MG>> 
> MG>> Although ptrace(PT_TRACE_ME,0,0,0) returned 0 the process did not stop after
> MG>> execve() and wait4() in parent (which was actually waiting for this stop)
> MG>> returned only after the child exit. No I idea why so far :-).
> MG>> 
> AY> 
> AY> As I understand SIGTRAP used to stop child process after execve(), but
> AY> this signal ignored:
> AY> 
> AY> citrin:~> sleep 300 &
> AY> citrin:~> procstat -i 1991 | fgrep TRAP
> AY>  1991 sleep            TRAP     -I-
> AY> 
> AY> Under FreeBSD 8, where ptrace works for me, this signal is not ignored:
> AY> x:~> sleep 300 &
> AY> x:~> procstat -i 78716 | fgrep TRAP
> AY> 78716 sleep            TRAP     ---
> 
> SIGTRAP is ignored by X window manager used by me, and this is inherited across
> forks/execs up to the truss.
> 
> IMHO truss should restore default signal handler for SIGTRAP.
> 
> With this patch truss works for me:
> 
> --- usr.bin/truss/main.c        (revision 225504)
> +++ usr.bin/truss/main.c        (working copy)
> _at__at_ -255,6 +255,11 _at__at_ main(int ac, char **av)
> 
>         if (trussinfo->pid == 0) {      /* Start a command ourselves */
>                 command = av;
> +               /*
> +                * SIGTRUP used to stop traced process after execve
> +                * un-ignore this signal (it can be ignored by parents)
> +                */
> +               signal(SIGTRAP, SIG_DFL);
>                 trussinfo->pid = setup_and_wait(command);
>                 signal(SIGINT, SIG_IGN);
>                 signal(SIGTERM, SIG_IGN);
This is quite a hack. The proper fix should go in kernel, otherwise
we cannot debug programs that decided to ignore SIGTRAP. The reason
there is that tdsendsignal() does nothing for attempt to deliver ignored
signal, and kern_execve() simply tries to send a signal to self.

BTW, it seems we might also have similar issues for threads that masks
SIGTRAP, now because issignal() does nothing in that case too. But lets
handle it by small steps.

Could you, please, test the change below ? For me, I still can truss(1)
or debug with gdb after the change applied. Does truss work for you
with only this change, without resetting SIGTRAP handler in truss process ?

commit 2ae586c039a55399edc3b34cd40410e0d690a16c
Author: Konstantin Belousov <kostik_at_pooma.home>
Date:   Tue Sep 20 00:25:07 2011 +0300

    Do not deliver SIGTRAP on exec as the normal signal, use ptracestop()
    on syscall exit path. Otherwise, if SIGTRAP is ignored, that tdsendsignal()
    do not want to deliver, and debugger never get a notification of exec.
    
    Found by:	Anton Yuzhaninov <citrin_at_citrin.ru>

diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c
index fe01142..4545848 100644
--- a/sys/kern/kern_exec.c
+++ b/sys/kern/kern_exec.c
_at__at_ -777,16 +777,6 _at__at_ interpret:
 	KNOTE_LOCKED(&p->p_klist, NOTE_EXEC);
 	p->p_flag &= ~P_INEXEC;
 
-	/*
-	 * If tracing the process, trap to the debugger so that
-	 * breakpoints can be set before the program executes.  We
-	 * have to use tdsignal() to deliver the signal to the current
-	 * thread since any other threads in this process will exit if
-	 * execve() succeeds.
-	 */
-	if (p->p_flag & P_TRACED)
-		tdsignal(td, SIGTRAP);
-
 	/* clear "fork but no exec" flag, as we _are_ execing */
 	p->p_acflag &= ~AFORK;
 
diff --git a/sys/kern/subr_syscall.c b/sys/kern/subr_syscall.c
index cb0d929..bba4479 100644
--- a/sys/kern/subr_syscall.c
+++ b/sys/kern/subr_syscall.c
_at__at_ -204,9 +204,17 _at__at_ syscallret(struct thread *td, int error, struct syscall_args *sa __unused)
 	 * is not the case, this code will need to be revisited.
 	 */
 	STOPEVENT(p, S_SCX, sa->code);
-	PTRACESTOP_SC(p, td, S_PT_SCX);
 	if (traced || (td->td_dbgflags & (TDB_EXEC | TDB_FORK)) != 0) {
 		PROC_LOCK(p);
+		/*
+		 * If tracing the execed process, trap to the debugger
+		 * so that breakpoints can be set before the program
+		 * executes.  If debugger requested tracing of syscall
+		 * returns, do it now too.
+		 */
+		if (traced && ((td->td_dbgflags & TDB_EXEC) != 0 ||
+		    (p->p_stops & S_PT_SCX) != 0))
+			ptracestop(td, SIGTRAP);
 		td->td_dbgflags &= ~(TDB_SCX | TDB_EXEC | TDB_FORK);
 		PROC_UNLOCK(p);
 	}