Re: [PATCH v6 6/6] perf util: use correct IP mapping to find srcline for hist entry

[Arnaldo Carvalho de Melo]

Em Wed, Oct 25, 2017 at 10:46:00AM +0900, Namhyung Kim escreveu:
> Hi Milian,
> 
> On Tue, Oct 24, 2017 at 10:51:43AM +0200, Milian Wolff wrote:
> > On Freitag, 20. Oktober 2017 07:15:33 CEST Namhyung Kim wrote:
> > > I looked into it and found a bug handling cumulative (children)
> > > entries.  For chilren entries that has no self period, the al->addr
> > > (so he->ip) ends up having an doubly-mapped address.
> > > 
> > > It seems to be there from the beginning but only affects entries that
> > > have no srclines - finding srcline itself is done using a different
> > > address but it will show the invalid address if no srcline was found.
> > > I think we should fix the commit c7405d85d7a3 ("perf tools: Update
> > > cpumode for each cumulative entry").
> > > 
> > > Could you please test the following patch works for you?
> > 
> > Sorry for the delay, nearly forgot about this mail. The patch below does help 
> > in my situation, thanks! Can you commit it please?
> 
> Sure, I'll add your Tested-by then.

Namhyung, I couldn't find a submission from you for this one, so I
tentatively added this to my perf/core branch, please let me know if you
want to reword this somehow.

- Arnaldo

commit 0485954310bf3490cb73936164ca03a0c5916773
Author: Namhyung Kim <namhyung@xxxxxxxxxx>
Date:   Fri Oct 20 14:15:33 2017 +0900

    perf callchain: Fix double mapping al->addr for children without self period
    
    Milian Wolff found a problem he described in [1] and that for him would
    get fixed:
    
    "Note how most of the large offset values are now gone. Most notably, we
    get proper srcline resolution for the random.h and complex headers."
    
    Then Namhyung found the root cause:
    
    "I looked into it and found a bug handling cumulative (children)
    entries.  For children entries that has no self period, the al->addr (so
    he->ip) ends up having an doubly-mapped address.
    
    It seems to be there from the beginning but only affects entries that
    have no srclines - finding srcline itself is done using a different
    address but it will show the invalid address if no srcline was found.  I
    think we should fix the commit c7405d85d7a3 ("perf tools: Update cpumode
    for each cumulative entry")."
    
    Reported-by: Milian Wolff <milian.wolff@xxxxxxxx>
    Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxxx>
    Tested-by: Milian Wolff <milian.wolff@xxxxxxxx>
    Cc: Jin Yao <yao.jin@xxxxxxxxxxxxxxx>
    Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
    Cc: kernel-team@xxxxxxx
    Fixes: c7405d85d7a3 ("perf tools: Update cpumode for each cumulative entry")
    Link: http://lkml.kernel.org/r/20171020051533.GA2746@sejong
    Signed-off-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>

diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index 3a3916934a92..837012147c7b 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -1091,10 +1091,7 @@ int fill_callchain_info(struct addr_location *al, struct callchain_cursor_node *
 	al->map = node->map;
 	al->sym = node->sym;
 	al->srcline = node->srcline;
-	if (node->map)
-		al->addr = node->map->map_ip(node->map, node->ip);
-	else
-		al->addr = node->ip;
+	al->addr = node->ip;
 
 	if (al->sym == NULL) {
 		if (hide_unresolved)