libbpf: Fix build with latest gcc/binutils with LTO

From: Patrick McCarty <patrick.mccarty@intel.com>

After updating to binutils 2.35, the build began to fail with an
assembler error. A bug was opened on the Red Hat Bugzilla a few days
later for the same issue.

Work around the problem by using the new `symver` attribute (introduced
in GCC 10) as needed, instead of the `COMPAT_VERSION` and
`DEFAULT_VERSION` macros, which expand to assembler directives.

Fixes: https://github.com/libbpf/libbpf/issues/338
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1863059
Fixes: https://bugzilla.opensuse.org/show_bug.cgi?id=1188749
Signed-off-by: Patrick McCarty <patrick.mccarty@intel.com>
Make the change conditional on GCC version
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 tools/lib/bpf/libbpf_internal.h | 23 +++++++++++++++++------
 tools/lib/bpf/xsk.c             |  4 ++--
 2 files changed, 19 insertions(+), 8 deletions(-)

On Fri, Aug 27, 2021 at 12:25 AM Michal Suchanek <msuchanek@suse.de> wrote:
>

> From: Patrick McCarty <patrick.mccarty@intel.com>

>

> After updating to binutils 2.35, the build began to fail with an

> assembler error. A bug was opened on the Red Hat Bugzilla a few days

> later for the same issue.

>

> Work around the problem by using the new `symver` attribute (introduced

> in GCC 10) as needed, instead of the `COMPAT_VERSION` and

> `DEFAULT_VERSION` macros, which expand to assembler directives.

>

> Fixes: https://github.com/libbpf/libbpf/issues/338


This is not a proper tag. We used the following form before:

  [0] Closes: https://github.com/libbpf/libbpf/issues/280

So it's a reference. And then mention in commit message that this was
initiated by the issue on Github ([0]), or something along those
lines.


> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1863059

> Fixes: https://bugzilla.opensuse.org/show_bug.cgi?id=1188749


These are also not proper Fixes: tags for kernel. It's fine if you
mention that this change fixes those bugs, but maybe use the reference
([1], [2]) style for that?

> Signed-off-by: Patrick McCarty <patrick.mccarty@intel.com>

> Make the change conditional on GCC version


This is not a tag, maybe remove this or make it part of the commit
message properly?

> Signed-off-by: Michal Suchanek <msuchanek@suse.de>

> ---

>  tools/lib/bpf/libbpf_internal.h | 23 +++++++++++++++++------

>  tools/lib/bpf/xsk.c             |  4 ++--

>  2 files changed, 19 insertions(+), 8 deletions(-)

>

> diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h

> index 016ca7cb4f8a..af0f3fb102c0 100644

> --- a/tools/lib/bpf/libbpf_internal.h

> +++ b/tools/lib/bpf/libbpf_internal.h

> @@ -86,20 +86,31 @@

>         (offsetof(TYPE, FIELD) + sizeof(((TYPE *)0)->FIELD))

>  #endif

>

> +#ifdef __GNUC__

> +# if __GNUC__ >= 10

> +#  define DEFAULT_VERSION(internal_name, api_name, version) \

> +__attribute__((__symver__(#api_name "@@" #version)))

> +#  define COMPAT_VERSION(internal_name, api_name, version) \

> +__attribute__((__symver__(#api_name "@" #version)))

> +# endif

> +#endif

> +

> +#if !defined(COMPAT_VERSION) || !defined(DEFAULT_VERSION)


This seems wrong. If __GNUC__ && __GNUC__ >= 10 we'll define
DEFAULT_VERSION and COMPAT_VERSION as if we are linking in shared
library mode. This will be wrong on new GCC *and* static linking mode.
I think the above declarations should be inside #ifdef SHARED section.

Also, can you please write it out as #if defined(__GNUC__) && __GNUC__
>= 10, instead of doubly nested #if/#ifdef condition?



>  /* Symbol versioning is different between static and shared library.

>   * Properly versioned symbols are needed for shared library, but

>   * only the symbol of the new version is needed for static library.

>   */

> -#ifdef SHARED

> -# define COMPAT_VERSION(internal_name, api_name, version) \

> +# ifdef SHARED

> +#  define COMPAT_VERSION(internal_name, api_name, version) \

>         asm(".symver " #internal_name "," #api_name "@" #version);

> -# define DEFAULT_VERSION(internal_name, api_name, version) \

> +#  define DEFAULT_VERSION(internal_name, api_name, version) \

>         asm(".symver " #internal_name "," #api_name "@@" #version);

> -#else

> -# define COMPAT_VERSION(internal_name, api_name, version)

> -# define DEFAULT_VERSION(internal_name, api_name, version) \

> +# else

> +#  define COMPAT_VERSION(internal_name, api_name, version)

> +#  define DEFAULT_VERSION(internal_name, api_name, version) \

>         extern typeof(internal_name) api_name \

>         __attribute__((alias(#internal_name)));

> +# endif

>  #endif

>

>  extern void libbpf_print(enum libbpf_print_level level,

> diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c

> index e9b619aa0cdf..a2111696ba91 100644

> --- a/tools/lib/bpf/xsk.c

> +++ b/tools/lib/bpf/xsk.c

> @@ -281,6 +281,7 @@ static int xsk_create_umem_rings(struct xsk_umem *umem, int fd,

>         return err;

>  }

>

> +DEFAULT_VERSION(xsk_umem__create_v0_0_4, xsk_umem__create, LIBBPF_0.0.4)

>  int xsk_umem__create_v0_0_4(struct xsk_umem **umem_ptr, void *umem_area,

>                             __u64 size, struct xsk_ring_prod *fill,

>                             struct xsk_ring_cons *comp,

> @@ -345,6 +346,7 @@ struct xsk_umem_config_v1 {

>         __u32 frame_headroom;

>  };

>

> +COMPAT_VERSION(xsk_umem__create_v0_0_2, xsk_umem__create, LIBBPF_0.0.2)

>  int xsk_umem__create_v0_0_2(struct xsk_umem **umem_ptr, void *umem_area,

>                             __u64 size, struct xsk_ring_prod *fill,

>                             struct xsk_ring_cons *comp,

> @@ -358,8 +360,6 @@ int xsk_umem__create_v0_0_2(struct xsk_umem **umem_ptr, void *umem_area,

>         return xsk_umem__create_v0_0_4(umem_ptr, umem_area, size, fill, comp,

>                                         &config);

>  }

> -COMPAT_VERSION(xsk_umem__create_v0_0_2, xsk_umem__create, LIBBPF_0.0.2)

> -DEFAULT_VERSION(xsk_umem__create_v0_0_4, xsk_umem__create, LIBBPF_0.0.4)

>

>  static enum xsk_prog get_xsk_prog(void)

>  {

> --

> 2.31.1

>