On Wed, Feb 22, 2017 at 7:08 PM, Taeung Song <treeze.tae...@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 <namhy...@kernel.org> > Cc: Jiri Olsa <jo...@redhat.com> > Signed-off-by: Taeung Song <treeze.tae...@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(¬es->src->lines); > } > > +static int symbol__free_source_code(struct symbol *sym) > +{ > + struct annotation *notes = symbol__annotation(sym); > + struct source_code *pos, *tmp; > + > + if (¬es->src->code == NULL) > + return -1; > + > + list_for_each_entry_safe(pos, tmp, ¬es->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(¬es->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, ¬es->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