[PATCH v2 4/5] Extend -Y option to decode PIDS in arguments and retvals
Ákos Uzonyi
uzonyi.akos at gmail.com
Wed Aug 25 15:57:43 UTC 2021
On Tue, 24 Aug 2021 at 21:27, Masatake YAMATO <yamato at redhat.com> wrote:
> From: <yamato at redhat.com>
>
> From: Masatake YAMATO <yamato at redhat.com>
>
> This change introduces -YY,--decode-pids=all option. In addition to
> what printed -Y option, the new options print command names for PIDS
> appeared in arguments and retvals.
>
> An example:
>
> getppid() = 3781395<strace>
> getpid() = 3781398<a.out>
> pidfd_open(1<systemd>, 0) = 3<pid:1<systemd>>
>
> * src/defs.h (maybe_printpid_comm): New function declaration.
> * src/strace.c (maybe_printpid_comm): New function printing
> command name for given PID.
> (usage): Add help message for the -YY option.
> (init): Add -YY and --decode-pids=all options.
> * src/pidns.c (printpid_translation,printpid,printpid_tgid_pgid): call
> `maybe_printpid_comm'.
> * src/filter_qualify.c (qualify_decode_pid): Handle "all" specified in
> --decode-pids=all.
> * src/time.c (printclockname): Print command name for pid.
> * src/util.c (printpidfd): Ditto.
> * doc/strace.1.in (.SS Output format): document the -YY option.
>
> Signed-off-by: Masatake YAMATO <yamato at redhat.com>
> ---
> doc/strace.1.in | 6 ++++++
> src/defs.h | 2 ++
> src/filter_qualify.c | 2 ++
> src/pidns.c | 11 +++++++++--
> src/strace.c | 24 ++++++++++++++++++++++--
> src/time.c | 4 +++-
> src/util.c | 1 +
> 7 files changed, 45 insertions(+), 5 deletions(-)
>
> diff --git a/doc/strace.1.in b/doc/strace.1.in
> index 99fba9076..28358f7df 100644
> --- a/doc/strace.1.in
> +++ b/doc/strace.1.in
> @@ -1089,6 +1089,12 @@ and PIDs associated with pidfd file descriptors.
> .BR \-\-decode\-pids = prefix
> Print command names for PIDs that appear as the prefixes of trace lines.
> .TP
> +.B \-YY
> +.TQ
> +.BR \-\-decode\-pids = all
> +Print command names for PIDs that appear in arguments and return values
> +in addition to what printed with \-Y option.
> +.TP
> .B \-\-pidns\-translation
> If strace and tracee are in different PID namespaces, print PIDs in
> strace's namespace, too.
> diff --git a/src/defs.h b/src/defs.h
> index 33f44dd64..5a4d418ee 100644
> --- a/src/defs.h
> +++ b/src/defs.h
> @@ -1909,4 +1909,6 @@ print_big_u64_addr(const uint64_t addr)
> # endif
>
> void maybe_load_task_comm(struct tcb *tcp);
> +/* print the content of /proc/$pid/comm. */
> +void maybe_printpid_comm(int pid);
> #endif /* !STRACE_DEFS_H */
> diff --git a/src/filter_qualify.c b/src/filter_qualify.c
> index 24e840c2f..b871176b1 100644
> --- a/src/filter_qualify.c
> +++ b/src/filter_qualify.c
> @@ -468,6 +468,8 @@ qualify_decode_pid(const char *const str)
> {
> if (strcmp(str, "prefix") == 0)
> decode_pids_enabled = 1;
> + else if (strcmp(str, "all") == 0)
> + decode_pids_enabled = 2;
> else
> error_msg_and_die("invalid --decode-pids = argument: '%s'", str);
> }
> diff --git a/src/pidns.c b/src/pidns.c
> index d3ebd0535..57f695d22 100644
> --- a/src/pidns.c
> +++ b/src/pidns.c
> @@ -586,6 +586,8 @@ printpid_translation(struct tcb *tcp, int pid, enum pid_type type)
> return;
>
> int strace_pid = translate_pid(tcp, pid, type, NULL);
> + if (strace_pid && (type == PT_TID || type == PT_TGID))
> + maybe_printpid_comm(strace_pid);
> if (strace_pid && strace_pid != pid)
> tprintf_comment("%d in strace's PID NS", strace_pid);
> }
> @@ -594,6 +596,9 @@ void
> printpid(struct tcb *tcp, int pid, enum pid_type type)
> {
> PRINT_VAL_D(pid);
> + if (!pidns_translation
> + && (type == PT_TID || type == PT_TGID))
> + maybe_printpid_comm(pid);
> printpid_translation(tcp, pid, type);
> }
>
> @@ -601,8 +606,10 @@ void
> printpid_tgid_pgid(struct tcb *tcp, int pid)
> {
> PRINT_VAL_D(pid);
> - if (pid > 0)
> + if (pid > 0) {
> + if (!pidns_translation)
> + maybe_printpid_comm(pid);
> printpid_translation(tcp, pid, PT_TGID);
> - else if (pid < -1)
> + } else if (pid < -1)
> printpid_translation(tcp, -pid, PT_PGID);
> }
Please note that even if pidns_translation is disabled, the process of
the tcp can still be in a different PID namespace. I would suggest to
modify printpid_translation, to do the translation not only when
pidns_translation is enabled, but also when decode_pids_enabled >= 2,
to be able to pass the correct PID (in strace's namespace) to
maybe_printpid_comm. With these modifications it won't be necessary to
call maybe_printpid_comm from other places than pidns_translation (and
maybe we could rename printpid_translation to something else, as it
also prints comm field now).
> diff --git a/src/strace.c b/src/strace.c
> index 26d11b271..da4178367 100644
> --- a/src/strace.c
> +++ b/src/strace.c
> @@ -53,7 +53,9 @@ bool stack_trace_enabled;
> #endif
>
> /* if this is non-zero
> - * print the process names read from /proc/$pid/comm files */
> + * print the process names read from /proc/$pid/comm files
> + * 1: prefix
> + * 2: all (prefix, arguments and return values) */
> unsigned decode_pids_enabled;
>
> #define my_tkill(tid, sig) syscall(__NR_tkill, (tid), (sig))
> @@ -421,6 +423,10 @@ Output format:\n\
> -Y, --decode-pids[=prefix]\n\
> print command names associated with the pids appeared\n\
> as prefixes of trace lines\n\
> + -YY, --decode-pids=all\n\
> + print command names associated with the pids appeared\n\
> + as arguments and return values in addition to what printed\n\
> + with -Y option.\n\
> "
> #ifdef ENABLE_SECONTEXT
> "\
> @@ -946,6 +952,19 @@ load_pid_comm(int pid, char *buf, size_t buf_size)
> fclose(fp);
> }
>
> +void
> +maybe_printpid_comm(int pid)
> +{
> + if (decode_pids_enabled < 2)
> + return;
> +
> + char buf[PROC_COMM_LEN];
> + load_pid_comm(pid, buf, sizeof(buf));
> + tprints("<");
> + tprints(buf);
> + tprints(">");
> +}
> +
> void
> maybe_load_task_comm(struct tcb *tcp)
> {
> @@ -2039,6 +2058,7 @@ init(int argc, char *argv[])
> static const char yflag_qual[] = "path";
> static const char yyflag_qual[] = "all";
> static const char Yflag_qual[] = "prefix";
> + static const char YYflag_qual[] = "all";
> static const char tflag_str[] = "format:time";
> static const char ttflag_str[] = "precision:us,format:time";
> static const char tttflag_str[] = "format:unix,precision:us";
> @@ -2532,7 +2552,7 @@ init(int argc, char *argv[])
> " be provided simultaneously");
> }
>
> - qualify_decode_pid(Yflag_qual);
> + qualify_decode_pid(Yflag_short == 1 ? Yflag_qual : YYflag_qual);
> }
> /* If --decode-pids option comes after -p, comm fields of tcbs
> * are not filled though tcbs are initialized. We must fill the
> diff --git a/src/time.c b/src/time.c
> index 2b0ded515..343342465 100644
> --- a/src/time.c
> +++ b/src/time.c
> @@ -253,7 +253,9 @@ printclockname(int clockid)
> tprints_arg_begin(CPUCLOCK_PERTHREAD(clockid) ?
> "MAKE_THREAD_CPUCLOCK" :
> "MAKE_PROCESS_CPUCLOCK");
> - PRINT_VAL_D(CPUCLOCK_PID(clockid));
> + int pid = CPUCLOCK_PID(clockid);
> + PRINT_VAL_D(pid);
> + maybe_printpid_comm(pid);
> tprint_arg_next();
> printxval(cpuclocknames, clockid & CLOCKFD_MASK,
> "CPUCLOCK_???");
I missed this occurence of PID printing after introducing the printpid
function, but we should use printpid here as well.
> diff --git a/src/util.c b/src/util.c
> index a22d4095f..d40184963 100644
> --- a/src/util.c
> +++ b/src/util.c
> @@ -642,6 +642,7 @@ printpidfd(pid_t pid_of_fd, int fd, const char *path)
>
> tprints("pid:");
> PRINT_VAL_D(pid);
> + maybe_printpid_comm(pid);
> return true;
> }
This is correct, as this PID is always in strace's namespace. But to
decrease code repetition a bit, we could also print this PID using
printpid with a NULL tcb argument (which means that the PID is in
strace's namespace).
More information about the Strace-devel
mailing list