| Message ID | 20240925150059.3955569-54-ardb+git@google.com (mailing list archive) |
|---|---|
| State | RFC, archived |
| Headers | show |
| Series | x86: Rely on toolchain for relocatable code | expand |
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().
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 --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);