Message-ID: <CAM9d7cigvru0aHx9a+KS=eFKeNzrQBATBVWOHa+GfiLSLV2-tQ@mail.gmail.com>
Date:   Wed, 22 Feb 2017 20:27:06 +0900
From:   Namhyung Kim <namhyung@...nel.org>
To:     Taeung Song <treeze.taeung@...il.com>
Cc:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Jiri Olsa <jolsa@...nel.org>, Ingo Molnar <mingo@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Wang Nan <wangnan0@...wei.com>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Jiri Olsa <jolsa@...hat.com>
Subject: 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@...il.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@...nel.org>
> Cc: Jiri Olsa <jolsa@...hat.com>
> Signed-off-by: Taeung Song <treeze.taeung@...il.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

Powered by blists - more mailing lists