From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751356AbdH2Rvs (ORCPT ); Tue, 29 Aug 2017 13:51:48 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55186 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751258AbdH2Rvq (ORCPT ); Tue, 29 Aug 2017 13:51:46 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com B7CA685376 Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=jpoimboe@redhat.com From: Josh Poimboeuf To: Ingo Molnar Cc: linux-kernel@vger.kernel.org, Arnd Bergmann , x86@kernel.org Subject: [PATCH] objtool: Handle GCC stack pointer adjustment bug Date: Tue, 29 Aug 2017 12:51:03 -0500 Message-Id: <6a41a96884c725e7f05413bb7df40cfe824b2444.1504028945.git.jpoimboe@redhat.com> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Tue, 29 Aug 2017 17:51:45 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 Reported-by: kbuild test robot Signed-off-by: Josh Poimboeuf --- tools/objtool/arch/x86/decode.c | 94 ++++++++++++----------------------------- tools/objtool/cfi.h | 2 +- tools/objtool/check.c | 81 ++++++++++++++++++++++++++--------- tools/objtool/check.h | 1 + 4 files changed, 88 insertions(+), 90 deletions(-) diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c index 7841e5d31973..0e8c8ec4fd4e 100644 --- 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 443ab2c69992..2fe883c665c7 100644 --- 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 3dffeb944523..f744617c9946 100644 --- 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 9f113016bf8c..47d9ea70a83d 100644 --- 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 { -- 2.13.5