On Thu, Apr 10, 2025 at 2:27 PM Ard Biesheuvel <ardb+...@google.com> 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 also ensures that -mnop-mcount works as expected when
> emitting the PLT based profiling calls.
>
> While at it, fix the 32-bit logic as well, and issue a PLT call unless
> PLTs are explicitly disabled.
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119386
>
> Signed-off-by: Ard Biesheuvel <a...@kernel.org>
>
> gcc/ChangeLog:
>
> PR target/119386
> * config/i386/i386.cc (x86_print_call_or_nop): Add @PLT suffix
> where appropriate.
> (x86_function_profiler): Fall through to x86_print_call_or_nop()
> for PIC codegen when flag_plt is set.
>
> gcc/testsuite/ChangeLog:
>
> PR target/119386
> * gcc.target/i386/pr119386-1.c: New test.
> * gcc.target/i386/pr119386-2.c: New test.
OK if there are no further comments in the next day or two.
BTW: Do you have commit rights?
Thanks,
Uros.
> ---
> gcc/config/i386/i386.cc | 12 ++++++++++--
> gcc/testsuite/gcc.target/i386/pr119386-1.c | 10 ++++++++++
> gcc/testsuite/gcc.target/i386/pr119386-2.c | 12 ++++++++++++
> 3 files changed, 32 insertions(+), 2 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/i386/pr119386-1.c
> create mode 100644 gcc/testsuite/gcc.target/i386/pr119386-2.c
>
> diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
> index 4f8380c4a58..20059b775b9 100644
> --- a/gcc/config/i386/i386.cc
> +++ b/gcc/config/i386/i386.cc
> @@ -23158,6 +23158,12 @@ 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)
> + {
> + gcc_assert (flag_plt);
> +
> + fprintf (file, "1:\tcall\t%s@PLT\n", target);
> + }
> else
> fprintf (file, "1:\tcall\t%s\n", target);
> }
> @@ -23321,7 +23327,7 @@ x86_function_profiler (FILE *file, int labelno
> ATTRIBUTE_UNUSED)
> break;
> case CM_SMALL_PIC:
> case CM_MEDIUM_PIC:
> - if (!ix86_direct_extern_access)
> + if (!flag_plt)
> {
> if (ASSEMBLER_DIALECT == ASM_INTEL)
> fprintf (file, "1:\tcall\t[QWORD PTR %s@GOTPCREL[rip]]\n",
> @@ -23352,7 +23358,9 @@ x86_function_profiler (FILE *file, int labelno
> ATTRIBUTE_UNUSED)
> "\tleal\t%sP%d@GOTOFF(%%ebx), %%" PROFILE_COUNT_REGISTER
> "\n",
> LPREFIX, labelno);
> #endif
> - if (ASSEMBLER_DIALECT == ASM_INTEL)
> + if (flag_plt)
> + x86_print_call_or_nop (file, mcount_name);
> + else if (ASSEMBLER_DIALECT == ASM_INTEL)
> fprintf (file, "1:\tcall\t[DWORD PTR %s@GOT[ebx]]\n", mcount_name);
> else
> fprintf (file, "1:\tcall\t*%s@GOT(%%ebx)\n", mcount_name);
> diff --git a/gcc/testsuite/gcc.target/i386/pr119386-1.c
> b/gcc/testsuite/gcc.target/i386/pr119386-1.c
> new file mode 100644
> index 00000000000..9a0dc64b5b9
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr119386-1.c
> @@ -0,0 +1,10 @@
> +/* PR target/119386 */
> +/* { dg-do compile { target *-*-linux* } } */
> +/* { dg-options "-O2 -fpic -pg" } */
> +/* { dg-final { scan-assembler "call\[ \t\]+mcount@PLT" } } */
> +
> +int
> +main ()
> +{
> + return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/i386/pr119386-2.c
> b/gcc/testsuite/gcc.target/i386/pr119386-2.c
> new file mode 100644
> index 00000000000..3ea978ecfdf
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr119386-2.c
> @@ -0,0 +1,12 @@
> +/* PR target/119386 */
> +/* { dg-do compile { target *-*-linux* } } */
> +/* { dg-options "-O2 -fpic -fno-plt -pg" } */
> +/* { dg-final { scan-assembler "call\[ \t\]+\\*mcount@GOTPCREL\\(" { target
> { ! ia32 } } } } */
> +/* { dg-final { scan-assembler "call\[ \t\]+\\*mcount@GOT\\(" { target ia32
> } } } */
> +
> +
> +int
> +main ()
> +{
> + return 0;
> +}
> --
> 2.49.0.504.g3bcea36a83-goog
>
