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;
