On March 3, 2016 8:51:39 AM PST, tip-bot for Josh Poimboeuf <tip...@zytor.com> wrote: >Commit-ID: 19072f23d1d785c093b7f81cb1fb161e7a13ecc0 >Gitweb: >http://git.kernel.org/tip/19072f23d1d785c093b7f81cb1fb161e7a13ecc0 >Author: Josh Poimboeuf <jpoim...@redhat.com> >AuthorDate: Wed, 2 Mar 2016 18:39:36 -0600 >Committer: Ingo Molnar <mi...@kernel.org> >CommitDate: Thu, 3 Mar 2016 16:13:00 +0100 > >x86/asm/decoder: Use explicitly signed chars > >When running objtool on a ppc64le host to analyze x86 binaries, it >reports a lot of false warnings like: > >ipc/compat_mq.o: warning: objtool: compat_SyS_mq_open()+0x91: can't >find jump dest instruction at .text+0x3a5 > >The warnings are caused by the x86 instruction decoder setting the >wrong >value for the jump instruction's immediate field because it assumes >that >"char == signed char", which isn't true for all architectures. When >converting char to int, gcc sign-extends on x86 but doesn't sign-extend >on ppc64le. > >According to the gcc man page, that's a feature, not a bug: > > > Each kind of machine has a default for what "char" should be. It is > > either like "unsigned char" by default or like "signed char" by > > default. > > > > Ideally, a portable program should always use "signed char" or > > "unsigned char" when it depends on the signedness of an object. > >Conform to the "standards" by changing the "char" casts to "signed >char". This results in no actual changes to the object code on x86. > >Note: the x86 decoder now lives in three different locations in the >kernel tree, which are all kept in sync via makefile checks and >warnings: in-kernel, perf, and objtool. This fixes all three >locations. >Eventually we should probably try to at least converge the two separate >"tools" locations into a single shared location. > >Signed-off-by: Josh Poimboeuf <jpoim...@redhat.com> >Cc: Adrian Hunter <adrian.hun...@intel.com> >Cc: Linus Torvalds <torva...@linux-foundation.org> >Cc: Masami Hiramatsu <masami.hiramatsu...@hitachi.com> >Cc: Michael Ellerman <m...@ellerman.id.au> >Cc: Peter Zijlstra <pet...@infradead.org> >Cc: Stephen Rothwell <s...@canb.auug.org.au> >Cc: Thomas Gleixner <t...@linutronix.de> >Link: >http://lkml.kernel.org/r/9dd4161719b20e6def9564646d68bfbe498c549f.1456962210.git.jpoim...@redhat.com >Signed-off-by: Ingo Molnar <mi...@kernel.org> >--- > arch/x86/lib/insn.c | 6 +++--- > tools/objtool/arch/x86/insn/insn.c | 6 +++--- > tools/perf/util/intel-pt-decoder/insn.c | 6 +++--- > 3 files changed, 9 insertions(+), 9 deletions(-) > >diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c >index 8f72b33..1a41693 100644 >--- a/arch/x86/lib/insn.c >+++ b/arch/x86/lib/insn.c >@@ -374,7 +374,7 @@ void insn_get_displacement(struct insn *insn) > if (mod == 3) > goto out; > if (mod == 1) { >- insn->displacement.value = get_next(char, insn); >+ insn->displacement.value = get_next(signed char, insn); > insn->displacement.nbytes = 1; > } else if (insn->addr_bytes == 2) { > if ((mod == 0 && rm == 6) || mod == 2) { >@@ -532,7 +532,7 @@ void insn_get_immediate(struct insn *insn) > > switch (inat_immediate_size(insn->attr)) { > case INAT_IMM_BYTE: >- insn->immediate.value = get_next(char, insn); >+ insn->immediate.value = get_next(signed char, insn); > insn->immediate.nbytes = 1; > break; > case INAT_IMM_WORD: >@@ -566,7 +566,7 @@ void insn_get_immediate(struct insn *insn) > goto err_out; > } > if (inat_has_second_immediate(insn->attr)) { >- insn->immediate2.value = get_next(char, insn); >+ insn->immediate2.value = get_next(signed char, insn); > insn->immediate2.nbytes = 1; > } > done: >diff --git a/tools/objtool/arch/x86/insn/insn.c >b/tools/objtool/arch/x86/insn/insn.c >index 47314a6..9f26eae 100644 >--- a/tools/objtool/arch/x86/insn/insn.c >+++ b/tools/objtool/arch/x86/insn/insn.c >@@ -374,7 +374,7 @@ void insn_get_displacement(struct insn *insn) > if (mod == 3) > goto out; > if (mod == 1) { >- insn->displacement.value = get_next(char, insn); >+ insn->displacement.value = get_next(signed char, insn); > insn->displacement.nbytes = 1; > } else if (insn->addr_bytes == 2) { > if ((mod == 0 && rm == 6) || mod == 2) { >@@ -532,7 +532,7 @@ void insn_get_immediate(struct insn *insn) > > switch (inat_immediate_size(insn->attr)) { > case INAT_IMM_BYTE: >- insn->immediate.value = get_next(char, insn); >+ insn->immediate.value = get_next(signed char, insn); > insn->immediate.nbytes = 1; > break; > case INAT_IMM_WORD: >@@ -566,7 +566,7 @@ void insn_get_immediate(struct insn *insn) > goto err_out; > } > if (inat_has_second_immediate(insn->attr)) { >- insn->immediate2.value = get_next(char, insn); >+ insn->immediate2.value = get_next(signed char, insn); > insn->immediate2.nbytes = 1; > } > done: >diff --git a/tools/perf/util/intel-pt-decoder/insn.c >b/tools/perf/util/intel-pt-decoder/insn.c >index 47314a6..9f26eae 100644 >--- a/tools/perf/util/intel-pt-decoder/insn.c >+++ b/tools/perf/util/intel-pt-decoder/insn.c >@@ -374,7 +374,7 @@ void insn_get_displacement(struct insn *insn) > if (mod == 3) > goto out; > if (mod == 1) { >- insn->displacement.value = get_next(char, insn); >+ insn->displacement.value = get_next(signed char, insn); > insn->displacement.nbytes = 1; > } else if (insn->addr_bytes == 2) { > if ((mod == 0 && rm == 6) || mod == 2) { >@@ -532,7 +532,7 @@ void insn_get_immediate(struct insn *insn) > > switch (inat_immediate_size(insn->attr)) { > case INAT_IMM_BYTE: >- insn->immediate.value = get_next(char, insn); >+ insn->immediate.value = get_next(signed char, insn); > insn->immediate.nbytes = 1; > break; > case INAT_IMM_WORD: >@@ -566,7 +566,7 @@ void insn_get_immediate(struct insn *insn) > goto err_out; > } > if (inat_has_second_immediate(insn->attr)) { >- insn->immediate2.value = get_next(char, insn); >+ insn->immediate2.value = get_next(signed char, insn); > insn->immediate2.nbytes = 1; > } > done:
It ought to be made specific as __s8 (or int8_t) really... -- Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.
