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  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:	Sun, 15 Jun 2014 21:46:31 +0900
From:	Namhyung Kim <namhyung@...nel.org>
To:	Jiri Olsa <jolsa@...nel.org>
Cc:	linux-kernel@...r.kernel.org,
	Arnaldo Carvalho de Melo <acme@...nel.org>,
	Corey Ashford <cjashfor@...ux.vnet.ibm.com>,
	David Ahern <dsahern@...il.com>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Ingo Molnar <mingo@...nel.org>,
	Paul Mackerras <paulus@...ba.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>
Subject: Re: [RFC/PATCH] perf tools: Fix segfault in cumulative.callchain
 report

Hi Jiri,

2014-06-15 (일), 10:35 +0200, Jiri Olsa:
> heya,
> not completely sure this is what we want to do, but have no
> streght to dive into annotation code ;-)

Currently, cumulative entries do not accounts annotation info so
annotation only works for normal entries.  I'm not sure we should
support it or not, but anyway it also needs some updates on the
annotation code IMHO.

So I'm okay with disabling annotation on pure cumulative entries.

Thanks,
Namhyung


> 
> ---
> When cumulative callchain mode is on, we could get samples with
> with no actual hits. This breaks the assumption of the annotation
> code, that each sample has annotation counts allocated and leads
> to segfault.
> 
> Fixing this by additional checks for annotation stats.
> 
> Cc: Arnaldo Carvalho de Melo <acme@...nel.org>
> Cc: Corey Ashford <cjashfor@...ux.vnet.ibm.com>
> Cc: David Ahern <dsahern@...il.com>
> Cc: Frederic Weisbecker <fweisbec@...il.com>
> Cc: Ingo Molnar <mingo@...nel.org>
> Cc: Namhyung Kim <namhyung@...nel.org>
> Cc: Paul Mackerras <paulus@...ba.org>
> Cc: Peter Zijlstra <a.p.zijlstra@...llo.nl>
> Signed-off-by: Jiri Olsa <jolsa@...nel.org>
> ---
>  tools/perf/ui/browsers/hists.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index 52c03fb..04a229a 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -17,6 +17,7 @@
>  #include "../util.h"
>  #include "../ui.h"
>  #include "map.h"
> +#include "annotate.h"
>  
>  struct hist_browser {
>  	struct ui_browser   b;
> @@ -1593,13 +1594,18 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
>  					 bi->to.sym->name) > 0)
>  				annotate_t = nr_options++;
>  		} else {
> -
>  			if (browser->selection != NULL &&
>  			    browser->selection->sym != NULL &&
> -			    !browser->selection->map->dso->annotate_warned &&
> -				asprintf(&options[nr_options], "Annotate %s",
> -					 browser->selection->sym->name) > 0)
> -				annotate = nr_options++;
> +			    !browser->selection->map->dso->annotate_warned) {
> +				struct annotation *notes;
> +
> +				notes = symbol__annotation(browser->selection->sym);
> +
> +				if (notes->src &&
> +				    asprintf(&options[nr_options], "Annotate %s",
> +						 browser->selection->sym->name) > 0)
> +					annotate = nr_options++;
> +			}
>  		}
>  
>  		if (thread != NULL &&
> @@ -1656,6 +1662,7 @@ retry_popup_menu:
>  
>  		if (choice == annotate || choice == annotate_t || choice == annotate_f) {
>  			struct hist_entry *he;
> +			struct annotation *notes;
>  			int err;
>  do_annotate:
>  			if (!objdump_path && perf_session_env__lookup_objdump(env))
> @@ -1679,6 +1686,10 @@ do_annotate:
>  				he->ms.map = he->branch_info->to.map;
>  			}
>  
> +			notes = symbol__annotation(he->ms.sym);
> +			if (!notes->src)
> +				continue;
> +
>  			/*
>  			 * Don't let this be freed, say, by hists__decay_entry.
>  			 */



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists