diff mbox series

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

Message ID 20240925150059.3955569-54-ardb+git@google.com
State New
Headers show
Series x86: Rely on toolchain for relocatable code | expand

Commit Message

Ard Biesheuvel Sept. 25, 2024, 3:01 p.m. UTC 
From: Ard Biesheuvel <ardb@kernel.org>

In some cases, the compiler may rely on indirect calls using GOT slots
as memory operands to emit function calls. This leaves it up to the
linker to relax the call to a direct call if possible, i.e., if the
destination address is known at link time and in range, which may not be
the case when building shared libraries for user space.

On x86, this may happen when building in PIC mode with ftrace enabled,
and given that vmlinux is a fully linked binary, this relaxation is
always possible, and therefore mandatory per the x86_64 psABI.

This means that the indirect calls to __fentry__ that are observeable in
vmlinux.o will have been converted to direct calls in vmlinux, and can
be treated as such by objtool.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 tools/objtool/check.c | 32 ++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

Comments

Josh Poimboeuf Oct. 1, 2024, 7:18 a.m. UTC | #1 
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?  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().
Ard Biesheuvel Oct. 1, 2024, 7:39 a.m. UTC | #2 
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
diff mbox series

Patch

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 04725bd83232..94a56099e22d 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1696,11 +1696,39 @@  static int add_call_destinations(struct objtool_file *file)
 	struct reloc *reloc;
 
 	for_each_insn(file, insn) {
-		if (insn->type != INSN_CALL)
+		if (insn->type != INSN_CALL &&
+		    insn->type != INSN_CALL_DYNAMIC)
 			continue;
 
 		reloc = insn_reloc(file, insn);
-		if (!reloc) {
+		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);
+			}
+
+		} else if (!reloc) {
 			dest_off = arch_jump_destination(insn);
 			dest = find_call_destination(insn->sec, dest_off);