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

Reply via email to