Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753215AbbF2N0b (ORCPT ); Mon, 29 Jun 2015 09:26:31 -0400 Received: from mga03.intel.com ([134.134.136.65]:48550 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751100AbbF2N00 (ORCPT ); Mon, 29 Jun 2015 09:26:26 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,698,1427785200"; d="scan'208";a="719601099" Message-ID: <559146E7.1070201@intel.com> Date: Mon, 29 Jun 2015 16:23:51 +0300 From: Adrian Hunter Organization: Intel Finland Oy, Registered Address: PL 281, 00181 Helsinki, Business Identity Code: 0357606 - 4, Domiciled in Helsinki User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Will Deacon , acme@redhat.com CC: linux-kernel@vger.kernel.org, kristina.martsenko@arm.com, Vladimir Nikulichev , Namhyung Kim , Andy Lutomirski Subject: Re: [PATCH v2] perf tools: don't adjust symbols in vDSO References: <1435396213-30208-1-git-send-email-will.deacon@arm.com> <55910989.8090608@intel.com> <55913A57.2000602@intel.com> In-Reply-To: <55913A57.2000602@intel.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3539 Lines: 90 On 29/06/15 15:30, Adrian Hunter wrote: > On 29/06/15 12:02, Adrian Hunter wrote: >> 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 >>> Cc: Adrian Hunter >>> Cc: Namhyung Kim >>> Cc: Andy Lutomirski >>> Reported-by: Kristina Martsenko >>> Signed-off-by: Will Deacon >>> --- >>> >>> 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. > > So what about just this instead: > > if (dso__is_vdso(dso)) > map->reloc = shdr.sh_addr - shdr.sh_offset; > No that's no good either :-( -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/