Messages in this thread Patch in this message | | | Date | Tue, 28 Apr 2020 16:16:14 +0200 | From | Peter Zijlstra <> | Subject | Re: x86 entry perf unwinding failure (missing IRET_REGS annotation on stack switch?) |
| |
On Tue, Apr 28, 2020 at 02:46:27PM +0200, Peter Zijlstra wrote: > On Tue, Apr 28, 2020 at 02:04:50AM -0500, Josh Poimboeuf wrote:
> > I'm thinking something like this should fix it. Peter, does this look > > ok? > > Unfortunate. But also, I fear, insufficient. Specifically consider > things like: > > ALTERNATIVE "jmp 1f", > "alt... > "..." > "...insn", X86_FEAT_foo > 1: > > This results in something like: > > > .text .altinstr_replacement > e8 xx ... > 90 > 90 > ... > 90 > > Where all our normal single byte nops (0x90) are unreachable with > undefined CFI, but the alternative might have CFI, which is never > propagated. > > We ran into this with the validate_alternative stuff from Alexandre.
> So rather than hacking around this issue, should we not make > create_orc() smarter? > > I'm trying to come up with something, but so far I'm just making a mess.
Like this, it's horrid, but it seems to work.
What do you think of the approach? I'll work on cleaning it up if you don't hate it too much ;-)
---
diff --git a/tools/objtool/orc_gen.c b/tools/objtool/orc_gen.c index 9d2bf2daaaa6..2a853ae994ea 100644 --- a/tools/objtool/orc_gen.c +++ b/tools/objtool/orc_gen.c @@ -10,17 +10,129 @@ #include "check.h" #include "warn.h" -int create_orc(struct objtool_file *file) +static bool same_cfi(struct cfi_state *a, struct cfi_state *b) { + return memcmp(a, b, sizeof(*a)); +} + +static struct instruction *next_insn_same_sec(struct objtool_file *file, + struct instruction *insn) +{ + struct instruction *next = list_next_entry(insn, list); + + if (!next || &next->list == &file->insn_list || next->sec != insn->sec) + return NULL; + + return next; +} + +struct alternative { + struct list_head list; struct instruction *insn; + bool skip_orig; +}; + +static int alt_cfi(struct objtool_file *file, + struct instruction *prev_insn, + struct instruction *orig_insn, + struct instruction *alt_insn) +{ + unsigned long orig_offset = orig_insn->offset; + unsigned long alt_offset = alt_insn->offset; + struct instruction *next_insn; + bool orig_orc, alt_orc; + + orig_orc = !prev_insn || same_cfi(&orig_insn->cfi, &prev_insn->cfi); + alt_orc = !prev_insn || same_cfi(&alt_insn->cfi, &prev_insn->cfi); + +again: + if ((orig_orc || alt_orc)) { + if (orig_insn->offset - orig_offset != alt_insn->offset - alt_offset) { + WARN_FUNC("alternative has unaligned ORC", orig_insn->sec, orig_insn->offset); + return -1; + } + + if (orig_insn->visited) { + if (same_cfi(&orig_insn->cfi, &alt_insn->cfi)) { + WARN_FUNC("alternative violates ORC invariance", orig_insn->sec, orig_insn->offset); + return -1; + } + } else { + /* + * We're in unreachable NOPs, allow the alternative to + * override the CFI/ORC data. + */ + orig_insn->cfi = alt_insn->cfi; + } + } + + next_insn = next_insn_same_sec(file, alt_insn); + if (!next_insn) + return 0; + + if (next_insn->offset == -1 /*FAKE_JUMP_OFFSET*/) + return 0; + + alt_orc = same_cfi(&alt_insn->cfi, &next_insn->cfi); + alt_insn = next_insn; + + do { + next_insn = next_insn_same_sec(file, orig_insn); + if (!next_insn) + return 0; + if (!next_insn->alt_group) + return 0; + + orig_orc = next_insn->visited && same_cfi(&orig_insn->cfi, &next_insn->cfi); + orig_insn = next_insn; + } while (orig_insn->offset - orig_offset < alt_insn->offset - alt_offset); + + goto again; +} + +static void orig_fill(struct objtool_file *file, + struct instruction *prev_insn, + struct instruction *insn) +{ + for (;;) { + if (!insn->visited) + insn->cfi = prev_insn->cfi; + + prev_insn = insn; + insn = next_insn_same_sec(file, insn); + if (!insn) + return; + if (!insn->alt_group) + return; + } +} + +int create_orc(struct objtool_file *file) +{ + struct instruction *insn, *prev_insn = NULL; for_each_insn(file, insn) { struct orc_entry *orc = &insn->orc; struct cfi_reg *cfa = &insn->cfi.cfa; struct cfi_reg *bp = &insn->cfi.regs[CFI_BP]; + int ret; orc->end = insn->cfi.end; + if (insn->alt_group && !insn->ignore_alts) { + struct alternative *alt; + + list_for_each_entry(alt, &insn->alts, list) { + if (alt->insn->offset == -1 /*FAKE_JUMP_OFFSET*/) + continue; + ret = alt_cfi(file, prev_insn, insn, alt->insn); + if (ret) + return ret; + } + + orig_fill(file, prev_insn, insn); + } + if (cfa->base == CFI_UNDEFINED) { orc->sp_reg = ORC_REG_UNDEFINED; continue; @@ -76,6 +188,8 @@ int create_orc(struct objtool_file *file) orc->sp_offset = cfa->offset; orc->bp_offset = bp->offset; orc->type = insn->cfi.type; + + prev_insn = insn; } return 0;
| |