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) <pet...@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);
