Messages in this thread | | | Date | Wed, 6 Jul 2022 12:50:23 -0300 | From | Arnaldo Carvalho de Melo <> | Subject | Re: [PATCH] perf trace: Fix SIGSEGV when processing augmented args |
| |
Em Wed, Jul 06, 2022 at 06:01:29PM +0530, Naveen N. Rao escreveu: > 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'.
I'll check and test, but I like it, two-liner even :-)
- Arnaldo
> > 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 || > >
--
- Arnaldo
| |