Messages in this thread Patch in this message | | | Date | Wed, 06 Jul 2022 18:01:29 +0530 | From | "Naveen N. Rao" <> | Subject | Re: [PATCH] perf trace: Fix SIGSEGV when processing augmented args |
| |
Hi Arnaldo,
Arnaldo Carvalho de Melo wrote: > Em Tue, Mar 15, 2022 at 02:52:05PM -0300, Arnaldo Carvalho de Melo escreveu: >> Em Tue, Mar 15, 2022 at 10:57:57PM +0530, Naveen N. Rao escreveu: >> > Yes, it looks like the current check in 'perf' isn't working. The below >> > patch also resolves the crash we are seeing: >> >> > diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c >> > index 2f1d20553a0aa3..86b459f4ebdd61 100644 >> > --- a/tools/perf/builtin-trace.c >> > +++ b/tools/perf/builtin-trace.c >> > @@ -2326,7 +2326,7 @@ static int trace__sys_enter(struct trace *trace, struct evsel *evsel, >> > * thinking that the extra 2 u64 args are the augmented filename, so just check >> > * here and avoid using augmented syscalls when the evsel is the raw_syscalls one. >> > */ >> > - if (evsel != trace->syscalls.events.sys_enter) >> > + if (strcmp(evsel__name(evsel), "raw_syscalls:sys_enter")) >> > augmented_args = syscall__augmented_args(sc, sample, &augmented_args_size, trace->raw_augmented_syscalls_args_size); >> > ttrace->entry_time = sample->time; >> > msg = ttrace->entry_str; >> >> Interesting, that should be equivalent :-\ humm, not really, understood, >> when processing perf.data files we don't setup >> trace->syscalls.events.sys_enter... >> >> switching from strcmp() to something cheaper but equivalent should be >> the fix for now. > > I'll add a trace->use_augmented_args boolean that will do this test > once, and then use it in this case and will audit to check if this > should be used in other places.
Does something like the below look reasonable?
I know this isn't quite what you proposed, but it fixes the problem for me while avoiding the need for a string comparison. I also think this addresses all uses in 'perf trace', though I didn't audit the need for a similar fix elsewhere in 'perf'.
Thanks, Naveen
--- diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c index d2de2a4073e7eb..352b88a51dec2d 100644 --- a/tools/perf/builtin-trace.c +++ b/tools/perf/builtin-trace.c @@ -4291,6 +4292,8 @@ static int trace__replay(struct trace *trace) goto out; } + trace->syscalls.events.sys_enter = evsel; + evsel = evlist__find_tracepoint_by_name(session->evlist, "raw_syscalls:sys_exit"); if (evsel == NULL) evsel = evlist__find_tracepoint_by_name(session->evlist, "syscalls:sys_exit"); @@ -4301,6 +4304,8 @@ static int trace__replay(struct trace *trace) goto out; } + trace->syscalls.events.sys_exit = evsel; + evlist__for_each_entry(session->evlist, evsel) { if (evsel->core.attr.type == PERF_TYPE_SOFTWARE && (evsel->core.attr.config == PERF_COUNT_SW_PAGE_FAULTS_MAJ ||
| |