Re: [PATCH 11/11] perf tools: Stop fallbacking to kallsyms for vdso symbols lookup

[Adrian Hunter]

On 18/10/18 1:55 AM, Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> 
> David reports that:
> 
> <quote>
> Perf has this hack where it uses the kernel symbol map as a backup when
> a symbol can't be found in the user's symbol table(s).

I don't think this is a complete fix because it exposes new problems.  This
code caters for branches from kernel space to user space and vice versa.
That is, since there is only one cpumode so it is certain to be wrong for
either 'ip' or 'addr' when they are not both in the kernel or both in userspace.

> 
> 
> Cc: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> Cc: David Ahern <dsahern@xxxxxxxxx>
> Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
> Cc: Namhyung Kim <namhyung@xxxxxxxxxx>
> Cc: Wang Nan <wangnan0@xxxxxxxxxx>
> Link: https://lkml.kernel.org/n/tip-cs7skq9pp0kjypiju6o7trse@xxxxxxxxxxxxxx
> Signed-off-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> ---
>  tools/perf/util/event.c | 21 ++-------------------
>  1 file changed, 2 insertions(+), 19 deletions(-)
> 
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index 0988eb3b844b..bc646185f8d9 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -1561,26 +1561,9 @@ struct map *thread__find_map(struct thread *thread, u8 cpumode, u64 addr,
>  
>  		return NULL;
>  	}
> -try_again:
> +
>  	al->map = map_groups__find(mg, al->addr);
> -	if (al->map == NULL) {
> -		/*
> -		 * If this is outside of all known maps, and is a negative
> -		 * address, try to look it up in the kernel dso, as it might be
> -		 * a vsyscall or vdso (which executes in user-mode).
> -		 *
> -		 * XXX This is nasty, we should have a symbol list in the
> -		 * "[vdso]" dso, but for now lets use the old trick of looking
> -		 * in the whole kernel symbol list.
> -		 */
> -		if (cpumode == PERF_RECORD_MISC_USER && machine &&
> -		    mg != &machine->kmaps &&
> -		    machine__kernel_ip(machine, al->addr)) {
> -			mg = &machine->kmaps;
> -			load_map = true;
> -			goto try_again;
> -		}
> -	} else {
> +	if (al->map != NULL) {
>  		/*
>  		 * Kernel maps might be changed when loading symbols so loading
>  		 * must be done prior to using kernel maps.
>