Re: New failure on aarch64 in Fedora Rawhide.

[Szabolcs Nagy <Szabolcs dot Nagy at arm dot com>]

On 09/04/2019 22:26, Carlos O'Donell wrote:
> Szabolcs,
> 
> https://kojipkgs.fedoraproject.org//work/tasks/2880/34082880/build.log
> 
> BUILDSTDERR: In file included from ../sysdeps/aarch64/sysdep.h:22,
> BUILDSTDERR:                  from ../sysdeps/unix/sysv/linux/aarch64/sysdep.h:26,
> BUILDSTDERR:                  from ../sysdeps/aarch64/nptl/tls.h:37,
> BUILDSTDERR:                  from ../include/errno.h:25,
> BUILDSTDERR:                  from ../sysdeps/unix/sysv/linux/netlink_assert_response.c:19:
> BUILDSTDERR: ../sysdeps/generic/sysdep.h:81: error: "CFI_RESTORE" redefined [-Werror]
> BUILDSTDERR:    81 | # define CFI_RESTORE(reg) \
> BUILDSTDERR:       |
> BUILDSTDERR: In file included from ../sysdeps/unix/sysdep.h:18,
> BUILDSTDERR:                  from ../sysdeps/unix/sysv/linux/aarch64/sysdep.h:25,
> BUILDSTDERR:                  from ../sysdeps/aarch64/nptl/tls.h:37,
> BUILDSTDERR:                  from ../include/errno.h:25,
> BUILDSTDERR:                  from ../sysdeps/unix/sysv/linux/netlink_assert_response.c:19:
> BUILDSTDERR: ../sysdeps/generic/sysdep.h:81: note: this is the location of the previous definition
> BUILDSTDERR:    81 | # define CFI_RESTORE(reg) \
> BUILDSTDERR:       |
> BUILDSTDERR: cc1: all warnings being treated as errors
> 
> This just came up in Fedora Rawhide.
> 
> We have no guards on systeps/generic/sysdep.h, I assume we want to include it multiple
> times with different macro API settings.
> 
> Note the difference in include path is:
> 
> sysdeps/aarch64/sysdep.h vs. sysdeps/unix/sysdep.h
> 
> In sysdeps/unix/sysv/linux/aarch64/sysdep.h we include both:
> 
>  25 #include <sysdeps/unix/sysdep.h>
>  26 #include <sysdeps/aarch64/sysdep.h>
> 
> Then in sysdeps/aarch64/sysdep.h we include:
> 
>  22 #include <sysdeps/generic/sysdep.h>
> 
> Then in sysdeps/unix/sysdeps.h we include:
> 
>  18 #include <sysdeps/generic/sysdep.h>
> 
> So we get two copies.
> 
> In general the project rule has been "Include the headers you need."
> 
> Guarding the macros with ifndef could lead to defaults being used incorrectly
> (macro API issues), and was the reason we enabled -Wundef, so doing that makes
> things less robust.
> 
> It would be really nice if we could just include what we needed once.
> 
> I would like to get to something like this:
> diff --git a/sysdeps/unix/sysv/linux/aarch64/sysdep.h b/sysdeps/unix/sysv/linux/aarch64/sysdep.h
> index 935c507f8cb36b2a..35f3dd65b3397001 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/sysdep.h
> +++ b/sysdeps/unix/sysv/linux/aarch64/sysdep.h
> @@ -22,9 +22,8 @@
>  /* Always enable vsyscalls on aarch64 */
>  #define ALWAYS_USE_VSYSCALL 1
>  
> -#include <sysdeps/unix/sysdep.h>
> -#include <sysdeps/aarch64/sysdep.h>
>  #include <sysdeps/unix/sysv/linux/generic/sysdep.h>
> +#include <sysdeps/aarch64/sysdep.h>
>  
>  /* Defines RTLD_PRIVATE_ERRNO and USE_DL_SYSINFO.  */
>  #include <dl-sysdep.h>
> ---
> We include the aarch64 sysdep.h, and then the Linux version, and we're done.
> This avoids the double inclusion, and appears to work just fine, matching
> what other arches do.
> 
> Thoughts?

This patch looks OK, but i don't yet understand the
failure: including sysdeps/generic/sysdep.h multiple
times should work, redefining a macro is valid if the
exact same pp token sequence is used.

Is there something that may change the definition of
CFI_RESTORE(reg) in sysdeps/generic/sysdep.h?
isn't inconsistent macro definition problematic
elsewhere?

I cannot reproduce this issue, would be nice to know
what causes it, it may be a gcc bug.