On Tue, 22 Jul 2014, Richard Biener wrote: > On Fri, 18 Jul 2014, Hans-Peter Nilsson wrote: > > > On Fri, 18 Jul 2014, Jan-Benedict Glaw wrote: > > It should be per-target because there *may* be port-specific > > constructs warned about by buggy previous-but-not-ancient > > gcc-versions, where working around the warnings would cause > > unwanted obfuscation. (IIRC gdb does something like this.) > > > > Is that the reason it's not the default, that there are such > > constructs in the non-port-specific parts? But then that would > > have already been noticed through use of the config-list.mk, no? > > The reason it's not the default is because the warnings are coming > from the host compiler which may be fairly old or even broken.
But that's the *general* reason -Werror is not always on, so I'm inclined to think that reply implies "and no-one has bothered to look into making an exception for non-release cross-builds". *Developers* (or rather, people cross-building non-released gcc source in their usual setup) don't use the fairly old or even broken host gcc versions that can be expected in use in the general public (well, the users that still want to build gcc from releases and not use pre-built binaries). > If we want it to make the default the we should restrict it > based on the host compiler (version). My point is that it's unnecessary; we should just enable it always *for cross-builds only* (native cases will see it for stage 2 anyway) and deal with the fallout. Bringing the non-fatal errors out in the open has value above the trouble dealing with any breakage. Also, exceptions should be simple to do per-target for the reasons mentions. (Actually, I ended up enabling both as the version checking was already there.) But, the above was written under the assumption that most targets *do* build these days with --enable-werror-always for most non-ancient gcc (say, 4.4 and up), but I no longer think that's the case after testing the patch. In the name of "dealing with the fallout": with the patch below (don't forget to re-generate configure) I get build errors in generic code r212915 for *both* x86_64 "gcc version 4.7.2 20120921 (Red Hat 4.7.2-2) (GCC)" for mmix-knuth-mmixware: g++ -c -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common -DHAVE_CONFIG_H -I. -I. -I/home/hp/gcctop/tmp/mbase13/gcc/gcc -I/home/hp/gcctop/tmp/mbase13/gcc/gcc/. -I/home/hp/gcctop/tmp/mbase13/gcc/gcc/../include -I/home/hp/gcctop/tmp/mbase13/gcc/gcc/../libcpp/include -I/home/hp/gcctop/tmp/mbase13/gcc/gcc/../libdecnumber -I/home/hp/gcctop/tmp/mbase13/gcc/gcc/../libdecnumber/dpd -I../libdecnumber -I/home/hp/gcctop/tmp/mbase13/gcc/gcc/../libbacktrace -o dwarf2out.o -MT dwarf2out.o -MMD -MP -MF ./.deps/dwarf2out.TPo /home/hp/gcctop/tmp/mbase13/gcc/gcc/dwarf2out.c In file included from /home/hp/gcctop/tmp/mbase13/gcc/gcc/real.h:25:0, from /home/hp/gcctop/tmp/mbase13/gcc/gcc/rtl.h:27, from /home/hp/gcctop/tmp/mbase13/gcc/gcc/dwarf2out.c:62: /home/hp/gcctop/tmp/mbase13/gcc/gcc/wide-int.h: In function 'void insert_wide_int(const wide_int&, unsigned char*, int)': /home/hp/gcctop/tmp/mbase13/gcc/gcc/wide-int.h:800:57: error: array subscript is above array bounds [-Werror=array-bounds] cc1plus: all warnings being treated as errors make[2]: *** [dwarf2out.o] Error 1 *and* "gcc version 4.4.4 20100630 (Red Hat 4.4.4-10) (GCC)" with cris-elf gets at the same revision: g++ -c -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common -DHAVE_CONFIG_H -DGENERATOR_FILE -I. -Ibuild -I/tmp/werralw/gcc/gcc -I/tmp/werralw/gcc/gcc/build -I/tmp/werralw/gcc/gcc/../include -I/tmp/werralw/gcc/gcc/../libcpp/include \ -o build/genpreds.o /tmp/werralw/gcc/gcc/genpreds.c cc1plus: warnings being treated as errors In file included from /tmp/werralw/gcc/gcc/rtl.h:28, from /tmp/werralw/gcc/gcc/genpreds.c:27: /tmp/werralw/gcc/gcc/vec.h: In static member function 'static size_t vec<T, A, vl_embed>::embedded_size(unsigned int) [with T = std::pair<unsigned int, const char*>, A = va_heap]': /tmp/werralw/gcc/gcc/vec.h:308: instantiated from 'static void va_heap::reserve(vec<T, va_heap, vl_embed>*&, unsigned int, bool) [with T = std::pair<unsigned int, const char*>]' /tmp/werralw/gcc/gcc/vec.h:1428: instantiated from 'bool vec<T, va_heap, vl_ptr>::reserve(unsigned int, bool) [with T = std::pair<unsigned int, const char*>]' /tmp/werralw/gcc/gcc/vec.h:1537: instantiated from 'T* vec<T, va_heap, vl_ptr>::safe_push(const T&) [with T = std::pair<unsigned int, const char*>]' /tmp/werralw/gcc/gcc/genpreds.c:1383: instantiated from here /tmp/werralw/gcc/gcc/vec.h:1048: error: invalid access to non-static data member 'vec<std::pair<unsigned int, const char*>, va_heap, vl_embed>::m_vecdata' of NULL object /tmp/werralw/gcc/gcc/vec.h:1048: error: (perhaps the 'offsetof' macro was used incorrectly) make[2]: *** [build/genpreds.o] Error 1 I guess both of these can be attributed to "fairly old or even broken" host gcc if you want to stretch it... Jan-Benedict, which host gcc version do you use when getting most targets to build with config-list.mk? Maybe we can just set the initial version to that instead of 4.4.4. For reference, the patch, which works as intended (-Werror in the gcc build directory for cross-builds by default, not affecting native builds and not at all for gcc < 4.4.4). (Vax is excepted, see J-B's previous post.) I'd ask for approval except now the fallout seems excessive as both my common setups fail building, while having reasonable testsuite results, and I don't know my way around the erroring part of the code: config: * aclocal.m4 (ACX_PROG_CC_WARNINGS_ARE_ERRORS): Evaluate parameter 1 at configure-time, not at autoconf-time. gcc: * configure.ac: For cross-builds of non-releases with gcc 4.4.4 and newer, enable -Werror. Provide per-target exceptions, and do except vax-*. (is_release): Move before all other "warnings and checkings" code. * configure, aclocal.m4: Regenerate. Index: config/warnings.m4 =================================================================== --- config/warnings.m4 (revision 212909) +++ config/warnings.m4 (working copy) @@ -98,7 +98,7 @@ AC_ARG_ENABLE(werror-always, [], [enable_werror_always=no]) AS_IF([test $enable_werror_always = yes], [acx_Var="$acx_Var${acx_Var:+ }-Werror"]) - m4_if($1, [manual],, + AS_IF([test "x$1" != xmanual], [AS_VAR_PUSHDEF([acx_GCCvers], [acx_cv_prog_cc_gcc_$1_or_newer])dnl AC_CACHE_CHECK([whether $CC is GCC >=$1], acx_GCCvers, [set fnord `echo $1 | tr '.' ' '` Index: gcc/configure.ac =================================================================== --- gcc/configure.ac (revision 212909) +++ gcc/configure.ac (working copy) @@ -349,6 +349,11 @@ AC_LANG_POP(C++) # Warnings and checking # --------------------- +is_release= +if test x"`cat $srcdir/DEV-PHASE`" != xexperimental; then + is_release=yes +fi + # Check $CC warning features (if it's GCC). # We want to use -pedantic, but we don't want warnings about # * 'long long' @@ -377,7 +382,32 @@ ACX_PROG_CC_WARNING_OPTS( ACX_PROG_CC_WARNING_ALMOST_PEDANTIC( m4_quote(m4_do([-Wno-long-long -Wno-variadic-macros ], [-Wno-overlength-strings])), [strict_warn]) -ACX_PROG_CC_WARNINGS_ARE_ERRORS([manual], [strict_warn]) + + +werror_gcc_version=manual + +# For cross-building, we will not have a stage 2 (where build-time +# warnings-as-errors is default on for all targets), so make sure we +# error on warnings for sufficiently new gcc anyway. +# Some targets are excepted. +if test x$host != x$target && test x$is_release = x ; then + case $target in + # These targets have (bogus) warnings for some target-specific + # construct and some fairly recent gcc. Also, working around the + # warning would obfuscate the source, so just never enable -Werror + # by default. + vax-*-*) ;; + *) + # The version here is just a random sufficiently old version + # that someone has shown warning-free for some target. + # Changing it to a newer version is expected, when changes to + # generic code upsets this version, and a newer gcc version is + # happy. + werror_gcc_version=4.4.4 ;; + esac +fi + +ACX_PROG_CC_WARNINGS_ARE_ERRORS([$werror_gcc_version], [strict_warn]) # The above macros do nothing if the compiler is not GCC. However, the # Makefile has more goo to add other flags, so these variables are used @@ -397,11 +427,6 @@ ACX_PROG_CC_WARNING_OPTS( [noexception_flags]) # Enable expensive internal checks -is_release= -if test x"`cat $srcdir/DEV-PHASE`" != xexperimental; then - is_release=yes -fi - AC_ARG_ENABLE(checking, [AS_HELP_STRING([[--enable-checking[=LIST]]], [enable expensive run-time checks. With LIST, brgds, H-P