linux-kernel - Re: [PATCH 1/2] perf symbols: Ignore mapping symbols on aarch64

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Wed, 14 Jan 2015 10:38:38 -0800
From:	Victor Kamensky <victor.kamensky@...aro.org>
To:	Will Deacon <will.deacon@....com>
Cc:	Arnaldo Carvalho de Melo <acme@...nel.org>,
	Namhyung Kim <namhyung@...nel.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Paul Mackerras <paulus@...ba.org>,
	Ingo Molnar <mingo@...hat.com>,
	Adrian Hunter <adrian.hunter@...el.com>,
	Jiri Olsa <jolsa@...hat.com>,
	Avi Kivity <avi@...udius-systems.com>,
	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
	Anton Blanchard <anton@...ba.org>,
	David Ahern <dsahern@...il.com>,
	Dave P Martin <Dave.Martin@....com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH 1/2] perf symbols: Ignore mapping symbols on aarch64

On 14 January 2015 at 03:22, Will Deacon <will.deacon@....com> wrote:
> On Tue, Jan 13, 2015 at 04:59:03PM +0000, Victor Kamensky wrote:
>> Aarch64 ELF files use mapping symbols with special names $x, $d
>> to identify regions of Aarch64 code (see Aarch64 ELF ABI - "ARM
>> IHI 0056B", section "4.5.4 Mapping symbols").
>>
>> The patch filters out these symbols at load time, similar to
>> "696b97a perf symbols: Ignore mapping symbols on ARM" changes
>> done for ARM before V8.
>>
>> Signed-off-by: Victor Kamensky <victor.kamensky@...aro.org>
>> Cc: Peter Zijlstra <a.p.zijlstra@...llo.nl>
>> Cc: Paul Mackerras <paulus@...ba.org>
>> Cc: Ingo Molnar <mingo@...hat.com>
>> Cc: Arnaldo Carvalho de Melo <acme@...nel.org>
>> Cc: Adrian Hunter <adrian.hunter@...el.com>
>> Cc: Jiri Olsa <jolsa@...hat.com>
>> Cc: Namhyung Kim <namhyung@...nel.org>
>> Cc: Avi Kivity <avi@...udius-systems.com>
>> Cc: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
>> Cc: Anton Blanchard <anton@...ba.org>
>> Cc: David Ahern <dsahern@...il.com>
>> Cc: Will Deacon <will.deacon@....com>
>> Cc: Dave Martin <Dave.Martin@....com>
>> ---
>>  tools/perf/util/symbol-elf.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
>> index 06fcd1b..1e188dd 100644
>> --- a/tools/perf/util/symbol-elf.c
>> +++ b/tools/perf/util/symbol-elf.c
>> @@ -862,6 +862,14 @@ int dso__load_sym(struct dso *dso, struct map *map,
>>                           !strcmp(elf_name, "$t"))
>>                               continue;
>>               }
>> +             /* Reject Aarch64 ELF "mapping symbols": these aren't unique and
>> +              * don't identify functions, so will confuse the profile
>> +              * output: */
>> +             if (ehdr.e_machine == EM_AARCH64) {
>> +                     if (!strcmp(elf_name, "$x") ||
>> +                         !strcmp(elf_name, "$d"))
>> +                             continue;
>> +             }
>
> Do we need to skip $x.* and $d.* too? I doubt GCC generates them, but they
> are permitted by the ELF ABI.

Fair enough. But I think it would need to be done for both EM_ARM and
EM_AARCH64. My above patch follows EM_ARM current case. Also it
seems that it would be quite rare case, as ABI suggests symbols
with dot notation should be used in manual asm where assembler does not
support multiple definitions of the same symbol.

Will something like the following suffice? I can add that as separate follow up
patch in this small series. Tested with explicit "$x.func1" symbol introduced in
manual asm.

>From fed6caab410ddcaf487ff23a3908eca129e50b89 Mon Sep 17 00:00:00 2001
From: Victor Kamensky <victor.kamensky@...aro.org>
Date: Wed, 14 Jan 2015 07:42:41 -0800
Subject: [PATCH 3/3] perf symbols: improve abi compliance in arm mapping
 symbols handling

Both Arm and Aarch64 ELF ABI allow mapping symbols be in from
either "$d" or "$d.<any>". But current code that handles mapping
symbols only deals with the first, dollar character and a single
letter, case.

The patch adds handling of the second case with period
followed by any characters.

Suggested-by: Will Deacon <will.deacon@....com>
Signed-off-by: Victor Kamensky <victor.kamensky@...aro.org>
---
 tools/perf/util/symbol-elf.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 1e188dd..ae92c27 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -859,7 +859,10 @@ int dso__load_sym(struct dso *dso, struct map *map,
         if (ehdr.e_machine == EM_ARM) {
             if (!strcmp(elf_name, "$a") ||
                 !strcmp(elf_name, "$d") ||
-                !strcmp(elf_name, "$t"))
+                !strcmp(elf_name, "$t") ||
+                !strncmp(elf_name, "$a.", 3) ||
+                !strncmp(elf_name, "$d.", 3) ||
+                !strncmp(elf_name, "$t.", 3))
                 continue;
         }
         /* Reject Aarch64 ELF "mapping symbols": these aren't unique and
@@ -867,7 +870,9 @@ int dso__load_sym(struct dso *dso, struct map *map,
          * output: */
         if (ehdr.e_machine == EM_AARCH64) {
             if (!strcmp(elf_name, "$x") ||
-                !strcmp(elf_name, "$d"))
+                !strcmp(elf_name, "$d") ||
+                !strncmp(elf_name, "$x.", 3) ||
+                !strncmp(elf_name, "$d.", 3))
                 continue;
         }

-- 
1.9.3

Thanks,
Victor

> Will
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/