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]
Message-ID: <20180104184120.GE14721@kernel.org>
Date:   Thu, 4 Jan 2018 15:41:20 -0300
From:   Arnaldo Carvalho de Melo <acme@...nel.org>
To:     Jin Yao <yao.jin@...ux.intel.com>
Cc:     jolsa@...nel.org, peterz@...radead.org, mingo@...hat.com,
        alexander.shishkin@...ux.intel.com, Linux-kernel@...r.kernel.org,
        ak@...ux.intel.com, kan.liang@...el.com, yao.jin@...el.com
Subject: Re: [PATCH] perf report: Fix a wrong offset issue when using
 /proc/kcore

Em Sat, Dec 30, 2017 at 12:26:52AM +0800, Jin Yao escreveu:
> When valid vmlinux is not found, perf report falls back to look
> at /proc/kcore. While in this case, it will report the impossible
> large offset. For example,

Thanks, tested and applied!

- Arnaldo
 
> perf record -b -e cycles:k find /etc/ > /dev/null
> perf report --stdio --branch-history
> 
>     22.77%  _vm_normal_page+18446603336221188162
>             |
>             ---page_remove_rmap +18446603336221188324
>                page_remove_rmap +18446603336221188487 (cycles:5)
>                unlock_page_memcg +18446603336221188096
>                page_remove_rmap +18446603336221188327 (cycles:1)
> 
> The issue is the value which is passed to parameter 'addr' in
> __get_srcline() is the objdump address. It's not correct if we calculate
> the offset by using 'addr - sym->start'.
> 
> This patch creates a new parameter 'ip' in __get_srcline(). It is not
> converted to objdump address.
> 
> With this patch, the perf report output is:
> 
>     22.77%  _vm_normal_page+66
>             |
>             ---page_remove_rmap +228
>                page_remove_rmap +391 (cycles:5)
>                unlock_page_memcg +0
>                page_remove_rmap +231 (cycles:1)
>                page_remove_rmap +236
> 
> Signed-off-by: Jin Yao <yao.jin@...ux.intel.com>
> ---
>  tools/perf/util/annotate.c |  3 ++-
>  tools/perf/util/machine.c  |  2 +-
>  tools/perf/util/map.c      |  2 +-
>  tools/perf/util/sort.c     | 16 ++++++++++------
>  tools/perf/util/srcline.c  |  9 +++++----
>  tools/perf/util/srcline.h  |  5 +++--
>  6 files changed, 22 insertions(+), 15 deletions(-)
> 
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 68e687d..28b233c 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -1960,7 +1960,8 @@ static void annotation__calc_lines(struct annotation *notes, struct map *map,
>  		if (percent_max <= 0.5)
>  			continue;
>  
> -		al->path = get_srcline(map->dso, start + al->offset, NULL, false, true);
> +		al->path = get_srcline(map->dso, start + al->offset, NULL,
> +				       false, true, start + al->offset);
>  		insert_source_line(&tmp_root, al);
>  	}
>  
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index 64d255f..b05a674 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -1726,7 +1726,7 @@ static char *callchain_srcline(struct map *map, struct symbol *sym, u64 ip)
>  		bool show_addr = callchain_param.key == CCKEY_ADDRESS;
>  
>  		srcline = get_srcline(map->dso, map__rip_2objdump(map, ip),
> -				      sym, show_sym, show_addr);
> +				      sym, show_sym, show_addr, ip);
>  		srcline__tree_insert(&map->dso->srclines, ip, srcline);
>  	}
>  
> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> index 6d40efd..8fe5703 100644
> --- a/tools/perf/util/map.c
> +++ b/tools/perf/util/map.c
> @@ -419,7 +419,7 @@ int map__fprintf_srcline(struct map *map, u64 addr, const char *prefix,
>  	if (map && map->dso) {
>  		srcline = get_srcline(map->dso,
>  				      map__rip_2objdump(map, addr), NULL,
> -				      true, true);
> +				      true, true, addr);
>  		if (srcline != SRCLINE_UNKNOWN)
>  			ret = fprintf(fp, "%s%s", prefix, srcline);
>  		free_srcline(srcline);
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index a00eacd..211e7f3 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -336,7 +336,7 @@ char *hist_entry__get_srcline(struct hist_entry *he)
>  		return SRCLINE_UNKNOWN;
>  
>  	return get_srcline(map->dso, map__rip_2objdump(map, he->ip),
> -			   he->ms.sym, true, true);
> +			   he->ms.sym, true, true, he->ip);
>  }
>  
>  static int64_t
> @@ -380,7 +380,8 @@ sort__srcline_from_cmp(struct hist_entry *left, struct hist_entry *right)
>  					   map__rip_2objdump(map,
>  							     left->branch_info->from.al_addr),
>  							 left->branch_info->from.sym,
> -							 true, true);
> +							 true, true,
> +							 left->branch_info->from.al_addr);
>  	}
>  	if (!right->branch_info->srcline_from) {
>  		struct map *map = right->branch_info->from.map;
> @@ -391,7 +392,8 @@ sort__srcline_from_cmp(struct hist_entry *left, struct hist_entry *right)
>  					     map__rip_2objdump(map,
>  							       right->branch_info->from.al_addr),
>  						     right->branch_info->from.sym,
> -						     true, true);
> +						     true, true,
> +						     right->branch_info->from.al_addr);
>  	}
>  	return strcmp(right->branch_info->srcline_from, left->branch_info->srcline_from);
>  }
> @@ -423,7 +425,8 @@ sort__srcline_to_cmp(struct hist_entry *left, struct hist_entry *right)
>  					   map__rip_2objdump(map,
>  							     left->branch_info->to.al_addr),
>  							 left->branch_info->from.sym,
> -							 true, true);
> +							 true, true,
> +							 left->branch_info->to.al_addr);
>  	}
>  	if (!right->branch_info->srcline_to) {
>  		struct map *map = right->branch_info->to.map;
> @@ -434,7 +437,8 @@ sort__srcline_to_cmp(struct hist_entry *left, struct hist_entry *right)
>  					     map__rip_2objdump(map,
>  							       right->branch_info->to.al_addr),
>  						     right->branch_info->to.sym,
> -						     true, true);
> +						     true, true,
> +						     right->branch_info->to.al_addr);
>  	}
>  	return strcmp(right->branch_info->srcline_to, left->branch_info->srcline_to);
>  }
> @@ -465,7 +469,7 @@ static char *hist_entry__get_srcfile(struct hist_entry *e)
>  		return no_srcfile;
>  
>  	sf = __get_srcline(map->dso, map__rip_2objdump(map, e->ip),
> -			 e->ms.sym, false, true, true);
> +			 e->ms.sym, false, true, true, e->ip);
>  	if (!strcmp(sf, SRCLINE_UNKNOWN))
>  		return no_srcfile;
>  	p = strchr(sf, ':');
> diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c
> index d19f05c..3c21fd0 100644
> --- a/tools/perf/util/srcline.c
> +++ b/tools/perf/util/srcline.c
> @@ -496,7 +496,8 @@ static struct inline_node *addr2inlines(const char *dso_name, u64 addr,
>  #define A2L_FAIL_LIMIT 123
>  
>  char *__get_srcline(struct dso *dso, u64 addr, struct symbol *sym,
> -		  bool show_sym, bool show_addr, bool unwind_inlines)
> +		  bool show_sym, bool show_addr, bool unwind_inlines,
> +		  u64 ip)
>  {
>  	char *file = NULL;
>  	unsigned line = 0;
> @@ -536,7 +537,7 @@ char *__get_srcline(struct dso *dso, u64 addr, struct symbol *sym,
>  
>  	if (sym) {
>  		if (asprintf(&srcline, "%s+%" PRIu64, show_sym ? sym->name : "",
> -					addr - sym->start) < 0)
> +					ip - sym->start) < 0)
>  			return SRCLINE_UNKNOWN;
>  	} else if (asprintf(&srcline, "%s[%" PRIx64 "]", dso->short_name, addr) < 0)
>  		return SRCLINE_UNKNOWN;
> @@ -550,9 +551,9 @@ void free_srcline(char *srcline)
>  }
>  
>  char *get_srcline(struct dso *dso, u64 addr, struct symbol *sym,
> -		  bool show_sym, bool show_addr)
> +		  bool show_sym, bool show_addr, u64 ip)
>  {
> -	return __get_srcline(dso, addr, sym, show_sym, show_addr, false);
> +	return __get_srcline(dso, addr, sym, show_sym, show_addr, false, ip);
>  }
>  
>  struct srcline_node {
> diff --git a/tools/perf/util/srcline.h b/tools/perf/util/srcline.h
> index 847b708..b2bb5502 100644
> --- a/tools/perf/util/srcline.h
> +++ b/tools/perf/util/srcline.h
> @@ -11,9 +11,10 @@ struct symbol;
>  
>  extern bool srcline_full_filename;
>  char *get_srcline(struct dso *dso, u64 addr, struct symbol *sym,
> -		  bool show_sym, bool show_addr);
> +		  bool show_sym, bool show_addr, u64 ip);
>  char *__get_srcline(struct dso *dso, u64 addr, struct symbol *sym,
> -		  bool show_sym, bool show_addr, bool unwind_inlines);
> +		  bool show_sym, bool show_addr, bool unwind_inlines,
> +		  u64 ip);
>  void free_srcline(char *srcline);
>  
>  /* insert the srcline into the DSO, which will take ownership */
> -- 
> 2.7.4

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