Messages in this thread Patch in this message | | | Date | Thu, 07 May 2020 18:10:23 +0200 | From | Peter Zijlstra <> | Subject | [RFC][PATCH 3/3] x86/entry, ORC: Teach objtool/unwind_orc about stack irq swizzles |
| |
Thomas would very much like objtool to understand and generate correct ORC unwind information for the minimal stack swizzle sequence:
mov %rsp, (%[ts]) mov %[ts], %rsp ... pop %rsp
This sequence works for the fp and guess unwinders -- all they need is that top-of-stack link set up by the first instruction.
The previous entry_64.S code worked with "UNWIND_HINT_REGS indirect=1" hints to inform the unwinder about the stack-swizzle, but because we've now already entered C, we can no longer point to a REGS. In fact, due to being in C we don't even have a reliable sp_offset to anything.
None of the existing UNWIND_HINT() functionality is quite sufficient to generate the right thing, but SP_INDIRECT is still the closest, so extend it.
When SP_INDIRECT is combined with .end=1 (which is otherwise unused, except for sp_reg == UNDEFINED):
- change it from (sp+sp_offset) to (sp)+sp_offset - have objtool preserve sp_offset from the previous state - change "pop %rsp" handling to restore the CFI state from before the hint.
NOTES:
- We now have an instruction with stackops and a hint; make hint take precedence over stackops.
- Due to the reverse search in "pop %rsp" we must fill_alternative_cfi() before validate_branch().
- This all isn't really pretty, but it works and gets Thomas the code sequence he wants.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> --- arch/x86/include/asm/irq_stack.h | 1 arch/x86/include/asm/unwind_hints.h | 41 +++++++++++++++++++++++++ arch/x86/kernel/unwind_orc.c | 11 +++++- tools/objtool/check.c | 58 +++++++++++++++++++++++++++--------- tools/objtool/orc_dump.c | 22 ++++++++----- 5 files changed, 110 insertions(+), 23 deletions(-)
--- a/arch/x86/include/asm/irq_stack.h +++ b/arch/x86/include/asm/irq_stack.h @@ -23,6 +23,7 @@ do { \ __this_cpu_add(irq_count, 1); \ asm volatile( \ "movq %%rsp, (%[ts]) \n" \ + UNWIND_HINT_STACK_EMPTY \ "movq %[ts], %%rsp \n" \ ASM_INSTR_BEGIN \ _asm " \n" \ --- a/arch/x86/include/asm/unwind_hints.h +++ b/arch/x86/include/asm/unwind_hints.h @@ -95,6 +95,47 @@ UNWIND_HINT type=UNWIND_HINT_TYPE_RET_OFFSET sp_offset=\sp_offset .endm +#else + +#define UNWIND_HINT(sp_reg, sp_offset, type, end) \ + "987: \n\t" \ + ".pushsection .discard.unwind_hints\n\t" \ + /* struct unwind_hint */ \ + ".long 987b - .\n\t" \ + ".short " __stringify(sp_offset) "\n\t" \ + ".byte " __stringify(sp_reg) "\n\t" \ + ".byte " __stringify(type) "\n\t" \ + ".byte " __stringify(end) "\n\t" \ + ".balign 4 \n\t" \ + ".popsection\n\t" + +/* + * Stack swizzling vs objtool/ORC: + * + * The canonical way of swizzling stack is: + * + * 1: mov %%rsp, (%[ts]) + * 2: mov %[ts], %%rsp + * ... + * 3: pop %%rsp + * + * Where: + * + * 1 - places a pointer to the previous stack at the top of the new stack; + * also see the unwinders. + * + * 2 - switches to the new stack, but to avoid hitting the CFA_UNDEFINED case, + * we need to tell objtool the stack pointer can be found at (%%rsp), + * UNWIND_HINT_STACK_EMPTY does so. + * + * 3 - restores the previous stack by popping the value stored by 1 into %%rsp, + * .... + * + * See arch/x86/include/asm/irq_stack.h + */ +#define UNWIND_HINT_STACK_EMPTY \ + UNWIND_HINT(ORC_REG_SP_INDIRECT, 0, ORC_TYPE_CALL, 1) + #endif /* __ASSEMBLY__ */ #endif /* _ASM_X86_UNWIND_HINTS_H */ --- a/arch/x86/kernel/unwind_orc.c +++ b/arch/x86/kernel/unwind_orc.c @@ -435,12 +435,16 @@ bool unwind_next_frame(struct unwind_sta break; case ORC_REG_SP_INDIRECT: - sp = state->sp + orc->sp_offset; + sp = state->sp; + if (!orc->end) + sp += orc->sp_offset; indirect = true; break; case ORC_REG_BP_INDIRECT: - sp = state->bp + orc->sp_offset; + sp = state->bp; + if (!orc->end) + sp += orc->sp_offset; indirect = true; break; @@ -489,6 +493,9 @@ bool unwind_next_frame(struct unwind_sta if (indirect) { if (!deref_stack_reg(state, sp, &sp)) goto err; + + if (orc->end) + sp += orc->sp_offset; } /* Find IP, SP and possibly regs: */ --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1720,8 +1720,8 @@ static void restore_reg(struct cfi_state * 41 5d pop %r13 * c3 retq */ -static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi, - struct stack_op *op) +static int update_cfi_state(struct objtool_file *file, struct instruction *insn, + struct cfi_state *cfi, struct stack_op *op) { struct cfi_reg *cfa = &cfi->cfa; struct cfi_reg *regs = cfi->regs; @@ -1898,6 +1898,29 @@ static int update_cfi_state(struct instr case OP_SRC_POP: case OP_SRC_POPF: + + if (op->dest.reg == CFI_SP && cfa->base == CFI_SP_INDIRECT && cfi->end) { + /* pop %rsp from a stack swizzle */ + + struct instruction *prev; + + for (prev = prev_insn_same_sym(file, insn); + prev; prev = prev_insn_same_sym(file, prev)) { + + if (prev->cfi.cfa.base != CFI_SP_INDIRECT) { + *cfi = prev->cfi; + break; + } + } + + if (!prev) { + WARN_FUNC("failed RESTORE_STACK", insn->sec, insn->offset); + return -1; + } + + break; + } + if (!cfi->drap && op->dest.reg == cfa->base) { /* pop %rbp */ @@ -2073,7 +2096,7 @@ static int update_cfi_state(struct instr return 0; } -static int handle_insn_ops(struct instruction *insn, struct insn_state *state) +static int handle_insn_ops(struct objtool_file *file, struct instruction *insn, struct insn_state *state) { struct stack_op *op; @@ -2085,9 +2108,11 @@ static int handle_insn_ops(struct instru return -1; } - res = update_cfi_state(insn, &state->cfi, op); - if (res) - return res; + if (!insn->hint) { + res = update_cfi_state(file, insn, &state->cfi, op); + if (res) + return res; + } if (op->dest.type == OP_DEST_PUSHF) { if (!state->uaccess_stack) { @@ -2319,16 +2344,26 @@ static int validate_branch(struct objtoo if (state.noinstr) state.instr += insn->instr; - if (insn->hint) - state.cfi = insn->cfi; - else + if (insn->hint) { + if (insn->cfi.cfa.base == CFI_SP_INDIRECT && insn->cfi.end) { + state.cfi.cfa.base = CFI_SP_INDIRECT; + state.cfi.end = true; + insn->cfi = state.cfi; + } else { + state.cfi = insn->cfi; + } + } else { insn->cfi = state.cfi; + } insn->visited |= visited; if (!insn->ignore_alts && !list_empty(&insn->alts)) { bool skip_orig = false; + if (insn->alt_group) + fill_alternative_cfi(file, insn); + list_for_each_entry(alt, &insn->alts, list) { if (alt->skip_orig) skip_orig = true; @@ -2341,14 +2376,11 @@ static int validate_branch(struct objtoo } } - if (insn->alt_group) - fill_alternative_cfi(file, insn); - if (skip_orig) return 0; } - if (handle_insn_ops(insn, &state)) + if (handle_insn_ops(file, insn, &state)) return 1; switch (insn->type) { --- a/tools/objtool/orc_dump.c +++ b/tools/objtool/orc_dump.c @@ -47,13 +47,19 @@ static const char *orc_type_name(unsigne } } -static void print_reg(unsigned int reg, int offset) +static void print_reg(unsigned int reg, int offset, bool end) { - if (reg == ORC_REG_BP_INDIRECT) - printf("(bp%+d)", offset); - else if (reg == ORC_REG_SP_INDIRECT) - printf("(sp%+d)", offset); - else if (reg == ORC_REG_UNDEFINED) + if (reg == ORC_REG_BP_INDIRECT) { + if (end) + printf("(bp)%+d", offset); + else + printf("(bp%+d)", offset); + } else if (reg == ORC_REG_SP_INDIRECT) { + if (end) + printf("(sp)%+d", offset); + else + printf("(sp%+d)", offset); + } else if (reg == ORC_REG_UNDEFINED) printf("(und)"); else printf("%s%+d", reg_name(reg), offset); @@ -195,11 +201,11 @@ int orc_dump(const char *_objname) printf(" sp:"); - print_reg(orc[i].sp_reg, orc[i].sp_offset); + print_reg(orc[i].sp_reg, orc[i].sp_offset, orc[i].end); printf(" bp:"); - print_reg(orc[i].bp_reg, orc[i].bp_offset); + print_reg(orc[i].bp_reg, orc[i].bp_offset, false); printf(" type:%s end:%d\n", orc_type_name(orc[i].type), orc[i].end);
| |