Re: [PATCH] perf tools: use map->map_ip to get srcline when listing symbol

[Andi Kleen <ak@linux.intel.com>]

On Thu, Aug 06, 2015 at 10:30:41AM -0700, Yannick Brosseau wrote:
> Revert 5550171b2a9f8df26ff483051d060db06376b26d and do the address
> mapping using map->map_ip instead of map__rip_2objdump when
> calling get_srcline

With that change the source line is not correctly reported anymore:

with your patch:

# Children      Self  Source:Line  Symbol                           Shared Object   
# ........  ........  ...........  ...............................  ................
#
    98.27%     0.00%  f1+4194306   [.] f1                           tcall           
            |

Without:

# Children      Self  Source:Line  Symbol                           Shared Object   
# ........  ........  ...........  ...............................  ................
#
    98.27%     0.00%  f1+2         [.] f1                           tcall           

and also the other way round when debug info is available the lines
in --branch-history output are not correctly resolved anymore.

before:

   99.35%     0.00%  f1+19        [.] f1                           tcall           
            |
            ---f1 tcall.c:12
               f1 tcall.c:12
               f2 tcall.c:7


after
   99.35%     5.87%  /home/ak/hl  [.] f2                           tcall           
            |          
            |--94.09%-- f2 +27
            |          f2 +0
            |          |          
            |          |--69.26%-- f1 +19
            |          |          f1 +14
            |          |          f2 +27


So the source:line improved, but the callgraph entries regressed.

Probably these two cases need to be handled differently.

BTW the commit description is not very clear.

-Andi

> 
> Signed-off-by: Yannick Brosseau <scientist@fb.com>
> ---
>  tools/perf/util/callchain.c | 4 ++--
>  tools/perf/util/machine.c   | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
> index 9f643ee..359fb6b 100644
> --- a/tools/perf/util/callchain.c
> +++ b/tools/perf/util/callchain.c
> @@ -830,8 +830,8 @@ char *callchain_list__sym_name(struct callchain_list *cl,
>  		if (callchain_param.key == CCKEY_ADDRESS &&
>  		    cl->ms.map && !cl->srcline)
>  			cl->srcline = get_srcline(cl->ms.map->dso,
> -						  map__rip_2objdump(cl->ms.map,
> -								    cl->ip),
> +						  cl->ms.map->map_ip(cl->ms.map,
> +								     cl->ip),
>  						  cl->ms.sym, false);
>  		if (cl->srcline)
>  			printed = scnprintf(bf, bfsize, "%s %s",
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index 7ff6827..80a5eda 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -1580,7 +1580,7 @@ static int add_callchain_ip(struct thread *thread,
>  		}
>  	}
>  
> -	return callchain_cursor_append(&callchain_cursor, al.addr, al.map, al.sym);
> +	return callchain_cursor_append(&callchain_cursor, ip, al.map, al.sym);
>  }
>  
>  struct branch_info *sample__resolve_bstack(struct perf_sample *sample,
> -- 
> 2.1.4
> 

-- 
ak@linux.intel.com -- Speaking for myself only