Commit-ID:  0d0970eef3b03ef08b19da5bc3044410731cf38f
Gitweb:     http://git.kernel.org/tip/0d0970eef3b03ef08b19da5bc3044410731cf38f
Author:     Josh Poimboeuf <jpoim...@redhat.com>
AuthorDate: Wed, 20 Sep 2017 16:24:32 -0500
Committer:  Ingo Molnar <mi...@kernel.org>
CommitDate: Sat, 23 Sep 2017 15:06:19 +0200

objtool: Handle another GCC stack pointer adjustment bug

The kbuild bot reported the following warning with GCC 4.4 and a
randconfig:

  net/socket.o: warning: objtool: compat_sock_ioctl()+0x1083: stack state 
mismatch: cfa1=7+160 cfa2=-1+0

This is caused by another GCC non-optimization, where it backs up and
restores the stack pointer for no apparent reason:

    2f91:       48 89 e0                mov    %rsp,%rax
    2f94:       4c 89 e7                mov    %r12,%rdi
    2f97:       4c 89 f6                mov    %r14,%rsi
    2f9a:       ba 20 00 00 00          mov    $0x20,%edx
    2f9f:       48 89 c4                mov    %rax,%rsp

This issue would have been happily ignored before the following commit:

  dd88a0a0c861 ("objtool: Handle GCC stack pointer adjustment bug")

But now that objtool is paying attention to such stack pointer writes
to/from a register, it needs to understand them properly.  In this case
that means recognizing that the "mov %rsp, %rax" instruction is
potentially a backup of the stack pointer.

Reported-by: kbuild test robot <fengguang...@intel.com>
Signed-off-by: Josh Poimboeuf <jpoim...@redhat.com>
Cc: Alexander Potapenko <gli...@google.com>
Cc: Andrey Ryabinin <aryabi...@virtuozzo.com>
Cc: Andy Lutomirski <l...@kernel.org>
Cc: Arnd Bergmann <a...@arndb.de>
Cc: Dmitriy Vyukov <dvyu...@google.com>
Cc: Linus Torvalds <torva...@linux-foundation.org>
Cc: Matthias Kaehlcke <m...@chromium.org>
Cc: Miguel Bernal Marin <miguel.bernal.ma...@linux.intel.com>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Thomas Gleixner <t...@linutronix.de>
Fixes: dd88a0a0c861 ("objtool: Handle GCC stack pointer adjustment bug")
Link: 
http://lkml.kernel.org/r/8c7aa8e9a36fbbb6655d9d8e7cea58958c912da8.1505942196.git.jpoim...@redhat.com
Signed-off-by: Ingo Molnar <mi...@kernel.org>
---
 tools/objtool/arch/x86/decode.c |  6 +++---
 tools/objtool/check.c           | 43 +++++++++++++++++++++++++++--------------
 2 files changed, 32 insertions(+), 17 deletions(-)

diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index 0e8c8ec..0f22768 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -208,14 +208,14 @@ int arch_decode_instruction(struct elf *elf, struct 
section *sec,
                break;
 
        case 0x89:
-               if (rex == 0x48 && modrm == 0xe5) {
+               if (rex_w && !rex_r && modrm_mod == 3 && modrm_reg == 4) {
 
-                       /* mov %rsp, %rbp */
+                       /* mov %rsp, reg */
                        *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;
+                       op->dest.reg = op_to_cfi_reg[modrm_rm][rex_b];
                        break;
                }
 
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index f744617..a0c518e 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1203,24 +1203,39 @@ static int update_insn_state(struct instruction *insn, 
struct insn_state *state)
                switch (op->src.type) {
 
                case OP_SRC_REG:
-                       if (op->src.reg == CFI_SP && op->dest.reg == CFI_BP) {
+                       if (op->src.reg == CFI_SP && op->dest.reg == CFI_BP &&
+                           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;
+                       }
 
-                               if (cfa->base == CFI_SP &&
-                                   regs[CFI_BP].base == CFI_CFA &&
-                                   regs[CFI_BP].offset == -cfa->offset) {
+                       else if (op->src.reg == CFI_SP &&
+                                op->dest.reg == CFI_BP && state->drap) {
 
-                                       /* mov %rsp, %rbp */
-                                       cfa->base = op->dest.reg;
-                                       state->bp_scratch = false;
-                               }
+                               /* drap: mov %rsp, %rbp */
+                               regs[CFI_BP].base = CFI_BP;
+                               regs[CFI_BP].offset = -state->stack_size;
+                               state->bp_scratch = false;
+                       }
 
-                               else if (state->drap) {
+                       else if (op->src.reg == CFI_SP && cfa->base == CFI_SP) {
 
-                                       /* drap: mov %rsp, %rbp */
-                                       regs[CFI_BP].base = CFI_BP;
-                                       regs[CFI_BP].offset = 
-state->stack_size;
-                                       state->bp_scratch = false;
-                               }
+                               /*
+                                * mov %rsp, %reg
+                                *
+                                * This is needed for the rare case where GCC
+                                * does:
+                                *
+                                *   mov    %rsp, %rax
+                                *   ...
+                                *   mov    %rax, %rsp
+                                */
+                               state->vals[op->dest.reg].base = CFI_CFA;
+                               state->vals[op->dest.reg].offset = 
-state->stack_size;
                        }
 
                        else if (op->dest.reg == cfa->base) {

Reply via email to