On Jul 22, 2014, at 1:40 PM, Hans-Peter Nilsson <h...@bitrange.com> wrote:
> 
> *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).

:-)  Speak for yourself.  I do cross, I deliver cross, and we just use the same 
old /bin/gcc that everyone else uses.  And, we might well deliver on OSes other 
than the one released last week.  In my case, /bin/gcc is 4.4.7.  Though, I 
usually develop on 4.6.3 and 4.8.2.  So, what I want is software that builds 
and works.  I object to any patch that causes gcc to not build.

Now, how do we ensure that gcc builds in the face of Werror?  This is easy, 
that option can only be added when the host compiler and version is in a 
whitelist of known good versions.  Why _must_ we do this?  Because a newer gcc 
_will_ add new checking, that new checking will break the build.  The only way 
to not break that build is to never ever add another warning to gcc, which, we 
are not going to sign up for, or, to have a white list that doesn’t include a 
version number of unreleased gccs.  That’s it.  If you are uninterested in 
testing if a particular host compiler works or not, then they by definition 
don’t want the build to fail.

This is isn’t theoretic.  I do cross builds on 4.4.7, I don’t want my builds to 
just break.  I’ve seen builds break with Werror with newer compilers.

To remain other than theoretic, I did a build of my tree with Werror.  Two 
problems in my code, happy to fix, I’ve fixed them, but… then it when on to 
break with the vec and offsetof problem that you’ve previously seen.  That’s 
not something I want to spend my days scratching my head at.  And this _is_ why 
the person to do the fixing, is the person that _adds_ that exact version to 
the white list.  Not some other random person.  Once it is added to the white 
list, sure, problems might creep up, and those we can deal with, as you say, 
it.  The obscure things, we should not build with Werror.  My build is 
apparently obscure, as it doesn’t build.  You want to add it to the white list 
for Werror, fix the build, _then_ add it to the list.

>> 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.

So, there are two schools of thought.  One is I wanna break the build to force 
people to do work for me that I refuse to do myself, and the other is I don’t 
want to break the build.  Which school do you come from?  I’m from the later.  
I am unsympathetic to the first, as if they wanted to contribute patches to gcc 
to make it nicer, they could, at any time.  If an older gcc generated nice 
warnings that a newer version doesn’t, they can compile with that version and 
contribute that work.  If an older version of gcc spat out wrong warnings, 
there is no point in breaking that build.  The warnings are wrong, they have 
been removed from gcc, and there is no reason to ask people to put crap into 
the source base to try and work around those warnings.  The proper solution is 
to remove that compiler version from the list of versions to include -Werror 
with.

You want to deal with the fallout, then build all the crosses on the host 
compiler you want to validate and then add that version to the whitelist after 
you have dealt with all the fall out…

> Bringing the non-fatal errors out in the open has value

But that isn’t overcome all the broken builds for all the people that 
experience all the broken builds.

> above the trouble dealing with any breakage.

To be very concrete, what you miss is that the people experiencing:

g++ -c   -g3 -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../../gcc/gcc 
-I../../gcc/gcc/build -I../../gcc/gcc/../include  
-I../../gcc/gcc/../libcpp/include  \
        -o build/genpreds.o ../../gcc/gcc/genpreds.c
cc1plus: warnings being treated as errors
In file included from ../../gcc/gcc/rtl.h:28,
                 from ../../gcc/gcc/genpreds.c:27:
../../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]’:
../../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*>]’
../../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*>]’
../../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*>]’
../../gcc/gcc/genpreds.c:1383:   instantiated from here
../../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
../../gcc/gcc/vec.h:1048: error: (perhaps the ‘offsetof’ macro was used 
incorrectly)
make: *** [build/genpreds.o] Error 1
g++ -c   -g3 -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../../gcc/gcc 
-I../../gcc/gcc/build -I../../gcc/gcc/../include  
-I../../gcc/gcc/../libcpp/include  \
        -o build/genopinit.o ../../gcc/gcc/genopinit.c
