| Message ID | bf1c5fb5-510f-8cc6-89d9-d3ca9e03ae8f@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | New failure on aarch64 in Fedora Rawhide. | expand |
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.
On Apr 09 2019, Carlos O'Donell <codonell@redhat.com> wrote: > 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. I don't see that failure, and I don't understand why the compiler complains. The definitions should be 100% identical. Why doesn't the compiler complain about the other macro redefinitions? Andreas.
On 4/10/19 7:04 AM, Andreas Schwab wrote: > On Apr 09 2019, Carlos O'Donell <codonell@redhat.com> wrote: > >> 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. > > I don't see that failure, and I don't understand why the compiler > complains. The definitions should be 100% identical. Why doesn't the > compiler complain about the other macro redefinitions? And the weirder thing is that a rebuild with the same compiler version worked. Maybe flaky hardware?
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>