From fea0cf842c7aa08950063264ab1cfbce4ba38c1b Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Mon, 9 Oct 2017 22:32:57 +0200 Subject: perf callchain: Refactor inline_list to operate on symbols This is a requirement to create real callchain entries for inlined frames. Since the list of inlines usually contains the target symbol too, i.e. the location where the frames get inlined to, we alias that symbol and reuse it as-is is. This ensures that other dependent functionality keeps working, most notably annotation of the target frames. For all other entries in the inline_list, a fake symbol is created. These are marked by new 'inlined' member which is set to true. Only those symbols are managed by the inline_list and get freed when the inline_list is deleted from within inline_node__delete. Signed-off-by: Milian Wolff Reviewed-by: Jiri Olsa Reviewed-by: Namhyung Kim Cc: David Ahern Cc: Peter Zijlstra Cc: Yao Jin Link: http://lkml.kernel.org/r/20171009203310.17362-4-milian.wolff@kdab.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/srcline.c | 93 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 63 insertions(+), 30 deletions(-) (limited to 'tools/perf/util/srcline.c') diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c index ed8e8d2de942..c0af61b355ed 100644 --- a/tools/perf/util/srcline.c +++ b/tools/perf/util/srcline.c @@ -33,29 +33,19 @@ static const char *dso__name(struct dso *dso) return dso_name; } -static int inline_list__append(char *filename, char *funcname, int line_nr, - struct inline_node *node, struct dso *dso) +static int inline_list__append(struct symbol *symbol, char *filename, + int line_nr, struct inline_node *node) { struct inline_list *ilist; - char *demangled; ilist = zalloc(sizeof(*ilist)); if (ilist == NULL) return -1; + ilist->symbol = symbol; ilist->filename = filename; ilist->line_nr = line_nr; - if (dso != NULL) { - demangled = dso__demangle_sym(dso, 0, funcname); - if (demangled == NULL) { - ilist->funcname = funcname; - } else { - ilist->funcname = demangled; - free(funcname); - } - } - if (callchain_param.order == ORDER_CALLEE) list_add_tail(&ilist->list, &node->val); else @@ -206,19 +196,56 @@ static void addr2line_cleanup(struct a2l_data *a2l) #define MAX_INLINE_NEST 1024 +static struct symbol *new_inline_sym(struct dso *dso, + struct symbol *base_sym, + const char *funcname) +{ + struct symbol *inline_sym; + char *demangled = NULL; + + if (dso) { + demangled = dso__demangle_sym(dso, 0, funcname); + if (demangled) + funcname = demangled; + } + + if (base_sym && strcmp(funcname, base_sym->name) == 0) { + /* reuse the real, existing symbol */ + inline_sym = base_sym; + /* ensure that we don't alias an inlined symbol, which could + * lead to double frees in inline_node__delete + */ + assert(!base_sym->inlined); + } else { + /* create a fake symbol for the inline frame */ + inline_sym = symbol__new(base_sym ? base_sym->start : 0, + base_sym ? base_sym->end : 0, + base_sym ? base_sym->binding : 0, + funcname); + if (inline_sym) + inline_sym->inlined = 1; + } + + free(demangled); + + return inline_sym; +} + static int inline_list__append_dso_a2l(struct dso *dso, - struct inline_node *node) + struct inline_node *node, + struct symbol *sym) { struct a2l_data *a2l = dso->a2l; - char *funcname = a2l->funcname ? strdup(a2l->funcname) : NULL; - char *filename = a2l->filename ? strdup(a2l->filename) : NULL; + struct symbol *inline_sym = new_inline_sym(dso, sym, a2l->funcname); - return inline_list__append(filename, funcname, a2l->line, node, dso); + return inline_list__append(inline_sym, strdup(a2l->filename), + a2l->line, node); } static int addr2line(const char *dso_name, u64 addr, char **file, unsigned int *line, struct dso *dso, - bool unwind_inlines, struct inline_node *node) + bool unwind_inlines, struct inline_node *node, + struct symbol *sym) { int ret = 0; struct a2l_data *a2l = dso->a2l; @@ -244,7 +271,7 @@ static int addr2line(const char *dso_name, u64 addr, if (unwind_inlines) { int cnt = 0; - if (node && inline_list__append_dso_a2l(dso, node)) + if (node && inline_list__append_dso_a2l(dso, node, sym)) return 0; while (bfd_find_inliner_info(a2l->abfd, &a2l->filename, @@ -255,7 +282,7 @@ static int addr2line(const char *dso_name, u64 addr, a2l->filename = NULL; if (node != NULL) { - if (inline_list__append_dso_a2l(dso, node)) + if (inline_list__append_dso_a2l(dso, node, sym)) return 0; // found at least one inline frame ret = 1; @@ -287,7 +314,7 @@ void dso__free_a2l(struct dso *dso) } static struct inline_node *addr2inlines(const char *dso_name, u64 addr, - struct dso *dso) + struct dso *dso, struct symbol *sym) { struct inline_node *node; @@ -300,7 +327,7 @@ static struct inline_node *addr2inlines(const char *dso_name, u64 addr, INIT_LIST_HEAD(&node->val); node->addr = addr; - if (!addr2line(dso_name, addr, NULL, NULL, dso, TRUE, node)) + if (!addr2line(dso_name, addr, NULL, NULL, dso, TRUE, node, sym)) goto out_free_inline_node; if (list_empty(&node->val)) @@ -340,7 +367,8 @@ static int addr2line(const char *dso_name, u64 addr, char **file, unsigned int *line_nr, struct dso *dso __maybe_unused, bool unwind_inlines __maybe_unused, - struct inline_node *node __maybe_unused) + struct inline_node *node __maybe_unused, + struct symbol *sym __maybe_unused) { FILE *fp; char cmd[PATH_MAX]; @@ -380,7 +408,8 @@ void dso__free_a2l(struct dso *dso __maybe_unused) } static struct inline_node *addr2inlines(const char *dso_name, u64 addr, - struct dso *dso __maybe_unused) + struct dso *dso __maybe_unused, + struct symbol *sym) { FILE *fp; char cmd[PATH_MAX]; @@ -408,13 +437,13 @@ static struct inline_node *addr2inlines(const char *dso_name, u64 addr, node->addr = addr; while (getline(&filename, &len, fp) != -1) { + if (filename_split(filename, &line_nr) != 1) { free(filename); goto out; } - if (inline_list__append(filename, NULL, line_nr, node, - NULL) != 0) + if (inline_list__append(sym, filename, line_nr, node) != 0) goto out; filename = NULL; @@ -454,7 +483,8 @@ char *__get_srcline(struct dso *dso, u64 addr, struct symbol *sym, if (dso_name == NULL) goto out; - if (!addr2line(dso_name, addr, &file, &line, dso, unwind_inlines, NULL)) + if (!addr2line(dso_name, addr, &file, &line, dso, + unwind_inlines, NULL, sym)) goto out; if (asprintf(&srcline, "%s:%u", @@ -500,7 +530,8 @@ char *get_srcline(struct dso *dso, u64 addr, struct symbol *sym, return __get_srcline(dso, addr, sym, show_sym, show_addr, false); } -struct inline_node *dso__parse_addr_inlines(struct dso *dso, u64 addr) +struct inline_node *dso__parse_addr_inlines(struct dso *dso, u64 addr, + struct symbol *sym) { const char *dso_name; @@ -508,7 +539,7 @@ struct inline_node *dso__parse_addr_inlines(struct dso *dso, u64 addr) if (dso_name == NULL) return NULL; - return addr2inlines(dso_name, addr, dso); + return addr2inlines(dso_name, addr, dso, sym); } void inline_node__delete(struct inline_node *node) @@ -518,7 +549,9 @@ void inline_node__delete(struct inline_node *node) list_for_each_entry_safe(ilist, tmp, &node->val, list) { list_del_init(&ilist->list); zfree(&ilist->filename); - zfree(&ilist->funcname); + /* only the inlined symbols are owned by the list */ + if (ilist->symbol && ilist->symbol->inlined) + symbol__delete(ilist->symbol); free(ilist); } -- cgit From 2be8832f3c51cf9e36a3e80ff57f4137505c2ba4 Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Mon, 9 Oct 2017 22:32:58 +0200 Subject: perf callchain: Refactor inline_list to store srcline string directly This is a preparation for the creation of real callchain entries for inlined frames. The rest of the perf code uses the srcline string. As such, using that also for the srcline API allows us to simplify some of the upcoming code. Most notably, it will allow us to cache the srcline for a given inline node and reuse it for different callchain entries. Signed-off-by: Milian Wolff Reviewed-by: Jiri Olsa Reviewed-by: Namhyung Kim Cc: David Ahern Cc: Peter Zijlstra Cc: Yao Jin Link: http://lkml.kernel.org/r/20171009203310.17362-5-milian.wolff@kdab.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/srcline.c | 54 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 40 insertions(+), 14 deletions(-) (limited to 'tools/perf/util/srcline.c') diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c index c0af61b355ed..f202fc7827df 100644 --- a/tools/perf/util/srcline.c +++ b/tools/perf/util/srcline.c @@ -33,8 +33,8 @@ static const char *dso__name(struct dso *dso) return dso_name; } -static int inline_list__append(struct symbol *symbol, char *filename, - int line_nr, struct inline_node *node) +static int inline_list__append(struct symbol *symbol, char *srcline, + struct inline_node *node) { struct inline_list *ilist; @@ -43,8 +43,7 @@ static int inline_list__append(struct symbol *symbol, char *filename, return -1; ilist->symbol = symbol; - ilist->filename = filename; - ilist->line_nr = line_nr; + ilist->srcline = srcline; if (callchain_param.order == ORDER_CALLEE) list_add_tail(&ilist->list, &node->val); @@ -54,6 +53,30 @@ static int inline_list__append(struct symbol *symbol, char *filename, return 0; } +/* basename version that takes a const input string */ +static const char *gnu_basename(const char *path) +{ + const char *base = strrchr(path, '/'); + + return base ? base + 1 : path; +} + +static char *srcline_from_fileline(const char *file, unsigned int line) +{ + char *srcline; + + if (!file) + return NULL; + + if (!srcline_full_filename) + file = gnu_basename(file); + + if (asprintf(&srcline, "%s:%u", file, line) < 0) + return NULL; + + return srcline; +} + #ifdef HAVE_LIBBFD_SUPPORT /* @@ -237,9 +260,12 @@ static int inline_list__append_dso_a2l(struct dso *dso, { struct a2l_data *a2l = dso->a2l; struct symbol *inline_sym = new_inline_sym(dso, sym, a2l->funcname); + char *srcline = NULL; - return inline_list__append(inline_sym, strdup(a2l->filename), - a2l->line, node); + if (a2l->filename) + srcline = srcline_from_fileline(a2l->filename, a2l->line); + + return inline_list__append(inline_sym, srcline, node); } static int addr2line(const char *dso_name, u64 addr, @@ -437,13 +463,15 @@ static struct inline_node *addr2inlines(const char *dso_name, u64 addr, node->addr = addr; while (getline(&filename, &len, fp) != -1) { + char *srcline; if (filename_split(filename, &line_nr) != 1) { free(filename); goto out; } - if (inline_list__append(sym, filename, line_nr, node) != 0) + srcline = srcline_from_fileline(filename, line_nr); + if (inline_list__append(sym, srcline, node) != 0) goto out; filename = NULL; @@ -487,16 +515,14 @@ char *__get_srcline(struct dso *dso, u64 addr, struct symbol *sym, unwind_inlines, NULL, sym)) goto out; - if (asprintf(&srcline, "%s:%u", - srcline_full_filename ? file : basename(file), - line) < 0) { - free(file); + srcline = srcline_from_fileline(file, line); + free(file); + + if (!srcline) goto out; - } dso->a2l_fails = 0; - free(file); return srcline; out: @@ -548,7 +574,7 @@ void inline_node__delete(struct inline_node *node) list_for_each_entry_safe(ilist, tmp, &node->val, list) { list_del_init(&ilist->list); - zfree(&ilist->filename); + free_srcline(ilist->srcline); /* only the inlined symbols are owned by the list */ if (ilist->symbol && ilist->symbol->inlined) symbol__delete(ilist->symbol); -- cgit From 11ea2515f32e783b9a7984c148e742c377383915 Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Mon, 9 Oct 2017 22:32:59 +0200 Subject: perf callchain: Create real callchain entries for inlined frames The inline_node structs are maintained by the new dso->inlines tree. This in turn keeps ownership of the fake symbols and srcline string representing an inline frame. This tree is sorted by address to allow quick lookups. All other entries of the symbol beside the function name are unused for inline frames. The advantage of this approach is that all existing users of the callchain API can now transparently display inlined frames without having to patch their code. Signed-off-by: Milian Wolff Reviewed-by: Jiri Olsa Reviewed-by: Namhyung Kim Cc: David Ahern Cc: Peter Zijlstra Cc: Yao Jin Link: http://lkml.kernel.org/r/20171009203310.17362-6-milian.wolff@kdab.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/srcline.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) (limited to 'tools/perf/util/srcline.c') diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c index f202fc7827df..8bea6621d657 100644 --- a/tools/perf/util/srcline.c +++ b/tools/perf/util/srcline.c @@ -583,3 +583,54 @@ void inline_node__delete(struct inline_node *node) free(node); } + +void inlines__tree_insert(struct rb_root *tree, struct inline_node *inlines) +{ + struct rb_node **p = &tree->rb_node; + struct rb_node *parent = NULL; + const u64 addr = inlines->addr; + struct inline_node *i; + + while (*p != NULL) { + parent = *p; + i = rb_entry(parent, struct inline_node, rb_node); + if (addr < i->addr) + p = &(*p)->rb_left; + else + p = &(*p)->rb_right; + } + rb_link_node(&inlines->rb_node, parent, p); + rb_insert_color(&inlines->rb_node, tree); +} + +struct inline_node *inlines__tree_find(struct rb_root *tree, u64 addr) +{ + struct rb_node *n = tree->rb_node; + + while (n) { + struct inline_node *i = rb_entry(n, struct inline_node, + rb_node); + + if (addr < i->addr) + n = n->rb_left; + else if (addr > i->addr) + n = n->rb_right; + else + return i; + } + + return NULL; +} + +void inlines__tree_delete(struct rb_root *tree) +{ + struct inline_node *pos; + struct rb_node *next = rb_first(tree); + + while (next) { + pos = rb_entry(next, struct inline_node, rb_node); + next = rb_next(&pos->rb_node); + rb_erase(&pos->rb_node, tree); + inline_node__delete(pos); + } +} -- cgit From b38775cf7678d7715b35dded3dcfab66e244baae Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Thu, 19 Oct 2017 13:38:33 +0200 Subject: perf report: Cache failed lookups of inlined frames When no inlined frames could be found for a given address, we did not store this information anywhere. That means we potentially do the costly inliner lookup repeatedly for cases where we know it can never succeed. This patch makes dso__parse_addr_inlines always return a valid inline_node. It will be empty when no inliners are found. This enables us to cache the empty list in the DSO, thereby improving the performance when many addresses fail to find the inliners. For my trivial example, the performance impact is already quite significant: Before: ~~~~~ Performance counter stats for 'perf report --stdio --inline -g srcline -s srcline' (5 runs): 594.804032 task-clock (msec) # 0.998 CPUs utilized ( +- 0.07% ) 53 context-switches # 0.089 K/sec ( +- 4.09% ) 0 cpu-migrations # 0.000 K/sec ( +-100.00% ) 5,687 page-faults # 0.010 M/sec ( +- 0.02% ) 2,300,918,213 cycles # 3.868 GHz ( +- 0.09% ) 4,395,839,080 instructions # 1.91 insn per cycle ( +- 0.00% ) 939,177,205 branches # 1578.969 M/sec ( +- 0.00% ) 11,824,633 branch-misses # 1.26% of all branches ( +- 0.10% ) 0.596246531 seconds time elapsed ( +- 0.07% ) ~~~~~ After: ~~~~~ Performance counter stats for 'perf report --stdio --inline -g srcline -s srcline' (5 runs): 113.111405 task-clock (msec) # 0.990 CPUs utilized ( +- 0.89% ) 29 context-switches # 0.255 K/sec ( +- 54.25% ) 0 cpu-migrations # 0.000 K/sec 5,380 page-faults # 0.048 M/sec ( +- 0.01% ) 432,378,779 cycles # 3.823 GHz ( +- 0.75% ) 670,057,633 instructions # 1.55 insn per cycle ( +- 0.01% ) 141,001,247 branches # 1246.570 M/sec ( +- 0.01% ) 2,346,845 branch-misses # 1.66% of all branches ( +- 0.19% ) 0.114222393 seconds time elapsed ( +- 1.19% ) ~~~~~ Signed-off-by: Milian Wolff Reviewed-by: Andi Kleen Cc: David Ahern Cc: Jin Yao Cc: Jiri Olsa Cc: Namhyung Kim Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/20171019113836.5548-3-milian.wolff@kdab.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/srcline.c | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) (limited to 'tools/perf/util/srcline.c') diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c index 8bea6621d657..fc3888664b20 100644 --- a/tools/perf/util/srcline.c +++ b/tools/perf/util/srcline.c @@ -353,17 +353,8 @@ static struct inline_node *addr2inlines(const char *dso_name, u64 addr, INIT_LIST_HEAD(&node->val); node->addr = addr; - if (!addr2line(dso_name, addr, NULL, NULL, dso, TRUE, node, sym)) - goto out_free_inline_node; - - if (list_empty(&node->val)) - goto out_free_inline_node; - + addr2line(dso_name, addr, NULL, NULL, dso, true, node, sym); return node; - -out_free_inline_node: - inline_node__delete(node); - return NULL; } #else /* HAVE_LIBBFD_SUPPORT */ @@ -480,11 +471,6 @@ static struct inline_node *addr2inlines(const char *dso_name, u64 addr, out: pclose(fp); - if (list_empty(&node->val)) { - inline_node__delete(node); - return NULL; - } - return node; } -- cgit From 21ac9d547fdde79c1e8692587d9044fde549214b Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Thu, 19 Oct 2017 13:38:34 +0200 Subject: perf report: Cache srclines for callchain nodes On one hand this ensures that the memory is properly freed when the DSO gets freed. On the other hand this significantly speeds up the processing of the callchain nodes when lots of srclines are requested. For one of my data files e.g.: Before: Performance counter stats for 'perf report -s srcline -g srcline --stdio': 52496.495043 task-clock (msec) # 0.999 CPUs utilized 634 context-switches # 0.012 K/sec 2 cpu-migrations # 0.000 K/sec 191,561 page-faults # 0.004 M/sec 165,074,498,235 cycles # 3.144 GHz 334,170,832,408 instructions # 2.02 insn per cycle 90,220,029,745 branches # 1718.591 M/sec 654,525,177 branch-misses # 0.73% of all branches 52.533273822 seconds time elapsedProcessed 236605 events and lost 40 chunks! After: Performance counter stats for 'perf report -s srcline -g srcline --stdio': 22606.323706 task-clock (msec) # 1.000 CPUs utilized 31 context-switches # 0.001 K/sec 0 cpu-migrations # 0.000 K/sec 185,471 page-faults # 0.008 M/sec 71,188,113,681 cycles # 3.149 GHz 133,204,943,083 instructions # 1.87 insn per cycle 34,886,384,979 branches # 1543.214 M/sec 278,214,495 branch-misses # 0.80% of all branches 22.609857253 seconds time elapsed Note that the difference is only this large when `--inline` is not passed. In such situations, we would use the inliner cache and thus do not run this code path that often. I think that this cache should actually be used in other places, too. When looking at the valgrind leak report for perf report, we see tons of srclines being leaked, most notably from calls to hist_entry__get_srcline. The problem is that get_srcline has many different formatting options (show_sym, show_addr, potentially even unwind_inlines when calling __get_srcline directly). As such, the srcline cannot easily be cached for all calls, or we'd have to add caches for all formatting combinations (6 so far). An alternative would be to remove the formatting options and handle that on a different level - i.e. print the sym/addr on demand wherever we actually output something. And the unwind_inlines could be moved into a separate function that does not return the srcline. Signed-off-by: Milian Wolff Reviewed-by: Andi Kleen Cc: David Ahern Cc: Jin Yao Cc: Jiri Olsa Cc: Namhyung Kim Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/20171019113836.5548-4-milian.wolff@kdab.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/srcline.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) (limited to 'tools/perf/util/srcline.c') diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c index fc3888664b20..c143c3bc1ef8 100644 --- a/tools/perf/util/srcline.c +++ b/tools/perf/util/srcline.c @@ -542,6 +542,72 @@ char *get_srcline(struct dso *dso, u64 addr, struct symbol *sym, return __get_srcline(dso, addr, sym, show_sym, show_addr, false); } +struct srcline_node { + u64 addr; + char *srcline; + struct rb_node rb_node; +}; + +void srcline__tree_insert(struct rb_root *tree, u64 addr, char *srcline) +{ + struct rb_node **p = &tree->rb_node; + struct rb_node *parent = NULL; + struct srcline_node *i, *node; + + node = zalloc(sizeof(struct srcline_node)); + if (!node) { + perror("not enough memory for the srcline node"); + return; + } + + node->addr = addr; + node->srcline = srcline; + + while (*p != NULL) { + parent = *p; + i = rb_entry(parent, struct srcline_node, rb_node); + if (addr < i->addr) + p = &(*p)->rb_left; + else + p = &(*p)->rb_right; + } + rb_link_node(&node->rb_node, parent, p); + rb_insert_color(&node->rb_node, tree); +} + +char *srcline__tree_find(struct rb_root *tree, u64 addr) +{ + struct rb_node *n = tree->rb_node; + + while (n) { + struct srcline_node *i = rb_entry(n, struct srcline_node, + rb_node); + + if (addr < i->addr) + n = n->rb_left; + else if (addr > i->addr) + n = n->rb_right; + else + return i->srcline; + } + + return NULL; +} + +void srcline__tree_delete(struct rb_root *tree) +{ + struct srcline_node *pos; + struct rb_node *next = rb_first(tree); + + while (next) { + pos = rb_entry(next, struct srcline_node, rb_node); + next = rb_next(&pos->rb_node); + rb_erase(&pos->rb_node, tree); + free_srcline(pos->srcline); + zfree(&pos); + } +} + struct inline_node *dso__parse_addr_inlines(struct dso *dso, u64 addr, struct symbol *sym) { -- cgit From b7b75a60b291cc699ca9bb2a8517a1b3b08bbeb1 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Tue, 31 Oct 2017 11:06:53 +0900 Subject: perf srcline: Fix memory leak in addr2inlines() When libbfd is not used, addr2inlines() executes `addr2line -i` and process output line by line. But it resets filename to NULL in the loop so getline() allocates additional memory everytime instead of realloc. Signed-off-by: Namhyung Kim Acked-by: Jiri Olsa Cc: Jin Yao Cc: Milian Wolff Cc: Peter Zijlstra Cc: kernel-team@lge.com Link: http://lkml.kernel.org/r/20171031020654.31163-1-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/srcline.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'tools/perf/util/srcline.c') diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c index c143c3bc1ef8..51dc49c65476 100644 --- a/tools/perf/util/srcline.c +++ b/tools/perf/util/srcline.c @@ -456,20 +456,17 @@ static struct inline_node *addr2inlines(const char *dso_name, u64 addr, while (getline(&filename, &len, fp) != -1) { char *srcline; - if (filename_split(filename, &line_nr) != 1) { - free(filename); + if (filename_split(filename, &line_nr) != 1) goto out; - } srcline = srcline_from_fileline(filename, line_nr); if (inline_list__append(sym, srcline, node) != 0) goto out; - - filename = NULL; } out: pclose(fp); + free(filename); return node; } -- cgit From 7285cf3325b4a1dfb336d31eebc27dfbc30fb9aa Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Tue, 31 Oct 2017 11:06:54 +0900 Subject: perf srcline: Show correct function name for srcline of callchains When libbfd is not used, it doesn't show proper function name and reuse the original symbol of the sample. That's because it passes the original sym to inline_list__append(). As `addr2line -f` returns function names as well, use that to create an inline_sym and pass it to inline_list__append(). For example, following data shows that inlined entries of main have same name (main). Before: $ perf report -g srcline -q | head 45.22% inlining libm-2.26.so [.] __hypot_finite | ---__hypot_finite ??:0 | |--44.15%--hypot ??:0 | main complex:589 | main complex:597 | main complex:654 | main complex:664 | main inlining.cpp:14 After: $ perf report -g srcline -q | head 45.22% inlining libm-2.26.so [.] __hypot_finite | ---__hypot_finite | |--44.15%--hypot | std::__complex_abs complex:589 (inlined) | std::abs complex:597 (inlined) | std::_Norm_helper::_S_do_it complex:654 (inlined) | std::norm complex:664 (inlined) | main inlining.cpp:14 Signed-off-by: Namhyung Kim Reviewed-by: Jiri Olsa Reviewed-by: Milian Wolff Cc: Jin Yao Cc: Peter Zijlstra Cc: kernel-team@lge.com Link: http://lkml.kernel.org/r/20171031020654.31163-2-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/srcline.c | 95 +++++++++++++++++++++++++++-------------------- 1 file changed, 55 insertions(+), 40 deletions(-) (limited to 'tools/perf/util/srcline.c') diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c index 51dc49c65476..ad1b46f1f2cf 100644 --- a/tools/perf/util/srcline.c +++ b/tools/perf/util/srcline.c @@ -10,7 +10,7 @@ #include "util/debug.h" #include "util/callchain.h" #include "srcline.h" - +#include "string2.h" #include "symbol.h" bool srcline_full_filename; @@ -77,6 +77,41 @@ static char *srcline_from_fileline(const char *file, unsigned int line) return srcline; } +static struct symbol *new_inline_sym(struct dso *dso, + struct symbol *base_sym, + const char *funcname) +{ + struct symbol *inline_sym; + char *demangled = NULL; + + if (dso) { + demangled = dso__demangle_sym(dso, 0, funcname); + if (demangled) + funcname = demangled; + } + + if (base_sym && strcmp(funcname, base_sym->name) == 0) { + /* reuse the real, existing symbol */ + inline_sym = base_sym; + /* ensure that we don't alias an inlined symbol, which could + * lead to double frees in inline_node__delete + */ + assert(!base_sym->inlined); + } else { + /* create a fake symbol for the inline frame */ + inline_sym = symbol__new(base_sym ? base_sym->start : 0, + base_sym ? base_sym->end : 0, + base_sym ? base_sym->binding : 0, + funcname); + if (inline_sym) + inline_sym->inlined = 1; + } + + free(demangled); + + return inline_sym; +} + #ifdef HAVE_LIBBFD_SUPPORT /* @@ -219,41 +254,6 @@ static void addr2line_cleanup(struct a2l_data *a2l) #define MAX_INLINE_NEST 1024 -static struct symbol *new_inline_sym(struct dso *dso, - struct symbol *base_sym, - const char *funcname) -{ - struct symbol *inline_sym; - char *demangled = NULL; - - if (dso) { - demangled = dso__demangle_sym(dso, 0, funcname); - if (demangled) - funcname = demangled; - } - - if (base_sym && strcmp(funcname, base_sym->name) == 0) { - /* reuse the real, existing symbol */ - inline_sym = base_sym; - /* ensure that we don't alias an inlined symbol, which could - * lead to double frees in inline_node__delete - */ - assert(!base_sym->inlined); - } else { - /* create a fake symbol for the inline frame */ - inline_sym = symbol__new(base_sym ? base_sym->start : 0, - base_sym ? base_sym->end : 0, - base_sym ? base_sym->binding : 0, - funcname); - if (inline_sym) - inline_sym->inlined = 1; - } - - free(demangled); - - return inline_sym; -} - static int inline_list__append_dso_a2l(struct dso *dso, struct inline_node *node, struct symbol *sym) @@ -432,10 +432,11 @@ static struct inline_node *addr2inlines(const char *dso_name, u64 addr, char cmd[PATH_MAX]; struct inline_node *node; char *filename = NULL; - size_t len; + char *funcname = NULL; + size_t filelen, funclen; unsigned int line_nr = 0; - scnprintf(cmd, sizeof(cmd), "addr2line -e %s -i %016"PRIx64, + scnprintf(cmd, sizeof(cmd), "addr2line -e %s -i -f %016"PRIx64, dso_name, addr); fp = popen(cmd, "r"); @@ -453,20 +454,34 @@ static struct inline_node *addr2inlines(const char *dso_name, u64 addr, INIT_LIST_HEAD(&node->val); node->addr = addr; - while (getline(&filename, &len, fp) != -1) { + /* addr2line -f generates two lines for each inlined functions */ + while (getline(&funcname, &funclen, fp) != -1) { char *srcline; + struct symbol *inline_sym; + + rtrim(funcname); + + if (getline(&filename, &filelen, fp) == -1) + goto out; if (filename_split(filename, &line_nr) != 1) goto out; srcline = srcline_from_fileline(filename, line_nr); - if (inline_list__append(sym, srcline, node) != 0) + inline_sym = new_inline_sym(dso, sym, funcname); + + if (inline_list__append(inline_sym, srcline, node) != 0) { + free(srcline); + if (inline_sym && inline_sym->inlined) + symbol__delete(inline_sym); goto out; + } } out: pclose(fp); free(filename); + free(funcname); return node; } -- cgit