Skip to content

Commit dd88a0a

Browse files
jpoimboeIngo Molnar
jpoimboe
authored and
Ingo Molnar
committed
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 <[email protected]> Reported-by: kbuild test robot <[email protected]> Signed-off-by: Josh Poimboeuf <[email protected]> Cc: Linus Torvalds <[email protected]> Cc: Peter Zijlstra <[email protected]> Cc: Thomas Gleixner <[email protected]> Link: http://lkml.kernel.org/r/6a41a96884c725e7f05413bb7df40cfe824b2444.1504028945.git.jpoimboe@redhat.com Signed-off-by: Ingo Molnar <[email protected]>
1 parent 4999348 commit dd88a0a

File tree

4 files changed

+88
-90
lines changed

4 files changed

+88
-90
lines changed

‎tools/objtool/arch/x86/decode.c‎

Lines changed: 26 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,8 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
8686
struct insn insn;
8787
int x86_64, sign;
8888
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,
90-
sib = 0;
89+
rex_x = 0, modrm = 0, modrm_mod = 0, modrm_rm = 0,
90+
modrm_reg = 0, sib = 0;
9191

9292
x86_64 = is_x86_64(elf);
9393
if (x86_64 == -1)
@@ -114,6 +114,7 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
114114
rex = insn.rex_prefix.bytes[0];
115115
rex_w = X86_REX_W(rex) >> 3;
116116
rex_r = X86_REX_R(rex) >> 2;
117+
rex_x = X86_REX_X(rex) >> 1;
117118
rex_b = X86_REX_B(rex);
118119
}
119120

