Messages in this thread | | | Date | Wed, 29 Apr 2020 18:41:36 +0200 (CEST) | From | Miroslav Benes <> | Subject | Re: [PATCH v2 02/14] objtool: Fix ORC vs alternatives |
| |
On Wed, 29 Apr 2020, Peter Zijlstra wrote:
> On Wed, Apr 29, 2020 at 04:33:31PM +0200, Miroslav Benes wrote: > > On Tue, 28 Apr 2020, Peter Zijlstra wrote: > > > /* > > > + * Alternatives should not contain any ORC entries, this in turn means they > > > + * should not contain any CFI ops, which implies all instructions should have > > > + * the same same CFI state. > > > + * > > > + * It is possible to constuct alternatives that have unreachable holes that go > > > + * unreported (because they're NOPs), such holes would result in CFI_UNDEFINED > > > + * states which then results in ORC entries, which we just said we didn't want. > > > + * > > > + * Avoid them by copying the CFI entry of the first instruction into the whole > > > + * alternative. > > > + */ > > > +static void fill_alternative_cfi(struct objtool_file *file, struct instruction *insn) > > > +{ > > > + struct instruction *first_insn = insn; > > > + int alt_group = insn->alt_group; > > > + > > > + sec_for_each_insn_continue(file, insn) { > > > + if (insn->alt_group != alt_group) > > > + break; > > > + insn->cfi = first_insn->cfi; > > > + } > > > +} > > > > If I am reading this and previous patch correctly... > > > > The function would copy cfi only to "orig" alternative (its insn->alts is > > non-empty, orig_insn->alt_group differs from new_insn->alt_group), right? > > Yes. > > > Would it make sense to do the same for "new" alternative, because of the > > invariant? It seems to me it is not processed anywhere that way. > > No. > > > Am I missing something? Whenever I try to read all this alternatives > > handling in objtool, I get lost pretty soon. > > We only care about the ORC covering the original range, because that is > the range we execute code from. The memory where we store the > alternative instructions (.altinstruction section) is never executed, > that is, RIP should never point there, so we don't need ORC data covering > it.
Aha, that's what I didn't realize (again). Note to myself (for the hundredth time): alternatives are not branches.
Thanks Miroslav
| |