Re: [RFC v5 12/57] objtool: check: Allow jumps from an alternative group to itself

[Julien Thierry]

On 1/21/20 5:33 PM, Josh Poimboeuf wrote:
> On Thu, Jan 09, 2020 at 04:02:15PM +0000, Julien Thierry wrote:
> > Alternatives can contain instructions that jump to another instruction
> > in the same alternative group. This is actually a common pattern on
> > arm64.
> >
> > Keep track of instructions jumping within their own alternative group
> > and carry on validating such branches.
> >
> > Signed-off-by: Julien Thierry <jthierry@xxxxxxxxxx>
> > ---
> >   tools/objtool/check.c | 48 ++++++++++++++++++++++++++++++++++---------
> >   tools/objtool/check.h |  1 +
> >   2 files changed, 39 insertions(+), 10 deletions(-)
> >
> > diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> > index 8f2ff030936d..c7b3f1e2a628 100644
> > --- a/tools/objtool/check.c
> > +++ b/tools/objtool/check.c
> > @@ -722,6 +722,14 @@ static int handle_group_alt(struct objtool_file *file,
> >   	sec_for_each_insn_from(file, insn) {
> >   		if (insn->offset >= special_alt->orig_off + special_alt->orig_len)
> >   			break;
> > +		/* Is insn a jump to an instruction within the alt_group */
> > +		if (insn->jump_dest && insn->sec == insn->jump_dest->sec &&
> > +		    (insn->type == INSN_JUMP_CONDITIONAL ||
> > +		     insn->type == INSN_JUMP_UNCONDITIONAL)) {
> > +			dest_off = insn->jump_dest->offset;
> > +			insn->intra_group_jump = special_alt->orig_off <= dest_off &&
> > +				dest_off < special_alt->orig_off + special_alt->orig_len;
> > +		}
>
> This patch adds some complexity, just so we can keep the
>
>    "don't know how to handle branch to middle of alternative instruction group"
>
> warning for the case where code from outside an alternative insn group
> is branching to inside the group.  But I've never actually seen that
> case in practice, and I get the feeling that warning isn't very useful
> or realistic.
>
> How about we just remove the warning?

I'm happy to remove it.

I was trying to look for a less intrusive place to only check for 
instructions jumping into alternatives. add_jump_destinations() could be 
an option, but it would require to duplicate some stuff done in 
add_special_section_alts(). So maybe just ignoring for now can be fine.

Thanks,

--
Julien Thierry