cc1plus: warnings being treated as errors
../../gcc/gcc/genopinit.c: In function ‘int main(int, char**)’:
../../gcc/gcc/genopinit.c:516: error: comparison between signed and unsigned 
integer expressions
make: *** [build/genopinit.o] Error 1
make: Target `all' not remade because of errors.

want to spend their time trying to puzzle out what went wrong, and why and how 
to fix it, and that there are a ton of these people and then all will hit it 
and you will waste all their time with a broken build.  You are not sensitive 
enough to this wastage.  You want the reports, great, there it is.  Now what?  
Causing 1 more person to waste their time on the issue now that I have (and 
apparently you have) doesn’t add value.  All we will likely do is to google it, 
find a web page that says gcc sucks, it doesn’t build, and to make it build you 
have to go edit some cryptic file and change it is exceedingly cryptic ways.  I 
don’t want that world.

> 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.)

Then remove 4.4 from the white list.  It doesn’t appear to work.  I can waste 
yet more time and try 4.6.3:

make -k
gcc -c -g -O2 -W -Wall -Wwrite-strings -Wstrict-prototypes -Wmissing-prototypes 
-Wold-style-definition -Wmissing-format-attribute -Wno-overlength-strings 
-pedantic -Wno-long-long -Werror  -DHAVE_CONFIG_H -I. -I../../gcc/fixincludes 
-I../include -I../../gcc/fixincludes/../include ../../gcc/fixincludes/server.c
../../gcc/fixincludes/server.c: In function ‘server_setup’:
../../gcc/fixincludes/server.c:195:10: error: ignoring return value of 
‘getcwd’, declared with attribute warn_unused_result [-Werror=unused-result]
cc1: all warnings being treated as errors
make[2]: *** [server.o] Error 1
make[2]: Target `all' not remade because of errors.
gcc -c -g -O2 -W -Wall -Wwrite-strings -Wstrict-prototypes -Wmissing-prototypes 
-Wold-style-definition -Wmissing-format-attribute -Wno-overlength-strings 
-pedantic -Wno-long-long -Werror  -DHAVE_CONFIG_H -I. 
-I../../../gcc/fixincludes -I../include -I../../../gcc/fixincludes/../include 
../../../gcc/fixincludes/server.c
../../../gcc/fixincludes/server.c: In function ‘server_setup’:
../../../gcc/fixincludes/server.c:195:10: error: ignoring return value of 
‘getcwd’, declared with attribute warn_unused_result [-Werror=unused-result]
cc1: all warnings being treated as errors
make[2]: *** [server.o] Error 1
g++  -I../../gcc/libcpp -I. -I../../gcc/libcpp/../include 
-I../../gcc/libcpp/include  -g -O2 -W -Wall -Wwrite-strings 
-Wmissing-format-attribute -pedantic -Wno-long-long -Werror -fno-exceptions 
-fno-rtti -I../../gcc/libcpp -I. -I../../gcc/libcpp/../include 
-I../../gcc/libcpp/include   -c -o directives.o -MT directives.o -MMD -MP -MF 
.deps/directives.Tpo ../../gcc/libcpp/directives.c
../../gcc/libcpp/directives.c: In function ‘void 
cpp_define_formatted(cpp_reader*, const char*, ...)’:
../../gcc/libcpp/directives.c:2380:28: error: ignoring return value of ‘int 
vasprintf(char**, const char*, __va_list_tag*)’, declared with attribute 
warn_unused_result [-Werror=unused-result]
cc1plus: all warnings being treated as errors
make[2]: *** [directives.o] Error 1
g++  -I../../gcc/libcpp -I. -I../../gcc/libcpp/../include 
-I../../gcc/libcpp/include  -g -O2 -W -Wall -Wwrite-strings 
-Wmissing-format-attribute -pedantic -Wno-long-long -Werror -fno-exceptions 
-fno-rtti -I../../gcc/libcpp -I. -I../../gcc/libcpp/../include 
-I../../gcc/libcpp/include   -c -o expr.o -MT expr.o -MMD -MP -MF 
.deps/expr.Tpo ../../gcc/libcpp/expr.c
../../gcc/libcpp/expr.c: In function ‘unsigned int 
cpp_classify_number(cpp_reader*, const cpp_token*, const char**, 
source_location)’:
../../gcc/libcpp/expr.c:672:18: error: format not a string literal and no 
format arguments [-Werror=format-security]
../../gcc/libcpp/expr.c:675:39: error: format not a string literal and no 
format arguments [-Werror=format-security]
cc1plus: all warnings being treated as errors
make[2]: *** [expr.o] Error 1
g++  -I../../gcc/libcpp -I. -I../../gcc/libcpp/../include 
-I../../gcc/libcpp/include  -g -O2 -W -Wall -Wwrite-strings 
-Wmissing-format-attribute -pedantic -Wno-long-long -Werror -fno-exceptions 
-fno-rtti -I../../gcc/libcpp -I. -I../../gcc/libcpp/../include 
-I../../gcc/libcpp/include   -c -o macro.o -MT macro.o -MMD -MP -MF 
.deps/macro.Tpo ../../gcc/libcpp/macro.c
../../gcc/libcpp/macro.c: In function ‘bool create_iso_definition(cpp_reader*, 
cpp_macro*)’:
../../gcc/libcpp/macro.c:2971:58: error: format not a string literal and no 
format arguments [-Werror=format-security]
../../gcc/libcpp/macro.c:2984:58: error: format not a string literal and no 
format arguments [-Werror=format-security]
cc1plus: all warnings being treated as errors
make[2]: *** [macro.o] Error 1

nope, doesn’t work either, you can remove it from the white list as well.  Now, 
even if both of these issues I hit are fixed that just means that one can add 
those two versions to the white list.  If 4.6.0 works and 4.6.3 works, I’m fine 
with adding all of 4.6.x on the assumption that the other versions won’t hit.

> 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.

Bingo.

> 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:

But that isn’t a fix.  It is a mere report that doing the change isn’t 
desirable.  The first person that hits a breakage should fix it and check it 
in, or, it harder, turn it off and file a bug report.  The closer of that bug 
report can fix it, and readable it.  So the question isn’t let’s imagine a 
world where 4.4 and later all work, let us ask instead on what version _is it 
known_ to work?

Reply via email to