[PATCH] Make strace correctly handle SIGTRAP produced by e.g. kill(2) and by trapping instruction.

Roland McGrath roland at redhat.com
Mon Dec 14 17:51:17 UTC 2009


Please don't repost the same patch again while ignoring the review I gave
before!  I'm repeating the same comments I've already given, that you
ignored completely.  Don't expect anything different from reposting the
same patch yet again.

There are a few kinds of older kernels you need to account for here.
You have this comment:

+				/*
+				 * NB: even if this "succeeds", we can
+				 * revert back to SIGTRAP if we later see
+				 * that it didnt really work.
+				 * Old kernels are known to lie here.
+				 */

But it is not clear to me exactly you are handling each of the following
cases.  Please clarify the scenario that will play out for each of these in
your code.

Some kernels have PTRACE_SETOPTIONS that actually only supports
PTRACE_O_TRACESYSGOOD and no other option bits, but does not reject
any bits.  So those will return 0 here even though PTRACE_O_TRACEEXEC
was ignored.

Some kernels (I think) have PTRACE_SETOPTIONS that checks for unsupported
bits, and do not support PTRACE_O_TRACEEXEC.  So those fail here with
EINVAL, but you could back off to just PTRACE_O_TRACESYSGOOD and win.

Because of the second case above, you can't assume that exec stops use
PTRACE_EVENT_EXEC until you actually see one.  Before you've seen one, a
SIGTRAP stop could be an exec stop.  You should always know when an exec
stop is coming, because you just traced the execve syscall exit with
successful return.

> +				/* It's post-exec ptrace stop.  */
> +				/* Set WSTOPSIG(status) = (SIGTRAP | 0x80).  */
> +				status |= 0x8000;

The sole purpose of this is for:

+		if (WSTOPSIG(status) != ptrace_stop_sig) {

not to match, right?  I think it would be more clear to write it as:

	/*
	 * Make our check below recognize this as a ptrace stop.
	 */
	status = W_STOPCODE(ptrace_stop_sig);

> +				 * Check some fields to make sure we see
> +				 * real SIGTRAP.
> +				 * Otherwise interpret it as ptrace stop.
> +				 * Real SIGTRAPs (int3 insn on x86, kill() etc)
> +				 * have these values:
> +				 * int3:                   kill -TRAP $pid:
> +				 * si_signo:5 (SIGTRAP)    si_signo:5 (SIGTRAP)
> +				 * si_errno:0              si_errno:(?)
> +				 * si_code:128 (SI_KERNEL) si_code:0 (SI_USER)
> +				 * si_pid:0                si_pid:(>0?)
> +				 * si_band:0               si_band:(?)
> +				 * Ptrace stops have garbage there instead.

This is way too fiddly.  These assumptions about siginfo_t settings for
real signals are not really valid across all machines and kernel versions.


Thanks,
Roland




More information about the Strace-devel mailing list