Message ID | bf1c5fb5-510f-8cc6-89d9-d3ca9e03ae8f@redhat.com |
---|---|
State | Not applicable |
Headers |
Received: (qmail 21418 invoked by alias); 9 Apr 2019 21:26:46 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 21406 invoked by uid 89); 9 Apr 2019 21:26:46 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-18.5 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy= X-HELO: mail-qt1-f169.google.com Return-Path: <codonell@redhat.com> To: Szabolcs Nagy <szabolcs.nagy@arm.com>, libc-alpha <libc-alpha@sourceware.org> From: Carlos O'Donell <codonell@redhat.com> Subject: New failure on aarch64 in Fedora Rawhide. Message-ID: <bf1c5fb5-510f-8cc6-89d9-d3ca9e03ae8f@redhat.com> Date: Tue, 9 Apr 2019 17:26:40 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit |
Commit Message
Carlos O'Donell
April 9, 2019, 9:26 p.m. UTC
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: --- 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?
Comments
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>