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 <jpoimboe@redhat.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/9dd4161719b20e6def9564646d68bfbe498c549f.1456962210.git.jpoimboe@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>

diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c
index 8f72b334aea03387ab7084d9998f497cf7287da2..1a416935bac98ffec19256448673335feb84bb74 100644 (file)
--- 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 47314a64399c8fc6246839cff82d7d75b0c92647..9f26eae6c9f067e8bdb4cfe17e7c16ec7180988b 100644 (file)
--- 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 47314a64399c8fc6246839cff82d7d75b0c92647..9f26eae6c9f067e8bdb4cfe17e7c16ec7180988b 100644 (file)
--- 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: