On Wed, Mar 11, 2026 at 09:24:22AM +0100, Miroslav Benes wrote:
> On Tue, 10 Mar 2026, Josh Poimboeuf wrote:
>
> > On Tue, Mar 10, 2026 at 11:47:41AM +0100, Miroslav Benes wrote:
> > > Hi,
> > >
> > > > @@ -3691,9 +3691,30 @@ static void checksum_update_insn(struct
> > > > objtool_file *file, struct symbol *func,
> > > > struct instruction *insn)
> > > > {
> > > > struct reloc *reloc = insn_reloc(file, insn);
> > > > + struct alternative *alt;
> > > > unsigned long offset;
> > > > struct symbol *sym;
> > > >
> > > > + for (alt = insn->alts; alt; alt = alt->next) {
> > > > + struct alt_group *alt_group = alt->insn->alt_group;
> > > > +
> > > > + checksum_update(func, insn, &alt->type,
> > > > sizeof(alt->type));
> > > > +
> > > > + if (alt_group && alt_group->orig_group) {
> > > > + struct instruction *alt_insn;
> > > > +
> > > > + checksum_update(func, insn,
> > > > &alt_group->feature, sizeof(alt_group->feature));
> > > > +
> > > > + for (alt_insn = alt->insn; alt_insn; alt_insn =
> > > > next_insn_same_sec(file, alt_insn)) {
> > > > + checksum_update_insn(file, func,
> > > > alt_insn);
> > > > + if (alt_insn == alt_group->last_insn)
> > > > + break;
> > > > + }
> > > > + } else {
> > > > + checksum_update(func, insn, &alt->insn->offset,
> > > > sizeof(alt->insn->offset));
> > > > + }
> > > > + }
> > > > +
> > >
> > > does this hunk belong to the patch? Unless I am missing something, it
> > > might be worth a separate one.
> >
> > It belongs, but I should have clarified that in the patch description.
> >
> > This hunk wasn't needed before because validate_branch() already
> > iterates all the alternatives, so it was calling checksum_update_insn()
> > for every instruction in the function, including the alternatives.
> >
> > Now that it's no longer called by validate_branch(),
> > checksum_update_insn() has to manually iterate the alternatives.
>
> After writing the email I had a suspicion it must have been something like
> above but failed to find it. Now I see that next_insn_to_validate() called
> in do_validate_branch() handles exactly that. Thanks for the pointer. The
> patch looks good to me then (and the rest as well as far as I can judge).
Actually, next_insn_to_validate() helps with an edge case for directing
code flow from the end of an alternative back to the original code.
The code which traverses the alternatives is in validate_insn():
if (insn->alts) {
for (alt = insn->alts; alt; alt = alt->next) {
TRACE_ALT_BEGIN(insn, alt, alt_name);
ret = validate_branch(file, func, alt->insn, *statep);
TRACE_ALT_END(insn, alt, alt_name);
if (ret) {
BT_INSN(insn, "(alt)");
return ret;
}
}
TRACE_ALT_INFO_NOADDR(insn, "/ ", "DEFAULT");
}
> I must admit that objtool has gotten so complex that I have a hard time to
> track everything in the code :).
The code hasn't changed *too* much, it's just that validate_branch() got
split up more when the tracing code went in, so things are organized a
bit differently. Most of that code is now in validate_insn().
--
Josh
