Re: [RFC PATCH 24/28] tools/objtool: Treat indirect ftrace calls as direct calls

[Ard Biesheuvel]

On Tue, 1 Oct 2024 at 09:18, Josh Poimboeuf <jpoimboe@xxxxxxxxxx> wrote:
>
> On Wed, Sep 25, 2024 at 05:01:24PM +0200, Ard Biesheuvel wrote:
> > +             if (insn->type == INSN_CALL_DYNAMIC) {
> > +                     if (!reloc)
> > +                             continue;
> > +
> > +                     /*
> > +                      * GCC 13 and older on x86 will always emit the call to
> > +                      * __fentry__ using a relaxable GOT-based symbol
> > +                      * reference when operating in PIC mode, i.e.,
> > +                      *
> > +                      *   call   *0x0(%rip)
> > +                      *             R_X86_64_GOTPCRELX  __fentry__-0x4
> > +                      *
> > +                      * where it is left up to the linker to relax this into
> > +                      *
> > +                      *   call   __fentry__
> > +                      *   nop
> > +                      *
> > +                      * if __fentry__ turns out to be DSO local, which is
> > +                      * always the case for vmlinux. Given that this
> > +                      * relaxation is mandatory per the x86_64 psABI, these
> > +                      * calls can simply be treated as direct calls.
> > +                      */
> > +                     if (arch_ftrace_match(reloc->sym->name)) {
> > +                             insn->type = INSN_CALL;
> > +                             add_call_dest(file, insn, reloc->sym, false);
> > +                     }
>
> Can the compiler also do this for non-fentry direct calls?

No, it is essentially an oversight in GCC that this happens at all,
and I fixed it [0] for GCC 14, i.e., to honour -mdirect-extern-access
when emitting these calls.

But even without that, it is peculiar at the very least that the
compiler would emit GOT based indirect calls at all.

Instead of

  call *__fentry__@GOTPCREL(%rip)

it should simply emit

  call __fentry__@PLT

and leave it up to the linker to resolve this directly or
lazily/eagerly via a PLT jump (assuming -fno-plt is not being used)

> If so would
> it make sense to generalize this by converting all
> INSN_CALL_DYNAMIC+reloc to INSN_CALL?
>
> And maybe something similar for add_jump_destinations().
>

I suppose that the pattern INSN_CALL_DYNAMIC+reloc is unambiguous, and
can therefore always be treated as INSN_CALL. But I don't anticipate
any other occurrences here, and if they do exist, they indicate some
other weirdness in the compiler, so perhaps it is better not to add
general support for these.


[0] https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=bde21de1205c0456f6df68c950fb7ee631fcfa93