objtool: Handle GCC stack pointer adjustment bug

Arnd Bergmann reported the following warning with GCC 7.1.1:

  fs/fs_pin.o: warning: objtool: pin_kill()+0x139: stack state mismatch: cfa1=7+88 cfa2=7+96

And the kbuild robot reported the following warnings with GCC 5.4.1:

  fs/fs_pin.o: warning: objtool: pin_kill()+0x182: return with modified stack frame
  fs/quota/dquot.o: warning: objtool: dquot_alloc_inode()+0x140: stack state mismatch: cfa1=7+120 cfa2=7+128
  fs/quota/dquot.o: warning: objtool: dquot_free_inode()+0x11a: stack state mismatch: cfa1=7+112 cfa2=7+120

Those warnings are caused by an unusual GCC non-optimization where it
uses an intermediate register to adjust the stack pointer.  It does:

  lea    0x8(%rsp), %rcx
  ...
  mov    %rcx, %rsp

Instead of the obvious:

  add    $0x8, %rsp

It makes no sense to use an intermediate register, so I opened a GCC bug
to track it:

  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81813

But it's not exactly a high-priority bug and it looks like we'll be
stuck with this issue for a while.  So for now we have to track register
values when they're loaded with stack pointer offsets.

This is kind of a big workaround for a tiny problem, but c'est la vie.
I hope to eventually create a GCC plugin to implement a big chunk of
objtool's functionality.  Hopefully at that point we'll be able to
remove of a lot of these GCC-isms from the objtool code.

Reported-by: Arnd Bergmann <arnd@arndb.de>
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/6a41a96884c725e7f05413bb7df40cfe824b2444.1504028945.git.jpoimboe@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>

diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index 7841e5d3197339171250d669a6dd05d0b75782b5..0e8c8ec4fd4e6ac1d101b2514d43d870762c293b 100644 (file)
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -86,8 +86,8 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
        struct insn insn;
        int x86_64, sign;
        unsigned char op1, op2, rex = 0, rex_b = 0, rex_r = 0, rex_w = 0,
-                     modrm = 0, modrm_mod = 0, modrm_rm = 0, modrm_reg = 0,
-                     sib = 0;
+                     rex_x = 0, modrm = 0, modrm_mod = 0, modrm_rm = 0,
+                     modrm_reg = 0, sib = 0;
 
        x86_64 = is_x86_64(elf);
        if (x86_64 == -1)
@@ -114,6 +114,7 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
                rex = insn.rex_prefix.bytes[0];
                rex_w = X86_REX_W(rex) >> 3;
                rex_r = X86_REX_R(rex) >> 2;
+               rex_x = X86_REX_X(rex) >> 1;
                rex_b = X86_REX_B(rex);
        }
 
@@ -217,6 +218,18 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
                        op->dest.reg = CFI_BP;
                        break;
                }
