Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1365924 > unrolled thread
| Started by | Anton Blanchard <anton@samba.org> |
|---|---|
| First post | 2016-03-29 09:10 +0200 |
| Last post | 2016-03-31 08:40 +0200 |
| Articles | 4 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH] perf jit: genelf makes assumptions about endian Anton Blanchard <anton@samba.org> - 2016-03-29 09:10 +0200
Re: [PATCH] perf jit: genelf makes assumptions about endian Michael Ellerman <mpe@ellerman.id.au> - 2016-03-30 04:40 +0200
Re: [PATCH] perf jit: genelf makes assumptions about endian Arnaldo Carvalho de Melo <acme@redhat.com> - 2016-03-30 23:20 +0200
[tip:perf/urgent] perf jit: genelf makes assumptions about endian tip-bot for Anton Blanchard <tipbot@zytor.com> - 2016-03-31 08:40 +0200
| From | Anton Blanchard <anton@samba.org> |
|---|---|
| Date | 2016-03-29 09:10 +0200 |
| Subject | [PATCH] perf jit: genelf makes assumptions about endian |
| Message-ID | <rhVWO-2De-21@gated-at.bofh.it> |
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
[toc] | [next] | [standalone]
| From | Michael Ellerman <mpe@ellerman.id.au> |
|---|---|
| Date | 2016-03-30 04:40 +0200 |
| Message-ID | <ried4-7fJ-9@gated-at.bofh.it> |
| In reply to | #1365924 |
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
[toc] | [prev] | [next] | [standalone]
| From | Arnaldo Carvalho de Melo <acme@redhat.com> |
|---|---|
| Date | 2016-03-30 23:20 +0200 |
| Message-ID | <rivGV-2PT-5@gated-at.bofh.it> |
| In reply to | #1366873 |
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
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for Anton Blanchard <tipbot@zytor.com> |
|---|---|
| Date | 2016-03-31 08:40 +0200 |
| Subject | [tip:perf/urgent] perf jit: genelf makes assumptions about endian |
| Message-ID | <riEqT-P8-43@gated-at.bofh.it> |
| In reply to | #1365924 |
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
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web