Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1586033 > unrolled thread

[PATCH 0/4] perf annotate: Fixes for line numbers and Introduce source_code

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/4] perf annotate: Fixes for line numbers and Introduce source_code Taeung Song <treeze.taeung@gmail.com> - 2017-02-22 11:10 +0100
    [PATCH 1/4] perf annotate: Remove needless regular expression for filename:linenr Taeung Song <treeze.taeung@gmail.com> - 2017-02-22 11:10 +0100
      Re: [PATCH 1/4] perf annotate: Remove needless regular expression for filename:linenr Namhyung Kim <namhyung@kernel.org> - 2017-02-22 11:50 +0100
        Re: [PATCH 1/4] perf annotate: Remove needless regular expression for  filename:linenr Taeung Song <treeze.taeung@gmail.com> - 2017-02-22 17:20 +0100
    [PATCH 4/4] perf annotate: Introduce source_code to collect actual code Taeung Song <treeze.taeung@gmail.com> - 2017-02-22 11:10 +0100
      Re: [PATCH 4/4] perf annotate: Introduce source_code to collect  actual code Namhyung Kim <namhyung@kernel.org> - 2017-02-22 12:30 +0100
        Re: [PATCH 4/4] perf annotate: Introduce source_code to collect  actual code Taeung Song <treeze.taeung@gmail.com> - 2017-02-22 17:50 +0100
      Re: [PATCH 4/4] perf annotate: Introduce source_code to collect  actual code Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> - 2017-02-24 07:00 +0100

#1586033 — [PATCH 0/4] perf annotate: Fixes for line numbers and Introduce source_code

Hi, :)

Currently perf-annotate have several problems.

  - Wrong line numbers on perf-annotate (both stdio and TUI)
  - Wrong sum of overhead(percent) matching source lines 
  - It's so confusing that the output is mixed with
    both source code and assembly code. (related to depending on 'objdump -S') 

So fix them regarding wrong line numbers
and Introduce new source_code collecting actual code,
not depending on 'objdump -S'.

In near future, I have a plan for new reable annotate view
base on source code (per function(sym)) using the 'struct source_code'. :)

I'd appreciate some feedback.

(Current v1 don't consider improvement of performance of annotate
and don't make code of annotate more compact so I'll consider them in v2) 

Thanks,
Taeung

Taeung Song (4):
  perf annotate: Remove needless regular expression for filename:linenr
  perf annotate: Align filename:linenr and more correct summary
  perf annotate: Change the method counting line numbers
  perf annotate: Introduce source_code to collect actual code

 tools/perf/util/annotate.c | 174 +++++++++++++++++++++++++++++++++++----------
 tools/perf/util/annotate.h |   7 ++
 2 files changed, 144 insertions(+), 37 deletions(-)

-- 
2.7.4

[toc] | [next] | [standalone]

#1586034 — [PATCH 1/4] perf annotate: Remove needless regular expression for filename:linenr

The commit e592488c01d5 ("perf annotate: Support source line
numbers in annotate") support source line numbers in annotate.

But we can get filename:line number by symbol__get_source_line()
Furthermore, the way can't exactly match source code lines
to actual line numbers.

For example,
Actual source code is as below

...
  21 };
  22
  23 unsigned int limited_wgt;
  24
  25 unsigned int get_cond_maxprice(int wgt)
  26 {
...

However, the output of annotate with the way about regmatch
is as below.

  4   Disassembly of section .text:

  6   0000000000400966 <get_cond_maxprice>:
  7   get_cond_maxprice():
  26  };

  28  unsigned int limited_wgt;

  30  unsigned int get_cond_maxprice(int wgt)
  31  {

The root cause is from objdump -S.
Because the source code of objdump used to print more code lines
than actual one code line according to a particular line number.
In the near future, I'll fix the problem about line numbers.

Cc: Andi Kleen <ak@linux.intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/util/annotate.c | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 273f21f..bc54e41 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -19,14 +19,12 @@
 #include "evsel.h"
 #include "block-range.h"
 #include "arch/common.h"
-#include <regex.h>
 #include <pthread.h>
 #include <linux/bitops.h>
 #include <sys/utsname.h>
 
 const char 	*disassembler_style;
 const char	*objdump_path;
-static regex_t	 file_lineno;
 
 static struct ins_ops *ins__find(struct arch *arch, const char *name);
 static void ins__sort(struct arch *arch);
@@ -1151,7 +1149,6 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,
 	char *line = NULL, *parsed_line, *tmp, *tmp2, *c;
 	size_t line_len;
 	s64 line_ip, offset = -1;
-	regmatch_t match[2];
 
 	if (getline(&line, &line_len, file) < 0)
 		return -1;
@@ -1169,12 +1166,6 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,
 	line_ip = -1;
 	parsed_line = line;
 
-	/* /filename:linenr ? Save line number and ignore. */
-	if (regexec(&file_lineno, line, 2, match, 0) == 0) {
-		*line_nr = atoi(line + match[1].rm_so);
-		return 0;
-	}
-
 	/*
 	 * Strip leading spaces:
 	 */
@@ -1235,11 +1226,6 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,
 	return 0;
 }
 