+
+               if (rex_w && !rex_b && modrm_mod == 3 && modrm_rm == 4) {
+
+                       /* mov reg, %rsp */
+                       *type = INSN_STACK;
+                       op->src.type = OP_SRC_REG;
+                       op->src.reg = op_to_cfi_reg[modrm_reg][rex_r];
+                       op->dest.type = OP_DEST_REG;
+                       op->dest.reg = CFI_SP;
+                       break;
+               }
+
                /* fallthrough */
        case 0x88:
                if (!rex_b &&
@@ -269,80 +282,28 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
                break;
 
        case 0x8d:
-               if (rex == 0x48 && modrm == 0x65) {
+               if (sib == 0x24 && rex_w && !rex_b && !rex_x) {
 
-                       /* lea disp(%rbp), %rsp */
+                       /* lea disp(%rsp), reg */
                        *type = INSN_STACK;
                        op->src.type = OP_SRC_ADD;
-                       op->src.reg = CFI_BP;
+                       op->src.reg = CFI_SP;
                        op->src.offset = insn.displacement.value;
                        op->dest.type = OP_DEST_REG;
-                       op->dest.reg = CFI_SP;
-                       break;
-               }
+                       op->dest.reg = op_to_cfi_reg[modrm_reg][rex_r];
 
-               if (rex == 0x48 && (modrm == 0xa4 || modrm == 0x64) &&
-                   sib == 0x24) {
+               } else if (rex == 0x48 && modrm == 0x65) {
 
-                       /* lea disp(%rsp), %rsp */
+                       /* lea disp(%rbp), %rsp */
                        *type = INSN_STACK;
                        op->src.type = OP_SRC_ADD;
-                       op->src.reg = CFI_SP;
+                       op->src.reg = CFI_BP;
                        op->src.offset = insn.displacement.value;
                        op->dest.type = OP_DEST_REG;
                        op->dest.reg = CFI_SP;
-                       break;
-               }
 
-               if (rex == 0x48 && modrm == 0x2c && sib == 0x24) {
-
-                       /* lea (%rsp), %rbp */
-                       *type = INSN_STACK;
-                       op->src.type = OP_SRC_REG;
-                       op->src.reg = CFI_SP;
-                       op->dest.type = OP_DEST_REG;
-                       op->dest.reg = CFI_BP;
-                       break;
-               }
-
-               if (rex == 0x4c && modrm == 0x54 && sib == 0x24 &&
-                   insn.displacement.value == 8) {
-
-                       /*
-                        * lea 0x8(%rsp), %r10
-                        *
-                        * Here r10 is the "drap" pointer, used as a stack
-                        * pointer helper when the stack gets realigned.
-                        */
-                       *type = INSN_STACK;
-                       op->src.type = OP_SRC_ADD;
-                       op->src.reg = CFI_SP;
-                       op->src.offset = 8;
-                       op->dest.type = OP_DEST_REG;
-                       op->dest.reg = CFI_R10;
-                       break;
-               }
-
-               if (rex == 0x4c && modrm == 0x6c && sib == 0x24 &&
-                   insn.displacement.value == 16) {
-
-                       /*
-                        * lea 0x10(%rsp), %r13
-                        *
-                        * Here r13 is the "drap" pointer, used as a stack
-                        * pointer helper when the stack gets realigned.
-                        */
-                       *type = INSN_STACK;
-                       op->src.type = OP_SRC_ADD;
-                       op->src.reg = CFI_SP;
-                       op->src.offset = 16;
-                       op->dest.type = OP_DEST_REG;
-                       op->dest.reg = CFI_R13;
-                       break;
-               }
-
-               if (rex == 0x49 && modrm == 0x62 &&
-                   insn.displacement.value == -8) {
+               } else if (rex == 0x49 && modrm == 0x62 &&
+                          insn.displacement.value == -8) {
 
                        /*
                         * lea -0x8(%r10), %rsp
@@ -356,11 +317,9 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
                        op->src.offset = -8;
                        op->dest.type = OP_DEST_REG;
                        op->dest.reg = CFI_SP;
-                       break;
-               }
 
-               if (rex == 0x49 && modrm == 0x65 &&
-                   insn.displacement.value == -16) {
+               } else if (rex == 0x49 && modrm == 0x65 &&
+                          insn.displacement.value == -16) {
 
                        /*
                         * lea -0x10(%r13), %rsp
@@ -374,7 +333,6 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
                        op->src.offset = -16;
                        op->dest.type = OP_DEST_REG;
                        op->dest.reg = CFI_SP;
-                       break;
                }
 
                break;

diff --git a/tools/objtool/cfi.h b/tools/objtool/cfi.h
index 443ab2c69992d15a9058f0d2a7f6b304e2b0b7d7..2fe883c665c7b5ed66d3d1fc6cd6b51d1a618c93 100644 (file)
--- a/tools/objtool/cfi.h
+++ b/tools/objtool/cfi.h
@@ -40,7 +40,7 @@
 #define CFI_R14                        14
 #define CFI_R15                        15
 #define CFI_RA                 16
-#define CFI_NUM_REGS   17
+#define CFI_NUM_REGS           17
 
 struct cfi_reg {
        int base;

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 3dffeb9445237429a004bb9657eb1a2ade2d3659..f744617c9946d7eaaa293b3aba5b63dddf19278b 100644 (file)
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -218,8 +218,10 @@ static void clear_insn_state(struct insn_state *state)
 
        memset(state, 0, sizeof(*state));
        state->cfa.base = CFI_UNDEFINED;
-       for (i = 0; i < CFI_NUM_REGS; i++)
+       for (i = 0; i < CFI_NUM_REGS; i++) {
                state->regs[i].base = CFI_UNDEFINED;
+               state->vals[i].base = CFI_UNDEFINED;
+       }
        state->drap_reg = CFI_UNDEFINED;
        state->drap_offset = -1;
 }
@@ -1201,24 +1203,47 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state)
                switch (op->src.type) {
 
                case OP_SRC_REG:
-                       if (cfa->base == op->src.reg && cfa->base == CFI_SP &&
-                           op->dest.reg == CFI_BP && regs[CFI_BP].base == CFI_CFA &&
-                           regs[CFI_BP].offset == -cfa->offset) {
-
-                               /* mov %rsp, %rbp */
-                               cfa->base = op->dest.reg;
-                               state->bp_scratch = false;
-                       } else if (state->drap) {
-
-                               /* drap: mov %rsp, %rbp */
-                               regs[CFI_BP].base = CFI_BP;
-                               regs[CFI_BP].offset = -state->stack_size;
-                               state->bp_scratch = false;
-                       } else if (!no_fp) {
-
-                               WARN_FUNC("unknown stack-related register move",
-                                         insn->sec, insn->offset);
-                               return -1;
+                       if (op->src.reg == CFI_SP && op->dest.reg == CFI_BP) {
+
+                               if (cfa->base == CFI_SP &&
+                                   regs[CFI_BP].base == CFI_CFA &&
+                                   regs[CFI_BP].offset == -cfa->offset) {
+
+                                       /* mov %rsp, %rbp */
+                                       cfa->base = op->dest.reg;
+                                       state->bp_scratch = false;
+                               }
+
+                               else if (state->drap) {
+
+                                       /* drap: mov %rsp, %rbp */
+                                       regs[CFI_BP].base = CFI_BP;
+                                       regs[CFI_BP].offset = -state->stack_size;
+                                       state->bp_scratch = false;
+                               }
+                       }
+
+                       else if (op->dest.reg == cfa->base) {
+
+                               /* mov %reg, %rsp */
+                               if (cfa->base == CFI_SP &&
+                                   state->vals[op->src.reg].base == CFI_CFA) {
+
+                                       /*
+                                        * This is needed for the rare case
+                                        * where GCC does something dumb like:
+                                        *
+                                        *   lea    0x8(%rsp), %rcx
+                                        *   ...
+                                        *   mov    %rcx, %rsp
+                                        */
+                                       cfa->offset = -state->vals[op->src.reg].offset;
+                                       state->stack_size = cfa->offset;
+
+                               } else {
+                                       cfa->base = CFI_UNDEFINED;
+                                       cfa->offset = 0;
+                               }
                        }
 
                        break;
@@ -1240,11 +1265,25 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state)
                                break;
                        }
 
-                       if (op->dest.reg != CFI_BP && op->src.reg == CFI_SP &&
-                           cfa->base == CFI_SP) {
+                       if (op->src.reg == CFI_SP && cfa->base == CFI_SP) {
 
                                /* drap: lea disp(%rsp), %drap */
                                state->drap_reg = op->dest.reg;
+
+                               /*
+                                * lea disp(%rsp), %reg
+                                *
+                                * This is needed for the rare case where GCC
+                                * does something dumb like:
+                                *
+                                *   lea    0x8(%rsp), %rcx
+                                *   ...
+                                *   mov    %rcx, %rsp
+                                */
+                               state->vals[op->dest.reg].base = CFI_CFA;
+                               state->vals[op->dest.reg].offset = \
+                                       -state->stack_size + op->src.offset;
+
                                break;
                        }
 

diff --git a/tools/objtool/check.h b/tools/objtool/check.h
index 9f113016bf8c8b5074cbeb5b759eaa8d10702183..47d9ea70a83d9f9326fb7bd68431c88337b5ce31 100644 (file)
--- a/tools/objtool/check.h
+++ b/tools/objtool/check.h
@@ -33,6 +33,7 @@ struct insn_state {
        bool bp_scratch;
        bool drap;
        int drap_reg, drap_offset;
+       struct cfi_reg vals[CFI_NUM_REGS];
 };
 
 struct instruction {