Wielaard: Looking forward to GCC6 – Many new warnings

Posted Feb 15, 2016 20:04 UTC (Mon) by juliank (guest, #45896)
In reply to: Wielaard: Looking forward to GCC6 – Many new warnings by mjw
Parent article: Wielaard: Looking forward to GCC6 – Many new warnings

Just adopt -Weverything from clang?

Wielaard: Looking forward to GCC6 – Many new warnings

Posted Feb 16, 2016 1:33 UTC (Tue) by pbonzini (subscriber, #60935) [Link] (43 responses)

-Weverything doesn't make much sense, for example -Wtemplates definitely shouldn't be part of it.

Wielaard: Looking forward to GCC6 – Many new warnings

Posted Feb 16, 2016 1:58 UTC (Tue) by Elv13 (subscriber, #106198) [Link] (41 responses)

The point of -Weverything is a case of opt-in vs. opt-out. You usually want those warnings, but many projects devs either don't know they exist, don't follow compilers development or don't bother enabling them. At least with -Weverything, you have to just disable all the noise. The main problem is using this with -Werror. When a new version of the compiler is released, it might catch new case and break the build. This is why enabling some -Werror-something only in debug builds make sense, but you may still want to see those -Weverything warnings in all modes. I would also like a -Weverything=gcc6 type option instead of the CMake module I carry across multiple projects to track compiler version and enable warnings (with a else if clang then -Weverything -Wno-...)

Wielaard: Looking forward to GCC6 – Many new warnings

Posted Feb 16, 2016 3:58 UTC (Tue) by Sesse (subscriber, #53779) [Link] (40 responses)

I would go as far as to say you should never enable -Werror in released source code. It tends to create blockers for distributions to move to newer GCC versions, for no good reason at all.

Of course, when developing, -Werror is great.

/* Steinar */

Wielaard: Looking forward to GCC6 – Many new warnings

Posted Feb 16, 2016 4:11 UTC (Tue) by neilbrown (subscriber, #359) [Link] (35 responses)

I like -Werror in source code releases. It encourages people to report warnings on platforms I haven't tested.

Of course I support some easy way to disable it like
make CWFLAGS=

Wielaard: Looking forward to GCC6 – Many new warnings

Posted Feb 16, 2016 6:24 UTC (Tue) by Sesse (subscriber, #53779) [Link] (28 responses)

I'm sure it's great for you; it's not great when you have a hundred projects like that breaking during a distro rebuild, requiring manual review and action (at the very least, turning off the warnings) for all of them. :-)

/* Steinar */

Wielaard: Looking forward to GCC6 – Many new warnings

Posted Feb 16, 2016 6:57 UTC (Tue) by torquay (guest, #92428) [Link] (23 responses)

it's not great when you have a hundred projects like that breaking during a distro rebuild, requiring manual review and action

It's clear that having per-project defaults is not scalable from a distro point of view. However, is the argument "it's bad for the distro, so the project is bad" ? I'd argue that it's the distro getting in the way, not the other way around.

Perhaps distros should get out of the business of providing mudballs requiring constant rebuilds of everything, and instead focus on providing a stable base operating system?

Ceterum Censeo

Posted Feb 16, 2016 7:57 UTC (Tue) by oldtomas (guest, #72579) [Link] (20 responses)

As in [https://en.wikipedia.org/wiki/Carthago_delenda_est].

Yeah, yeah. We know. Some disagree. Now back to GCC6, shall we?

Ceterum Censeo

Posted Feb 16, 2016 8:54 UTC (Tue) by torquay (guest, #92428) [Link] (19 responses)

Umm? Project XYZ wants to use -Werror (from GCC6 and beyond) because the developer is (rightly) paranoid about bugs. Distro throws the dummy and says no, because it would take too much effort in existing as a distro. The problem here is not the developer, but the rather arbitrary overriding of developers' choices.

There is no burning of Carthage here, but merely an indication that distros in their current state are not fit for purpose. Rather than changing Project XYZ to fit into an arbitrary definition of a good project (which changes from distro to distro), how about we let the project developer compile the project as he/she sees fit, and then just sandbox the result? Let the distro evolve to provide the sandbox and the security policies that go with it.

Ceterum Censeo

Posted Feb 16, 2016 9:42 UTC (Tue) by epa (subscriber, #39769) [Link]

It is common for projects to have build targets make test, which can be done as part of package building every time, and make maintainer-test, which turns on extra checks and doodads intended for the program's maintainer or developers. Building with -Werror could be done as part of maintainer-test only.

It would sure be useful for compiler warnings in package building to be reported back upstream somehow, and automatically - but the amount of spam this would generate makes it impractical unless you have somebody employed full time on build cleanup janitorial tasks.

Ceterum Censeo

Posted Feb 16, 2016 11:37 UTC (Tue) by oldtomas (guest, #72579) [Link] (17 responses)

Look, I don't want to enter a spat about this with you.

Thing is, for me Project XYZ's developer isn't God; sometimes I trust the distro's maintainer more (especially for those programs on my box which get less attention from me). The few (alas, too few: my capacity is far too small) which get more attention I do compile myself.

Why do I trust the packager more?

Focus. The maintainer's focus is to integrate the program into a consistent whole, whereas Project XYZ's maintainer's focus sometimes is "my program is the single most important thing out there", which is understandable, but counterproductive[1]. As long as the distro's maintainer makes her decisions transparent to me, I'm infinitely thankful for her doing this job for me. And (for me, still) this job is becoming ever more essential the more strident those "open source thingies" are becoming out there. Sometimes they remind me of spoiled little children[1].

Now I know you strongly disagree -- and I don't think each of us will convince the other; so having clarified our standpoints enough, let's leave it at that. If you want to have the last word, go ahead... this is *my* last one.

[1] There are great exceptions to that, for sure.

Ceterum Censeo

Posted Feb 16, 2016 17:23 UTC (Tue) by mpr22 (subscriber, #60784) [Link] (15 responses)

Given the choice between trusting someone who turns on compiler warnings and puts -Werror in CFLAGS, and someone who turns off compiler warnings and/or removes -Werror from CFLAGS, I would definitely prefer to trust the former.

Ceterum Censeo

Posted Feb 16, 2016 18:50 UTC (Tue) by bronson (subscriber, #4806) [Link] (11 responses)

So, the type of person that creates the Debian SSL bug?

Pretty sure you've set up a false dichotomy there.

Ceterum Censeo

Posted Feb 17, 2016 11:29 UTC (Wed) by itvirta (guest, #49997) [Link] (10 responses)

That would be the type of people who deliberately access uninitialized memory.
They are surely not to be trusted, but that has little to do with warnings.

Ceterum Censeo

Posted Feb 17, 2016 12:05 UTC (Wed) by juliank (guest, #45896) [Link] (4 responses)

Fun fact: We use uninitialized memory in APT. That is, for the binary (mmap()able) cache, we allocate memory and initialise parts of that.

We store all of it though, and calculate a checksum of it all, including the not yet initialised parts.

Ceterum Censeo

Posted Feb 17, 2016 19:20 UTC (Wed) by ksandstr (guest, #60862) [Link] (3 responses)

However, POSIX initializes fresh mmap() memory to all zeroes, but uninitialized automatic arrays in a function are truly uninitialized in that they contain arbitrary detritus from the thread stack (whatever that was un-/initialized to). In essence, APT performs a well-defined function that yields consistent results despite "you" thinking it doesn't.

Ceterum Censeo

Posted Feb 17, 2016 19:43 UTC (Wed) by juliank (guest, #45896) [Link] (2 responses)

Thing is, we don't really mmap() for writing anymore (only for reading), but use new[] instead (or malloc, idk right now). We then do a nearly-atomic write to temporary cache & rename transaction.

Valgrind currently complains about that, but oh well, it's at least clearly identifiable. Should still get fixed at some point.

Ceterum Censeo

Posted Feb 17, 2016 20:52 UTC (Wed) by ksandstr (guest, #60862) [Link] (1 responses)

Whether it's using new[] or malloc is, in fact, very important here: new[] initializes values to the default constructor of the array subscript type (zeroes for integral and floating-point types; the same rule is applied to all members of structs to make up an implicit default constructor), while malloc() does no initialization at all.

So if you're seeing consistent results, you're either using new[] or malloc() in a process that hasn't freed much of the memory it's allocated and written yet. But sure enough, valgrind does complain about uninitialized values when reading from new[]'d arrays -- this should be reported and corrected.

Back in 2006 it was still realistic that valgrind wouldn't become yet another mindlessly-followed lint. It had innovations like data tracking instrumentation and what-not. Alas.

Ceterum Censeo

Posted Feb 21, 2016 13:34 UTC (Sun) by ms_43 (subscriber, #99293) [Link]

> new[] initializes values to the default constructor of the array subscript type

yes (if an explicit initialization expression is omitted).

https://github.com/cplusplus/draft/blob/master/source/exp...

> (zeroes for integral and floating-point types;

no, that would be zero-initialization!

operator new performs default-initialization, which calls an applicable no-argument constructor for class types, otherwise does nothing.

https://github.com/cplusplus/draft/blob/01c0772e922e5edcd...

Ceterum Censeo

Posted Feb 17, 2016 16:05 UTC (Wed) by bronson (subscriber, #4806) [Link] (4 responses)

The direct reason the SSL bug entered Debian was to get rid of the warning.

Or, if you're saying that it's normal to go commenting out lines of code to eradicate warnings -- because it's ultimately the fault of the person who wrote the questionable code -- then I guess this is a fine example of the problem. Thank you!

Remember, warning-driven development only works when you're using a perfect compiler.

Ceterum Censeo

Posted Feb 17, 2016 21:20 UTC (Wed) by itvirta (guest, #49997) [Link] (3 responses)

Well, of course commenting out lines without understanding what they do is either silly or plain stupid.

But in that particular case, the ultimate reason seems to be that someone tried to be smarter than they should have
been, and did something that is both wrong (in the language sense) and not very useful either.

Wrong, since as far as I can find, the point was to read uninitialized memory, and that results in undefined behaviour
with the full demons-flying-from-ones-mouth possibility included.

Not very useful since the whole thing was to "safeguard" against not having a good RNG by creating a really shitty RNG.
Same goes for the fact that the library used the PID to seed the RNG. PIDs aren't really secret or random and make for
a rather shitty RNG. Had the library not used such a weak addition, the final screw-up might have been detected sooner,
since all the outputs would have been exactly the same, instead of having the 2^15 different possibilities (or whatever
the number was).

All of this goes with a big IIRC disclaimer, so feel free to point me wrong.
However, I do find the custom of solely blaming a silly mistake while ignoring the fucked-up design really tiring.

Ceterum Censeo

Posted Feb 17, 2016 23:58 UTC (Wed) by tialaramex (subscriber, #21167) [Link] (2 responses)

Version of the story which is popular and you're repeating to us:

* OpenSSL is so crappy it relies on uninitialised memory reads to do random number generation even though, er, that can't work on Unix
* Hero Debian Guy fixes the uninitialised reads
* Somehow this breaks OpenSSL because it needed uninitialised reads, even though er, they didn't work so... um.

Actual reality of the story

* OpenSSL is so crappy it has several similar-looking C functions that actually do different stuff
* Debian Guy spots that function A does a harmless uninitialised read and fixes it, arguably making the code more "correct" but with no functional change.
* Idiot Debian Guy sees a similar function B and blindly applies the same "fix".
* The "fix" to this function B disables its entire purpose, effectively removing the PRNG feature almost entirely

I don't know if either story has anything to tell us about GCC6. But I do know that spreading the former story is harmful.

Ceterum Censeo

Posted Feb 19, 2016 14:13 UTC (Fri) by cortana (subscriber, #24596) [Link] (1 responses)

Your version of the story misses out several details that were covered in Debian, OpenSSL, and a lack of cooperation posted to a certain web site some time ago.

Ceterum Censeo

Posted Feb 19, 2016 19:12 UTC (Fri) by bronson (subscriber, #4806) [Link]

It was way better than the guesses itvirta posted though! And nice and succinct.

Ceterum Censeo

Posted Feb 17, 2016 0:17 UTC (Wed) by nix (subscriber, #2304) [Link] (2 responses)

You mean the sort of person who makes compiler warnings fatal on every machine the thing is compiled on, even though many warnings vary per-architecture and almost all vary per-compiler-version?

Yeah, I'd trust that sort of person a whole lot -- to make mistakes through ignorance.

Ceterum Censeo

Posted Feb 18, 2016 21:48 UTC (Thu) by mpr22 (subscriber, #60784) [Link] (1 responses)

My C and C++ code builds without warnings on my system, because I do all my development with -Werror enabled. Thus, if my C and C++ code produces warnings when compiled on someone else's system, then either their toolchain is (defective|deficient|misconfigured) or there is an issue with my code which needs to be properly examined by a human, and that someone else should probably not be distributing binaries to other people until it has been.

Ceterum Censeo

Posted Feb 18, 2016 21:54 UTC (Thu) by Sesse (subscriber, #53779) [Link]

That's a false dichotomy. What's much more likely is that someone used a different compiler which has a different set of warnings. Not all the world is GCC.

Most warnings, especially in code that's been around for a while, are _not_ signs of actual bugs. (It's good style to fix them nevertheless, because some are, but that should not block the compile for an end user.)

/* Steinar */

Ceterum Censeo

Posted Feb 16, 2016 18:16 UTC (Tue) by drag (guest, #31333) [Link]

> Thing is, for me Project XYZ's developer isn't God; sometimes I trust the distro's maintainer more

Distro maintainers can still work with projects and review code for them without needing to be involved in modifying them to be packaged for a specific distribution.

Arguably distro maintainers working to get changes into upstream and systematically eliminating distro-specific packaging requirements and build processes should allow a greater degree of collaboration between distributions. This should help significantly to maintain those packages which are critical, but do not warrant the full time development staff that many other types of software requires.

> The maintainer's focus is to integrate the program into a consistent whole, whereas Project XYZ's maintainer's focus sometimes is "my program is the single most important thing out there", which is understandable, but counterproductive[1].

It depends on the nature of the application.

Some applications are end-user focus and thus are considerably higher priority then some other software that focuses on infrastructure of the OS. Think about the difference between Firefox versus some sort of XML parsing library.

Once my imaginary XML parsing library works, is fast, and is stable there doesn't really need to have a full time team developing on it. You still need maintainers, but new development is discouraged by the nature of it being used by a wide variety of other applications. 'If it isn't broke don't fix it' is the name of the game in this instance. It's purpose in life is just to serve as a basis for new development. Having it be almost 'abandonware' and only be maintained for security purposes is can be a advantage.

In comparison Firefox browser has the triple duty of being a end-user facing application, dealing with a ever evolving set of standards and practices, as well as facing new threats and problems related to users needing to accessing unpredictable sources of unvetted and potentially malicious data on the internet. In this case is it is the job of the OS to bend for the application rather then the other way around. Firefox is significantly more important then the rest of the OS in a lot of ways and almost all situations in which it gets used.

Since the primary purpose of a operating system is to make developing and using end-user applications easier (since it is always the job of computers to 'do something' rather then sitting there consuming electricity for the sake of correctness), then it's the job of distributions to bend to the will of users and the applications they consume. It is true that a OS, as part of it's job description of 'making life easier for users', does have to enforce a certain amount of correctness on applications, however the ideal solution is to 'make it easier to do it right' rather then trying to second guess developers and modify their programs to suit the expectations of distributions.

Sometimes it's unavoidable, but in my mind if distributions have to hack on software then they are the new upstream, the new developers, and thus catering to the whims of distribution packagers is just as troublesome as catering to the whims of the original developer. This also has obvious scalability issues as you can't expect distributions to be experts in all the software in the known universe. A far better solution is to have distributions work together to get changes into upstream rather then trying to solve the problem 'downstream'.

Wielaard: Looking forward to GCC6 – Many new warnings

Posted Feb 16, 2016 10:10 UTC (Tue) by roblucid (guest, #48964) [Link]

The point of rebuild, is to ensure you can build & modify the software delivered.
From end users point of view, a distro's security updates are a real benefit, building from source & re-updating soon gets boring, especially if you have the problem of embedded bundled libraries.

Wielaard: Looking forward to GCC6 – Many new warnings

Posted Feb 17, 2016 21:09 UTC (Wed) by Sesse (subscriber, #53779) [Link]

Sure, when all upstream app developers commit to making their own packages for the 10–15 architectures people care about on Linux, and keep them up-to-date wrt. security and such.

/* Steinar */

Wielaard: Looking forward to GCC6 – Many new warnings

Posted Feb 16, 2016 9:22 UTC (Tue) by error27 (subscriber, #8346) [Link]

There should be a get_maintainer.pl for the whole distro so that reporting bugs can be done almost automatically. In the kernel I auto generate bug reports, look them over and hit send.

Wielaard: Looking forward to GCC6 – Many new warnings

Posted Feb 16, 2016 10:03 UTC (Tue) by roblucid (guest, #48964) [Link] (2 responses)

Packaging lets you apply patches, and tailor the build process. When building software from the developers, it's a good idea to actually look at their documentation and options chosen, hopefully the distro want to make packages with consistent build & feature set and selects the config appropriately. Turn off -Werror in the package, there once.

Wielaard: Looking forward to GCC6 – Many new warnings

Posted Feb 16, 2016 22:28 UTC (Tue) by Wol (subscriber, #4433) [Link] (1 responses)

aiui, LibreOffice actually has config options in the build system for "each" distro.

So you can take an upstream git pull, and run make for your distro. So the distro can tweak things, and they go upstream so the user can see what's going on - that is, if they can understand the build system ... :-)

Cheers,
Wol

Wielaard: Looking forward to GCC6 – Many new warnings

Posted Feb 21, 2016 13:48 UTC (Sun) by ms_43 (subscriber, #99293) [Link]

> aiui, LibreOffice actually has config options in the build system for "each" distro.

Where "distro" means "Linux", "MacOSX", "Win32" or "Android".

https://cgit.freedesktop.org/libreoffice/core/tree/distro...

Wielaard: Looking forward to GCC6 – Many new warnings

Posted Feb 16, 2016 7:02 UTC (Tue) by lkundrak (subscriber, #43452) [Link] (5 responses)

Also, it breaks the build. Sounds like a bad deal.

Wielaard: Looking forward to GCC6 – Many new warnings

Posted Feb 18, 2016 10:02 UTC (Thu) by mjw (subscriber, #16740) [Link] (4 responses)

I am somewhat surprised by the hostility against -Werror. Personally I use it religiously in my projects and take pride in having the build explicitly break if a warning slips through. I do select the set of warnings (normally at least -Wall and -Wextra and then some of the non-default ones after making sure they catch some real bugs, and then fix any false positive, like some of those in this blog post did). My experience with distros is actually pretty positive. Package maintainers do warn (pun intended!) us when the build breaks because a new warning is detected on some setup we didn't test yet ourselves and we work together to get the build clean again. In fact one real bug was found when Debian tried a mass rebuild with gcc6 because of -Wmisleading-indentation. It was an obscure bug that would probably not trigger in normal situations, but it was nice to find it because it would have been a pain to diagnose otherwise. You do have to have a good relationship with the package maintainers of your project to make -Werror work, but then it really is IMHO a pretty sweet deal.

Wielaard: Looking forward to GCC6 – Many new warnings

Posted Feb 18, 2016 13:44 UTC (Thu) by pizza (subscriber, #46) [Link] (3 responses)

> I am somewhat surprised by the hostility against -Werror.

It's awesome for the tail end of development work leading up to a release, but IMO it has no place in a release package if that package is intended to be compiled by random third parties. The same code on different versions of the compiler (or even the same compiler on different platforms) will result in different sets of warnings.

Wielaard: Looking forward to GCC6 – Many new warnings

Posted Feb 18, 2016 17:58 UTC (Thu) by mjw (subscriber, #16740) [Link]

But that is the whole point isn't it? If your setup is so different from the supported setups of the project as released that you get new warnings then you do need to carefully check them and ideally discuss with the project maintainers how to resolve them. In my experience that is exactly what happens. Distros that have architecture support not yet seen by the project contribute fixes back and we work together to make sure newer compiler warnings are fixed (which often means there was some subtle bug in the code in the first place). It grows the project to include these new environments as supported. And it makes sure no subtle bugs slip in because people ignore warnings. IMHO it is exactly as making sure your testsuite is zero fail and if not to abort the build so nothing is ever released that is known broken.

Wielaard: Looking forward to GCC6 – Many new warnings

Posted Mar 3, 2016 21:20 UTC (Thu) by nix (subscriber, #2304) [Link] (1 responses)

There is one special case where this doesn't apply: where the software is arch-dependent enough and delicate enough that it should only be compiled with specific compilers (or with compilers in a specific version range) and on supported arches. This is particularly true where the software is crucial and any new warnings are quite likely a sign of impending disaster. On such systems, -Werror is actually really useful.

glibc is the only such software I can think of right now, on Linux at least.

Wielaard: Looking forward to GCC6 – Many new warnings

Posted Mar 3, 2016 23:00 UTC (Thu) by bronson (subscriber, #4806) [Link]

Hyper-optimized codecs and graphics code... but that's still very specialized. Anyone who isn't sure if -Werror is necessary should ship with it off.

Wielaard: Looking forward to GCC6 – Many new warnings

Posted Feb 16, 2016 7:24 UTC (Tue) by josh (subscriber, #17465) [Link] (3 responses)

I'd love to see a -Werror=gcc-6 option or similar, such that any warning introduced after gcc-6 will *not* turn into an error. In that circumstance, enabling -Werror for the specific compiler version you know you've fixed all the warnings for seems fine.

Wielaard: Looking forward to GCC6 – Many new warnings

Posted Feb 16, 2016 7:49 UTC (Tue) by andresfreund (subscriber, #69562) [Link] (1 responses)

That sounds bothersome for GCC's authors: I bet immediately people would complain because GCC +1 issues new warnings in existing categories, because of e.g. better control flow analysis. Preventing that problem sounds like it might make changes noticeably more painful.

Wielaard: Looking forward to GCC6 – Many new warnings

Posted Feb 16, 2016 17:52 UTC (Tue) by k3ninho (subscriber, #50375) [Link]

>That sounds bothersome for GCC's authors
I'm sure that we can imagine a layer of indirection which manages the mapping of versions to classes of error, holding a matrix of version and features as the release notes would list. And that might (maybe) appear in some man page content. What free/open source projects have been nearly-adequate at doing is explaining the orthodoxy of how a user is expected to use their tool.

You know that thing where a newcomer to a community shows up and complains that feature X looks like it should do Y but instead it's structured to do A, B and/or C? There's a habitual gap in the community record that makes sense of the journey from one problem to a solution, to broader appeal and then on to a growing userbase and contributory community. There's almost room for a maxim: "the current solution looks like it does because we started over there, went round the houses, painted a few bike-sheds and ended up where we are now; of course you know where you want to be and, if you were going there, you wouldn't want to be starting from here."

In light of that polish, the shorthand for someone taking on a compiler upgrade and not evaluating the full extent of changes they will face remains the less-friendly RTFM.

K3n.

Wielaard: Looking forward to GCC6 – Many new warnings

Posted Feb 19, 2016 0:18 UTC (Fri) by dirtyepic (guest, #30178) [Link]

In half the cases it's not a new option throwing a warning at you, it's an old option that learned some new tricks (or likely, some new false positives).

Wielaard: Looking forward to GCC6 – Many new warnings

Posted Feb 18, 2016 9:49 UTC (Thu) by mjw (subscriber, #16740) [Link]

I agree that -Weverything is pretty silly. Not all warnings are equal. Some are like -Wtemplate or -Weffc++ are really about coding style (and don't normally signal a possible bug). And then there are warnings like -Wwrite-strings that change the type of some constructs effectively changing the code. Most people are probably not interested in such warnings in the first place.

But for other categories of warnings (where there is the potential that it does indeed signal a subtle bug that might need some analysis even if it could be a false positive) it would be really nice to have some way to enable them all and just see what warning results you get. Is it really too hard to define two or three categories of warnings that can easily be enabled all at once?

I certainly appreciate the GCC documentation. It is pretty good. And it was fun to compare the info pages of the current and new version of GCC to get an overview of all the new warning options to try out. I learned a lot! But an easier way to discover new warnings (especially those not enabled by default or through -Wall or -Wextra) would be appreciated.