On Sun, 6 Apr 2025 at 12:58, Alexander Monakov <amona...@ispras.ru> wrote:
>
> On Fri, 4 Apr 2025, Ard Biesheuvel wrote:
>
> > From: Ard Biesheuvel <a...@kernel.org>
> >
> > Commit bde21de1205 ("i386: Honour -mdirect-extern-access when calling
> > __fentry__") updated the logic that emits mcount() / __fentry__() calls
> > into function prologues when profiling is enabled, to avoid GOT-based
> > indirect calls when a direct call would suffice.
> >
> > There are two problems with that change:
> > - it relies on -mdirect-extern-access rather than -fno-plt to decide
> > whether or not a direct [PLT based] call is appropriate;
> > - for the PLT case, it falls through to x86_print_call_or_nop(), which
> > does not emit the @PLT suffix, resulting in the wrong relocation to be
> > used (R_X86_64_PC32 instead of R_X86_64_PLT32)
> >
> > Fix this by testing flag_plt instead of ix86_direct_extern_access, and
> > updating x86_print_call_or_nop() to take flag_pic and flag_plt into
> > account. This ensures that -mnop-mcount works as expected when emitting
> > the PLT based profiling calls.
> >
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119386
> >
> > Signed-off-by: Ard Biesheuvel <a...@kernel.org>
> >
> > gcc/ChangeLog:
> >
> > * config/i386/i386.cc (x86_function_profiler): Take
> > ix86_direct_extern_access into account when generating calls
> > to __fentry__()
>
> Wrong changelog (reused from the earlier patch?)
> The entry should mention PR category/number.
>
Thanks,
How should I generate this PR, and this changelog?
> > --- a/gcc/config/i386/i386.cc
> > +++ b/gcc/config/i386/i386.cc
> > @@ -23154,6 +23154,8 @@ x86_print_call_or_nop (FILE *file, const char
> > *target)
> > if (flag_nop_mcount || !strcmp (target, "nop"))
> > /* 5 byte nop: nopl 0(%[re]ax,%[re]ax,1) */
> > fprintf (file, "1:" ASM_BYTE "0x0f, 0x1f, 0x44, 0x00, 0x00\n");
> > + else if (!TARGET_PECOFF && flag_pic && flag_plt)
> > + fprintf (file, "1:\tcall\t%s@PLT\n", target);
>
> I guess this is written this way because the caller is supposed to handle the
> !flat_plt case, but then this should be an assert:
>
> else if (!TARGET_PECOFF && flag_pic)
> {
> gcc_assert (flat_plt);
> fprintf (file, "1:\tcall\t%s@PLT\n", target);
> }
>
I'll update this.
> Note that this patch touches only 64-bit codegen, -m32 will yield
> call *mcount@GOT as before (the previous patch also touched only 64-bit case).
>
I will mention this in the commit log.
