[tip:x86/urgent] objtool: Handle another GCC stack pointer adjustment bug
From: tip-bot for Josh Poimboeuf
Date: Sat Sep 23 2017 - 09:34:31 EST
Commit-ID: 0d0970eef3b03ef08b19da5bc3044410731cf38f
Gitweb: http://git.kernel.org/tip/0d0970eef3b03ef08b19da5bc3044410731cf38f
Author: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
AuthorDate: Wed, 20 Sep 2017 16:24:32 -0500
Committer: Ingo Molnar <mingo@xxxxxxxxxx>
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.wu@xxxxxxxxx>
Signed-off-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
Cc: Alexander Potapenko <glider@xxxxxxxxxx>
Cc: Andrey Ryabinin <aryabinin@xxxxxxxxxxxxx>
Cc: Andy Lutomirski <luto@xxxxxxxxxx>
Cc: Arnd Bergmann <arnd@xxxxxxxx>
Cc: Dmitriy Vyukov <dvyukov@xxxxxxxxxx>
Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Cc: Matthias Kaehlcke <mka@xxxxxxxxxxxx>
Cc: Miguel Bernal Marin <miguel.bernal.marin@xxxxxxxxxxxxxxx>
Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
Fixes: dd88a0a0c861 ("objtool: Handle GCC stack pointer adjustment bug")
Link: http://lkml.kernel.org/r/8c7aa8e9a36fbbb6655d9d8e7cea58958c912da8.1505942196.git.jpoimboe@xxxxxxxxxx
Signed-off-by: Ingo Molnar <mingo@xxxxxxxxxx>
---
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) {