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

Ard Biesheuvel
2024-10-01T07:39:59Z

On Tue, 1 Oct 2024 at 09:18, Josh Poimboeuf <jpoimboe@kernel.org> 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