Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit dd88a0a0 authored by Josh Poimboeuf's avatar Josh Poimboeuf Committed by Ingo Molnar
Browse files

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: default avatarArnd Bergmann <arnd@arndb.de>
Reported-by: default avatarkbuild test robot <fengguang.wu@intel.com>
Signed-off-by: default avatarJosh 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: default avatarIngo Molnar <mingo@kernel.org>
parent 49993489

tools/objtool/arch/x86/decode.c

+26 −68
Original line number Diff line number Diff line
@@ -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,79 +282,27 @@ 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 &&

		} else if (rex == 0x49 && modrm == 0x62 &&

			   insn.displacement.value == -8) {



			/*
@@ -356,10 +317,8 @@ 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 &&

		} else if (rex == 0x49 && modrm == 0x65 &&

			   insn.displacement.value == -16) {



			/*
@@ -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;

tools/objtool/check.c

+60 −21
Original line number Diff line number Diff line
@@ -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 &&

			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) {

				}



				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;

			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;

			}

tools/objtool/check.h

+1 −0
Original line number Diff line number Diff line
@@ -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 {

tools/objtool/cfi.h

+1 −1

File changed.

Contains only whitespace changes.