[PATCH] compiler-gcc.h: Disable __retain on gcc-11

[PATCH] compiler-gcc.h: Disable __retain on gcc-11

[Geert Uytterhoeven]

All my gcc-11 compilers (Ubuntu 11.4.0-1ubuntu1~22.04) claim to support
the __retain__ attribute, but only riscv64-linux-gnu-gcc-11 and
x86_64-linux-gnu-gcc-11 (not x86_64-linux-gnux32-gcc-11!) actually do.
The arm-linux-gnueabi-gcc-11.5.0 compiler from kernel.org crosstool
fails in the same way:

    error: ‘retain’ attribute ignored [-Werror=attributes]

All my gcc-12 compilers seem to support the attribute.

Play it safe, and disable __retain unconditionally on gcc-11.

Fixes: 0a5d3258d7c97295 ("compiler_types.h: Define __retain for __attribute__((__retain__))")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 include/linux/compiler-gcc.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index f805adaa316e9a21..bedeb76b63a34745 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -145,3 +145,12 @@
 #if GCC_VERSION < 90100
 #undef __alloc_size__
 #endif
+
+/*
+ * Most 11.x compilers claim to support it, but only riscv64-linux-gnu-gcc and
+ * x86_64-linux-gnu-gcc actually do.
+ */
+#if GCC_VERSION < 120000
+#undef __retain
+#define __retain
+#endif
-- 
2.34.1

Re: [PATCH] compiler-gcc.h: Disable __retain on gcc-11

[Miguel Ojeda]

On Tue, Sep 24, 2024 at 3:55 PM Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
>
> All my gcc-11 compilers (Ubuntu 11.4.0-1ubuntu1~22.04) claim to support
> the __retain__ attribute, but only riscv64-linux-gnu-gcc-11 and
> x86_64-linux-gnu-gcc-11 (not x86_64-linux-gnux32-gcc-11!) actually do.
> The arm-linux-gnueabi-gcc-11.5.0 compiler from kernel.org crosstool
> fails in the same way:
>
>     error: ‘retain’ attribute ignored [-Werror=attributes]

That appears to be the case indeed:

    https://godbolt.org/z/78Gj94vMW

