Re: [PATCH] configure.ac: add detection of symver gcc attribute

[Giulio Benetti]

Hello Andreas,

thank you for taking care,

On 17/02/24 22:00, Andreas Grünbacher wrote:
> Hello,
>
> this looks mostly reasonable, but
>
> Am Fr., 16. Feb. 2024 um 22:11 Uhr schrieb Giulio Benetti
> <giulio.benetti@benettiengineering.com>:
> > From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> >
> > On non-ELF platforms, such as microblaze, builds will fail when trying
> > to add symver information because  __attribute__((symver ..)) is not
> > supported even though __has_attribute(__symver__) returns true.
> >
> > Support for symver needs to be detected via a compile test since
> > __has_attribute can report false positives [0].
> >
> > Add a configure compile check for __attribute__((symver ..)) to ensure
> > it is supported and define a variable to advertise support.
> >
> > [0] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101766#c1
> >
> > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> > Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
> > ---
> >   configure.ac       | 15 +++++++++++++++
> >   libattr/syscalls.c |  2 +-
> >   2 files changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/configure.ac b/configure.ac
> > index 7e362e9..98477b5 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -57,6 +57,21 @@ AS_CASE([$host_os],
> >          [linux*], [os_linux=yes])
> >   AM_CONDITIONAL([OS_LINUX], [test "x$os_linux" = "xyes"])
> >
> > +AC_CACHE_CHECK(whether __attribute__((__symver__())) is supported,
> > +       gcc_cv_symver_attribute,
> > +       [cat > conftest.c <<EOF
> > +void foo (void) {}
> > +__typeof(foo) foo __attribute__ ((__symver__("foo@foo")));
> > +EOF
> > +       gcc_cv_symver_attribute=no
> > +       if ${CC-cc} -Werror -S conftest.c -o conftest.s >/dev/null 2>&1; then \
> > +           gcc_cv_symver_attribute=yes
> > +       fi
> > +       rm -f conftest.[cs]
> > +])
> > +AS_IF([test $gcc_cv_symver_attribute = yes],
> > +      [AC_DEFINE(HAVE_SYMVER_ATTRIBUTE, [], [GCC supports symver attribute])])
> > +
> >   AC_CONFIG_COMMANDS([include/attr],
> >          [dnl
> >          rm -rf include/attr
> > diff --git a/libattr/syscalls.c b/libattr/syscalls.c
> > index 907560a..caa70bc 100644
> > --- a/libattr/syscalls.c
> > +++ b/libattr/syscalls.c
> > @@ -32,7 +32,7 @@
> >    * fall back to traditional .symver asm directive otherwise.
> >    */
> >   #ifdef __has_attribute
> > -# if __has_attribute(__symver__)
> > +# if defined(HAVE_SYMVER_ATTRIBUTE)
>
> checking for __has_attribute doesn't make sense when
> HAVE_SYMVER_ATTRIBUTE is defined now.

You're right, need to check if __has_attribute is defined only when
checking for __no_reorder.

> Could you please fix the if nesting?

Sure, send V2 asap.

Best regards
--
Giulio Benetti
CEO&CTO@Benetti Engineering sas

> >   #  define SYMVER(cn, vn) __typeof(cn) cn __attribute__((__symver__(vn)))
> >   # elif __has_attribute(__no_reorder__)
> >      /*
> > --
> > 2.34.1
>
> Thanks,
> Andreas