* [PATCH] perf jit: genelf makes assumptions about endian
@ 2016-03-29 6:59 Anton Blanchard
2016-03-30 2:38 ` Michael Ellerman
2016-03-31 6:34 ` [tip:perf/urgent] " tip-bot for Anton Blanchard
0 siblings, 2 replies; 4+ messages in thread
From: Anton Blanchard @ 2016-03-29 6:59 UTC (permalink / raw)
To: eranian, sukadev, acme, mpe, cel; +Cc: linuxppc-dev, linux-kernel
Commit 9b07e27f88b9 ("perf inject: Add jitdump mmap injection support")
incorrectly assumed that PowerPC is big endian only.
Simplify things by consolidating the define of GEN_ELF_ENDIAN and checking
for __BYTE_ORDER == __BIG_ENDIAN.
The PowerPC checks were also incorrect, they do not match what gcc
emits. We should first look for __powerpc64__, then __powerpc__.
Fixes: 9b07e27f88b9 ("perf inject: Add jitdump mmap injection support")
Signed-off-by: Anton Blanchard <anton@samba.org>
---
diff --git a/tools/perf/util/genelf.h b/tools/perf/util/genelf.h
index cd67e64..2fbeb59 100644
--- a/tools/perf/util/genelf.h
+++ b/tools/perf/util/genelf.h
@@ -9,36 +9,32 @@ int jit_add_debug_info(Elf *e, uint64_t code_addr, void *debug, int nr_debug_ent
#if defined(__arm__)
#define GEN_ELF_ARCH EM_ARM
-#define GEN_ELF_ENDIAN ELFDATA2LSB
#define GEN_ELF_CLASS ELFCLASS32
#elif defined(__aarch64__)
#define GEN_ELF_ARCH EM_AARCH64
-#define GEN_ELF_ENDIAN ELFDATA2LSB
#define GEN_ELF_CLASS ELFCLASS64
#elif defined(__x86_64__)
#define GEN_ELF_ARCH EM_X86_64
-#define GEN_ELF_ENDIAN ELFDATA2LSB
#define GEN_ELF_CLASS ELFCLASS64
#elif defined(__i386__)
#define GEN_ELF_ARCH EM_386
-#define GEN_ELF_ENDIAN ELFDATA2LSB
#define GEN_ELF_CLASS ELFCLASS32
-#elif defined(__ppcle__)
-#define GEN_ELF_ARCH EM_PPC
-#define GEN_ELF_ENDIAN ELFDATA2LSB
-#define GEN_ELF_CLASS ELFCLASS64
-#elif defined(__powerpc__)
-#define GEN_ELF_ARCH EM_PPC64
-#define GEN_ELF_ENDIAN ELFDATA2MSB
-#define GEN_ELF_CLASS ELFCLASS64
-#elif defined(__powerpcle__)
+#elif defined(__powerpc64__)
#define GEN_ELF_ARCH EM_PPC64
-#define GEN_ELF_ENDIAN ELFDATA2LSB
#define GEN_ELF_CLASS ELFCLASS64
+#elif defined(__powerpc__)
+#define GEN_ELF_ARCH EM_PPC
+#define GEN_ELF_CLASS ELFCLASS32
#else
#error "unsupported architecture"
#endif
+#if __BYTE_ORDER == __BIG_ENDIAN
+#define GEN_ELF_ENDIAN ELFDATA2MSB
+#else
+#define GEN_ELF_ENDIAN ELFDATA2LSB
+#endif
+
#if GEN_ELF_CLASS == ELFCLASS64
#define elf_newehdr elf64_newehdr
#define elf_getshdr elf64_getshdr
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] perf jit: genelf makes assumptions about endian
2016-03-29 6:59 [PATCH] perf jit: genelf makes assumptions about endian Anton Blanchard
@ 2016-03-30 2:38 ` Michael Ellerman
2016-03-30 21:15 ` Arnaldo Carvalho de Melo
2016-03-31 6:34 ` [tip:perf/urgent] " tip-bot for Anton Blanchard
1 sibling, 1 reply; 4+ messages in thread
From: Michael Ellerman @ 2016-03-30 2:38 UTC (permalink / raw)
To: Anton Blanchard, eranian, sukadev, acme, cel; +Cc: linuxppc-dev, linux-kernel
On Tue, 2016-03-29 at 17:59 +1100, Anton Blanchard wrote:
> Commit 9b07e27f88b9 ("perf inject: Add jitdump mmap injection support")
> incorrectly assumed that PowerPC is big endian only.
>
> Simplify things by consolidating the define of GEN_ELF_ENDIAN and checking
> for __BYTE_ORDER == __BIG_ENDIAN.
>
> The PowerPC checks were also incorrect, they do not match what gcc
> emits. We should first look for __powerpc64__, then __powerpc__.
>
> Fixes: 9b07e27f88b9 ("perf inject: Add jitdump mmap injection support")
> Signed-off-by: Anton Blanchard <anton@samba.org>
The diff's a little hard to read because you're pulling the endian logic out,
if I remove that I get something like:
> #elif defined(__i386__)
> #define GEN_ELF_ARCH EM_386
> #define GEN_ELF_CLASS ELFCLASS32
> -#elif defined(__ppcle__)
> -#define GEN_ELF_ARCH EM_PPC
> -#define GEN_ELF_CLASS ELFCLASS64
> -#elif defined(__powerpc__)
> -#define GEN_ELF_ARCH EM_PPC64
> -#define GEN_ELF_CLASS ELFCLASS64
> -#elif defined(__powerpcle__)
> +#elif defined(__powerpc64__)
> #define GEN_ELF_ARCH EM_PPC64
> #define GEN_ELF_CLASS ELFCLASS64
> +#elif defined(__powerpc__)
> +#define GEN_ELF_ARCH EM_PPC
> +#define GEN_ELF_CLASS ELFCLASS32
> #else
> #error "unsupported architecture"
> #endif
Which looks correct to me.
And the consolidation of the endian logic is "obviously correct", so:
Acked-by: Michael Ellerman <mpe@ellerman.id.au>
cheers
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] perf jit: genelf makes assumptions about endian
2016-03-30 2:38 ` Michael Ellerman
@ 2016-03-30 21:15 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 4+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-03-30 21:15 UTC (permalink / raw)
To: Michael Ellerman
Cc: Anton Blanchard, eranian, sukadev, cel, linuxppc-dev, linux-kernel
Em Wed, Mar 30, 2016 at 01:38:20PM +1100, Michael Ellerman escreveu:
> On Tue, 2016-03-29 at 17:59 +1100, Anton Blanchard wrote:
>
> > Commit 9b07e27f88b9 ("perf inject: Add jitdump mmap injection support")
> > incorrectly assumed that PowerPC is big endian only.
> >
> > Simplify things by consolidating the define of GEN_ELF_ENDIAN and checking
> > for __BYTE_ORDER == __BIG_ENDIAN.
> >
> > The PowerPC checks were also incorrect, they do not match what gcc
> > emits. We should first look for __powerpc64__, then __powerpc__.
> >
> > Fixes: 9b07e27f88b9 ("perf inject: Add jitdump mmap injection support")
> > Signed-off-by: Anton Blanchard <anton@samba.org>
>
> The diff's a little hard to read because you're pulling the endian logic out,
> if I remove that I get something like:
Yeah, I'm taking this patch, but would be better next time to break it
down in two, one doing the reorg, the other doing the actual fix...
Thanks,
- Arnaldo
> > #elif defined(__i386__)
> > #define GEN_ELF_ARCH EM_386
> > #define GEN_ELF_CLASS ELFCLASS32
> > -#elif defined(__ppcle__)
> > -#define GEN_ELF_ARCH EM_PPC
> > -#define GEN_ELF_CLASS ELFCLASS64
> > -#elif defined(__powerpc__)
> > -#define GEN_ELF_ARCH EM_PPC64
> > -#define GEN_ELF_CLASS ELFCLASS64
> > -#elif defined(__powerpcle__)
> > +#elif defined(__powerpc64__)
> > #define GEN_ELF_ARCH EM_PPC64
> > #define GEN_ELF_CLASS ELFCLASS64
> > +#elif defined(__powerpc__)
> > +#define GEN_ELF_ARCH EM_PPC
> > +#define GEN_ELF_CLASS ELFCLASS32
> > #else
> > #error "unsupported architecture"
> > #endif
>
> Which looks correct to me.
>
> And the consolidation of the endian logic is "obviously correct", so:
>
> Acked-by: Michael Ellerman <mpe@ellerman.id.au>
>
> cheers
^ permalink raw reply [flat|nested] 4+ messages in thread
* [tip:perf/urgent] perf jit: genelf makes assumptions about endian
2016-03-29 6:59 [PATCH] perf jit: genelf makes assumptions about endian Anton Blanchard
2016-03-30 2:38 ` Michael Ellerman
@ 2016-03-31 6:34 ` tip-bot for Anton Blanchard
1 sibling, 0 replies; 4+ messages in thread
From: tip-bot for Anton Blanchard @ 2016-03-31 6:34 UTC (permalink / raw)
To: linux-tip-commits
Cc: anton, eranian, cel, sukadev, mingo, hpa, tglx, mpe, acme, linux-kernel
Commit-ID: 9f56c092b99b40ce3cf4c6d0134ff7e513c9f1a6
Gitweb: http://git.kernel.org/tip/9f56c092b99b40ce3cf4c6d0134ff7e513c9f1a6
Author: Anton Blanchard <anton@samba.org>
AuthorDate: Tue, 29 Mar 2016 17:59:44 +1100
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 30 Mar 2016 18:12:06 -0300
perf jit: genelf makes assumptions about endian
Commit 9b07e27f88b9 ("perf inject: Add jitdump mmap injection support")
incorrectly assumed that PowerPC is big endian only.
Simplify things by consolidating the define of GEN_ELF_ENDIAN and checking
for __BYTE_ORDER == __BIG_ENDIAN.
The PowerPC checks were also incorrect, they do not match what gcc
emits. We should first look for __powerpc64__, then __powerpc__.
Signed-off-by: Anton Blanchard <anton@samba.org>
Acked-by: Michael Ellerman <mpe@ellerman.id.au>
Cc: Carl Love <cel@us.ibm.com>
Cc: Stephane Eranian <eranian@google.com>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: linuxppc-dev@lists.ozlabs.org
Fixes: 9b07e27f88b9 ("perf inject: Add jitdump mmap injection support")
Link: http://lkml.kernel.org/r/20160329175944.33a211cc@kryten
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/genelf.h | 24 ++++++++++--------------
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/tools/perf/util/genelf.h b/tools/perf/util/genelf.h
index cd67e64..2fbeb59 100644
--- a/tools/perf/util/genelf.h
+++ b/tools/perf/util/genelf.h
@@ -9,36 +9,32 @@ int jit_add_debug_info(Elf *e, uint64_t code_addr, void *debug, int nr_debug_ent
#if defined(__arm__)
#define GEN_ELF_ARCH EM_ARM
-#define GEN_ELF_ENDIAN ELFDATA2LSB
#define GEN_ELF_CLASS ELFCLASS32
#elif defined(__aarch64__)
#define GEN_ELF_ARCH EM_AARCH64
-#define GEN_ELF_ENDIAN ELFDATA2LSB
#define GEN_ELF_CLASS ELFCLASS64
#elif defined(__x86_64__)
#define GEN_ELF_ARCH EM_X86_64
-#define GEN_ELF_ENDIAN ELFDATA2LSB
#define GEN_ELF_CLASS ELFCLASS64
#elif defined(__i386__)
#define GEN_ELF_ARCH EM_386
-#define GEN_ELF_ENDIAN ELFDATA2LSB
#define GEN_ELF_CLASS ELFCLASS32
-#elif defined(__ppcle__)
-#define GEN_ELF_ARCH EM_PPC
-#define GEN_ELF_ENDIAN ELFDATA2LSB
-#define GEN_ELF_CLASS ELFCLASS64
-#elif defined(__powerpc__)
-#define GEN_ELF_ARCH EM_PPC64
-#define GEN_ELF_ENDIAN ELFDATA2MSB
-#define GEN_ELF_CLASS ELFCLASS64
-#elif defined(__powerpcle__)
+#elif defined(__powerpc64__)
#define GEN_ELF_ARCH EM_PPC64
-#define GEN_ELF_ENDIAN ELFDATA2LSB
#define GEN_ELF_CLASS ELFCLASS64
+#elif defined(__powerpc__)
+#define GEN_ELF_ARCH EM_PPC
+#define GEN_ELF_CLASS ELFCLASS32
#else
#error "unsupported architecture"
#endif
+#if __BYTE_ORDER == __BIG_ENDIAN
+#define GEN_ELF_ENDIAN ELFDATA2MSB
+#else
+#define GEN_ELF_ENDIAN ELFDATA2LSB
+#endif
+
#if GEN_ELF_CLASS == ELFCLASS64
#define elf_newehdr elf64_newehdr
#define elf_getshdr elf64_getshdr
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-03-31 6:35 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-29 6:59 [PATCH] perf jit: genelf makes assumptions about endian Anton Blanchard
2016-03-30 2:38 ` Michael Ellerman
2016-03-30 21:15 ` Arnaldo Carvalho de Melo
2016-03-31 6:34 ` [tip:perf/urgent] " tip-bot for Anton Blanchard
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).