Re: Should -Wmaybe-uninitialized be included in -Wall?
On Wed, 10 Jul 2013, Manuel López-Ibáñez wrote: This points to other ideas: 1) how about adding a helper switch to show what is included in Wall? such as -Wall-print Doesn't gcc -Q -Wall --help=warnings give you this? Yes, but...how would I know to use this? For example, gcc --help does not refer to -Q at all, so how would one know to use it. Yes, it's listed in gcc.info, though I am not sure that is easy enough to find. Gerald
Re: Should -Wmaybe-uninitialized be included in -Wall?
On Wed, Jul 10, 2013 at 06:11:11PM +0200, Andi Kleen wrote: FWIW basically -Werror -Wall defines a compiler version specific variant of C. May be great for individual developers, but it's always a serious mistake in any distributed Makefile. Not always. Any project large enough (or serious enough about build reproducibility) to include its own toolchain can be written in that compiler-version-specific subset and nonetheless be worked on by more than one person. This is not uncommon in the BSDs, for example; see instances of WARNS=4. It's an uncommon use case (and, I think, not a justification for changing -Wall), but it does exist and it is useful. --Jed
Re: Should -Wmaybe-uninitialized be included in -Wall?
On Thu, Jul 11, 2013 at 12:42 AM, Ryan Hill dirtye...@gentoo.org wrote: On Wed, 10 Jul 2013 07:49:18 -0500 Gabriel Dos Reis g...@integrable-solutions.net wrote: If we include a warning in -Wall then it is because we believe it to be generally useful and likely to uncover common bugs/mistakes. It is therefore reasonable for users to issue -Wall -Werror even in application delivery mode. Arg, no. -Werror is very useful for development and I'm sure that code quality increases because of it, but it should never be enabled by default for releases. I think about 80% of the bugs we've had filed so far for packages failing to build against 4.8 are due to -Werror. The fact that you have 80% failing to build is not in itself an argument for not including -Werror in release mode. The real issue is whether those warnings uncovered any real bugs. If they don't, then either we are emitting too many false positives, or those packages should have turned off the offending diagnostics. Also, several distros patch gcc to enable additional warnings by default (eg. Debian, Ubuntu, and Gentoo enable -Wformat=security) that upstream may not see or be interested in. It's a big enough headache that we had to ban use of -Werror from our tree (instead we flag important warnings and output them at the end of the build). Well, the issue isn't as clear cut as you make it sound. Some distros build service monitor compiler outputs for some of these warnings and abort builds even if the package does not supply -Werror. -- Gaby
Re: Should -Wmaybe-uninitialized be included in -Wall?
Jeff Law l...@redhat.com writes: On 07/10/2013 04:51 AM, Andreas Arnez wrote: OK, I may be biased, because I have *only* seen false positives with this warning so far. Others may have made better experience with it. It's found numerous bugs across many projects. The reduction in bug reports against GCC which turn out to be uninitialized variables in the user's code has dropped dramatically over the last decade (if only the same could be said for user malloc errors reported against glibc :( Interesting. Upstream GCC seems to have added this option to -Wall on April 26th in 2011. Have those projects been using the option explicitly before? What you're seeing may be an indication that most of the real bugs have been found squashed and what remains may be mostly false positives. Actually, I believe most of the maybe uninitialized false positives I saw were with code that hasn't been touched since many years. They suddenly appeared due to the introduction of this option in -Wall or due to regressions in GCC. My understanding of -Wall (so far) was that it shouldn't include warnings with false positives. No, that's not true at all. -Wall is a set of warnings that the GCC developers have found useful over time and strive to avoid triggering in our own code. We've also seen that -Wall maps reasonably well to problems other projects want to fix in their own code. We certainly evaluate whether or not the warning generates too many false positives when we decide whether or not to include it in -Wall. However, we certainly allow warnings which generate false positives to be in -Wall. Then maybe that's what should be documented for -Wall instead of the current (misleading) description. Particuarly I suggest to add a disclaimer about possible false positives, and to remove the easy to avoid. So instead of this: This enables all the warnings about constructions that some users consider questionable, and that are easy to avoid (or modify to prevent the warning), even in conjunction with macros. Maybe more something like this: This enables all the warnings about constructions that GCC developers consider questionable, or which GCC isn't sure about whether they might be questionable.
Re: Should -Wmaybe-uninitialized be included in -Wall?
On Thu, Jul 11, 2013 at 11:11:28AM +0200, Andreas Arnez wrote: On 07/10/2013 04:51 AM, Andreas Arnez wrote: OK, I may be biased, because I have *only* seen false positives with this warning so far. Others may have made better experience with it. It's found numerous bugs across many projects. The reduction in bug reports against GCC which turn out to be uninitialized variables in the user's code has dropped dramatically over the last decade (if only the same could be said for user malloc errors reported against glibc :( Interesting. Upstream GCC seems to have added this option to -Wall on April 26th in 2011. Have those projects been using the option explicitly before? You are misreading that change. On April 26th 2011 the -Wuninitialized option was just split into -Wuninitialized and -Wmaybe-uninitialized, to allow people to specify e.g. -Wno-error=maybe-uninitialized. So the warning was included in -Wall before as well, for many many years. Jakub
Re: Should -Wmaybe-uninitialized be included in -Wall?
Hi, On Thu, 11 Jul 2013, Gabriel Dos Reis wrote: Arg, no. -Werror is very useful for development and I'm sure that code quality increases because of it, but it should never be enabled by default for releases. I think about 80% of the bugs we've had filed so far for packages failing to build against 4.8 are due to -Werror. The fact that you have 80% failing to build is not in itself an argument for not including -Werror in release mode. The real issue is whether those warnings uncovered any real bugs. If they don't, then either we are emitting too many false positives, or those packages should have turned off the offending diagnostics. Not quite. The problem is that the compiler is used for two completely different things: 1) by developers to develop their code, a development too. They will use a certain version of GCC, hence enabling -Wall -Werror is indeed good practice; even with the expectation that the developer will eventually change the used GCC version implying different errors; this will merely result in the source base to be improved by that developer to also conform to the new warnings (and hopefully not in a way that it warns with the old compiler again) 2) by users of software (which includes distributors as one of the largest consumers of random source codes) as sort of install tool. This will most of the time _not_ be the version of the compiler that the developer used when the software was released. As we already determined adding -Werror effectively makes the software require a very specific GCC version, but only because of warnings. Individual consumers of software usually are not inherently interested in such warnings, and distributors have different ways of enforcing policies related to source code quality (e.g. by parsing build logs). Adding -Werror by developers for their released software is hence neither required, nor even advised, it'll merely make life harder for consumers. Then, of course, there's a mixture between consumers and developers, who might indeed be interested in warnings with the intention of helping the sources developers. Well, as they are developers themself, they should know a way to add -Werror themself. So, no, -Werror for delivered stuff is no good idea. Also, several distros patch gcc to enable additional warnings by default (eg. Debian, Ubuntu, and Gentoo enable -Wformat=security) that upstream may not see or be interested in. It's a big enough headache that we had to ban use of -Werror from our tree (instead we flag important warnings and output them at the end of the build). Well, the issue isn't as clear cut as you make it sound. Some distros build service monitor compiler outputs for some of these warnings and abort builds even if the package does not supply -Werror. Yes, one reason why -Werror for application delivery is unnecessary. Ciao, Michael.
Re: Should -Wmaybe-uninitialized be included in -Wall?
Jeff Law l...@redhat.com writes: On 07/09/2013 07:56 AM, Andreas Arnez wrote: Andrew Haley a...@redhat.com writes: On 07/09/2013 12:59 PM, Andreas Arnez wrote: With this situation at hand, I wonder whether it's a good idea to keep maybe-uninitialized included in -Wall. Projects which have been using -Wall -Werror successfully for many years are now forced to investigate non-existing bugs in their code. But maybe-uninitialized is very useful, and it's not really inappropriate for -Wall. The warning would be extremely useful, if false positives didn't obscure real problems. In its current state I consider its usefulness limited. Depends on your point of view. This topic has been hashed through repeatedly on this list. OK, I may be biased, because I have *only* seen false positives with this warning so far. Others may have made better experience with it. I personally like -Wall -Werror. While we do run into false positives and the set of false positives does change from release to release as a result of optimizations, I believe there's been an overall improvement in the quality of the codebases that use -Wall -Werror. Yes, I fully agree on the usefulness of -Wall -Werror and its contribution to the code base quality increase. I certainly see fewer bugs these days blamed on the compiler which are in fact uninitialized variables. That's good. But there are still some known false positives with maybe uninitialized. If there weren't, then this option could go away and be covered by -Wuninitialized instead. I'd like to revamp how we do path isolation to eliminate more of the false positives, but this warning is by its nature going to generate false positives. Right, that's the point. My understanding of -Wall (so far) was that it shouldn't include warnings with false positives. What I would ask is that folks continue to file bugs for false positives. While I can't guarantee we'll fix them, they do provide a good base of tests for path sensitive analysis. Yes, I can see how the current policy of including this warning in -Wall may help getting more coverage. I'd also suggest that when an initialization is added merely to avoid the warning that a comment be added to the code so that folks know it's merely to avoid the warning. Adding a bogus initialization is usually not a reasonable response to this warning anyway. For instance, consider cases where certain elements of an array (like in bug 57832) or of a large struct (like reported in bug 55288) are falsely claimed to may be uninitialized. Also consider cases where -Wmaybe-uninitialized reported the wrong variable, like in bug 55985. Besides, a bogus initialization would also obscure helpful warnings, like those emitted from -Wuninitialized. OK, it appears that most people (silently or explicitly) accept the inclusion of warnings with false positives in -Wall. I'm not hung up on this. It would just be nice to reflect this appropriately in the documentation.
Re: Should -Wmaybe-uninitialized be included in -Wall?
Tom Tromey tro...@redhat.com writes: gdb only enables it for the development branch, not for releases. If you're building from CVS you're expected to know how to either fix these problems or disable -Werror. Typically the fix is trivial; if you look through the archives you'll see fixes along these lines. Right -- except that false positives from -Wmaybe-uninitialized may actually not be trivial to fix, as demonstrated by the various GCC bugs reported against this option. I was mentioning GDB as an example mainly because of GCC bug 57287, GCC 4.9.0 fails to build GDB on Ubuntu 12.04, where somebody has obviously stumbled across this. Comment 12 in that bug hints at a false positive that's still present with upstream GCC, AFAIK.
Re: Should -Wmaybe-uninitialized be included in -Wall?
On Tue, Jul 9, 2013 at 4:57 PM, Jeff Law l...@redhat.com wrote: I personally like -Wall -Werror. While we do run into false positives and the set of false positives does change from release to release as a result of optimizations, I believe there's been an overall improvement in the quality of the codebases that use -Wall -Werror. I certainly see fewer bugs these days blamed on the compiler which are in fact uninitialized variables. Amen. If we include a warning in -Wall then it is because we believe it to be generally useful and likely to uncover common bugs/mistakes. It is therefore reasonable for users to issue -Wall -Werror even in application delivery mode. (I know I systematically turn it on for some of my programs and when delivering code base to my students.) In fact, if a warning generates too many false positives or annoying diagnostics for idiomatic constructs, then it is a good indication that it not ready for -Wall. -- Gaby
Re: Should -Wmaybe-uninitialized be included in -Wall?
Andrew Haley a...@redhat.com writes: On 07/09/2013 12:59 PM, Andreas Arnez wrote: With this situation at hand, I wonder whether it's a good idea to keep maybe-uninitialized included in -Wall. Projects which have been using -Wall -Werror successfully for many years are now forced to investigate non-existing bugs in their code. But maybe-uninitialized is very useful, and it's not really inappropriate for -Wall. I would question the appropriateness of using -Wall -Werror in production code. Unfortunately a number of projects do that. I regularly have to remove -Werror from shipped makefiles, and it's primarily due to this warning. Maybe not erroring on the maybe by default would be a good idea? -Andi -- a...@linux.intel.com -- Speaking for myself only
Re: Should -Wmaybe-uninitialized be included in -Wall?
On Jul 10, 2013, at 10:42 AM, Andi Kleen wrote: Andrew Haley a...@redhat.com writes: On 07/09/2013 12:59 PM, Andreas Arnez wrote: With this situation at hand, I wonder whether it's a good idea to keep maybe-uninitialized included in -Wall. Projects which have been using -Wall -Werror successfully for many years are now forced to investigate non-existing bugs in their code. But maybe-uninitialized is very useful, and it's not really inappropriate for -Wall. I would question the appropriateness of using -Wall -Werror in production code. Unfortunately a number of projects do that. I regularly have to remove -Werror from shipped makefiles, and it's primarily due to this warning. Maybe not erroring on the maybe by default would be a good idea? I would rather see such warnings fixed. They exist for a good reason. (If there are false warnings, those are compiler bugs to fix. But in most cases I have seen, the warnings are legit.) Changing -Werror to mean error out on *most* warnings would be a very serious mistake. paul
Re: Should -Wmaybe-uninitialized be included in -Wall?
On 07/10/2013 04:51 AM, Andreas Arnez wrote: Jeff Law l...@redhat.com writes: OK, I may be biased, because I have *only* seen false positives with this warning so far. Others may have made better experience with it. It's found numerous bugs across many projects. The reduction in bug reports against GCC which turn out to be uninitialized variables in the user's code has dropped dramatically over the last decade (if only the same could be said for user malloc errors reported against glibc :( What you're seeing may be an indication that most of the real bugs have been found squashed and what remains may be mostly false positives. When you run into false positives, are you creating bug reports for them? That's the best way to keep things improving. I certainly want to continue to see the precision of all maybe-foo warnings improve. That's good. But there are still some known false positives with maybe uninitialized. If there weren't, then this option could go away and be covered by -Wuninitialized instead. False positives for maybe-uninitialized uses are a fact of life without heroic analysis. However, I continue to see opportunities to improve GCC's ability to identify and optimize away unexecutable paths through the CFG. In my experience, identification and elimination of those paths consistently leads to better precision of maybe-foo style warnings. I'd like to revamp how we do path isolation to eliminate more of the false positives, but this warning is by its nature going to generate false positives. Right, that's the point. My understanding of -Wall (so far) was that it shouldn't include warnings with false positives. No, that's not true at all. -Wall is a set of warnings that the GCC developers have found useful over time and strive to avoid triggering in our own code. We've also seen that -Wall maps reasonably well to problems other projects want to fix in their own code. We certainly evaluate whether or not the warning generates too many false positives when we decide whether or not to include it in -Wall. However, we certainly allow warnings which generate false positives to be in -Wall. Adding a bogus initialization is usually not a reasonable response to this warning anyway. For instance, consider cases where certain elements of an array (like in bug 57832) or of a large struct (like reported in bug 55288) are falsely claimed to may be uninitialized. Also consider cases where -Wmaybe-uninitialized reported the wrong variable, like in bug 55985. Besides, a bogus initialization would also obscure helpful warnings, like those emitted from -Wuninitialized. I'm well aware of these issues. There was a great article on this topic that I've wanted to hunt down again and be able to reference from time to time, but haven't seen again. The general idea of the article was these bogus initializations can in some cases result in longer term maintenance problems. They claimed that the bogus initialization could hide real uninitialized uses as the nearby code was changed over time. If the bogus initialized value wasn't chosen very carefully incorrect behaviour would result. In many cases where was no safe value to use for the bogus initialization. Jeff
Re: Should -Wmaybe-uninitialized be included in -Wall?
On Wed, Jul 10, 2013 at 9:42 AM, Andi Kleen a...@firstfloor.org wrote: Andrew Haley a...@redhat.com writes: On 07/09/2013 12:59 PM, Andreas Arnez wrote: With this situation at hand, I wonder whether it's a good idea to keep maybe-uninitialized included in -Wall. Projects which have been using -Wall -Werror successfully for many years are now forced to investigate non-existing bugs in their code. But maybe-uninitialized is very useful, and it's not really inappropriate for -Wall. I would question the appropriateness of using -Wall -Werror in production code. Unfortunately a number of projects do that. I regularly have to remove -Werror from shipped makefiles, and it's primarily due to this warning. Maybe not erroring on the maybe by default would be a good idea? No. People expect that -Werror turns warnings into errors. That is what we have documented for years. Starting to special case these things is a royal road to confusion, and a slippery slope. -- Gaby
Re: Should -Wmaybe-uninitialized be included in -Wall?
No. People expect that -Werror turns warnings into errors. That is what we have documented for years. Starting to special case these things is a royal road to confusion, and a slippery slope. Ok, I will keep removing -Werrors from Makefiles then. FWIW basically -Werror -Wall defines a compiler version specific variant of C. May be great for individual developers, but it's always a serious mistake in any distributed Makefile. -Andi
Re: Should -Wmaybe-uninitialized be included in -Wall?
On 07/10/2013 05:11 PM, Andi Kleen wrote: FWIW basically -Werror -Wall defines a compiler version specific variant of C. May be great for individual developers, but it's always a serious mistake in any distributed Makefile. I could not have put it any better. Andrew.
Re: Should -Wmaybe-uninitialized be included in -Wall?
On 10 July 2013 17:11, Andi Kleen wrote: FWIW basically -Werror -Wall defines a compiler version specific variant of C. May be great for individual developers, but it's always a serious mistake in any distributed Makefile. That's a very nice way to put it.
Re: Should -Wmaybe-uninitialized be included in -Wall?
On Wed, Jul 10, 2013 at 07:42:55AM -0700, Andi Kleen wrote: Andrew Haley a...@redhat.com writes: On 07/09/2013 12:59 PM, Andreas Arnez wrote: With this situation at hand, I wonder whether it's a good idea to keep maybe-uninitialized included in -Wall. Projects which have been using -Wall -Werror successfully for many years are now forced to investigate non-existing bugs in their code. But maybe-uninitialized is very useful, and it's not really inappropriate for -Wall. I would question the appropriateness of using -Wall -Werror in production code. Unfortunately a number of projects do that. I regularly have to remove -Werror from shipped makefiles, and it's primarily due to this warning. Maybe not erroring on the maybe by default would be a good idea? But that would be very inconsistent with what -Werror does. Projects that don't want to error out on maybe uninitialized warnings can easily use -Wall -Werror -Wno-error=maybe-uninitialized, still get the warning but not fatal. Jakub
Re: Should -Wmaybe-uninitialized be included in -Wall?
On 07/10/2013 10:29 AM, Jonathan Wakely wrote: On 10 July 2013 17:11, Andi Kleen wrote: FWIW basically -Werror -Wall defines a compiler version specific variant of C. May be great for individual developers, but it's always a serious mistake in any distributed Makefile. That's a very nice way to put it. Yup. Particularly when one considers that new warnings tend to show up in -Wall from one release to the next. I've often suggested that for organizations/projects that are sensitive to additions to -Wall that they use an explicit set of -W options rather than the often changing -Wall. jeff
Re: Should -Wmaybe-uninitialized be included in -Wall?
On Jul 10, 2013, at 12:44 PM, Jeff Law wrote: On 07/10/2013 10:29 AM, Jonathan Wakely wrote: On 10 July 2013 17:11, Andi Kleen wrote: FWIW basically -Werror -Wall defines a compiler version specific variant of C. May be great for individual developers, but it's always a serious mistake in any distributed Makefile. That's a very nice way to put it. Yup. Particularly when one considers that new warnings tend to show up in -Wall from one release to the next. I've often suggested that for organizations/projects that are sensitive to additions to -Wall that they use an explicit set of -W options rather than the often changing -Wall. True. But even if you do that, you may still get new warnings for new compiler releases because of changes/improvements in the checkers. It seems to me there are two cases. One is releases, where you want to maximize the odds that an install will work. For that you clearly don't want -Werror, and you might want to trim back the warnings. The other is the development phase, where you want to weed out questionable code. So for development builds you want lots of warnings, and possible -Werror as well to increase the odds that flagged code will be seen and fixed. paul
Re: Should -Wmaybe-uninitialized be included in -Wall?
On 07/10/2013 05:48 PM, paul_kon...@dell.com wrote: It seems to me there are two cases. One is releases, where you want to maximize the odds that an install will work. For that you clearly don't want -Werror, and you might want to trim back the warnings. The other is the development phase, where you want to weed out questionable code. So for development builds you want lots of warnings, and possible -Werror as well to increase the odds that flagged code will be seen and fixed. There's a third case: release environments where the developers track GCC development. Andrew.
Re: Should -Wmaybe-uninitialized be included in -Wall?
What about introducing a new blanket warning kind that excludes anything with false positives? something like -WALL ? David On Wed, Jul 10, 2013 at 3:51 AM, Andreas Arnez ar...@linux.vnet.ibm.com wrote: Jeff Law l...@redhat.com writes: On 07/09/2013 07:56 AM, Andreas Arnez wrote: Andrew Haley a...@redhat.com writes: On 07/09/2013 12:59 PM, Andreas Arnez wrote: With this situation at hand, I wonder whether it's a good idea to keep maybe-uninitialized included in -Wall. Projects which have been using -Wall -Werror successfully for many years are now forced to investigate non-existing bugs in their code. But maybe-uninitialized is very useful, and it's not really inappropriate for -Wall. The warning would be extremely useful, if false positives didn't obscure real problems. In its current state I consider its usefulness limited. Depends on your point of view. This topic has been hashed through repeatedly on this list. OK, I may be biased, because I have *only* seen false positives with this warning so far. Others may have made better experience with it. I personally like -Wall -Werror. While we do run into false positives and the set of false positives does change from release to release as a result of optimizations, I believe there's been an overall improvement in the quality of the codebases that use -Wall -Werror. Yes, I fully agree on the usefulness of -Wall -Werror and its contribution to the code base quality increase. I certainly see fewer bugs these days blamed on the compiler which are in fact uninitialized variables. That's good. But there are still some known false positives with maybe uninitialized. If there weren't, then this option could go away and be covered by -Wuninitialized instead. I'd like to revamp how we do path isolation to eliminate more of the false positives, but this warning is by its nature going to generate false positives. Right, that's the point. My understanding of -Wall (so far) was that it shouldn't include warnings with false positives. What I would ask is that folks continue to file bugs for false positives. While I can't guarantee we'll fix them, they do provide a good base of tests for path sensitive analysis. Yes, I can see how the current policy of including this warning in -Wall may help getting more coverage. I'd also suggest that when an initialization is added merely to avoid the warning that a comment be added to the code so that folks know it's merely to avoid the warning. Adding a bogus initialization is usually not a reasonable response to this warning anyway. For instance, consider cases where certain elements of an array (like in bug 57832) or of a large struct (like reported in bug 55288) are falsely claimed to may be uninitialized. Also consider cases where -Wmaybe-uninitialized reported the wrong variable, like in bug 55985. Besides, a bogus initialization would also obscure helpful warnings, like those emitted from -Wuninitialized. OK, it appears that most people (silently or explicitly) accept the inclusion of warnings with false positives in -Wall. I'm not hung up on this. It would just be nice to reflect this appropriately in the documentation.
Re: Should -Wmaybe-uninitialized be included in -Wall?
On Wed, Jul 10, 2013 at 2:01 PM, Xinliang David Li davi...@google.com wrote: What about introducing a new blanket warning kind that excludes anything with false positives? something like -WALL ? I am doubtful more ropes is the answer. -- Gaby
Re: Should -Wmaybe-uninitialized be included in -Wall?
On Wed, Jul 10, 2013 at 12:05 PM, Gabriel Dos Reis g...@integrable-solutions.net wrote: On Wed, Jul 10, 2013 at 2:01 PM, Xinliang David Li davi...@google.com wrote: What about introducing a new blanket warning kind that excludes anything with false positives? something like -WALL ? I am doubtful more ropes is the answer. Reason? David -- Gaby
Re: Should -Wmaybe-uninitialized be included in -Wall?
On Wed, Jul 10, 2013 at 2:27 PM, Xinliang David Li davi...@google.com wrote: On Wed, Jul 10, 2013 at 12:05 PM, Gabriel Dos Reis g...@integrable-solutions.net wrote: On Wed, Jul 10, 2013 at 2:01 PM, Xinliang David Li davi...@google.com wrote: What about introducing a new blanket warning kind that excludes anything with false positives? something like -WALL ? I am doubtful more ropes is the answer. Reason? It adds one too-many options to existing minefield without solving the underlying fundamental problem. -- Gaby
Re: Should -Wmaybe-uninitialized be included in -Wall?
There are two fundamental problems: 1) uninit warning has false positives. 2) people disagree what is the expected behavior of -Wall. 1) can only be solved by improving the analysis. The new option is a reasonable way to solve 2), unless you think the only way to solve it is to change the behavior of -Wall, or ask concerned user to explicitly use -Wno-error=... or white list the warning options they care. David On Wed, Jul 10, 2013 at 12:36 PM, Gabriel Dos Reis g...@integrable-solutions.net wrote: On Wed, Jul 10, 2013 at 2:27 PM, Xinliang David Li davi...@google.com wrote: On Wed, Jul 10, 2013 at 12:05 PM, Gabriel Dos Reis g...@integrable-solutions.net wrote: On Wed, Jul 10, 2013 at 2:01 PM, Xinliang David Li davi...@google.com wrote: What about introducing a new blanket warning kind that excludes anything with false positives? something like -WALL ? I am doubtful more ropes is the answer. Reason? It adds one too-many options to existing minefield without solving the underlying fundamental problem. -- Gaby
Re: Should -Wmaybe-uninitialized be included in -Wall?
Xinliang David Li davi...@google.com writes: What about introducing a new blanket warning kind that excludes anything with false positives? something like -WALL ? This still doesn't help if any new compiler version could ever add a new warning. -Andi -- a...@linux.intel.com -- Speaking for myself only
Re: Should -Wmaybe-uninitialized be included in -Wall?
On Wed, Jul 10, 2013 at 2:49 PM, Xinliang David Li davi...@google.com wrote: There are two fundamental problems: 1) uninit warning has false positives. 2) people disagree what is the expected behavior of -Wall. 1) can only be solved by improving the analysis. I think we should focus on this. While we can't attain perfection here, there is room for improvement for the cases that actually matter. The new option is a reasonable way to solve 2), unless you think the only way to solve it is to change the behavior of -Wall, or ask concerned user to explicitly use -Wno-error=... or white list the warning options they care. I do consider that -Wall should be 'reasonably' free of annoying false positives, and that should be fixed by improving the detection analysis, of by removing the switch from -Wall. This is the recipe we've applied so far, and it usually works. Of course, a workaround, while waiting for the improvement, is to list the diagnostics that are at fault in -Wno-error=. In general, I think we are too eager to add new switches, but we have no framework in place to test the coherence of the various switches we keep adding. A key attractive aspect of -Wall is that one does not need to know the gazillion switches that are activated. One can make a case that everyone should know all of them, but that I think it is misguided to require every user to know everything we put in -Wall. -- Gaby
Re: Should -Wmaybe-uninitialized be included in -Wall?
On Wed, Jul 10, 2013 at 1:20 PM, Gabriel Dos Reis g...@integrable-solutions.net wrote: On Wed, Jul 10, 2013 at 2:49 PM, Xinliang David Li davi...@google.com wrote: There are two fundamental problems: 1) uninit warning has false positives. 2) people disagree what is the expected behavior of -Wall. 1) can only be solved by improving the analysis. I think we should focus on this. While we can't attain perfection here, there is room for improvement for the cases that actually matter. The new option is a reasonable way to solve 2), unless you think the only way to solve it is to change the behavior of -Wall, or ask concerned user to explicitly use -Wno-error=... or white list the warning options they care. I do consider that -Wall should be 'reasonably' free of annoying false positives, and that should be fixed by improving the detection analysis, of by removing the switch from -Wall. This is the recipe we've applied so far, and it usually works. Of course, a workaround, while waiting for the improvement, is to list the diagnostics that are at fault in -Wno-error=. In general, I think we are too eager to add new switches, but we have no framework in place to test the coherence of the various switches we keep adding. A key attractive aspect of -Wall is that one does not need to know the gazillion switches that are activated. One can make a case that everyone should know all of them, but that I think it is misguided to require every user to know everything we put in -Wall. This points to other ideas: 1) how about adding a helper switch to show what is included in Wall? such as -Wall-print 2) how about making -Wall configurable -- a default config file is looked at by the compiler, but user can change the config or use a different config they like. David -- Gaby
Re: Should -Wmaybe-uninitialized be included in -Wall?
This points to other ideas: 1) how about adding a helper switch to show what is included in Wall? such as -Wall-print Doesn't gcc -Q -Wall --help=warnings give you this? Otherwise, I think it is a bug. 2) how about making -Wall configurable -- a default config file is looked at by the compiler, but user can change the config or use a different config they like. Isn't this exactly like using -Wno-error=* or -Wno-* ? It is a small step to encode those in some file and pass the content of the file to gcc as extra command-line options. To be honest, I agree with Gabriel here. And I would go a step forward, I would say that we are too timid with the warnings we enable by default or by -Wall. We should warn more agressively, and let users disable the warnings that they don't care about or are too buggy for their taste. Because, at the end, it is a matter of taste. If -Wmaybe-uninitialized saved your neck you will wonder why it is not on by default. If it only shows false positives for your code, you will wonder why it exists at all. -Wmaybe-uninitialized is able to catch real bugs, and the false positives have easy work-arounds. It has some bugs, yes, but one can always work-around them. Despite this very long thread, we don't actually get that many complaints about it (we do get a lot of complaints about false negatives, in particular PR18501). Cheers, Manuel.
Re: Should -Wmaybe-uninitialized be included in -Wall?
On Wed, Jul 10, 2013 at 2:37 PM, Manuel López-Ibáñez lopeziba...@gmail.com wrote: This points to other ideas: 1) how about adding a helper switch to show what is included in Wall? such as -Wall-print Doesn't gcc -Q -Wall --help=warnings give you this? Yes it does work as expected. Otherwise, I think it is a bug. 2) how about making -Wall configurable -- a default config file is looked at by the compiler, but user can change the config or use a different config they like. Isn't this exactly like using -Wno-error=* or -Wno-* ? It is a small step to encode those in some file and pass the content of the file to gcc as extra command-line options. yes. To be honest, I agree with Gabriel here. And I would go a step forward, I would say that we are too timid with the warnings we enable by default or by -Wall. We should warn more agressively, and let users disable the warnings that they don't care about or are too buggy for their taste. Because, at the end, it is a matter of taste. If -Wmaybe-uninitialized saved your neck you will wonder why it is not on by default. If it only shows false positives for your code, you will wonder why it exists at all. -Wmaybe-uninitialized is able to catch real bugs, and the false positives have easy work-arounds. It has some bugs, yes, but one can always work-around them. Despite this very long thread, we don't actually get that many complaints about it (we do get a lot of complaints about false negatives, in particular PR18501). I agree with you here. thanks, David Cheers, Manuel.
Re: Should -Wmaybe-uninitialized be included in -Wall?
On Wed, Jul 10, 2013 at 4:37 PM, Manuel López-Ibáñez lopeziba...@gmail.com wrote: To be honest, I agree with Gabriel here. And I would go a step forward, I would say that we are too timid with the warnings we enable by default or by -Wall. We should warn more agressively, and let users disable the warnings that they don't care about or are too buggy for their taste. Because, at the end, it is a matter of taste. We have to be careful here. If a warning switch spews too much noise, especially when building idiomatic codes or the standard library for example, then I don't think it is a matter of taste; it just doesn't have its place in -Wall. And yes, we had had such examples. -- Gaby
Re: Should -Wmaybe-uninitialized be included in -Wall?
On Wed, 10 Jul 2013 07:49:18 -0500 Gabriel Dos Reis g...@integrable-solutions.net wrote: If we include a warning in -Wall then it is because we believe it to be generally useful and likely to uncover common bugs/mistakes. It is therefore reasonable for users to issue -Wall -Werror even in application delivery mode. Arg, no. -Werror is very useful for development and I'm sure that code quality increases because of it, but it should never be enabled by default for releases. I think about 80% of the bugs we've had filed so far for packages failing to build against 4.8 are due to -Werror. Also, several distros patch gcc to enable additional warnings by default (eg. Debian, Ubuntu, and Gentoo enable -Wformat=security) that upstream may not see or be interested in. It's a big enough headache that we had to ban use of -Werror from our tree (instead we flag important warnings and output them at the end of the build). -- Ryan Hillpsn: dirtyepic_sk gcc-porting/toolchain/wxwidgets @ gentoo.org 47C3 6D62 4864 0E49 8E9E 7F92 ED38 BD49 957A 8463 signature.asc Description: PGP signature
Should -Wmaybe-uninitialized be included in -Wall?
When building gdb with newer gcc versions I frequently stumble across maybe-uninitialized false positives, like the ones documented in bug 57237. Various bugs address similar issues, and in bug 56526 Jakub Jelinek wrote: Maybe-uninitialized warnings have tons of known false positives, while the predicated analysis can handle the simplest cases, it can't handle anything more complicated. Even the description of this option states: These warnings are made optional because GCC is not smart enough to see all the reasons why the code might be correct in spite of appearing to have an error. With this situation at hand, I wonder whether it's a good idea to keep maybe-uninitialized included in -Wall. Projects which have been using -Wall -Werror successfully for many years are now forced to investigate non-existing bugs in their code.
Re: Should -Wmaybe-uninitialized be included in -Wall?
On 07/09/2013 12:59 PM, Andreas Arnez wrote: With this situation at hand, I wonder whether it's a good idea to keep maybe-uninitialized included in -Wall. Projects which have been using -Wall -Werror successfully for many years are now forced to investigate non-existing bugs in their code. But maybe-uninitialized is very useful, and it's not really inappropriate for -Wall. I would question the appropriateness of using -Wall -Werror in production code. Andrew.
Re: Should -Wmaybe-uninitialized be included in -Wall?
On 9 July 2013 13:04, Andrew Haley wrote: On 07/09/2013 12:59 PM, Andreas Arnez wrote: With this situation at hand, I wonder whether it's a good idea to keep maybe-uninitialized included in -Wall. Projects which have been using -Wall -Werror successfully for many years are now forced to investigate non-existing bugs in their code. But maybe-uninitialized is very useful, and it's not really inappropriate for -Wall. I would question the appropriateness of using -Wall -Werror in production code. Me too, but for those who prefer it there is -Wno-error=maybe-uninitialized so they can keep everything else as an error.
Re: Should -Wmaybe-uninitialized be included in -Wall?
On 07/09/2013 02:56 PM, Andreas Arnez wrote: What matters is whether *some* stages of production code development use this combination of options. It could certainly be argued whether it should also be a project's configure default, like currently the case for gdb. It's not a problem for GDB because they track GCC development. Andrew.
Re: Should -Wmaybe-uninitialized be included in -Wall?
Andrew Haley a...@redhat.com writes: On 07/09/2013 12:59 PM, Andreas Arnez wrote: With this situation at hand, I wonder whether it's a good idea to keep maybe-uninitialized included in -Wall. Projects which have been using -Wall -Werror successfully for many years are now forced to investigate non-existing bugs in their code. But maybe-uninitialized is very useful, and it's not really inappropriate for -Wall. The warning would be extremely useful, if false positives didn't obscure real problems. In its current state I consider its usefulness limited. The reason I consider the warning (currently) inappropriate for -Wall is that it does not seem to behave as reliably and predictably as the other -Wall options. The description of -Wall says: This enables all the warnings about constructions that some users consider questionable, and that are easy to avoid (or modify to prevent the warning) [...] Note the easy to avoid. If everybody agrees that -Wall should include maybe-uninitialized, then I suggest to change the wording here. I would question the appropriateness of using -Wall -Werror in production code. What matters is whether *some* stages of production code development use this combination of options. It could certainly be argued whether it should also be a project's configure default, like currently the case for gdb.
Re: Should -Wmaybe-uninitialized be included in -Wall?
Andrew I would question the appropriateness of using -Wall -Werror in Andrew production code. Andreas What matters is whether *some* stages of production code Andreas development use this combination of options. It could Andreas certainly be argued whether it should also be a project's Andreas configure default, like currently the case for gdb. gdb only enables it for the development branch, not for releases. If you're building from CVS you're expected to know how to either fix these problems or disable -Werror. Typically the fix is trivial; if you look through the archives you'll see fixes along these lines. Tom
Re: Should -Wmaybe-uninitialized be included in -Wall?
On 07/09/2013 07:56 AM, Andreas Arnez wrote: Andrew Haley a...@redhat.com writes: On 07/09/2013 12:59 PM, Andreas Arnez wrote: With this situation at hand, I wonder whether it's a good idea to keep maybe-uninitialized included in -Wall. Projects which have been using -Wall -Werror successfully for many years are now forced to investigate non-existing bugs in their code. But maybe-uninitialized is very useful, and it's not really inappropriate for -Wall. The warning would be extremely useful, if false positives didn't obscure real problems. In its current state I consider its usefulness limited. Depends on your point of view. This topic has been hashed through repeatedly on this list. I personally like -Wall -Werror. While we do run into false positives and the set of false positives does change from release to release as a result of optimizations, I believe there's been an overall improvement in the quality of the codebases that use -Wall -Werror. I certainly see fewer bugs these days blamed on the compiler which are in fact uninitialized variables. I'd like to revamp how we do path isolation to eliminate more of the false positives, but this warning is by its nature going to generate false positives. What I would ask is that folks continue to file bugs for false positives. While I can't guarantee we'll fix them, they do provide a good base of tests for path sensitive analysis. I'd also suggest that when an initialization is added merely to avoid the warning that a comment be added to the code so that folks know it's merely to avoid the warning. Jeff
