Eric Gallager <eg...@gwmail.gwu.edu> writes: > On Wed, Oct 9, 2024 at 4:54 AM Christophe Lyon > <christophe.l...@linaro.org> wrote: >> >> On Wed, 9 Oct 2024 at 03:05, Eric Gallager <eg...@gwmail.gwu.edu> wrote: >> > >> > On Tue, Oct 8, 2024 at 6:25 AM Richard Sandiford >> > <richard.sandif...@arm.com> wrote: >> > > >> > > Christophe Lyon <christophe.l...@linaro.org> writes: >> > > > When --enable-werror is enabled when running the top-level configure, >> > > > it passes --enable-werror-always to subdirs. Some of them, like >> > > > libgcc, ignore it. >> > > > >> > > > This patch adds support for it, enabled only for aarch64, to avoid >> > > > breaking bootstrap for other targets. >> > > > >> > > > The patch also adds -Wno-prio-ctor-dtor to avoid a warning when >> > > > compiling lse_init.c >> > > > >> > > > libgcc/ >> > > > * Makefile.in (WERROR): New. >> > > > * config/aarch64/t-aarch64: Handle WERROR. Always use >> > > > -Wno-prio-ctor-dtor. >> > > > * configure.ac: Add support for --enable-werror-always. >> > > > * configure: Regenerate. >> > > > --- >> > > > libgcc/Makefile.in | 1 + >> > > > libgcc/config/aarch64/t-aarch64 | 1 + >> > > > libgcc/configure | 31 +++++++++++++++++++++++++++++++ >> > > > libgcc/configure.ac | 5 +++++ >> > > > 4 files changed, 38 insertions(+) >> > > > >> > > > [...] >> > > > diff --git a/libgcc/configure.ac b/libgcc/configure.ac >> > > > index 4e8c036990f..6b3ea2aea5c 100644 >> > > > --- a/libgcc/configure.ac >> > > > +++ b/libgcc/configure.ac >> > > > @@ -13,6 +13,7 @@ sinclude(../config/unwind_ipinfo.m4) >> > > > sinclude(../config/gthr.m4) >> > > > sinclude(../config/sjlj.m4) >> > > > sinclude(../config/cet.m4) >> > > > +sinclude(../config/warnings.m4) >> > > > >> > > > AC_INIT([GNU C Runtime Library], 1.0,,[libgcc]) >> > > > AC_CONFIG_SRCDIR([static-object.mk]) >> > > > @@ -746,6 +747,10 @@ AC_SUBST(HAVE_STRUB_SUPPORT) >> > > > # Determine what GCC version number to use in filesystem paths. >> > > > GCC_BASE_VER >> > > > >> > > > +# Only enable with --enable-werror-always until existing warnings are >> > > > +# corrected. >> > > > +ACX_PROG_CC_WARNINGS_ARE_ERRORS([manual]) >> > > >> > > It looks like this is borrowed from libcpp and/or libdecnumber. >> > > Those are a bit different from libgcc in that they're host libraries >> > > that can be built with any supported compiler (including non-GCC ones). >> > > In constrast, libgcc can only be built with the corresponding version >> > > of GCC. The usual restrictions on -Werror -- only use it during stages >> > > 2 and 3, or if the user explicitly passes --enable-werror -- don't apply >> > > in libgcc's case. We should always be building with the "right" version >> > > of GCC (even for Canadian crosses) and so should always be able to use >> > > -Werror. >> > > >> > > So personally, I think we should just go with: >> > > >> > > diff --git a/libgcc/config/aarch64/t-aarch64 >> > > b/libgcc/config/aarch64/t-aarch64 >> > > index b70e7b94edd..ae1588ce307 100644 >> > > --- a/libgcc/config/aarch64/t-aarch64 >> > > +++ b/libgcc/config/aarch64/t-aarch64 >> > > @@ -30,3 +30,4 @@ LIB2ADDEH += \ >> > > $(srcdir)/config/aarch64/__arm_za_disable.S >> > > >> > > SHLIB_MAPFILES += $(srcdir)/config/aarch64/libgcc-sme.ver >> > > +LIBGCC2_CFLAGS += $(WERROR) -Wno-prio-ctor-dtor >> > > >> > > ...this, but with $(WERROR) replaced by -Werror. >> > > >> > > At least, it would be a good way of finding out if there's a case >> > > I've forgotten :) >> > > >> > > Let's see what others think though. >> > >> > I think it would be worthwhile to test this assumption first; I have a >> > vague memory of having seen warnings in libgcc previously that would >> > presumably get turned into errors if -Werror were applied >> > unconditionally... >> > >> Sorry, it's not clear to me what you mean by "test this assumption" ? >> Do you mean I should push the patch with unconditional -Werror and >> monitor what happens for a while? >> Or investigate more / other targets? >> Or wait for others to commit? >> > > I mean, I think we should try the original approach of having it be > enableable manually first, let some people test by enabling it > manually, and then if they all report back success, then we can go > ahead with the unconditional -Werror version of it.
I can see the attraction. But that would mean adding code only to take it out later. Plus, the original patch would enable -Werror for stages 2 and 3 anyway, so everyone who bootstraps would still be testing the -Werror path, whether they'd chosen to or not. My point is that stage 1 isn't really a different case from stages 2 and 3 for libgcc, since in all three cases, it's the recently built gcc that is being used to build libgcc. libgcc also gets the majority of its command-line flags directly from the gcc directory (via libgcc.mvars), rather than from libgcc's own configure line. How about going for an unconditional -Werror (for aarch64 only), but with a pre-approval for anyone with commit access to revert it if it breaks their build? Thanks, Richard
