Re: [PATCH] perf probe: Fix segfault when glob matching function without debuginfo

[Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>]

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.pt@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 <wangnan0@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.pt@hitachi.com