[PATCH] Add option to trace child program's threads only
Thomas S.J
sjthomas888 at gmail.com
Wed Jan 27 16:07:54 UTC 2021
Thanks for your comments.
I shall take care of the white-space in the updated diff.
I will add the test-cases in the strace test-suite taking
reference(rhyming with) of some existing test-case.
I shall also remove the errormsg for followthread when only -F option
is given, in the updated diff.
On Wed, Jan 6, 2021 at 11:45 PM Ákos Uzonyi <uzonyi.akos at gmail.com> wrote:
>
> Hi,
>
> I'm not a maintainer, just sharing some thoughts on your patch:
>
> The patch seems to be whitespace corrupted by gmail. Using "git
> send-mail" is a good way to avoid this problem.
>
> Note that the clone syscall can be used to create processes too, not
> just threads. The documentation of PTRACE_O_TRACECLONE says that it
> does not trace all clone calls (it excludes fork and vfork like clone
> calls), so it might not be a problem, but I think it's worth thinking
> about whether those exceptional cases are enough to filter out
> non-thread processes.
>
> It would be nice if the tests you mentioned were integrated in the
> strace test suite.
>
> On Wed, 6 Jan 2021 at 17:07, Thomas S.J <sjthomas888 at gmail.com> wrote:
> >
> > As per issue #141 in Strace's github page, there was a
> > requirement/discussion on adding a feature that would allow a user to
> > attach strace to all the threads of the tracee program but does not
> > trace the child process.
> >
> > The following patch adds the above described option.
> >
> > I have executed the following test-cases:
> > 1. Usage help
> > 2. Traced a program having child process
> > 3. Traced a program having only threads
> > 4. Traced a program having threads and child process
> > 5. Run strace with combination of conflicting options
> >
> > I have also run the scripts/checkpatch.pl script (from the Linux
> > kernel repository) on the patch to verify coding style.
> >
> > Please review and comment on the diff below.
> >
> > From bb3635e53beba0b9f1c70b3441cb2fd618f2c058 Mon Sep 17 00:00:00 2001
> > From: S J Thomas <sjthomas888 at gmail.com>
> > Date: Mon, 4 Jan 2021 20:26:41 +0530
> > Subject: [PATCH v5] Add option to follow only threads
> >
> > ---
> > strace.c | 29 +++++++++++++++++++++++------
> > 1 file changed, 23 insertions(+), 6 deletions(-)
> >
> > diff --git a/strace.c b/strace.c
> > index 8adbe45f..99e07902 100644
> > --- a/strace.c
> > +++ b/strace.c
> > @@ -66,6 +66,7 @@ const unsigned int syscall_trap_sig = SIGTRAP | 0x80;
> >
> > cflag_t cflag = CFLAG_NONE;
> > bool followfork;
> > +bool followthreads;
> > bool output_separately;
> > unsigned int ptrace_setoptions = PTRACE_O_TRACESYSGOOD | PTRACE_O_TRACEEXEC
> > | PTRACE_O_TRACEEXIT;
> > @@ -297,6 +298,8 @@ Tracing:\n\
> > follow forks\n\
> > -ff, --follow-forks --output-separately\n\
> > follow forks with output into separate files\n\
> > + --follow-threads\n\
> > + follow threads only, not child processes\n\
> > -I INTERRUPTIBLE, --interruptible=INTERRUPTIBLE\n\
> > 1, anywhere: no signals are blocked\n\
> > 2, waiting: fatal signals are blocked while decoding syscall
> > (default)\n\
> > @@ -1201,7 +1204,7 @@ attach_tcb(struct tcb *const tcp)
> > DIR *dir;
> > unsigned int ntid = 0, nerr = 0;
> >
> > - if (followfork && tcp->pid != strace_child &&
> > + if ((followfork || followthreads) && tcp->pid != strace_child &&
> > xsprintf(procdir, task_path, get_proc_pid(tcp)) > 0 &&
> > (dir = opendir(procdir)) != NULL) {
> > struct_dirent *de;
> > @@ -2030,6 +2033,7 @@ init(int argc, char *argv[])
> > GETOPT_DAEMONIZE,
> > GETOPT_HEX_STR,
> > GETOPT_FOLLOWFORKS,
> > + GETOPT_FOLLOWTHREADS,
> > GETOPT_OUTPUT_SEPARATELY,
> > GETOPT_TS,
> > GETOPT_PIDNS_TRANSLATION,
> > @@ -2060,6 +2064,7 @@ init(int argc, char *argv[])
> > { "daemonized", optional_argument, 0, GETOPT_DAEMONIZE },
> > { "env", required_argument, 0, 'E' },
> > { "follow-forks", no_argument, 0, GETOPT_FOLLOWFORKS },
> > + { "follow-threads", no_argument, 0, GETOPT_FOLLOWTHREADS },
> > { "output-separately", no_argument, 0,
> > GETOPT_OUTPUT_SEPARATELY },
> > { "help", no_argument, 0, 'h' },
> > @@ -2178,6 +2183,9 @@ init(int argc, char *argv[])
> > case GETOPT_FOLLOWFORKS:
> > followfork = true;
> > break;
> > + case GETOPT_FOLLOWTHREADS:
> > + followthreads = true;
> > + break;
> > case GETOPT_OUTPUT_SEPARATELY:
> > output_separately = true;
> > break;
> > @@ -2439,9 +2447,9 @@ init(int argc, char *argv[])
> > if (nprocs && (!argc || debug_flag))
> > error_msg("--seccomp-bpf is not enabled for processes"
> > " attached with -p");
> > - if (!followfork) {
> > + if (!(followfork || followthreads)) {
> > error_msg("--seccomp-bpf cannot be used without "
> > - "-f/--follow-forks, disabling");
> > + "-f/--follow-forks or --follow-threads, disabling");
> > seccomp_filtering = false;
> > }
> > }
> > @@ -2454,6 +2462,13 @@ init(int argc, char *argv[])
> > "please use -f/--follow-forks instead");
> > followfork = true;
> > }
> > + if (followthreads) {
> > + error_msg("deprecated option -F ignored");
> > + } else {
> > + error_msg("option -F is deprecated, "
> > + "please use --follow-threads instead");
> > + followthreads = true;
> > + }
> > }
>
> I think you don't need to set followthreads if the -F option is given.
>
>
> > if (output_separately && cflag) {
> > @@ -2502,7 +2517,7 @@ init(int argc, char *argv[])
> > }
> >
> > if (!outfname) {
> > - if (output_separately && !followfork)
> > + if (output_separately && !followfork && !followthreads)
> > error_msg("--output-separately has no effect "
> > "without -o/--output");
> > if (open_append)
> > @@ -2559,6 +2574,8 @@ init(int argc, char *argv[])
> > ptrace_setoptions |= PTRACE_O_TRACECLONE |
> > PTRACE_O_TRACEFORK |
> > PTRACE_O_TRACEVFORK;
> > + if (followthreads)
> > + ptrace_setoptions |= PTRACE_O_TRACECLONE;
> >
> > if (seccomp_filtering)
> > check_seccomp_filter();
> > @@ -2693,7 +2710,7 @@ init(int argc, char *argv[])
> > * -p PID1,PID2: yes (there are already more than one pid)
> > */
> > print_pid_pfx = outfname && !output_separately &&
> > - ((followfork && !output_separately) || nprocs > 1);
> > + (((followfork || followthreads) && !output_separately) || nprocs > 1);
> > }
> >
> > static struct tcb *
> > @@ -2806,7 +2823,7 @@ maybe_allocate_tcb(const int pid, int status)
> > }
> > return NULL;
> > }
> > - if (followfork) {
> > + if (followfork || followthreads) {
> > /* We assume it's a fork/vfork/clone child */
> > struct tcb *tcp = alloctcb(pid);
> > after_successful_attach(tcp, post_attach_sigstop);
> > --
> > 2.28.0
> --
> Strace-devel mailing list
> Strace-devel at lists.strace.io
> https://lists.strace.io/mailman/listinfo/strace-devel
More information about the Strace-devel
mailing list