Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753194AbbF2Mcr (ORCPT ); Mon, 29 Jun 2015 08:32:47 -0400 Received: from mga03.intel.com ([134.134.136.65]:2439 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752980AbbF2Mcp (ORCPT ); Mon, 29 Jun 2015 08:32:45 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,698,1427785200"; d="scan'208";a="755098280" Message-ID: <55913A57.2000602@intel.com> Date: Mon, 29 Jun 2015 15:30:15 +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: Adrian Hunter , 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> In-Reply-To: <55910989.8090608@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: 3500 Lines: 89 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; Although shdr is ".dynsym" whereas ".text" is what we typically care about. Don't know if they could be inconsistent. -- 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/