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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 30 Oct 2017 17:03:47 -0300
From:   Arnaldo Carvalho de Melo <acme@...nel.org>
To:     Namhyung Kim <namhyung@...nel.org>
Cc:     Milian Wolff <milian.wolff@...b.com>, jolsa@...nel.org,
        Linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org,
        Arnaldo Carvalho de Melo <acme@...hat.com>,
        Yao Jin <yao.jin@...ux.intel.com>,
        Jiri Olsa <jolsa@...hat.com>, kernel-team@....com
Subject: Re: [PATCH v6 6/6] perf util: use correct IP mapping to find srcline
 for hist entry

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@...nel.org>
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@...b.com>
    Signed-off-by: Namhyung Kim <namhyung@...nel.org>
    Tested-by: Milian Wolff <milian.wolff@...b.com>
    Cc: Jin Yao <yao.jin@...ux.intel.com>
    Cc: Jiri Olsa <jolsa@...hat.com>
    Cc: kernel-team@....com
    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@...hat.com>

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)