Re: [PATCH] perf, tools, report: Add support for srcfile sort key

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

Em Mon, Aug 10, 2015 at 03:32:47PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Mon, Aug 10, 2015 at 01:12:41PM -0300, Arnaldo Carvalho de Melo escreveu:
> > (gdb) fr 2
> > #2  0x00000000004bb812 in sort__srcline_cmp (left=0x1bfa090, right=0x7fffffffc0a0) at util/sort.c:306
> > 306				hists__new_col_len(right->hists, HISTC_SRCLINE,
> > (gdb) p right->hists
> > $7 = (struct hists *) 0x0
> > (gdb) p left->hists
> > $8 = (struct hists *) 0x18d66e0

> > Due to this both "-s srcline" and "-s srcfile" crashes here.
 
> Ok, this one needs to be applied before yours and Andi's:
 
> +++ b/tools/perf/util/hist.c
> @@ -761,6 +761,7 @@ iter_add_next_cumulative_entry(struct hist_entry_iter *iter,
>  	struct hist_entry **he_cache = iter->priv;
>  	struct hist_entry *he;
>  	struct hist_entry he_tmp = {
> +		.hists = evsel__hists(evsel),
>  		.cpu = al->cpu,
> ---------

> Because you will use that stack synthesized he_tmp entry as a parameter
> to the cmp() function, that will end up trying to access he_tmp->hists,
> that was NULL, b00m.

all this is moot, as we need to update that column length elsewhere,
together with all the other fileds, please see:

  ://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/commit/?h=perf/core&id=e8e6d37e73e6b950c891c780745460b87f4755b6

And now, with it really updating the width, we get to another problem,
in one case I got a line like:

  0.00%  JS_GetInstancePrivate(JSContext*, JSObject*, JSClass*, JS /usr/lib64/libmozjs-24.so 0x1a0bd1 d [.] JS_GetInstancePrivate(JSContext*, JSObject*, JSClass*, JS::Value*)   

Which is really strange, i.e. it mixed up demangled stuff with the
srcfile, i.e. normal lines for:

 perf report -v -s srcfile,dso,symbol --stdio

Look like:

     1.94%  malloc.c    /usr/lib64/libc-2.20.so   0x80f91 d [.] _int_malloc

Anyway, will continue this later, together with checking why it doesn't resolve
many things, that one may be related, in some cases, to functions in assembly
source files, for now it updates correctly the column width for "srcline" and
"srcfile".

- Arnaldo

> - Arnaldo
>   
> > > 
> > > 
> > > >From c052d17ae45ac08a381192191473ed3c4308c0c5 Mon Sep 17 00:00:00 2001
> > > From: Namhyung Kim <namhyung@kernel.org>
> > > Date: Sun, 9 Aug 2015 12:28:01 +0900
> > > Subject: [PATCH] perf tools: Update srcline column length
> > > 
> > > It didn't calculate the column length so the default of header length
> > > used only.  Update the length when we get the srcline actually.
> > > 
> > > Cc: Andi Kleen <andi@firstfloor.org>
> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > ---
> > >  tools/perf/util/sort.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> > > index 5177088a71d3..85e7e3e75d39 100644
> > > --- a/tools/perf/util/sort.c
> > > +++ b/tools/perf/util/sort.c
> > > @@ -291,6 +291,8 @@ sort__srcline_cmp(struct hist_entry *left, struct hist_entry *right)
> > >  			left->srcline = get_srcline(map->dso,
> > >  					   map__rip_2objdump(map, left->ip),
> > >  						    left->ms.sym, true);
> > > +			hists__new_col_len(left->hists, HISTC_SRCLINE,
> > > +					   strlen(left->srcline));
> > >  		}
> > >  	}
> > >  	if (!right->srcline) {
> > > @@ -301,6 +303,8 @@ sort__srcline_cmp(struct hist_entry *left, struct hist_entry *right)
> > >  			right->srcline = get_srcline(map->dso,
> > >  					     map__rip_2objdump(map, right->ip),
> > >  						     right->ms.sym, true);
> > > +			hists__new_col_len(right->hists, HISTC_SRCLINE,
> > > +					   strlen(right->srcline));
> > >  		}
> > >  	}
> > >  	return strcmp(right->srcline, left->srcline);
> > > -- 
> > > 2.5.0