The `.section` does not get emitted, so the warning appears to be
right, but we cannot trust `__has_attribute` for this :(

> Fixes: 0a5d3258d7c97295 ("compiler_types.h: Define __retain for __attribute__((__retain__))")

Nit: 12 char hash.

> +/*
> + * Most 11.x compilers claim to support it, but only riscv64-linux-gnu-gcc and
> + * x86_64-linux-gnu-gcc actually do.
> + */

Just to confirm: did you try all? If not, perhaps we should say "at
least X does not work" instead.

> +#if GCC_VERSION < 120000
> +#undef __retain
> +#define __retain
> +#endif

Should this go into the conditional in `compiler_types.h` instead? And
perhaps the `__has__attribute` test removed for GCC?

Even if we keep it here, I think at least a comment there should be
added, since it says GCC >= 11 supports it, which can be confusing if
one is not aware of this other thing in this file.

Thanks!

Cheers,
Miguel

Re: [PATCH] compiler-gcc.h: Disable __retain on gcc-11

[Geert Uytterhoeven]

Hi Miguel,

On Tue, Sep 24, 2024 at 7:21 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
> On Tue, Sep 24, 2024 at 3:55 PM Geert Uytterhoeven
> <geert+renesas@glider.be> wrote:
> > All my gcc-11 compilers (Ubuntu 11.4.0-1ubuntu1~22.04) claim to support
> > the __retain__ attribute, but only riscv64-linux-gnu-gcc-11 and
> > x86_64-linux-gnu-gcc-11 (not x86_64-linux-gnux32-gcc-11!) actually do.
> > The arm-linux-gnueabi-gcc-11.5.0 compiler from kernel.org crosstool
> > fails in the same way:
> >
> >     error: ‘retain’ attribute ignored [-Werror=attributes]
>
> That appears to be the case indeed:
>
>     https://godbolt.org/z/78Gj94vMW
>
> The `.section` does not get emitted, so the warning appears to be
> right, but we cannot trust `__has_attribute` for this :(
>
> > Fixes: 0a5d3258d7c97295 ("compiler_types.h: Define __retain for __attribute__((__retain__))")
>
> Nit: 12 char hash.

I should really update and resend my patch to sync all (different) values and
ranges spread across the Linux kernel documentation. And didn't we reach
2^24 objects in git a while ago, so conflicts are imminent?

> > +/*
> > + * Most 11.x compilers claim to support it, but only riscv64-linux-gnu-gcc and
> > + * x86_64-linux-gnu-gcc actually do.
> > + */
>
> Just to confirm: did you try all? If not, perhaps we should say "at
> least X does not work" instead.

What's your definition of "all"? ;-)

  1. All compilers available on Ubuntu 22.04LTS:

      aarch64-linux-gnu-gcc-11
      alpha-linux-gnu-gcc-11
      arm-linux-gnueabi-gcc-11
      arm-linux-gnueabihf-gcc-11
      hppa64-linux-gnu-gcc-11
      hppa-linux-gnu-gcc-11
      i686-linux-gnu-gcc-11
      m68k-linux-gnu-gcc-11
      powerpc64le-linux-gnu-gcc-11
      powerpc64-linux-gnu-gcc-11
      powerpc-linux-gnu-gcc-11
      riscv64-linux-gnu-gcc-11
      s390x-linux-gnu-gcc-11
      sh4-linux-gnu-gcc-11
      sparc64-linux-gnu-gcc-11
      x86_64-linux-gnu-gcc-11
      x86_64-linux-gnux32-gcc-11

  2. A few from kernel.org crosstool:

      ia64-linux-gcc-11.1.0
      sh2eb-linux-muslfdpic-gcc-11.2.0
      arm-linux-gnueabi-gcc-11.5.0

  3. A compiler from the J-Core folks:

      sh2eb-linux-muslfdpic-gcc-11.2.0

> > +#if GCC_VERSION < 120000
> > +#undef __retain
> > +#define __retain
> > +#endif
>
> Should this go into the conditional in `compiler_types.h` instead? And
> perhaps the `__has__attribute` test removed for GCC?

AFAIK, the gcc-specifics are in compiler-gcc.h...

> Even if we keep it here, I think at least a comment there should be
> added, since it says GCC >= 11 supports it, which can be confusing if
> one is not aware of this other thing in this file.

Fair enough....

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] compiler-gcc.h: Disable __retain on gcc-11

[Tony Ambardar]

Hello Geert,

On Tue, Sep 24, 2024 at 08:30:30PM +0200, Geert Uytterhoeven wrote:
> Hi Miguel,
> 
> On Tue, Sep 24, 2024 at 7:21 PM Miguel Ojeda
> <miguel.ojeda.sandonis@gmail.com> wrote:
> > On Tue, Sep 24, 2024 at 3:55 PM Geert Uytterhoeven
> > <geert+renesas@glider.be> wrote:
> > > All my gcc-11 compilers (Ubuntu 11.4.0-1ubuntu1~22.04) claim to support
> > > the __retain__ attribute, but only riscv64-linux-gnu-gcc-11 and
> > > x86_64-linux-gnu-gcc-11 (not x86_64-linux-gnux32-gcc-11!) actually do.
> > > The arm-linux-gnueabi-gcc-11.5.0 compiler from kernel.org crosstool
> > > fails in the same way:

[snip]

> What's your definition of "all"? ;-)
> 
>   1. All compilers available on Ubuntu 22.04LTS:
> 
>       aarch64-linux-gnu-gcc-11
>       alpha-linux-gnu-gcc-11
>       arm-linux-gnueabi-gcc-11
>       arm-linux-gnueabihf-gcc-11
>       hppa64-linux-gnu-gcc-11
>       hppa-linux-gnu-gcc-11
>       i686-linux-gnu-gcc-11
>       m68k-linux-gnu-gcc-11
>       powerpc64le-linux-gnu-gcc-11
>       powerpc64-linux-gnu-gcc-11
>       powerpc-linux-gnu-gcc-11
>       riscv64-linux-gnu-gcc-11
>       s390x-linux-gnu-gcc-11
>       sh4-linux-gnu-gcc-11
>       sparc64-linux-gnu-gcc-11
>       x86_64-linux-gnu-gcc-11
>       x86_64-linux-gnux32-gcc-11
> 
>   2. A few from kernel.org crosstool:
> 
>       ia64-linux-gcc-11.1.0
>       sh2eb-linux-muslfdpic-gcc-11.2.0
>       arm-linux-gnueabi-gcc-11.5.0
> 
>   3. A compiler from the J-Core folks:
> 
>       sh2eb-linux-muslfdpic-gcc-11.2.0
> 

Thank you for testing the gcc-11 edge so thoroughly. For my testing, I
used a handful of gcc versions across 9 to 13, but ironically only
tried gcc 11 on x86_64 (which works... :-\ ).

> > > +#if GCC_VERSION < 120000
> > > +#undef __retain
> > > +#define __retain
> > > +#endif
> >
> > Should this go into the conditional in `compiler_types.h` instead? And
> > perhaps the `__has__attribute` test removed for GCC?
> 
> AFAIK, the gcc-specifics are in compiler-gcc.h...
> 

Right, I agree it's best left there.

> > Even if we keep it here, I think at least a comment there should be
> > added, since it says GCC >= 11 supports it, which can be confusing if
> > one is not aware of this other thing in this file.
> 
> Fair enough....

Yes, some clarifying text would be helpful e.g.
"Optional: only supported since gcc >= 11 (partial), clang >= 13"

Otherwise LGTM, so:

Reviewed-by: Tony Ambardar <tony.ambardar@gmail.com>

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

Re: [PATCH] compiler-gcc.h: Disable __retain on gcc-11

[Miguel Ojeda]

On Tue, Sep 24, 2024 at 8:30 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> I should really update and resend my patch to sync all (different) values and
> ranges spread across the Linux kernel documentation. And didn't we reach
> 2^24 objects in git a while ago, so conflicts are imminent?

Please do so! :)