@@ -217,6 +218,18 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
217218
op->dest.reg = CFI_BP;
218219
break;
219220
}
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+
220233
/* fallthrough */
221234
case 0x88:
222235
if (!rex_b &&
@@ -269,80 +282,28 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
269282
break;
270283

271284
case 0x8d:
272-
if (rex == 0x48 && modrm == 0x65) {
285+
if (sib == 0x24 && rex_w && !rex_b && !rex_x) {
273286

274-
/* lea disp(%rbp), %rsp */
287+
/* lea disp(%rsp), reg */
275288
*type = INSN_STACK;
276289
op->src.type = OP_SRC_ADD;
277-
op->src.reg = CFI_BP;
290+
op->src.reg = CFI_SP;
278291
op->src.offset = insn.displacement.value;
279292
op->dest.type = OP_DEST_REG;
280-
op->dest.reg = CFI_SP;
281-
break;
282-
}
293+
op->dest.reg = op_to_cfi_reg[modrm_reg][rex_r];
283294

284-
if (rex == 0x48 && (modrm == 0xa4 || modrm == 0x64) &&
285-
sib == 0x24) {
295+
} else if (rex == 0x48 && modrm == 0x65) {
286296

287-
/* lea disp(%rsp), %rsp */
297+
/* lea disp(%rbp), %rsp */
288298
*type = INSN_STACK;
289299
op->src.type = OP_SRC_ADD;
290-
op->src.reg = CFI_SP;
300+
op->src.reg = CFI_BP;
291301
op->src.offset = insn.displacement.value;
292302
op->dest.type = OP_DEST_REG;
293303
op->dest.reg = CFI_SP;
294-
break;
295-
}
296304

297-
if (rex == 0x48 && modrm == 0x2c && sib == 0x24) {
298-
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) {
305+
} else if (rex == 0x49 && modrm == 0x62 &&
306+
insn.displacement.value == -8) {
346307

347308
/*
348309
* lea -0x8(%r10), %rsp
@@ -356,11 +317,9 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
356317
op->src.offset = -8;
357318
op->dest.type = OP_DEST_REG;
358319
op->dest.reg = CFI_SP;
359-
break;
360-
}
361320

362-
if (rex == 0x49 && modrm == 0x65 &&
363-
insn.displacement.value == -16) {
321+
} else if (rex == 0x49 && modrm == 0x65 &&
322+
insn.displacement.value == -16) {
364323

365324
/*
366325
* lea -0x10(%r13), %rsp
@@ -374,7 +333,6 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
374333
op->src.offset = -16;
375334
op->dest.type = OP_DEST_REG;
376335
op->dest.reg = CFI_SP;
377-
break;
378336
}
379337

380338
break;

‎tools/objtool/cfi.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
#define CFI_R14 14
4141
#define CFI_R15 15
4242
#define CFI_RA 16
43-
#define CFI_NUM_REGS 17
43+
#define CFI_NUM_REGS 17
4444

4545
struct cfi_reg {
4646
int base;

‎tools/objtool/check.c‎

Lines changed: 60 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -218,8 +218,10 @@ static void clear_insn_state(struct insn_state *state)
218218

219219
memset(state, 0, sizeof(*state));
220220
state->cfa.base = CFI_UNDEFINED;
221-
for (i = 0; i < CFI_NUM_REGS; i++)
221+
for (i = 0; i < CFI_NUM_REGS; i++) {
222222
state->regs[i].base = CFI_UNDEFINED;
223+
state->vals[i].base = CFI_UNDEFINED;
224+
}
223225
state->drap_reg = CFI_UNDEFINED;
224226
state->drap_offset = -1;
225227
}
@@ -1201,24 +1203,47 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state)
12011203
switch (op->src.type) {
12021204

12031205
case OP_SRC_REG:
1204-
if (cfa->base == op->src.reg && cfa->base == CFI_SP &&
1205-
op->dest.reg == CFI_BP && regs[CFI_BP].base == CFI_CFA &&
1206-
regs[CFI_BP].offset == -cfa->offset) {
1207-
1208-
/* mov %rsp, %rbp */
1209-
cfa->base = op->dest.reg;
1210-
state->bp_scratch = false;
1211-
} else if (state->drap) {
1212-
1213-
/* drap: mov %rsp, %rbp */
1214-
regs[CFI_BP].base = CFI_BP;
1215-
regs[CFI_BP].offset = -state->stack_size;
1216-
state->bp_scratch = false;
1217-
} else if (!no_fp) {
1218-
1219-
WARN_FUNC("unknown stack-related register move",
1220-
insn->sec, insn->offset);
1221-
return -1;
1206+
if (op->src.reg == CFI_SP && op->dest.reg == CFI_BP) {
1207+
1208+
if (cfa->base == CFI_SP &&
1209+
regs[CFI_BP].base == CFI_CFA &&
1210+
regs[CFI_BP].offset == -cfa->offset) {
1211+
1212+
/* mov %rsp, %rbp */
1213+
cfa->base = op->dest.reg;
1214+
state->bp_scratch = false;
1215+
}
1216+
1217+
else if (state->drap) {
1218+
1219+
/* drap: mov %rsp, %rbp */
1220+
regs[CFI_BP].base = CFI_BP;
1221+
regs[CFI_BP].offset = -state->stack_size;
1222+
state->bp_scratch = false;
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+
}
12221247
}
12231248

12241249
break;
@@ -1240,11 +1265,25 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state)
12401265
break;
12411266
}
12421267

1243-
if (op->dest.reg != CFI_BP && op->src.reg == CFI_SP &&
1244-
cfa->base == CFI_SP) {
1268+
if (op->src.reg == CFI_SP && cfa->base == CFI_SP) {
12451269

12461270
/* drap: lea disp(%rsp), %drap */
12471271
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+
12481287
break;
12491288
}
12501289

‎tools/objtool/check.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ struct insn_state {
3333
bool bp_scratch;
3434
bool drap;
3535
int drap_reg, drap_offset;
36+
struct cfi_reg vals[CFI_NUM_REGS];
3637
};
3738

3839
struct instruction {

0 commit comments

Comments
 (0)