-static __attribute__((constructor)) void symbol__init_regexpr(void)
-{
-	regcomp(&file_lineno, "^/[^:]+:([0-9]+)", REG_EXTENDED);
-}
-
 static void delete_last_nop(struct symbol *sym)
 {
 	struct annotation *notes = symbol__annotation(sym);
@@ -1435,7 +1421,7 @@ int symbol__disassemble(struct symbol *sym, struct map *map, const char *arch_na
 	snprintf(command, sizeof(command),
 		 "%s %s%s --start-address=0x%016" PRIx64
 		 " --stop-address=0x%016" PRIx64
-		 " -l -d %s %s -C %s 2>/dev/null|grep -v %s|expand",
+		 " -d %s %s -C %s 2>/dev/null|grep -v %s|expand",
 		 objdump_path ? objdump_path : "objdump",
 		 disassembler_style ? "-M " : "",
 		 disassembler_style ? disassembler_style : "",
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1586057 — Re: [PATCH 1/4] perf annotate: Remove needless regular expression for filename:linenr

Hi Taeung,

On Wed, Feb 22, 2017 at 7:08 PM, Taeung Song <treeze.taeung@gmail.com> wrote:
> The commit e592488c01d5 ("perf annotate: Support source line
> numbers in annotate") support source line numbers in annotate.
>
> But we can get filename:line number by symbol__get_source_line()
> Furthermore, the way can't exactly match source code lines
> to actual line numbers.
>
> For example,
> Actual source code is as below
>
> ...
>   21 };
>   22
>   23 unsigned int limited_wgt;
>   24
>   25 unsigned int get_cond_maxprice(int wgt)
>   26 {
> ...
>
> However, the output of annotate with the way about regmatch
> is as below.
>
>   4   Disassembly of section .text:
>
>   6   0000000000400966 <get_cond_maxprice>:
>   7   get_cond_maxprice():
>   26  };
>
>   28  unsigned int limited_wgt;
>
>   30  unsigned int get_cond_maxprice(int wgt)
>   31  {
>
> The root cause is from objdump -S.
> Because the source code of objdump used to print more code lines
> than actual one code line according to a particular line number.
> In the near future, I'll fix the problem about line numbers.

So what's the purpose of this patch?  You just removed the line
number from the output, it changed behavior for what?  If you want
to fix a problem of line number, please do it here..

Thanks,
Namhyung


>
> Cc: Andi Kleen <ak@linux.intel.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
> ---
>  tools/perf/util/annotate.c | 16 +---------------
>  1 file changed, 1 insertion(+), 15 deletions(-)
>
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 273f21f..bc54e41 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -19,14 +19,12 @@
>  #include "evsel.h"
>  #include "block-range.h"
>  #include "arch/common.h"
> -#include <regex.h>
>  #include <pthread.h>
>  #include <linux/bitops.h>
>  #include <sys/utsname.h>
>
>  const char     *disassembler_style;
>  const char     *objdump_path;
> -static regex_t  file_lineno;
>
>  static struct ins_ops *ins__find(struct arch *arch, const char *name);
>  static void ins__sort(struct arch *arch);
> @@ -1151,7 +1149,6 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,
>         char *line = NULL, *parsed_line, *tmp, *tmp2, *c;
>         size_t line_len;
>         s64 line_ip, offset = -1;
> -       regmatch_t match[2];
>
>         if (getline(&line, &line_len, file) < 0)
>                 return -1;
> @@ -1169,12 +1166,6 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,
>         line_ip = -1;
>         parsed_line = line;
>
> -       /* /filename:linenr ? Save line number and ignore. */
> -       if (regexec(&file_lineno, line, 2, match, 0) == 0) {
> -               *line_nr = atoi(line + match[1].rm_so);
> -               return 0;
> -       }
> -
>         /*
>          * Strip leading spaces:
>          */
> @@ -1235,11 +1226,6 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,
>         return 0;
>  }
>
> -static __attribute__((constructor)) void symbol__init_regexpr(void)
> -{
> -       regcomp(&file_lineno, "^/[^:]+:([0-9]+)", REG_EXTENDED);
> -}
> -
>  static void delete_last_nop(struct symbol *sym)
>  {
>         struct annotation *notes = symbol__annotation(sym);
> @@ -1435,7 +1421,7 @@ int symbol__disassemble(struct symbol *sym, struct map *map, const char *arch_na
>         snprintf(command, sizeof(command),
>                  "%s %s%s --start-address=0x%016" PRIx64
>                  " --stop-address=0x%016" PRIx64
> -                " -l -d %s %s -C %s 2>/dev/null|grep -v %s|expand",
> +                " -d %s %s -C %s 2>/dev/null|grep -v %s|expand",
>                  objdump_path ? objdump_path : "objdump",
>                  disassembler_style ? "-M " : "",
>                  disassembler_style ? disassembler_style : "",
> --
> 2.7.4
>



-- 
Thanks,
Namhyung

[toc] | [prev] | [next] | [standalone]

#1586289 — Re: [PATCH 1/4] perf annotate: Remove needless regular expression for filename:linenr

Hi Namhyung,

On 02/22/2017 07:47 PM, Namhyung Kim wrote:
> Hi Taeung,
>
> On Wed, Feb 22, 2017 at 7:08 PM, Taeung Song <treeze.taeung@gmail.com> wrote:
>> The commit e592488c01d5 ("perf annotate: Support source line
>> numbers in annotate") support source line numbers in annotate.
>>
>> But we can get filename:line number by symbol__get_source_line()
>> Furthermore, the way can't exactly match source code lines
>> to actual line numbers.
>>
>> For example,
>> Actual source code is as below
>>
>> ...
>>   21 };
>>   22
>>   23 unsigned int limited_wgt;
>>   24
>>   25 unsigned int get_cond_maxprice(int wgt)
>>   26 {
>> ...
>>
>> However, the output of annotate with the way about regmatch
>> is as below.
>>
>>   4   Disassembly of section .text:
>>
>>   6   0000000000400966 <get_cond_maxprice>:
>>   7   get_cond_maxprice():
>>   26  };
>>
>>   28  unsigned int limited_wgt;
>>
>>   30  unsigned int get_cond_maxprice(int wgt)
>>   31  {
>>
>> The root cause is from objdump -S.
>> Because the source code of objdump used to print more code lines
>> than actual one code line according to a particular line number.
>> In the near future, I'll fix the problem about line numbers.
>
> So what's the purpose of this patch?  You just removed the line
> number from the output, it changed behavior for what?  If you want
> to fix a problem of line number, please do it here..
>
> Thanks,
> Namhyung
>
>

The purpose of this patch was to remove needless code about line number.
To avoid big size of commit, I made several patches as parts..

But as you said I also think it is better to remake this patch
preferentially centering important points of the patchset!

Thanks
Taeung

[toc] | [prev] | [next] | [standalone]

#1586035 — [PATCH 4/4] perf annotate: Introduce source_code to collect actual code

The output of perf-annotate has a problem.
It is so confusing that the output is mixed with
both source code and assembly code.
IMHO, we need readable annotate view based on source code,
not mixed view. (not depending on 'objdump -S')

And to do that, we can collect actual source code per function(sym)
using addr2line() and we can handle 'struct source_code'
that contains each line of code.

In near future, it would be used for new annotate view based on
actual source code per function(sym).

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/util/annotate.c | 117 +++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/annotate.h |   7 +++
 2 files changed, 124 insertions(+)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index acd500b..93abc1e 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1590,6 +1590,116 @@ static void symbol__free_source_line(struct symbol *sym, int len)
 	zfree(&notes->src->lines);
 }
 
+static int symbol__free_source_code(struct symbol *sym)
+{
+	struct annotation *notes = symbol__annotation(sym);
+	struct source_code *pos, *tmp;
+
+	if (&notes->src->code == NULL)
+		return -1;
+
+	list_for_each_entry_safe(pos, tmp, &notes->src->code, node) {
+		list_del(&pos->node);
+		zfree(&pos->code_line);
+		free(pos);
+	}
+	return 0;
+}
+
+static int parse_srcline(char *srcline, char **path, int *line_nr)
+{
+	char *sep;
+
+	if (!strcmp(srcline, SRCLINE_UNKNOWN))
+		return -1;
+
+	sep = strchr(srcline, ':');
+	if (sep) {
+		*sep = '\0';
+		*path = srcline;
+		*line_nr = strtoul(++sep, NULL, 0);
+	} else
+		return -1;
+
+	return 0;
+}
+
+static bool symbol__get_source_code(struct symbol *sym, struct map *map)
+{
+	FILE *file;
+	int first_linenr, start_linenr, end_linenr;
+	size_t len;
+	char *line = NULL, *path, *start_srcline, *end_srcline;
+	u64 start = map__rip_2objdump(map, sym->start);
+	u64 end = map__rip_2objdump(map, sym->end - 1);
+	bool bef_fullpath = srcline_full_filename;
+	bool ret = false;
+	struct annotation *notes = symbol__annotation(sym);
+
+	srcline_full_filename = true;
+	start_srcline = get_srcline(map->dso, start, NULL, false);
+	end_srcline = get_srcline(map->dso, end, NULL, false);
+	srcline_full_filename = bef_fullpath;
+
+	if (parse_srcline(start_srcline, &path, &start_linenr) < 0)
+		return false;
+	if (parse_srcline(end_srcline, &path, &end_linenr) < 0)
+		return false;
+
+	/* To read a function header for the sym */
+	if (start_linenr > 4)
+		first_linenr = start_linenr - 4;
+	else
+		first_linenr = 1;
+
+	if (access(path, R_OK) != 0)
+		return false;
+
+	file = fopen(path, "r");
+	if (!file)
+		return false;
+
+	INIT_LIST_HEAD(&notes->src->code);
+
+	while (!feof(file)) {
+		int nr;
+		char *c, *parsed_line;
+		struct source_code *code;
+
+		if (getline(&line, &len, file) < 0) {
+			symbol__free_source_code(sym);
+			break;
+		}
+
+		if (++nr < first_linenr)
+			continue;
+
+		parsed_line = rtrim(line);
+		c = strchr(parsed_line, '\n');
+		if (c)
+			*c = '\0';
+		code = zalloc(sizeof(*code));
+		if (!code) {
+			symbol__free_source_code(sym);
+			break;
+		}
+		code->nr = nr;
+		code->code_line = strdup(parsed_line);
+		list_add_tail(&code->node, &notes->src->code);
+
+		if (nr == end_linenr) {
+			ret = true;
+			break;
+		}
+	}
+
+	free(line);
+	fclose(file);
+	free_srcline(start_srcline);
+	free_srcline(end_srcline);
+	return ret;
+}
+
 /* Get the filename:line for the colored entries */
 static int symbol__get_source_line(struct symbol *sym, struct map *map,
 				   struct perf_evsel *evsel,
@@ -1862,6 +1972,7 @@ int symbol__tty_annotate(struct symbol *sym, struct map *map,
 	struct dso *dso = map->dso;
 	struct rb_root source_line = RB_ROOT;
 	u64 len;
+	bool has_code = false;
 
 	if (symbol__disassemble(sym, map, perf_evsel__env_arch(evsel), 0) < 0)
 		return -1;
@@ -1874,11 +1985,17 @@ int symbol__tty_annotate(struct symbol *sym, struct map *map,
 		print_summary(&source_line, dso->long_name);
 	}
 
+	if (symbol_conf.annotate_src)
+		has_code = symbol__get_source_code(sym, map);
+
 	symbol__annotate_printf(sym, map, evsel, full_paths,
 				min_pcnt, max_lines, 0);
 	if (print_lines)
 		symbol__free_source_line(sym, len);
 
+	if (has_code)
+		symbol__free_source_code(sym);
+
 	disasm__purge(&symbol__annotation(sym)->src->source);
 
 	return 0;
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 09776b5..7231f46 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -95,6 +95,12 @@ struct cyc_hist {
 	u16	reset;
 };
 
+struct source_code {
+	struct list_head node;
+	int nr;
+	char *code_line;
+};
+
 struct source_line_samples {
 	double		percent;
 	double		percent_sum;
@@ -123,6 +129,7 @@ struct source_line {
  */
 struct annotated_source {
 	struct list_head   source;
+	struct list_head   code;
 	struct source_line *lines;
 	int    		   nr_histograms;
 	size_t		   sizeof_sym_hist;
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1586082 — Re: [PATCH 4/4] perf annotate: Introduce source_code to collect actual code

On Wed, Feb 22, 2017 at 7:08 PM, Taeung Song <treeze.taeung@gmail.com> wrote:
> The output of perf-annotate has a problem.
> It is so confusing that the output is mixed with
> both source code and assembly code.
> IMHO, we need readable annotate view based on source code,
> not mixed view. (not depending on 'objdump -S')
>
> And to do that, we can collect actual source code per function(sym)
> using addr2line() and we can handle 'struct source_code'
> that contains each line of code.
>
> In near future, it would be used for new annotate view based on
> actual source code per function(sym).

I think this is just a preparation so you'd be better sending it with
your new annotate view patchset later.

Thanks,
Namhyung

>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
> ---
>  tools/perf/util/annotate.c | 117 +++++++++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/annotate.h |   7 +++
>  2 files changed, 124 insertions(+)
>
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index acd500b..93abc1e 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -1590,6 +1590,116 @@ static void symbol__free_source_line(struct symbol *sym, int len)
>         zfree(&notes->src->lines);
>  }
>
> +static int symbol__free_source_code(struct symbol *sym)
> +{
> +       struct annotation *notes = symbol__annotation(sym);
> +       struct source_code *pos, *tmp;
> +
> +       if (&notes->src->code == NULL)
> +               return -1;
> +
> +       list_for_each_entry_safe(pos, tmp, &notes->src->code, node) {
> +               list_del(&pos->node);
> +               zfree(&pos->code_line);
> +               free(pos);
> +       }
> +       return 0;
> +}
> +
> +static int parse_srcline(char *srcline, char **path, int *line_nr)
> +{
> +       char *sep;
> +
> +       if (!strcmp(srcline, SRCLINE_UNKNOWN))
> +               return -1;
> +
> +       sep = strchr(srcline, ':');
> +       if (sep) {
> +               *sep = '\0';
> +               *path = srcline;
> +               *line_nr = strtoul(++sep, NULL, 0);
> +       } else
> +               return -1;
> +
> +       return 0;
> +}
> +
> +static bool symbol__get_source_code(struct symbol *sym, struct map *map)
> +{
> +       FILE *file;
> +       int first_linenr, start_linenr, end_linenr;
> +       size_t len;
> +       char *line = NULL, *path, *start_srcline, *end_srcline;
> +       u64 start = map__rip_2objdump(map, sym->start);
> +       u64 end = map__rip_2objdump(map, sym->end - 1);
> +       bool bef_fullpath = srcline_full_filename;
> +       bool ret = false;
> +       struct annotation *notes = symbol__annotation(sym);
> +
> +       srcline_full_filename = true;
> +       start_srcline = get_srcline(map->dso, start, NULL, false);
> +       end_srcline = get_srcline(map->dso, end, NULL, false);
> +       srcline_full_filename = bef_fullpath;
> +
> +       if (parse_srcline(start_srcline, &path, &start_linenr) < 0)
> +               return false;
> +       if (parse_srcline(end_srcline, &path, &end_linenr) < 0)
> +               return false;
> +
> +       /* To read a function header for the sym */
> +       if (start_linenr > 4)
> +               first_linenr = start_linenr - 4;
> +       else
> +               first_linenr = 1;
> +
> +       if (access(path, R_OK) != 0)
> +               return false;
> +
> +       file = fopen(path, "r");
> +       if (!file)
> +               return false;
> +
> +       INIT_LIST_HEAD(&notes->src->code);
> +
> +       while (!feof(file)) {
> +               int nr;
> +               char *c, *parsed_line;
> +               struct source_code *code;
> +
> +               if (getline(&line, &len, file) < 0) {
> +                       symbol__free_source_code(sym);
> +                       break;
> +               }
> +
> +               if (++nr < first_linenr)
> +                       continue;
> +
> +               parsed_line = rtrim(line);
> +               c = strchr(parsed_line, '\n');
> +               if (c)
> +                       *c = '\0';
> +               code = zalloc(sizeof(*code));
> +               if (!code) {
> +                       symbol__free_source_code(sym);
> +                       break;
> +               }
> +               code->nr = nr;
> +               code->code_line = strdup(parsed_line);
> +               list_add_tail(&code->node, &notes->src->code);
> +
> +               if (nr == end_linenr) {
> +                       ret = true;
> +                       break;
> +               }
> +       }
> +
> +       free(line);
> +       fclose(file);
> +       free_srcline(start_srcline);
> +       free_srcline(end_srcline);
> +       return ret;
> +}
> +
>  /* Get the filename:line for the colored entries */
>  static int symbol__get_source_line(struct symbol *sym, struct map *map,
>                                    struct perf_evsel *evsel,
> @@ -1862,6 +1972,7 @@ int symbol__tty_annotate(struct symbol *sym, struct map *map,
>         struct dso *dso = map->dso;
>         struct rb_root source_line = RB_ROOT;
>         u64 len;
> +       bool has_code = false;
>
>         if (symbol__disassemble(sym, map, perf_evsel__env_arch(evsel), 0) < 0)
>                 return -1;
> @@ -1874,11 +1985,17 @@ int symbol__tty_annotate(struct symbol *sym, struct map *map,
>                 print_summary(&source_line, dso->long_name);
>         }
>
> +       if (symbol_conf.annotate_src)
> +               has_code = symbol__get_source_code(sym, map);
> +
>         symbol__annotate_printf(sym, map, evsel, full_paths,
>                                 min_pcnt, max_lines, 0);
>         if (print_lines)
>                 symbol__free_source_line(sym, len);
>
> +       if (has_code)
> +               symbol__free_source_code(sym);
> +
>         disasm__purge(&symbol__annotation(sym)->src->source);
>
>         return 0;
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index 09776b5..7231f46 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -95,6 +95,12 @@ struct cyc_hist {
>         u16     reset;
>  };
>
> +struct source_code {
> +       struct list_head node;
> +       int nr;
> +       char *code_line;
> +};
> +
>  struct source_line_samples {
>         double          percent;
>         double          percent_sum;
> @@ -123,6 +129,7 @@ struct source_line {
>   */
>  struct annotated_source {
>         struct list_head   source;
> +       struct list_head   code;
>         struct source_line *lines;
>         int                nr_histograms;
>         size_t             sizeof_sym_hist;
> --
> 2.7.4
>



-- 
Thanks,
Namhyung

[toc] | [prev] | [next] | [standalone]

#1586310 — Re: [PATCH 4/4] perf annotate: Introduce source_code to collect actual code

On 02/22/2017 08:27 PM, Namhyung Kim wrote:
> On Wed, Feb 22, 2017 at 7:08 PM, Taeung Song <treeze.taeung@gmail.com> wrote:
>> The output of perf-annotate has a problem.
>> It is so confusing that the output is mixed with
>> both source code and assembly code.
>> IMHO, we need readable annotate view based on source code,
>> not mixed view. (not depending on 'objdump -S')
>>
>> And to do that, we can collect actual source code per function(sym)
>> using addr2line() and we can handle 'struct source_code'
>> that contains each line of code.
>>
>> In near future, it would be used for new annotate view based on
>> actual source code per function(sym).
>
> I think this is just a preparation so you'd be better sending it with
> your new annotate view patchset later.
>
> Thanks,
> Namhyung

I got it!
I thought it is needed to separate a big commit into several commits
so I sent this patch before including new annotate view.

But as you said, it seems like just a preparation without important 
point(new view).
So I'll send v2 with new annotate view !

Thanks,
Taeung

[toc] | [prev] | [next] | [standalone]

#1587308 — Re: [PATCH 4/4] perf annotate: Introduce source_code to collect actual code

Hi Taeung,

On Wednesday 22 February 2017 03:38 PM, Taeung Song wrote:
> +	INIT_LIST_HEAD(&notes->src->code);
> +
> +	while (!feof(file)) {
> +		int nr;
> +		char *c, *parsed_line;
> +		struct source_code *code;
> +
> +		if (getline(&line, &len, file) < 0) {
> +			symbol__free_source_code(sym);
> +			break;
> +		}
> +
> +		if (++nr < first_linenr)

Please initialize variable nr. I got a compilation error:

util/annotate.c: In function ‘symbol__tty_annotate’:
util/annotate.c:1674:6: error: ‘nr’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
   if (++nr < first_linenr)
      ^
util/annotate.c:1665:7: note: ‘nr’ was declared here
   int nr;
       ^

Ravi

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web