> What's your definition of "all"? ;-)

That is definitely a nice list -- thanks!

> AFAIK, the gcc-specifics are in compiler-gcc.h...
>
> Fair enough....

Sounds good.

Cheers,
Miguel

Re: [PATCH] compiler-gcc.h: Disable __retain on gcc-11

[Andreas Schwab]

On Sep 24 2024, Geert Uytterhoeven wrote:

> All my gcc-11 compilers (Ubuntu 11.4.0-1ubuntu1~22.04) claim to support
> the __retain__ attribute, but only riscv64-linux-gnu-gcc-11 and
> x86_64-linux-gnu-gcc-11 (not x86_64-linux-gnux32-gcc-11!) actually do.
> The arm-linux-gnueabi-gcc-11.5.0 compiler from kernel.org crosstool
> fails in the same way:
>
>     error: ‘retain’ attribute ignored [-Werror=attributes]
>
> All my gcc-12 compilers seem to support the attribute.

That ultimately depends on binutils support for SHF_GNU_RETAIN (2.36+).

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH] compiler-gcc.h: Disable __retain on gcc-11

[Tony Ambardar]

On Tue, Sep 24, 2024 at 07:29:46PM +0200, Andreas Schwab wrote:
> On Sep 24 2024, Geert Uytterhoeven wrote:
> 
> > All my gcc-11 compilers (Ubuntu 11.4.0-1ubuntu1~22.04) claim to support
> > the __retain__ attribute, but only riscv64-linux-gnu-gcc-11 and
> > x86_64-linux-gnu-gcc-11 (not x86_64-linux-gnux32-gcc-11!) actually do.
> > The arm-linux-gnueabi-gcc-11.5.0 compiler from kernel.org crosstool
> > fails in the same way:
> >
> >     error: ‘retain’ attribute ignored [-Werror=attributes]
> >
> > All my gcc-12 compilers seem to support the attribute.
> 
> That ultimately depends on binutils support for SHF_GNU_RETAIN (2.36+).

IIRC from working on the original patch, older linkers which do not
support this 'R' flag would simply ignore it and not error out, making
binutils compatibility less of an issue.

> 
> -- 
> Andreas Schwab, schwab@linux-m68k.org
> GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
> "And now for something completely different."

Re: [PATCH] compiler-gcc.h: Disable __retain on gcc-11

[Andreas Schwab]

On Sep 30 2024, Tony Ambardar wrote:

> On Tue, Sep 24, 2024 at 07:29:46PM +0200, Andreas Schwab wrote:
>> On Sep 24 2024, Geert Uytterhoeven wrote:
>> 
>> > All my gcc-11 compilers (Ubuntu 11.4.0-1ubuntu1~22.04) claim to support
>> > the __retain__ attribute, but only riscv64-linux-gnu-gcc-11 and
>> > x86_64-linux-gnu-gcc-11 (not x86_64-linux-gnux32-gcc-11!) actually do.
>> > The arm-linux-gnueabi-gcc-11.5.0 compiler from kernel.org crosstool
>> > fails in the same way:
>> >
>> >     error: ‘retain’ attribute ignored [-Werror=attributes]
>> >
>> > All my gcc-12 compilers seem to support the attribute.
>> 
>> That ultimately depends on binutils support for SHF_GNU_RETAIN (2.36+).
>
> IIRC from working on the original patch, older linkers which do not
> support this 'R' flag would simply ignore it and not error out,

The assembler gives a fatal error for unrecognized section attributes
since binutils 2.15, and emitted a warning before that (but the gcc
check uses --fatal-warnings).

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."