Re: [PATCH v2] perf tools: don't adjust symbols in vDSO

[Adrian Hunter <adrian.hunter@intel.com>]

On 27/06/15 12:10, Will Deacon wrote:
> Commit 922d0e4d9f04 ("perf tools: Adjust symbols in VDSO") changed the
> ELF symbol parsing so that the vDSO is treated the same as ET_EXEC and
> ET_REL binaries despite being an ET_DYN. This was a partial workaround
> to deal with older x86 vDSOs being prelinked at a high address that
> didn't correspond to the map, so using object-relative offsets and
> adding the base of the map allowed symbol resolution to succeed.
> 
> Unfortunately, this causes objdump not to produce any output in
> conjunction with perf annotate, which cheerfully passes the absolute
> address of the map symbol.
> 
> This patch fixes the problem by avoiding adjustment of vDSO symbols and
> instead setting the map->pgoff field to correspond to the virtual load
> address specified in the vDSO ELF header.
> 
> Cc: Vladimir Nikulichev <nvs@tbricks.com>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Reported-by: Kristina Martsenko <kristina.martsenko@arm.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
> 
> v1->v2: Adjust map->pgoff in ELF loader to avoid breaking symbol lookup
>         on older kernels.
> 
>  tools/perf/util/map.c        | 5 ++---
>  tools/perf/util/symbol-elf.c | 9 ++++++++-
>  2 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> index a14f08f41686..6ba38293be88 100644
> --- a/tools/perf/util/map.c
> +++ b/tools/perf/util/map.c
> @@ -173,10 +173,9 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,
>  				filename = newfilename;
>  		}
>  
> -		if (vdso) {
> -			pgoff = 0;
> +		if (vdso)
>  			dso = vdso__dso_findnew(machine, thread);
> -		} else
> +		else
>  			dso = __dsos__findnew(&machine->user_dsos, filename);
>  
>  		if (dso == NULL)
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index a7ab6063e038..83f8ba232575 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -706,7 +706,6 @@ int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
>  		GElf_Shdr shdr;
>  		ss->adjust_symbols = (ehdr.e_type == ET_EXEC ||
>  				ehdr.e_type == ET_REL ||
> -				dso__is_vdso(dso) ||
>  				elf_section_by_name(elf, &ehdr, &shdr,
>  						     ".gnu.prelink_undo",
>  						     NULL) != NULL);
> @@ -824,6 +823,14 @@ int dso__load_sym(struct dso *dso, struct map *map,
>  	sec = syms_ss->symtab;
>  	shdr = syms_ss->symshdr;
>  
> +	/*
> +	 * Older x86 kernels prelink the vDSO at a high address, so
> +	 * we need to reflect that in map->pgoff in order to talk to
> +	 * objdump.
> +	 */
> +	if (dso__is_vdso(dso))
> +		map->pgoff = shdr.sh_addr - shdr.sh_offset;

In the case of perf tools, maps map memory addresses to file offsets.
That is used to read from the object file, so you can't change the map.