On 2015/05/29 18:45, Wang Nan wrote: > Commit 4c859351226c920b227fec040a3b447f0d482af3 ("perf probe: Support > glob wildcards for function name") introduces segfault problems when > debuginfo is not available: > > # perf probe 'sys_w*' > Added new events: > Segmentation fault > > The first problem resides in find_probe_trace_events_from_map(). In > that function, find_probe_functions() is called to match each symbol > against glob to find the number of matching functions, but still use > map__for_each_symbol_by_name() to find 'struct symbol' for matching > functions. Unfortunately, map__for_each_symbol_by_name() does > exact matching by searching in an rbtree. It doesn't know glob > matching, and not easy for it to support it because it use rbtree based > binary search, but we are unable to ensure all names matched by the > glob (any glob passed by user) reside in one subtree. > > This patch drops map__for_each_symbol_by_name(). Since there is no > rbtree again, re-matching all symbols costs a lot. This patch avoid it > by saving all matching results into an array (syms). > > The second problem is the lost of tp->realname. In > __add_probe_trace_events(), if pev->point.function is glob, the event > name should be set to tev->point.realname. This patch ensures its > existence by strdup sym->name instead of leaving a NULL pointer there.
Good catch! Acked-by: Masami Hiramatsu <masami.hiramatsu...@hitachi.com> Thank you! > > After this patch: > > # perf probe 'sys_w*' > Added new events: > probe:sys_waitid (on sys_w*) > probe:sys_wait4 (on sys_w*) > probe:sys_waitpid (on sys_w*) > probe:sys_write (on sys_w*) > probe:sys_writev (on sys_w*) > > You can now use it in all perf tools, such as: > > perf record -e probe:sys_writev -aR sleep 1 > > Signed-off-by: Wang Nan <wangn...@huawei.com> > --- > tools/perf/util/probe-event.c | 26 +++++++++++++++++++++----- > 1 file changed, 21 insertions(+), 5 deletions(-) > > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c > index 9052e7b..4f1bc78 100644 > --- a/tools/perf/util/probe-event.c > +++ b/tools/perf/util/probe-event.c > @@ -2493,7 +2493,8 @@ close_out: > return ret; > } > > -static int find_probe_functions(struct map *map, char *name) > +static int find_probe_functions(struct map *map, char *name, > + struct symbol **syms) > { > int found = 0; > struct symbol *sym; > @@ -2503,8 +2504,11 @@ static int find_probe_functions(struct map *map, char > *name) > return 0; > > map__for_each_symbol(map, sym, tmp) { > - if (strglobmatch(sym->name, name)) > + if (strglobmatch(sym->name, name)) { > found++; > + if (syms && found < probe_conf.max_probes) > + syms[found - 1] = sym; > + } > } > > return found; > @@ -2527,11 +2531,12 @@ static int find_probe_trace_events_from_map(struct > perf_probe_event *pev, > struct map *map = NULL; > struct ref_reloc_sym *reloc_sym = NULL; > struct symbol *sym; > + struct symbol **syms = NULL; > struct probe_trace_event *tev; > struct perf_probe_point *pp = &pev->point; > struct probe_trace_point *tp; > int num_matched_functions; > - int ret, i; > + int ret, i, j; > > map = get_target_map(pev->target, pev->uprobes); > if (!map) { > @@ -2539,11 +2544,17 @@ static int find_probe_trace_events_from_map(struct > perf_probe_event *pev, > goto out; > } > > + syms = malloc(sizeof(struct symbol *) * probe_conf.max_probes); > + if (!syms) { > + ret = -ENOMEM; > + goto out; > + } > + > /* > * Load matched symbols: Since the different local symbols may have > * same name but different addresses, this lists all the symbols. > */ > - num_matched_functions = find_probe_functions(map, pp->function); > + num_matched_functions = find_probe_functions(map, pp->function, syms); > if (num_matched_functions == 0) { > pr_err("Failed to find symbol %s in %s\n", pp->function, > pev->target ? : "kernel"); > @@ -2574,7 +2585,9 @@ static int find_probe_trace_events_from_map(struct > perf_probe_event *pev, > > ret = 0; > > - map__for_each_symbol_by_name(map, pp->function, sym) { > + for (j = 0; j < num_matched_functions; j++) { > + sym = syms[j]; > + > tev = (*tevs) + ret; > tp = &tev->point; > if (ret == num_matched_functions) { > @@ -2598,6 +2611,8 @@ static int find_probe_trace_events_from_map(struct > perf_probe_event *pev, > tp->symbol = strdup_or_goto(sym->name, nomem_out); > tp->offset = pp->offset; > } > + tp->realname = strdup_or_goto(sym->name, nomem_out); > + > tp->retprobe = pp->retprobe; > if (pev->target) > tev->point.module = strdup_or_goto(pev->target, > @@ -2628,6 +2643,7 @@ static int find_probe_trace_events_from_map(struct > perf_probe_event *pev, > > out: > put_target_map(map, pev->uprobes); > + free(syms); > return ret; > > nomem_out: > -- Masami HIRAMATSU Linux Technology Research Center, System Productivity Research Dept. Center for Technology Innovation - Systems Engineering Hitachi, Ltd., Research & Development Group E-mail: masami.hiramatsu...@hitachi.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/