diff options
author | Josh Poimboeuf <jpoimboe@redhat.com> | 2017-08-29 13:51:03 -0400 |
---|---|---|
committer | Ingo Molnar <mingo@kernel.org> | 2017-08-30 04:48:41 -0400 |
commit | dd88a0a0c8615417fe6b4285769b5b772de87279 (patch) | |
tree | 3ec72a80a155dff0949ced1b01510685450b0496 | |
parent | 499934898fcd15e4337dc858be6c09cd9fd74e85 (diff) |
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>
-rw-r--r-- | tools/objtool/arch/x86/decode.c | 94 | ||||
-rw-r--r-- | tools/objtool/cfi.h | 2 | ||||
-rw-r--r-- | tools/objtool/check.c | 81 | ||||
-rw-r--r-- | 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, | |||
86 | struct insn insn; | 86 | struct insn insn; |
87 | int x86_64, sign; | 87 | int x86_64, sign; |
88 | unsigned char op1, op2, rex = 0, rex_b = 0, rex_r = 0, rex_w = 0, | 88 | unsigned char op1, op2, rex = 0, rex_b = 0, rex_r = 0, rex_w = 0, |
89 | modrm = 0, modrm_mod = 0, modrm_rm = 0, modrm_reg = 0, | 89 | rex_x = 0, modrm = 0, modrm_mod = 0, modrm_rm = 0, |
90 | sib = 0; | 90 | modrm_reg = 0, sib = 0; |
91 | 91 | ||
92 | x86_64 = is_x86_64(elf); | 92 | x86_64 = is_x86_64(elf); |
93 | if (x86_64 == -1) | 93 | if (x86_64 == -1) |
@@ -114,6 +114,7 @@ int arch_decode_instruction(struct elf *elf, struct section *sec, | |||
114 | rex = insn.rex_prefix.bytes[0]; | 114 | rex = insn.rex_prefix.bytes[0]; |
115 | rex_w = X86_REX_W(rex) >> 3; | 115 | rex_w = X86_REX_W(rex) >> 3; |
116 | rex_r = X86_REX_R(rex) >> 2; | 116 | rex_r = X86_REX_R(rex) >> 2; |
117 | rex_x = X86_REX_X(rex) >> 1; | ||
117 | rex_b = X86_REX_B(rex); | 118 | rex_b = X86_REX_B(rex); |
118 | } | 119 | } |
119 | 120 | ||
@@ -217,6 +218,18 @@ int arch_decode_instruction(struct elf *elf, struct section *sec, | |||
217 | op->dest.reg = CFI_BP; | 218 | op->dest.reg = CFI_BP; |
218 | break; | 219 | break; |
219 | } | 220 | } |
221 | |||
222 | if (rex_w && !rex_b && modrm_mod == 3 && modrm_rm == 4) { | ||
223 | |||
224 | /* mov reg, %rsp */ | ||
225 | *type = INSN_STACK; | ||
226 | op->src.type = OP_SRC_REG; | ||
227 | op->src.reg = op_to_cfi_reg[modrm_reg][rex_r]; | ||
228 | op->dest.type = OP_DEST_REG; | ||
229 | op->dest.reg = CFI_SP; | ||
230 | break; | ||
231 | } | ||
232 | |||
220 | /* fallthrough */ | 233 | /* fallthrough */ |
221 | case 0x88: | 234 | case 0x88: |
222 | if (!rex_b && | 235 | if (!rex_b && |
@@ -269,80 +282,28 @@ int arch_decode_instruction(struct elf *elf, struct section *sec, | |||
269 | break; | 282 | break; |
270 | 283 | ||
271 | case 0x8d: | 284 | case 0x8d: |
272 | if (rex == 0x48 && modrm == 0x65) { | 285 | if (sib == 0x24 && rex_w && !rex_b && !rex_x) { |
273 | 286 | ||
274 | /* lea disp(%rbp), %rsp */ | 287 | /* lea disp(%rsp), reg */ |
275 | *type = INSN_STACK; | 288 | *type = INSN_STACK; |
276 | op->src.type = OP_SRC_ADD; | 289 | op->src.type = OP_SRC_ADD; |
277 | op->src.reg = CFI_BP; | 290 | op->src.reg = CFI_SP; |
278 | op->src.offset = insn.displacement.value; | 291 | op->src.offset = insn.displacement.value; |
279 | op->dest.type = OP_DEST_REG; | 292 | op->dest.type = OP_DEST_REG; |
280 | op->dest.reg = CFI_SP; | 293 | op->dest.reg = op_to_cfi_reg[modrm_reg][rex_r]; |
281 | break; | ||
282 | } | ||
283 | 294 | ||
284 | if (rex == 0x48 && (modrm == 0xa4 || modrm == 0x64) && | 295 | } else if (rex == 0x48 && modrm == 0x65) { |
285 | sib == 0x24) { | ||
286 | 296 | ||
287 | /* lea disp(%rsp), %rsp */ | 297 | /* lea disp(%rbp), %rsp */ |
288 | *type = INSN_STACK; | 298 | *type = INSN_STACK; |
289 | op->src.type = OP_SRC_ADD; | 299 | op->src.type = OP_SRC_ADD; |
290 | op->src.reg = CFI_SP; | 300 | op->src.reg = CFI_BP; |
291 | op->src.offset = insn.displacement.value; | 301 | op->src.offset = insn.displacement.value; |
292 | op->dest.type = OP_DEST_REG; | 302 | op->dest.type = OP_DEST_REG; |
293 | op->dest.reg = CFI_SP; | 303 | op->dest.reg = CFI_SP; |
294 | break; | ||
295 | } | ||
296 | 304 | ||
297 | if (rex == 0x48 && modrm == 0x2c && sib == 0x24) { | 305 | } else if (rex == 0x49 && modrm == 0x62 && |
298 | 306 | insn.displacement.value == -8) { | |
299 | /* lea (%rsp), %rbp */ | ||
300 | *type = INSN_STACK; | ||
301 | op->src.type = OP_SRC_REG; | ||
302 | op->src.reg = CFI_SP; | ||
303 | op->dest.type = OP_DEST_REG; | ||
304 | op->dest.reg = CFI_BP; | ||
305 | break; | ||
306 | } | ||
307 | |||
308 | if (rex == 0x4c && modrm == 0x54 && sib == 0x24 && | ||
309 | insn.displacement.value == 8) { | ||
310 | |||
311 | /* | ||
312 | * lea 0x8(%rsp), %r10 | ||
313 | * | ||
314 | * Here r10 is the "drap" pointer, used as a stack | ||
315 | * pointer helper when the stack gets realigned. | ||
316 | */ | ||
317 | *type = INSN_STACK; | ||
318 | op->src.type = OP_SRC_ADD; | ||
319 | op->src.reg = CFI_SP; | ||
320 | op->src.offset = 8; | ||
321 | op->dest.type = OP_DEST_REG; | ||
322 | op->dest.reg = CFI_R10; | ||
323 | break; | ||
324 | } | ||
325 | |||
326 | if (rex == 0x4c && modrm == 0x6c && sib == 0x24 && | ||
327 | insn.displacement.value == 16) { | ||
328 | |||
329 | /* | ||
330 | * lea 0x10(%rsp), %r13 | ||
331 | * | ||
332 | * Here r13 is the "drap" pointer, used as a stack | ||
333 | * pointer helper when the stack gets realigned. | ||
334 | */ | ||
335 | *type = INSN_STACK; | ||
336 | op->src.type = OP_SRC_ADD; | ||
337 | op->src.reg = CFI_SP; | ||
338 | op->src.offset = 16; | ||
339 | op->dest.type = OP_DEST_REG; | ||
340 | op->dest.reg = CFI_R13; | ||
341 | break; | ||
342 | } | ||
343 | |||
344 | if (rex == 0x49 && modrm == 0x62 && | ||
345 | insn.displacement.value == -8) { | ||
346 | 307 | ||
347 | /* | 308 | /* |
348 | * lea -0x8(%r10), %rsp | 309 | * lea -0x8(%r10), %rsp |
@@ -356,11 +317,9 @@ int arch_decode_instruction(struct elf *elf, struct section *sec, | |||
356 | op->src.offset = -8; | 317 | op->src.offset = -8; |
357 | op->dest.type = OP_DEST_REG; | 318 | op->dest.type = OP_DEST_REG; |
358 | op->dest.reg = CFI_SP; | 319 | op->dest.reg = CFI_SP; |
359 | break; | ||
360 | } | ||
361 | 320 | ||
362 | if (rex == 0x49 && modrm == 0x65 && | 321 | } else if (rex == 0x49 && modrm == 0x65 && |
363 | insn.displacement.value == -16) { | 322 | insn.displacement.value == -16) { |
364 | 323 | ||
365 | /* | 324 | /* |
366 | * lea -0x10(%r13), %rsp | 325 | * lea -0x10(%r13), %rsp |
@@ -374,7 +333,6 @@ int arch_decode_instruction(struct elf *elf, struct section *sec, | |||
374 | op->src.offset = -16; | 333 | op->src.offset = -16; |
375 | op->dest.type = OP_DEST_REG; | 334 | op->dest.type = OP_DEST_REG; |
376 | op->dest.reg = CFI_SP; | 335 | op->dest.reg = CFI_SP; |
377 | break; | ||
378 | } | 336 | } |
379 | 337 | ||
380 | break; | 338 | 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 @@ | |||
40 | #define CFI_R14 14 | 40 | #define CFI_R14 14 |
41 | #define CFI_R15 15 | 41 | #define CFI_R15 15 |
42 | #define CFI_RA 16 | 42 | #define CFI_RA 16 |
43 | #define CFI_NUM_REGS 17 | 43 | #define CFI_NUM_REGS 17 |
44 | 44 | ||
45 | struct cfi_reg { | 45 | struct cfi_reg { |
46 | int base; | 46 | 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) | |||
218 | 218 | ||
219 | memset(state, 0, sizeof(*state)); | 219 | memset(state, 0, sizeof(*state)); |
220 | state->cfa.base = CFI_UNDEFINED; | 220 | state->cfa.base = CFI_UNDEFINED; |
221 | for (i = 0; i < CFI_NUM_REGS; i++) | 221 | for (i = 0; i < CFI_NUM_REGS; i++) { |
222 | state->regs[i].base = CFI_UNDEFINED; | 222 | state->regs[i].base = CFI_UNDEFINED; |
223 | state->vals[i].base = CFI_UNDEFINED; | ||
224 | } | ||
223 | state->drap_reg = CFI_UNDEFINED; | 225 | state->drap_reg = CFI_UNDEFINED; |
224 | state->drap_offset = -1; | 226 | state->drap_offset = -1; |
225 | } | 227 | } |
@@ -1201,24 +1203,47 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state) | |||
1201 | switch (op->src.type) { | 1203 | switch (op->src.type) { |
1202 | 1204 | ||
1203 | case OP_SRC_REG: | 1205 | case OP_SRC_REG: |
1204 | if (cfa->base == op->src.reg && cfa->base == CFI_SP && | 1206 | if (op->src.reg == CFI_SP && op->dest.reg == CFI_BP) { |
1205 | op->dest.reg == CFI_BP && regs[CFI_BP].base == CFI_CFA && | 1207 | |
1206 | regs[CFI_BP].offset == -cfa->offset) { | 1208 | if (cfa->base == CFI_SP && |
1207 | 1209 | regs[CFI_BP].base == CFI_CFA && | |
1208 | /* mov %rsp, %rbp */ | 1210 | regs[CFI_BP].offset == -cfa->offset) { |
1209 | cfa->base = op->dest.reg; | 1211 | |
1210 | state->bp_scratch = false; | 1212 | /* mov %rsp, %rbp */ |
1211 | } else if (state->drap) { | 1213 | cfa->base = op->dest.reg; |
1212 | 1214 | state->bp_scratch = false; | |
1213 | /* drap: mov %rsp, %rbp */ | 1215 | } |
1214 | regs[CFI_BP].base = CFI_BP; | 1216 | |
1215 | regs[CFI_BP].offset = -state->stack_size; | 1217 | else if (state->drap) { |
1216 | state->bp_scratch = false; | 1218 | |
1217 | } else if (!no_fp) { | 1219 | /* drap: mov %rsp, %rbp */ |
1218 | 1220 | regs[CFI_BP].base = CFI_BP; | |
1219 | WARN_FUNC("unknown stack-related register move", | 1221 | regs[CFI_BP].offset = -state->stack_size; |
1220 | insn->sec, insn->offset); | 1222 | state->bp_scratch = false; |
1221 | return -1; | 1223 | } |
1224 | } | ||
1225 | |||
1226 | else if (op->dest.reg == cfa->base) { | ||
1227 | |||
1228 | /* mov %reg, %rsp */ | ||
1229 | if (cfa->base == CFI_SP && | ||
1230 | state->vals[op->src.reg].base == CFI_CFA) { | ||
1231 | |||
1232 | /* | ||
1233 | * This is needed for the rare case | ||
1234 | * where GCC does something dumb like: | ||
1235 | * | ||
1236 | * lea 0x8(%rsp), %rcx | ||
1237 | * ... | ||
1238 | * mov %rcx, %rsp | ||
1239 | */ | ||
1240 | cfa->offset = -state->vals[op->src.reg].offset; | ||
1241 | state->stack_size = cfa->offset; | ||
1242 | |||
1243 | } else { | ||
1244 | cfa->base = CFI_UNDEFINED; | ||
1245 | cfa->offset = 0; | ||
1246 | } | ||
1222 | } | 1247 | } |
1223 | 1248 | ||
1224 | break; | 1249 | break; |
@@ -1240,11 +1265,25 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state) | |||
1240 | break; | 1265 | break; |
1241 | } | 1266 | } |
1242 | 1267 | ||
1243 | if (op->dest.reg != CFI_BP && op->src.reg == CFI_SP && | 1268 | if (op->src.reg == CFI_SP && cfa->base == CFI_SP) { |
1244 | cfa->base == CFI_SP) { | ||
1245 | 1269 | ||
1246 | /* drap: lea disp(%rsp), %drap */ | 1270 | /* drap: lea disp(%rsp), %drap */ |
1247 | state->drap_reg = op->dest.reg; | 1271 | state->drap_reg = op->dest.reg; |
1272 | |||
1273 | /* | ||
1274 | * lea disp(%rsp), %reg | ||
1275 | * | ||
1276 | * This is needed for the rare case where GCC | ||
1277 | * does something dumb like: | ||
1278 | * | ||
1279 | * lea 0x8(%rsp), %rcx | ||
1280 | * ... | ||
1281 | * mov %rcx, %rsp | ||
1282 | */ | ||
1283 | state->vals[op->dest.reg].base = CFI_CFA; | ||
1284 | state->vals[op->dest.reg].offset = \ | ||
1285 | -state->stack_size + op->src.offset; | ||
1286 | |||
1248 | break; | 1287 | break; |
1249 | } | 1288 | } |
1250 | 1289 | ||
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 { | |||
33 | bool bp_scratch; | 33 | bool bp_scratch; |
34 | bool drap; | 34 | bool drap; |
35 | int drap_reg, drap_offset; | 35 | int drap_reg, drap_offset; |
36 | struct cfi_reg vals[CFI_NUM_REGS]; | ||
36 | }; | 37 | }; |
37 | 38 | ||
38 | struct instruction { | 39 | struct instruction { |