Re: [PATCH v2] perf trace: Fix perf trace tracing itself, creating feedback loops

[Arnaldo Carvalho de Melo <acme@kernel.org>]

On Thu, Nov 14, 2024 at 02:57:02PM -0300, Arnaldo Carvalho de Melo wrote:
> On Tue, Nov 12, 2024 at 02:07:36PM -0300, Arnaldo Carvalho de Melo wrote:
> > On Tue, Oct 29, 2024 at 10:24:31PM -0700, Howard Chu wrote:
> > > There exists a pids_filtered map in augmented_raw_syscalls.bpf.c that
> > > ceases to provide functionality after the BPF skeleton migration:
> > > commit 5e6da6be3082 ("perf trace: Migrate BPF augmentation to use a skeleton")
> > 
> > Thanks, applied to perf-tools-next,
> 
> There is a problem with this patch, I'm investigating it now:
> 
> root@number:~# perf trace -e syscalls:sys_enter_exit_group true
> perf: Segmentation fault
> Obtained 12 stack frames.
> perf() [0x6229a1]
> perf() [0x622a8b]
> /lib64/libc.so.6(+0x40d00) [0x7f704364fd00]
> perf() [0x4b5f2e]
> perf() [0x4ba3e6]
> perf() [0x4c03bc]
> perf() [0x4c0663]
> perf() [0x4c07bc]
> perf() [0x4c0b05]
> /lib64/libc.so.6(+0x2a088) [0x7f7043639088]
> /lib64/libc.so.6(__libc_start_main+0x8b) [0x7f704363914b]
> perf() [0x412265]
> Segmentation fault (core dumped)
> root@number:~#
> 
> (gdb) run trace -e syscalls:sys_enter_exit_group true
> Starting program: /root/bin/perf trace -e syscalls:sys_enter_exit_group true
> 
> This GDB supports auto-downloading debuginfo from the following URLs:
>   <https://debuginfod.fedoraproject.org/>
> Enable debuginfod for this session? (y or [n]) y
> Debuginfod has been enabled.
> To make this setting permanent, add 'set debuginfod enabled on' to .gdbinit.
> [Thread debugging using libthread_db enabled]
> Using host libthread_db library "/lib64/libthread_db.so.1".
> [Detaching after fork from child process 1965197]
> 
> Program received signal SIGSEGV, Segmentation fault.
> trace__run (trace=0x7fffffffa5e0, argc=1, argv=0x7fffffffde90) at builtin-trace.c:4330
> 4330		trace->filter_pids.map = trace->skel->maps.pids_filtered;
> (gdb) bt
> #0  trace__run (trace=0x7fffffffa5e0, argc=1, argv=0x7fffffffde90) at builtin-trace.c:4330
> #1  0x00000000004ba3e6 in cmd_trace (argc=1, argv=0x7fffffffde90) at builtin-trace.c:5490
> #2  0x00000000004c03bc in run_builtin (p=0xec4068 <commands+648>, argc=4, argv=0x7fffffffde90) at perf.c:351
> #3  0x00000000004c0663 in handle_internal_command (argc=4, argv=0x7fffffffde90) at perf.c:404
> #4  0x00000000004c07bc in run_argv (argcp=0x7fffffffdc7c, argv=0x7fffffffdc70) at perf.c:448
> #5  0x00000000004c0b05 in main (argc=4, argv=0x7fffffffde90) at perf.c:560
> (gdb) p trace->skel
> $1 = (struct augmented_raw_syscalls_bpf *) 0x0
> (gdb)
> 
> I.e. when we specify a syscall event tracepoint, we expect to use
> augmentation, but are not setting it up.

We can't assume that trace->skel will be always set, as it is only
initialized if trace->trace_syscalls is set, so I added this patch on
top of yours:

⬢ [acme@toolbox perf-tools-next]$ git diff
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 0f8678142fbfdbcb..14794725669d49ee 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -4327,7 +4327,8 @@ static int trace__run(struct trace *trace, int argc, const char **argv)
                }
        }
 
-       trace->filter_pids.map = trace->skel->maps.pids_filtered;
+       if (trace->skel)
+               trace->filter_pids.map = trace->skel->maps.pids_filtered;
 #endif
        err = trace__set_filter_pids(trace);
        if (err < 0)