| Message ID | alpine.DEB.2.21.1901312237250.16995@stedding.saclay.inria.fr |
|---|---|
| State | New |
| Headers | show |
| Series | Move -Wmaybe-uninitialized to -Wextra | expand |
Hi Marc, On Fri, Feb 01, 2019 at 12:32:45PM +0100, Marc Glisse wrote: > -Wmaybe-uninitialized generates false positives, we can tweak the compiler > to reduce them, but there will always be some, that's in the nature of > this warning. That is true for *every* warning; if not, it should be an error, not a warning. > My opinion is that -Wmaybe-uninitialized would serve its purpose better as > part of -Wextra. +1 > People tend to use -Wall with -Werror (either explicitly > or implicitly by modifying the code until all warnings are gone). What I > see currently in projects where I participate is that > -Wmaybe-uninitialized is making things worse. People don't investigate > deeply the cause of the warning, they just go for whatever "quick-fix" > makes the compiler shut up. Quite often, this involves extra code that is > less readable and performs worse, while it didn't even "fix" what caused > the warning, it just randomly ended up with code where the compiler > doesn't warn (possibly because the function got bigger, which changed > inlining decisions...). Yes, using -Werror is usually a terrible idea. > Note that similar arguments may apply to some other warnings that somehow > made their way into -Wall when they shouldn't have, but for now I am only > proposing to move -Wmaybe-uninitialized. Some people tend to consider that > if a warning is not part of -Wall, it might as well not exist. Obviously I > disagree with that. If it is not part of -Wall and not of -W, and not special purpose, then it might as well not exist. Segher
On Fri, Feb 01, 2019 at 07:19:25AM -0600, Segher Boessenkool wrote: > Hi Marc, > > On Fri, Feb 01, 2019 at 12:32:45PM +0100, Marc Glisse wrote: > > -Wmaybe-uninitialized generates false positives, we can tweak the compiler > > to reduce them, but there will always be some, that's in the nature of > > this warning. > > That is true for *every* warning; if not, it should be an error, not a > warning. > > > My opinion is that -Wmaybe-uninitialized would serve its purpose better as > > part of -Wextra. > > +1 +1 from me too. > > People tend to use -Wall with -Werror (either explicitly > > or implicitly by modifying the code until all warnings are gone). What I > > see currently in projects where I participate is that > > -Wmaybe-uninitialized is making things worse. People don't investigate > > deeply the cause of the warning, they just go for whatever "quick-fix" > > makes the compiler shut up. Quite often, this involves extra code that is > > less readable and performs worse, while it didn't even "fix" what caused > > the warning, it just randomly ended up with code where the compiler > > doesn't warn (possibly because the function got bigger, which changed > > inlining decisions...). > > Yes, using -Werror is usually a terrible idea. > > > Note that similar arguments may apply to some other warnings that somehow > > made their way into -Wall when they shouldn't have, but for now I am only > > proposing to move -Wmaybe-uninitialized. Some people tend to consider that > > if a warning is not part of -Wall, it might as well not exist. Obviously I > > disagree with that. > > If it is not part of -Wall and not of -W, and not special purpose, then it > might as well not exist. There are warnings that *do* make sense, but have issues e.g. with macro expansion, so will be outside -Wall/-Wextra unless that's fixed. E.g. -Wlogical-op, -Wduplicated-conds, or a warning I posted to some PR called -Wsame-arguments I think, etc. Marek
On Fri, Feb 01, 2019 at 09:01:53AM -0500, Marek Polacek wrote: > On Fri, Feb 01, 2019 at 07:19:25AM -0600, Segher Boessenkool wrote: > > > Some people tend to consider that > > > if a warning is not part of -Wall, it might as well not exist. Obviously I > > > disagree with that. > > > > If it is not part of -Wall and not of -W, and not special purpose, then it > > might as well not exist. > > There are warnings that *do* make sense, but have issues e.g. with macro > expansion, so will be outside -Wall/-Wextra unless that's fixed. E.g. > -Wlogical-op, -Wduplicated-conds, or a warning I posted to some PR > called -Wsame-arguments I think, etc. Yes, we agree on that. I'm just saying such general-purpose warnings are not used much until they are part of -Wall or -W. Segher
On 2/1/19 7:01 AM, Marek Polacek wrote: > On Fri, Feb 01, 2019 at 07:19:25AM -0600, Segher Boessenkool wrote: >> Hi Marc, >> >> On Fri, Feb 01, 2019 at 12:32:45PM +0100, Marc Glisse wrote: >>> -Wmaybe-uninitialized generates false positives, we can tweak the compiler >>> to reduce them, but there will always be some, that's in the nature of >>> this warning. >> >> That is true for *every* warning; if not, it should be an error, not a >> warning. >> >>> My opinion is that -Wmaybe-uninitialized would serve its purpose better as >>> part of -Wextra. >> >> +1 > > +1 from me too. I disagree strongly. If we move it to Wextra it's going to see a lot less usage in real world codebases and potentially lead to the re-introduction of a class of bugs that we've largely helped stomp out. It's also the case that maybe uninitialized vs is uninitialized is really just a function of CFG shape. Give me any "maybe uninitialized" case and I can turn it into a "is uninitialized" with simple block duplication of the forms done by jump threading, path isolation, superblock formation, etc. > >>> People tend to use -Wall with -Werror (either explicitly >>> or implicitly by modifying the code until all warnings are gone). What I >>> see currently in projects where I participate is that >>> -Wmaybe-uninitialized is making things worse. People don't investigate >>> deeply the cause of the warning, they just go for whatever "quick-fix" >>> makes the compiler shut up. Quite often, this involves extra code that is >>> less readable and performs worse, while it didn't even "fix" what caused >>> the warning, it just randomly ended up with code where the compiler >>> doesn't warn (possibly because the function got bigger, which changed >>> inlining decisions...). >> >> Yes, using -Werror is usually a terrible idea. Generally agreed in released versions of any code. -Werror *may* be appropriate in development versions depending on the project's policies, procedures and quality of codebase. Jeff
On Fri, 1 Feb 2019, Jeff Law wrote: > On 2/1/19 7:01 AM, Marek Polacek wrote: >> On Fri, Feb 01, 2019 at 07:19:25AM -0600, Segher Boessenkool wrote: >>> Hi Marc, >>> >>> On Fri, Feb 01, 2019 at 12:32:45PM +0100, Marc Glisse wrote: >>>> -Wmaybe-uninitialized generates false positives, we can tweak the compiler >>>> to reduce them, but there will always be some, that's in the nature of >>>> this warning. >>> >>> That is true for *every* warning; if not, it should be an error, not a >>> warning. >>> >>>> My opinion is that -Wmaybe-uninitialized would serve its purpose better as >>>> part of -Wextra. >>> >>> +1 >> >> +1 from me too. > I disagree strongly. I am not surprised, but I had to at least start the conversation. Would you mind providing a patch that changes the definition of -Wall, since the current one doesn't quite match reality anymore? Also, what do you recommend people do when they hit false positives? > If we move it to Wextra it's going to see a lot less usage in real world > codebases I am very tempted by the strawman: should we deprecate -Wextra since nobody uses it? (Sorry) Ideally serious projects would use (parts of) -Wextra, at least occasionally, and with circumspection. But some warnings like -Wmaybe-uninitialized are dangerous tools in the hands of quickfix developers, and I am indeed trying to keep it out of their hands... > and potentially lead to the re-introduction of a class of bugs that > we've largely helped stomp out. That's very optimistic. -Wmaybe-uninitialized only detects a very small proportion of uninitialized uses. Also, my experience is that people have stomped out the warning, not the bugs. In some cases they even introduced bugs to stomp out false warnings, or made it harder to detect real bugs in the future, so the warning did more harm than good. I am mostly working on large C++ header-only template-heavy scientific libraries, it is quite possible that people who handle different types of code bases have a different experience, and -Wmaybe-uninitialized may have had a more positive impact on other projects. > It's also the case that maybe uninitialized vs is uninitialized is > really just a function of CFG shape. Give me any "maybe uninitialized" > case and I can turn it into a "is uninitialized" with simple block > duplication of the forms done by jump threading, path isolation, > superblock formation, etc. Hmm, you know those things better than me, so you are probably right, but I am not seeing it. We omit "maybe" if, starting from the entry of the function, and barring exceptions, we know the statement will always be executed. If you take a false positive for maybe-uninitialized, i.e. a statement in a dead branch, I don't see how block duplication can make it so the statement is now always executed. The natural way to remove "maybe" is through function cloning or outlining. Then you can create functions that are never called, and any warning about those is a false positive. There is a matter of statistics. In practice maybe-uninitialized has way more false positives than uninitialized, which makes it more problematic. But if you prefer to move both to -Wextra (this is the current default when front-ends don't override it), that's ok with me ;-)
On Fri, Feb 01, 2019 at 12:27:57PM -0700, Jeff Law wrote: > On 2/1/19 7:01 AM, Marek Polacek wrote: > > On Fri, Feb 01, 2019 at 07:19:25AM -0600, Segher Boessenkool wrote: > >> On Fri, Feb 01, 2019 at 12:32:45PM +0100, Marc Glisse wrote: > >>> My opinion is that -Wmaybe-uninitialized would serve its purpose better as > >>> part of -Wextra. > >> > >> +1 > > > > +1 from me too. > I disagree strongly. If we move it to Wextra it's going to see a lot > less usage in real world codebases and potentially lead to the > re-introduction of a class of bugs that we've largely helped stomp out. The usual workaround, especially for programs that build with multiple (i.e. older) versions of GCC, is to initialise any such variable (to an either or not useful value) early. This doesn't fix the actual problem usually (which is that your control flow is too complex). > It's also the case that maybe uninitialized vs is uninitialized is > really just a function of CFG shape. Give me any "maybe uninitialized" > case and I can turn it into a "is uninitialized" with simple block > duplication of the forms done by jump threading, path isolation, > superblock formation, etc. Are you saying that -Wmaybe-uninitialized should be included in -Wuninitialized, since it has no extra false positives? That wasn't true at all historically: -Wuninitialized has false positives on paths that cannot be executed because of function preconditions, but -Wmaybe-uninitialized used to warn for things that can be locally proven to never execute, like if (a) b = 42; ... if (a) f(b); > >> Yes, using -Werror is usually a terrible idea. > Generally agreed in released versions of any code. -Werror *may* be > appropriate in development versions depending on the project's policies, > procedures and quality of codebase. IMO it is more useful it is much more useful if you make your build system less noisy so that problems are more obvious, instead of breaking the build for no reason all the time (see PR89162, etc. etc.) Segher
On 2/1/19 4:32 AM, Marc Glisse wrote: > Hello, > > first, I expect this to be controversial, so feel free to complain. I don't feel too strongly about whether -Wmaybe-uninitialized should be in -Wall or in -Wextra, and I might even be somewhat more inclined to expect to find it in the latter. But since you sound like you are gearing up for proposing other changes in the same vein down the line where I might have stronger opinions, I should comment. [It's a bit of a long-winded response because I've been thinking about this topic a lot lately.] In general, I think a discussion of warning groupings is worth having (even needed) at the right time, but stage 4 doesn't strike me as the most opportune occasion. Specifically for -Wmaybe-uninitialized, the option has been in -Wall for many releases, and no major changes to it have been made recently that I know. So what I'm missing in this proposal is: why now? What has changed to make this a pressing issue now? Has its rate of false positives gone up significantly? If so, by how much and why? > 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), even in conjunction with > macros." > > And the description of -Wmaybe-uninitialized "For an automatic variable, > if there exists a path from the function entry to a use of the variable > that is initialized, but there exist some other paths for which the > variable is not initialized, the compiler emits a warning if it cannot > prove the uninitialized paths are not executed at run time. 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." > > -Wmaybe-uninitialized generates false positives, we can tweak the > compiler to reduce them, but there will always be some, that's in the > nature of this warning. Please be clear about what you mean by false positives. Is it that the warning triggers contrary to the documentation ("a path exists where the variable is uninitialized along with one where it is"), or that the path to the use of the variable does exist but we (though not GCC) can tell from the context that it cannot be reached? (The latter wouldn't qualify as a false positive as the term is defined in literature; i.e., the warning works as designed, we just don't like or agree with it.) In practice, false positives (and negatives) of both kinds, whether they fit the formal definition or the informal one, are the nature of virtually all non-trivial static diagnostics, certainly all those that depend on control or data flow analysis. Some are due to bugs or limitations in the implementation of the warning. Others are inherent in the technology. Is this warning more prone to one kind than others? If so, is it because it's implemented poorly, or that its implementation hasn't kept up with the improvements to the optimizers, or has the infrastructure it depends on become ill-suited for it to avoid some of the false positives (as formally defined), or is it something else? > These false positives are not easy to avoid, as required to be part of > -Wall. Avoiding them, when it is possible at all, requires not just a > syntactic tweak, like adding parentheses, but a semantic change that can > make the code worse. Initializing something that does not need it is > extra code (increases code size and running time). It also prevents > better tools from detecting true uninitialized uses, either static > analyzers or runtime checkers (sanitizer, valgrind). I don't find the argument very compelling that diagnosing potential bugs should be avoided because the fix (or the suppression in the case of a false positive) could be wrong. The risk exists with all diagnostics. I also take issue with the suggestion that dynamic analysis is necessarily a superior mechanism for detecting bugs. They each have their strengths and weaknesses. They are not an either-or proposition but rather complementary solutions. Lastly, in the case of uninitialized variables, the usual solution of initializing them is trivial and always safe (some coding styles even require it). Initializing scalars has a negligible performance impact in practice, and, if it's thought to be necessary, can easily be done conditionally (as in, based on some macro) so that the uninitialized accesses can still be detected by dynamic analysis. > This message concentrates on the negatives, but that doesn't mean I > consider -Wmaybe-uninitialized as useless. It can find true > uninitialized uses. And even some false positives can point at places > where we can help the compiler generate better code (say with a default > __builtin_unreachable case in a switch). I did in the past contribute > patches to make it warn more often, and I might do so again in the future. This paragraph makes me think you equate false positives from this warning with those from -Wuninitialized. They are (obviously) not the same. "May be unitialized" doesn't preclude the possibility that the variable is not used uninitialized. I'm not just nit-picking here either. It's important to use the same vocabulary. > My opinion is that -Wmaybe-uninitialized would serve its purpose better > as part of -Wextra. People tend to use -Wall with -Werror (either > explicitly or implicitly by modifying the code until all warnings are > gone). What I see currently in projects where I participate is that > -Wmaybe-uninitialized is making things worse. People don't investigate > deeply the cause of the warning, they just go for whatever "quick-fix" > makes the compiler shut up. Quite often, this involves extra code that > is less readable and performs worse, while it didn't even "fix" what > caused the warning, it just randomly ended up with code where the > compiler doesn't warn (possibly because the function got bigger, which > changed inlining decisions...). I agree that we need to be mindful of warnings leading to unnecessary and perhaps inappropriate code changes (as the author of a few warnings with this potential it has been on mind a lot). At the same time, not taking the time to properly analyze diagnostics and come up with the appropriate solutions seems like a problem that's orthogonal to the warnings themselves. Disabling warnings in hopes of solving what's really a problem with the development process or a culture of a subset of projects doesn't seem like an optimal solution. > If the warning is not enabled by default so much but only when people > are ready to investigate any warning thoroughly, the quickfix mentality > is less likely to be present. People using -Wmaybe-uninitialized need to > be willing to ignore false positives, or disable them with pragmas. > > Note that similar arguments may apply to some other warnings that > somehow made their way into -Wall when they shouldn't have, but for now > I am only proposing to move -Wmaybe-uninitialized. Some people tend to > consider that if a warning is not part of -Wall, it might as well not > exist. Obviously I disagree with that. The description of -Wall is vague enough that arguments about which warnings should or shouldn't be enabled by it are unavoidably subject to individual biases and unlikely to lead to a consensus, either among ourselves or, even less likely, among users. A shared definition of a false positive should be one of the very first steps to coming closer to a consensus. Real world (as opposed to anecdotal) data on the rates of actual rates of false positives and negatives vs true positives would be also most helpful, as would some consensus of the severity of the bugs the true positives expose, as well as some objective measure of the ease of suppression. There probably are others but these would be a start. But one size never fits all, so even if we do manage to come to a consensus among ourselves, it will not make everyone happy(*). I wonder of we need a better solution than just two (or even four, f you count enabled-by-default and disabled- by-default) more or less arbitrarily selected buckets of warnings. Martin [*] There are nearly 30 open bugs in Bugzilla about options people think should or shouldn't be enabled by -Wall or -Weextra, including bug 89072 that request to enable -Wall and -Werror by default. > ------- > > Now the actual patch. Surprisingly, the middle-end puts both > Wuninitialized and Wmaybe-uninitialized in Wextra, it is the C-family of > front-ends that puts them in Wall. It also makes Wuninitialized enable > Wmaybe-uninitialized, which is backwards (it made sense historically), > Wuninitialized has much fewer false positives, and if we are willing to > be warned about possibly uninitialized uses, we certainly also want > warnings about uninitialized uses that are certain. So I am switching > the enabling relation between those 2, and enabling only Wuninitialized > at Wall. > > If the patch gets in, this will of course require a mention in the > release notes. > > I changed a set of tests based on a mix of grep and seeing what failed > make check. The exact list may not be optimal. > > gcc/ChangeLog: > > 2019-02-01 Marc Glisse <marc.glisse@inria.fr> > > * common.opt (Wuninitialized): Enable with Wmaybe-uninitialized. > (Wmaybe-uninitialized): Enable with Wextra. > * doc/invoke.texi: Update implications between Wuninitialized, > Wmaybe-uninitialized, Wall and Wextra. > > gcc/c-family/ChangeLog: > > 2019-02-01 Marc Glisse <marc.glisse@inria.fr> > > * c.opt (Wmaybe-uninitialized): Enable with Wextra. > > gcc/testsuite/ChangeLog: > > 2019-02-01 Marc Glisse <marc.glisse@inria.fr> > > * c-c++-common/pr69543-1.c: Use -Wmaybe-uninitialized. > * c-c++-common/pr69543-2.c: Likewise. > * c-c++-common/pr69543-3.c: Likewise. > * c-c++-common/pr69543-4.c: Likewise. > * c-c++-common/uninit-17.c: Likewise. > * g++.dg/pr48484.C: Likewise. > * g++.dg/uninit-pred-1_b.C: Likewise. > * g++.dg/uninit-pred-2_b.C: Likewise. > * g++.dg/uninit-pred-3_b.C: Likewise. > * g++.dg/warn/Wuninitialized-5.C: Likewise. > * g++.dg/warn/Wuninitialized-6.C: Likewise. > * g++.dg/warn/Wuninitialized-9.C: Likewise. > * gcc.dg/gomp/pr72781.c: Likewise. > * gcc.dg/pr18501.c: Likewise. > * gcc.dg/pr39666-2.c: Likewise. > * gcc.dg/pr45083.c: Likewise. > * gcc.dg/pr57149.c: Likewise. > * gcc.dg/pr59924.c: Likewise. > * gcc.dg/pr69543.c: Likewise. > * gcc.dg/uninit-11-O0.c: Likewise. > * gcc.dg/uninit-11.c: Likewise. > * gcc.dg/uninit-16.c: Likewise. > * gcc.dg/uninit-17.c: Likewise. > * gcc.dg/uninit-18.c: Likewise. > * gcc.dg/uninit-19.c: Likewise. > * gcc.dg/uninit-6-O0.c: Likewise. > * gcc.dg/uninit-pr19430-2.c: Likewise. > * gcc.dg/uninit-pr19430-O0.c: Likewise. > * gcc.dg/uninit-pr19430.c: Likewise. > * gcc.dg/uninit-pr20644-O0.c: Likewise. > * gcc.dg/uninit-pred-2_a.c: Likewise. > * gcc.dg/uninit-pred-2_b.c: Likewise. > * gcc.dg/uninit-pred-2_c.c: Likewise. > * gcc.dg/uninit-pred-3_a.c: Likewise. > * gcc.dg/uninit-pred-3_b.c: Likewise. > * gcc.dg/uninit-pred-3_c.c: Likewise. > * gcc.dg/uninit-pred-3_d.c: Likewise. > * gcc.dg/uninit-pred-3_e.c: Likewise. > * gcc.dg/uninit-pred-4_a.c: Likewise. > * gcc.dg/uninit-pred-4_b.c: Likewise. > * gcc.dg/uninit-pred-5_a.c: Likewise. > * gcc.dg/uninit-pred-5_b.c: Likewise. > * gcc.dg/uninit-pred-6_a.c: Likewise. > * gcc.dg/uninit-pred-6_b.c: Likewise. > * gcc.dg/uninit-pred-6_c.c: Likewise. > * gcc.dg/uninit-pred-6_d.c: Likewise. > * gcc.dg/uninit-pred-6_e.c: Likewise. > * gcc.dg/uninit-pred-7_a.c: Likewise. > * gcc.dg/uninit-pred-7_b.c: Likewise. > * gcc.dg/uninit-pred-7_c.c: Likewise. > * gcc.dg/uninit-pred-7_d.c: Likewise. > * gcc.dg/uninit-pred-8_a.c: Likewise. > * gcc.dg/uninit-pred-8_b.c: Likewise. > * gcc.dg/uninit-pred-8_c.c: Likewise. > * gcc.dg/uninit-pred-8_d.c: Likewise. > * gcc.dg/uninit-pred-9_a.c: Likewise. > * gcc.dg/uninit-pred-9_b.c: Likewise. > * gcc.dg/uninit-suppress_2.c: Likewise. > * gfortran.dg/pr25923.f90: Likewise. > * gfortran.dg/pr39666-2.f90: Likewise. >
On Sat, 2 Feb 2019, Martin Sebor wrote: > I don't feel too strongly about whether -Wmaybe-uninitialized should > be in -Wall or in -Wextra, and I might even be somewhat more inclined > to expect to find it in the latter. But since you sound like you are > gearing up for proposing other changes in the same vein down the line > where I might have stronger opinions, I should comment. > > [It's a bit of a long-winded response because I've been thinking about > this topic a lot lately.] Thank you for taking the time to write this down. > In general, I think a discussion of warning groupings is worth having > (even needed) at the right time, but stage 4 doesn't strike me as > the most opportune occasion. > > Specifically for -Wmaybe-uninitialized, the option has been in -Wall > for many releases, and no major changes to it have been made recently > that I know. So what I'm missing in this proposal is: why now? What > has changed to make this a pressing issue now? Has its rate of false > positives gone up significantly? If so, by how much and why? I've been wanting to post this for years, but I only got motivated enough recently after seeing negative effects of -Wmaybe-uninitialized in two projects within a few days. I don't think the rate of false positives has gone up significantly in gcc-9, they just randomly appear or disappear from one release to the next. In some sense, for projects that support multiple compiler versions, each new gcc release makes the number of false positives (on at least one version) go up, but that doesn't count as the warning getting worse. The discussion and/or the change don't need to happen now, but if I didn't post this patch while motivated, it might be years before I actually do it, so I ignored stage 4. > Please be clear about what you mean by false positives. Is it that > the warning triggers contrary to the documentation ("a path exists > where the variable is uninitialized along with one where it is"), > or that the path to the use of the variable does exist but we > (though not GCC) can tell from the context that it cannot be reached? The latter. > (The latter wouldn't qualify as a false positive as the term is defined > in literature; i.e., the warning works as designed, we just don't like > or agree with it.) Indeed the definition of false positive is important. In some sense, no warning is ever a false positive, when its definition is "the compiler decided to warn". But that makes the warning useless. -Wmaybe-uninitialized does not say there is an uninitialized use, it says the control flow is too complicated or the compiler not clever enough (or there is indeed an uninitialized use for some input, and only this subcase is a bug). Another warning, -Wstrict-overflow (after it moved to the middle-end), was more of an optimization note than a true warning: "warning: assuming your code is valid when generating code", Duh! Was I supposed to make it invalid to quiet the warning? > In practice, false positives (and negatives) of both kinds, whether > they fit the formal definition or the informal one, are the nature > of virtually all non-trivial static diagnostics, certainly all those > that depend on control or data flow analysis. Some are due to bugs > or limitations in the implementation of the warning. Others are > inherent in the technology. Yes, and I argue that these warnings belong in a different "level" of warnings than the trivial warnings. > Is this warning more prone to one kind than others? If so, is it > because it's implemented poorly, or that its implementation hasn't kept > up with the improvements to the optimizers, or has the infrastructure it > depends on become ill-suited for it to avoid some of the false positives > (as formally defined), or is it something else? I am mostly looking at what I see in practice. I regularly see false positives of -Wstrict-overflow (less now that I neutered parts of it) and -Wmaybe-uninitialized. And when I have strange crashes that later turn out to be caused by uninitialized memory uses, it is very seldom that -Wmaybe-uninitialized helps detect them. IIRC, last time, I had slightly more success with -Wreturn-local-addr -O3 -fkeep-inline-functions -fkeep-static-functions. I think several of the new middle-end warnings you added are about string manipulation. I don't do any of that, which may explain why I am not seeing them. Also, we are not using gcc-9 much yet. >> These false positives are not easy to avoid, as required to be part of >> -Wall. Avoiding them, when it is possible at all, requires not just a >> syntactic tweak, like adding parentheses, but a semantic change that can >> make the code worse. Initializing something that does not need it is extra >> code (increases code size and running time). It also prevents better tools >> from detecting true uninitialized uses, either static analyzers or runtime >> checkers (sanitizer, valgrind). > > I don't find the argument very compelling that diagnosing potential > bugs should be avoided because the fix (or the suppression in > the case of a false positive) could be wrong. The risk exists with > all diagnostics. Yes, but in practice I haven't seen people get the parentheses wrong after a warning from -Wparentheses. > I also take issue with the suggestion that dynamic analysis is > necessarily a superior mechanism for detecting bugs. They each have > their strengths and weaknesses. They are not an either-or proposition > but rather complementary solutions. I didn't claim they were necessarily superior, and I started my enumeration of potential better tools with static analyzers, i.e. like gcc's -Wmaybe-uninitialized, but really focused on this and not on optimization. Dynamic analysis has the advantage that it avoids dead branches and thus a whole class of false positives, but indeed it is also a weakness that it misses a lot of possible paths not covered during testing. I agree 100% that they are complementary. > Lastly, in the case of uninitialized variables, the usual solution > of initializing them is trivial and always safe (some coding styles > even require it). Here it shows that we don't work with the same type of code at all. If I am using a boost::optional, i.e. a class with a buffer and a boolean that says if the buffer is initialized, how do I initialize the (private) buffer? Or should boost itself zero out the buffer whenever the boolean is set to false? The variables can easily be hidden behind dozens of levels of abstraction that make them hard to initialize, and there may be nothing meaningful to initialize them with. Uninitialized also includes clobbered (out-of-scope for instance) in gcc, where it isn't clear what you are supposed to initialize to quiet the warning. By the way, we usually try to avoid imposing a coding style, although that's not always true. > Initializing scalars has a negligible performance > impact in practice, and, if it's thought to be necessary, can easily > be done conditionally (as in, based on some macro) so that > the uninitialized accesses can still be detected by dynamic analysis. I'd rather avoid increasing yet again the uses of macros, but yes, when initializing to quiet what has been identified as a false positive, that seems desirable. And it can be arrays instead of scalars. >> This message concentrates on the negatives, but that doesn't mean I >> consider -Wmaybe-uninitialized as useless. It can find true uninitialized >> uses. And even some false positives can point at places where we can help >> the compiler generate better code (say with a default __builtin_unreachable >> case in a switch). I did in the past contribute patches to make it warn >> more often, and I might do so again in the future. > > This paragraph makes me think you equate false positives from this > warning with those from -Wuninitialized. Uh? No, I don't know what gave you this impression. > "May be unitialized" doesn't preclude the possibility that the variable > is not used uninitialized. But if the variable can never be used uninitialized and gcc is simply unable to prove it, the warning is bad. It may not be a "false positive" depending on your definition, but it is undesirable. >> My opinion is that -Wmaybe-uninitialized would serve its purpose better as >> part of -Wextra. People tend to use -Wall with -Werror (either explicitly >> or implicitly by modifying the code until all warnings are gone). What I >> see currently in projects where I participate is that -Wmaybe-uninitialized >> is making things worse. People don't investigate deeply the cause of the >> warning, they just go for whatever "quick-fix" makes the compiler shut up. >> Quite often, this involves extra code that is less readable and performs >> worse, while it didn't even "fix" what caused the warning, it just randomly >> ended up with code where the compiler doesn't warn (possibly because the >> function got bigger, which changed inlining decisions...). > > I agree that we need to be mindful of warnings leading to unnecessary > and perhaps inappropriate code changes (as the author of a few warnings > with this potential it has been on mind a lot). > > At the same time, not taking the time to properly analyze diagnostics > and come up with the appropriate solutions seems like a problem that's > orthogonal to the warnings themselves. Disabling warnings in hopes of > solving what's really a problem with the development process or > a culture of a subset of projects doesn't seem like an optimal solution. I agree with that. But if I complain about the mindless quickfixes, the reply is that customers ask for a library that does not warn, and the developers do not have infinite time, so they do the minimum to avoid warnings. This is strongly linked to the warning levels. One could recommend that customers include the library with -isystem to disable the warnings inside the library, bug that's fragile (though it has gotten quite good), and most importantly it also disables some warnings that are relevant to the user. >> If the warning is not enabled by default so much but only when people are >> ready to investigate any warning thoroughly, the quickfix mentality is less >> likely to be present. People using -Wmaybe-uninitialized need to be willing >> to ignore false positives, or disable them with pragmas. >> >> Note that similar arguments may apply to some other warnings that somehow >> made their way into -Wall when they shouldn't have, but for now I am only >> proposing to move -Wmaybe-uninitialized. Some people tend to consider that >> if a warning is not part of -Wall, it might as well not exist. Obviously I >> disagree with that. > > The description of -Wall is vague enough that arguments about which > warnings should or shouldn't be enabled by it are unavoidably subject > to individual biases and unlikely to lead to a consensus, either among > ourselves or, even less likely, among users. True. To me, one of the key documented properties of -Wall is being "easy to avoid". -Wall -Werror should be a usable combination. Anything not easy to avoid belongs in a different class, say -Wextra. We can rename that to W1/W2/... if Wall is too charged with historical meaning. One issue is that, especially in a large, long-lived code base, people want to know when a change (or a new compiler) introduces a new warning. However, they do not want to have to go through hundreds or pre-existing, already analysed warnings. The easiest way to do that is to make sure there are no warnings at all (-Werror). For speculative warnings, that's really not convenient. I don't think there is a good solution to this (marking with a pragma the known false positives may be hard when instantiating some class somewhere causes a warning in some almost unrelated place, and you want the marking to be narrow enough that it won't also disable other warnings), and it is independent on whether the warning is in -Wall or -Wextra. I just have a feeling that splitting the easy and the not-easy warnings may help developers handle them differently. > A shared definition of a false positive should be one of the very first > steps to coming closer to a consensus. Real world (as opposed to > anecdotal) data on the rates of actual rates of false positives and > negatives vs true positives would be also most helpful, as would some > consensus of the severity of the bugs the true positives expose, as well > as some objective measure of the ease of suppression. There probably > are others but these would be a start. This data is going to be super hard to get. Most projects have been compiling for years and tweaking their code to avoid some warnings. We do not get to see the code that people originally write, we can only see what they commit.
On Fri, Feb 1, 2019 at 8:28 PM Jeff Law <law@redhat.com> wrote: > > On 2/1/19 7:01 AM, Marek Polacek wrote: > > On Fri, Feb 01, 2019 at 07:19:25AM -0600, Segher Boessenkool wrote: > >> Hi Marc, > >> > >> On Fri, Feb 01, 2019 at 12:32:45PM +0100, Marc Glisse wrote: > >>> -Wmaybe-uninitialized generates false positives, we can tweak the compiler > >>> to reduce them, but there will always be some, that's in the nature of > >>> this warning. > >> > >> That is true for *every* warning; if not, it should be an error, not a > >> warning. > >> > >>> My opinion is that -Wmaybe-uninitialized would serve its purpose better as > >>> part of -Wextra. > >> > >> +1 > > > > +1 from me too. > I disagree strongly. If we move it to Wextra it's going to see a lot > less usage in real world codebases and potentially lead to the > re-introduction of a class of bugs that we've largely helped stomp out. > > It's also the case that maybe uninitialized vs is uninitialized is > really just a function of CFG shape. Give me any "maybe uninitialized" > case and I can turn it into a "is uninitialized" with simple block > duplication of the forms done by jump threading, path isolation, > superblock formation, etc. Agreed. Also note that we consider everything executed when function entry is executed as not "maybe" even though functions can be called conditionally. So there's the artificial function boundary issue when distinguishing maybe from must-be stuff as well. Richard. > > > > >>> People tend to use -Wall with -Werror (either explicitly > >>> or implicitly by modifying the code until all warnings are gone). What I > >>> see currently in projects where I participate is that > >>> -Wmaybe-uninitialized is making things worse. People don't investigate > >>> deeply the cause of the warning, they just go for whatever "quick-fix" > >>> makes the compiler shut up. Quite often, this involves extra code that is > >>> less readable and performs worse, while it didn't even "fix" what caused > >>> the warning, it just randomly ended up with code where the compiler > >>> doesn't warn (possibly because the function got bigger, which changed > >>> inlining decisions...). > >> > >> Yes, using -Werror is usually a terrible idea. > Generally agreed in released versions of any code. -Werror *may* be > appropriate in development versions depending on the project's policies, > procedures and quality of codebase. > > > Jeff
On Sat, Feb 2, 2019 at 9:20 PM Segher Boessenkool <segher@kernel.crashing.org> wrote: > > On Fri, Feb 01, 2019 at 12:27:57PM -0700, Jeff Law wrote: > > On 2/1/19 7:01 AM, Marek Polacek wrote: > > > On Fri, Feb 01, 2019 at 07:19:25AM -0600, Segher Boessenkool wrote: > > >> On Fri, Feb 01, 2019 at 12:32:45PM +0100, Marc Glisse wrote: > > >>> My opinion is that -Wmaybe-uninitialized would serve its purpose better as > > >>> part of -Wextra. > > >> > > >> +1 > > > > > > +1 from me too. > > I disagree strongly. If we move it to Wextra it's going to see a lot > > less usage in real world codebases and potentially lead to the > > re-introduction of a class of bugs that we've largely helped stomp out. > > The usual workaround, especially for programs that build with multiple > (i.e. older) versions of GCC, is to initialise any such variable (to an > either or not useful value) early. This doesn't fix the actual problem > usually (which is that your control flow is too complex). > > > It's also the case that maybe uninitialized vs is uninitialized is > > really just a function of CFG shape. Give me any "maybe uninitialized" > > case and I can turn it into a "is uninitialized" with simple block > > duplication of the forms done by jump threading, path isolation, > > superblock formation, etc. > > Are you saying that -Wmaybe-uninitialized should be included in > -Wuninitialized, since it has no extra false positives? That wasn't true > at all historically: -Wuninitialized has false positives on paths that > cannot be executed because of function preconditions, but > -Wmaybe-uninitialized used to warn for things that can be locally proven to > never execute, like > if (a) > b = 42; > ... > if (a) > f(b); IMHO the main reason of most false positives is that we rely on SSA default-defs in PHIs which are spurious when definition and use are not in domination relation as in if (p) a = 1; if (p) ... = a; the uninit pass tries to catch those cases but that's obviously hard. > > >> Yes, using -Werror is usually a terrible idea. > > Generally agreed in released versions of any code. -Werror *may* be > > appropriate in development versions depending on the project's policies, > > procedures and quality of codebase. > > IMO it is more useful it is much more useful if you make your build system > less noisy so that problems are more obvious, instead of breaking the build > for no reason all the time (see PR89162, etc. etc.) > > > Segher
>> In practice, false positives (and negatives) of both kinds, whether >> they fit the formal definition or the informal one, are the nature >> of virtually all non-trivial static diagnostics, certainly all those >> that depend on control or data flow analysis. Some are due to bugs >> or limitations in the implementation of the warning. Others are >> inherent in the technology. > > Yes, and I argue that these warnings belong in a different "level" of > warnings than the trivial warnings. Introducing more levels sounds fine to me. I wouldn't want to see a more permissive default; my preference would be the opposite, leaving it to projects to adjust. > >> Lastly, in the case of uninitialized variables, the usual solution >> of initializing them is trivial and always safe (some coding styles >> even require it). > > Here it shows that we don't work with the same type of code at all. If I > am using a boost::optional, i.e. a class with a buffer and a boolean > that says if the buffer is initialized, how do I initialize the > (private) buffer? Or should boost itself zero out the buffer whenever > the boolean is set to false? The variables can easily be hidden behind > dozens of levels of abstraction that make them hard to initialize, and > there may be nothing meaningful to initialize them with. Uninitialized > also includes clobbered (out-of-scope for instance) in gcc, where it > isn't clear what you are supposed to initialize to quiet the warning. You're right that this is hard to imagine without first hand experience with the problem. If this is a common pattern with the warning in C++ class templates in general, a representative test case would help get a better appreciation of the problem and might also give us an idea of a better solution. (If there is one in Bugzilla please point me at it.) >>> This message concentrates on the negatives, but that doesn't mean I >>> consider -Wmaybe-uninitialized as useless. It can find true >>> uninitialized uses. And even some false positives can point at places >>> where we can help the compiler generate better code (say with a >>> default __builtin_unreachable case in a switch). I did in the past >>> contribute patches to make it warn more often, and I might do so >>> again in the future. >> >> This paragraph makes me think you equate false positives from this >> warning with those from -Wuninitialized. > > Uh? No, I don't know what gave you this impression. It's common to view as a false positive (or "bad" as you write below) every instance of a warning designed to detect or prevent bugs that doesn't indicate one (as opposed to warnings designed to help write better/clearer code like -Wparentheses). >> "May be unitialized" doesn't preclude the possibility that the >> variable is not used uninitialized. > > But if the variable can never be used uninitialized and gcc is simply > unable to prove it, the warning is bad. It may not be a "false positive" > depending on your definition, but it is undesirable. Ideally each instance of every warning designed to find bugs would point out one. But 100% accuracy or a zero rate of the undesirable instances is not attainable in general, and reducing their rate at the expense of the helpful ones also isn't the best tradeoff either. The challenge is striking the right balance between their ratios. (Only if that can't done then it might be time to consider disabling/demoting the warning. I don't have the impression we are at that point with -Wmaybe-uninitialized but I haven't done any research.) >>> My opinion is that -Wmaybe-uninitialized would serve its purpose >>> better as part of -Wextra. People tend to use -Wall with -Werror >>> (either explicitly or implicitly by modifying the code until all >>> warnings are gone). What I see currently in projects where I >>> participate is that -Wmaybe-uninitialized is making things worse. >>> People don't investigate deeply the cause of the warning, they just >>> go for whatever "quick-fix" makes the compiler shut up. Quite often, >>> this involves extra code that is less readable and performs worse, >>> while it didn't even "fix" what caused the warning, it just randomly >>> ended up with code where the compiler doesn't warn (possibly because >>> the function got bigger, which changed inlining decisions...). >> >> I agree that we need to be mindful of warnings leading to unnecessary >> and perhaps inappropriate code changes (as the author of a few warnings >> with this potential it has been on mind a lot). >> >> At the same time, not taking the time to properly analyze diagnostics >> and come up with the appropriate solutions seems like a problem that's >> orthogonal to the warnings themselves. Disabling warnings in hopes of >> solving what's really a problem with the development process or >> a culture of a subset of projects doesn't seem like an optimal solution. > > I agree with that. But if I complain about the mindless quickfixes, the > reply is that customers ask for a library that does not warn, and the > developers do not have infinite time, so they do the minimum to avoid > warnings. This is strongly linked to the warning levels. > > One could recommend that customers include the library with -isystem to > disable the warnings inside the library, bug that's fragile (though it > has gotten quite good), and most importantly it also disables some > warnings that are relevant to the user. Right, that wouldn't be much better than disabling the warning on the command line (or us removing it from -Wall and perhaps also from -Wextra). So since mindless quick fixes aren't the way to go and assuming we agree that warnings have value even with some noise, is demoting them to lower levels because they're not always used properly the best solution? No predetermined system of warning levels is going to make everyone happy. Different projects have different constraints and tolerances for noise, or even resources to fix real bugs -- GCC with over 400 wrong-code bugs in Open status being a case in point, so a level that works for one, like a library, may be overly pedantic for an application. More warning levels might help. Warning profiles (say one for system libraries or freestanding code like the kernel, another for C++ class libraries, yet another for applications) might be another approach. Others might be worth brainstorming about. >>> If the warning is not enabled by default so much but only when people >>> are ready to investigate any warning thoroughly, the quickfix >>> mentality is less likely to be present. People using >>> -Wmaybe-uninitialized need to be willing to ignore false positives, >>> or disable them with pragmas. >>> >>> Note that similar arguments may apply to some other warnings that >>> somehow made their way into -Wall when they shouldn't have, but for >>> now I am only proposing to move -Wmaybe-uninitialized. Some people >>> tend to consider that if a warning is not part of -Wall, it might as >>> well not exist. Obviously I disagree with that. >> >> The description of -Wall is vague enough that arguments about which >> warnings should or shouldn't be enabled by it are unavoidably subject >> to individual biases and unlikely to lead to a consensus, either among >> ourselves or, even less likely, among users. > > True. To me, one of the key documented properties of -Wall is being > "easy to avoid". -Wall -Werror should be a usable combination. Anything > not easy to avoid belongs in a different class, say -Wextra. We can > rename that to W1/W2/... if Wall is too charged with historical meaning. I would have expected dealing with -Wmaybe-uninitialized to always be easy. Your comment about std::optional suggests there are cases when it isn't. Can we look at some of those cases and see if there is something we can do to make it easier? >> A shared definition of a false positive should be one of the very >> first steps to coming closer to a consensus. Real world (as opposed >> to anecdotal) data on the rates of actual rates of false positives and >> negatives vs true positives would be also most helpful, as would some >> consensus of the severity of the bugs the true positives expose, as >> well as some objective measure of the ease of suppression. There >> probably are others but these would be a start. > > This data is going to be super hard to get. Most projects have been > compiling for years and tweaking their code to avoid some warnings. We > do not get to see the code that people originally write, we can only see > what they commit. It's not perfect but we should be able to get a rough idea based on bug reports in Bugzilla. Many distros rebuild the world before a GCC release and new instances of build-breaking warnings tend to get filed there. Meta-bugs can be helpful as the first step. A finer-grained classification by search terms (e.g., "bogus" "spurious", etc.) helps narrow it down even more. Martin
On Mon, 4 Feb 2019, Martin Sebor wrote: >>> In practice, false positives (and negatives) of both kinds, whether >>> they fit the formal definition or the informal one, are the nature >>> of virtually all non-trivial static diagnostics, certainly all those >>> that depend on control or data flow analysis. Some are due to bugs >>> or limitations in the implementation of the warning. Others are >>> inherent in the technology. >> >> Yes, and I argue that these warnings belong in a different "level" of >> warnings than the trivial warnings. > > Introducing more levels sounds fine to me. I wouldn't want to see > a more permissive default; my preference would be the opposite, > leaving it to projects to adjust. And I'd rather suggest as default something that can be combined with -Werror (not that you should do it, just that you could), leaving the non-trivial warnings that require motivation to investigate properly to people who have at least the motivation to enable extra flags. We can only agree to disagree... >>> Lastly, in the case of uninitialized variables, the usual solution >>> of initializing them is trivial and always safe (some coding styles >>> even require it). >> >> Here it shows that we don't work with the same type of code at all. If I am >> using a boost::optional, i.e. a class with a buffer and a boolean that says >> if the buffer is initialized, how do I initialize the (private) buffer? Or >> should boost itself zero out the buffer whenever the boolean is set to >> false? The variables can easily be hidden behind dozens of levels of >> abstraction that make them hard to initialize, and there may be nothing >> meaningful to initialize them with. Uninitialized also includes clobbered >> (out-of-scope for instance) in gcc, where it isn't clear what you are >> supposed to initialize to quiet the warning. > > You're right that this is hard to imagine without first hand experience > with the problem. If this is a common pattern with the warning in C++ > class templates in general, a representative test case would help get > a better appreciation of the problem and might also give us an idea > of a better solution. (If there is one in Bugzilla please point me > at it.) Looking for "optional" and "-Wmaybe-uninitialized" shows https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78044 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635 Google also gives https://www.boost.org/doc/libs/1_69_0/libs/optional/doc/html/boost_optional/tutorial/gotchas/false_positive_with__wmaybe_uninitialized.html https://sourceware.org/ml/gdb-patches/2017-05/msg00130.html etc And that's just for using a type called 'optional' (3 implementations of it). > It's common to view as a false positive (or "bad" as you write below) > every instance of a warning designed to detect or prevent bugs that doesn't > indicate one (as opposed to warnings designed to help write > better/clearer code like -Wparentheses). Well, warnings can't be completely arbitrary, they have to serve a purpose. Preventing some control flows where the use is not dominated by the initialization clearly enough could be one, but in my experience, on average, this does not lead to better programs for any metric besides the number of warnings. Again, experience can vary. > Ideally each instance of every warning designed to find bugs would > point out one. But 100% accuracy or a zero rate of the undesirable > instances is not attainable in general, and reducing their rate at > the expense of the helpful ones also isn't the best tradeoff either. > The challenge is striking the right balance between their ratios. Yes. > (Only if that can't done then it might be time to consider > disabling/demoting the warning. I don't have the impression > we are at that point with -Wmaybe-uninitialized but I haven't > done any research.) It seems to depend on the type of code base, unsurprisingly. > So since mindless quick fixes aren't the way to go and assuming we > agree that warnings have value even with some noise, is demoting > them to lower levels because they're not always used properly > the best solution? No predetermined system of warning levels is > going to make everyone happy. Different projects have different > constraints and tolerances for noise, or even resources to fix > real bugs -- GCC with over 400 wrong-code bugs in Open status > being a case in point, so a level that works for one, like > a library, may be overly pedantic for an application. True. The main point is that there is a feeling that "gcc recommends using -Wall and fixing **all** those warnings **easily**" that does not exist (to the same level) with -Wextra. I guess that's not such a strong argument (you can probably feel that I've already given up). Coincidentally, yesterday, I had to debug a crash in one of my programs. No warning, whatever I tried. At first, -fsanitize=address didn't give anything either. Then I found out about ASAN_OPTIONS=detect_stack_use_after_return=1, which pointed out super precisely which local variable I was accessing from where, which was then trivial to understand. So there are cases where I wish we were more verbose by default. Unless I misunderstand, this is disabled because it slows things down a bit. However the slowdown does not seem that big, people who care could always tweak it and are unlikely to use the defaults anyway. But I have almost no experience with it yet.
Hi, On Mon, Feb 04 2019, Marc Glisse wrote: > On Mon, 4 Feb 2019, Martin Sebor wrote: >> ... >> You're right that this is hard to imagine without first hand experience >> with the problem. If this is a common pattern with the warning in C++ >> class templates in general, a representative test case would help get >> a better appreciation of the problem and might also give us an idea >> of a better solution. (If there is one in Bugzilla please point me >> at it.) > > Looking for "optional" and "-Wmaybe-uninitialized" shows > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78044 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635 > > Google also gives > https://www.boost.org/doc/libs/1_69_0/libs/optional/doc/html/boost_optional/tutorial/gotchas/false_positive_with__wmaybe_uninitialized.html > https://sourceware.org/ml/gdb-patches/2017-05/msg00130.html > etc > > And that's just for using a type called 'optional' (3 implementations of > it). from my very quick reading of the first googled testcase, I assume the instance of the optional class got SRAed and a warning was generated for what originally was a class member, which indeed is not easy to initialize on its own in order to avoid the warning. Would it perhaps make sense to split the -Wmaybe-uninitialized warning into two, one for scalars that are scalars in the original code and one for SRA-created scalars and move only the latter to -Wextra? Martin
On 2/4/19, Martin Sebor <msebor@gmail.com> wrote: >>> In practice, false positives (and negatives) of both kinds, whether >>> they fit the formal definition or the informal one, are the nature >>> of virtually all non-trivial static diagnostics, certainly all those >>> that depend on control or data flow analysis. Some are due to bugs >>> or limitations in the implementation of the warning. Others are >>> inherent in the technology. >> >> Yes, and I argue that these warnings belong in a different "level" of >> warnings than the trivial warnings. > > Introducing more levels sounds fine to me. I wouldn't want to see > a more permissive default; my preference would be the opposite, > leaving it to projects to adjust. For reference, besides -Wuninitialized and -Wmaybe-uninitialized, clang also has its uninitialized warnings split further into -Wsometimes-uninitialized and -Wconditional-uninitialized. -Wsometimes-uninitialized relies on the integration of their static analyzer into their FE and gets stuff that gcc puts under -Wjump-misses-init and -Wswitch-unreachable instead. -Wconditional-uninitialized is their super-aggressive variant and warns about just about everything. It intentionally includes false positives as described in the comments on this bug: https://bugs.llvm.org/show_bug.cgi?id=38856 If gcc wants to introduce more levels to -Wuninitialized, I'd start by considering the additional flags clang supports. > >> >>> Lastly, in the case of uninitialized variables, the usual solution >>> of initializing them is trivial and always safe (some coding styles >>> even require it). >> >> Here it shows that we don't work with the same type of code at all. If I >> am using a boost::optional, i.e. a class with a buffer and a boolean >> that says if the buffer is initialized, how do I initialize the >> (private) buffer? Or should boost itself zero out the buffer whenever >> the boolean is set to false? The variables can easily be hidden behind >> dozens of levels of abstraction that make them hard to initialize, and >> there may be nothing meaningful to initialize them with. Uninitialized >> also includes clobbered (out-of-scope for instance) in gcc, where it >> isn't clear what you are supposed to initialize to quiet the warning. > > You're right that this is hard to imagine without first hand experience > with the problem. If this is a common pattern with the warning in C++ > class templates in general, a representative test case would help get > a better appreciation of the problem and might also give us an idea > of a better solution. (If there is one in Bugzilla please point me > at it.) > >>>> This message concentrates on the negatives, but that doesn't mean I >>>> consider -Wmaybe-uninitialized as useless. It can find true >>>> uninitialized uses. And even some false positives can point at places >>>> where we can help the compiler generate better code (say with a >>>> default __builtin_unreachable case in a switch). I did in the past >>>> contribute patches to make it warn more often, and I might do so >>>> again in the future. >>> >>> This paragraph makes me think you equate false positives from this >>> warning with those from -Wuninitialized. >> >> Uh? No, I don't know what gave you this impression. > > It's common to view as a false positive (or "bad" as you write below) > every instance of a warning designed to detect or prevent bugs that > doesn't indicate one (as opposed to warnings designed to help write > better/clearer code like -Wparentheses). > >>> "May be unitialized" doesn't preclude the possibility that the >>> variable is not used uninitialized. >> >> But if the variable can never be used uninitialized and gcc is simply >> unable to prove it, the warning is bad. It may not be a "false positive" >> depending on your definition, but it is undesirable. > > Ideally each instance of every warning designed to find bugs would > point out one. But 100% accuracy or a zero rate of the undesirable > instances is not attainable in general, and reducing their rate at > the expense of the helpful ones also isn't the best tradeoff either. > The challenge is striking the right balance between their ratios. > (Only if that can't done then it might be time to consider > disabling/demoting the warning. I don't have the impression > we are at that point with -Wmaybe-uninitialized but I haven't > done any research.) > >>>> My opinion is that -Wmaybe-uninitialized would serve its purpose >>>> better as part of -Wextra. People tend to use -Wall with -Werror >>>> (either explicitly or implicitly by modifying the code until all >>>> warnings are gone). What I see currently in projects where I >>>> participate is that -Wmaybe-uninitialized is making things worse. >>>> People don't investigate deeply the cause of the warning, they just >>>> go for whatever "quick-fix" makes the compiler shut up. Quite often, >>>> this involves extra code that is less readable and performs worse, >>>> while it didn't even "fix" what caused the warning, it just randomly >>>> ended up with code where the compiler doesn't warn (possibly because >>>> the function got bigger, which changed inlining decisions...). >>> >>> I agree that we need to be mindful of warnings leading to unnecessary >>> and perhaps inappropriate code changes (as the author of a few warnings >>> with this potential it has been on mind a lot). >>> >>> At the same time, not taking the time to properly analyze diagnostics >>> and come up with the appropriate solutions seems like a problem that's >>> orthogonal to the warnings themselves. Disabling warnings in hopes of >>> solving what's really a problem with the development process or >>> a culture of a subset of projects doesn't seem like an optimal solution. >> >> I agree with that. But if I complain about the mindless quickfixes, the >> reply is that customers ask for a library that does not warn, and the >> developers do not have infinite time, so they do the minimum to avoid >> warnings. This is strongly linked to the warning levels. >> >> One could recommend that customers include the library with -isystem to >> disable the warnings inside the library, bug that's fragile (though it >> has gotten quite good), and most importantly it also disables some >> warnings that are relevant to the user. > > Right, that wouldn't be much better than disabling the warning on > the command line (or us removing it from -Wall and perhaps also > from -Wextra). > > So since mindless quick fixes aren't the way to go and assuming we > agree that warnings have value even with some noise, is demoting > them to lower levels because they're not always used properly > the best solution? No predetermined system of warning levels is > going to make everyone happy. Different projects have different > constraints and tolerances for noise, or even resources to fix > real bugs -- GCC with over 400 wrong-code bugs in Open status > being a case in point, so a level that works for one, like > a library, may be overly pedantic for an application. > > More warning levels might help. Warning profiles (say one for > system libraries or freestanding code like the kernel, another > for C++ class libraries, yet another for applications) might > be another approach. Others might be worth brainstorming about. > >>>> If the warning is not enabled by default so much but only when people >>>> are ready to investigate any warning thoroughly, the quickfix >>>> mentality is less likely to be present. People using >>>> -Wmaybe-uninitialized need to be willing to ignore false positives, >>>> or disable them with pragmas. >>>> >>>> Note that similar arguments may apply to some other warnings that >>>> somehow made their way into -Wall when they shouldn't have, but for >>>> now I am only proposing to move -Wmaybe-uninitialized. Some people >>>> tend to consider that if a warning is not part of -Wall, it might as >>>> well not exist. Obviously I disagree with that. >>> >>> The description of -Wall is vague enough that arguments about which >>> warnings should or shouldn't be enabled by it are unavoidably subject >>> to individual biases and unlikely to lead to a consensus, either among >>> ourselves or, even less likely, among users. >> >> True. To me, one of the key documented properties of -Wall is being >> "easy to avoid". -Wall -Werror should be a usable combination. Anything >> not easy to avoid belongs in a different class, say -Wextra. We can >> rename that to W1/W2/... if Wall is too charged with historical meaning. > > I would have expected dealing with -Wmaybe-uninitialized to always > be easy. Your comment about std::optional suggests there are cases > when it isn't. Can we look at some of those cases and see if there > is something we can do to make it easier? > >>> A shared definition of a false positive should be one of the very >>> first steps to coming closer to a consensus. Real world (as opposed >>> to anecdotal) data on the rates of actual rates of false positives and >>> negatives vs true positives would be also most helpful, as would some >>> consensus of the severity of the bugs the true positives expose, as >>> well as some objective measure of the ease of suppression. There >>> probably are others but these would be a start. >> >> This data is going to be super hard to get. Most projects have been >> compiling for years and tweaking their code to avoid some warnings. We >> do not get to see the code that people originally write, we can only see >> what they commit. > > It's not perfect but we should be able to get a rough idea based > on bug reports in Bugzilla. Many distros rebuild the world before > a GCC release and new instances of build-breaking warnings tend to > get filed there. Meta-bugs can be helpful as the first step. > A finer-grained classification by search terms (e.g., "bogus" > "spurious", etc.) helps narrow it down even more. > > Martin >
On 2/4/19, Martin Jambor <mjambor@suse.cz> wrote: > Hi, > > On Mon, Feb 04 2019, Marc Glisse wrote: >> On Mon, 4 Feb 2019, Martin Sebor wrote: >>> > > ... > >>> You're right that this is hard to imagine without first hand experience >>> with the problem. If this is a common pattern with the warning in C++ >>> class templates in general, a representative test case would help get >>> a better appreciation of the problem and might also give us an idea >>> of a better solution. (If there is one in Bugzilla please point me >>> at it.) >> >> Looking for "optional" and "-Wmaybe-uninitialized" shows >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78044 >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635 >> >> Google also gives >> https://www.boost.org/doc/libs/1_69_0/libs/optional/doc/html/boost_optional/tutorial/gotchas/false_positive_with__wmaybe_uninitialized.html >> https://sourceware.org/ml/gdb-patches/2017-05/msg00130.html >> etc >> >> And that's just for using a type called 'optional' (3 implementations of >> it). > > from my very quick reading of the first googled testcase, I assume the > instance of the optional class got SRAed and a warning was generated for > what originally was a class member, which indeed is not easy to > initialize on its own in order to avoid the warning. > > Would it perhaps make sense to split the -Wmaybe-uninitialized warning > into two, one for scalars that are scalars in the original code and one > for SRA-created scalars and move only the latter to -Wextra? What would the new warning names be once split? > > Martin >
On Mon, 4 Feb 2019, Martin Jambor wrote: >> Looking for "optional" and "-Wmaybe-uninitialized" shows >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78044 >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635 >> >> Google also gives >> https://www.boost.org/doc/libs/1_69_0/libs/optional/doc/html/boost_optional/tutorial/gotchas/false_positive_with__wmaybe_uninitialized.html >> https://sourceware.org/ml/gdb-patches/2017-05/msg00130.html >> etc >> >> And that's just for using a type called 'optional' (3 implementations of >> it). > > from my very quick reading of the first googled testcase, I assume the > instance of the optional class got SRAed and a warning was generated for > what originally was a class member, which indeed is not easy to > initialize on its own in order to avoid the warning. Hmm, SRA and -Wmaybe-uninitialized, I saw that recently in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66459 > Would it perhaps make sense to split the -Wmaybe-uninitialized warning > into two, one for scalars that are scalars in the original code and one > for SRA-created scalars and move only the latter to -Wextra? If SRA is really the main source of false positives (I don't know about that), maybe. But I am afraid we will also miss a significant proportion of the already rare true positives that -Wmaybe-uninitialized currently finds. I really have no idea if such a split would work great or badly (sorry, I am not being very useful).
On 2/4/19 11:24 PM, Marc Glisse wrote: > On Mon, 4 Feb 2019, Martin Jambor wrote: > >>> Looking for "optional" and "-Wmaybe-uninitialized" shows >>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78044 >>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635 >>> >>> Google also gives >>> https://www.boost.org/doc/libs/1_69_0/libs/optional/doc/html/boost_optional/tutorial/gotchas/false_positive_with__wmaybe_uninitialized.html >>> >>> https://sourceware.org/ml/gdb-patches/2017-05/msg00130.html >>> etc >>> >>> And that's just for using a type called 'optional' (3 implementations of >>> it). >> >> from my very quick reading of the first googled testcase, I assume the >> instance of the optional class got SRAed and a warning was generated for >> what originally was a class member, which indeed is not easy to >> initialize on its own in order to avoid the warning. > > Hmm, SRA and -Wmaybe-uninitialized, I saw that recently in > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66459 > >> Would it perhaps make sense to split the -Wmaybe-uninitialized warning >> into two, one for scalars that are scalars in the original code and one >> for SRA-created scalars and move only the latter to -Wextra? > > If SRA is really the main source of false positives (I don't know about > that), maybe. But I am afraid we will also miss a significant proportion > of the already rare true positives that -Wmaybe-uninitialized currently > finds. I really have no idea if such a split would work great or badly > (sorry, I am not being very useful). I've had some discussions with Kees and others in this space. The general consensus is that for pure scalars that everyone does a pretty good job at generating good maybe-uninitialized warnings. LLVM and GCC take different approaches, both are valid and both have been useful at giving developers actionable warnings. However, the general consensus is that for aggregates or anything living in memory is that most compilers aren't doing a particularly good job across the board in this space. Not enough objects are being thoroughly analyzed and those that are analyzed give too many false positives, etc. On the Microsoft side, they've chosen to focus on improving DSE and just initializing everything in memory (presumably flag controlled). It's not clear where LLVM is going to go in this space. ISTM that most of the analysis to do a good job at DSE-ing away the redundant initializer code should play directly into doing a good job at maybe-uninit warnings for objects in memory. However, I don't really have the time to explore here. I would generally support some experimentation in the overall space, that might include allowing for different default settings for uninit warnings of pure scalars vs aggregates/addressables. FWIW, I've been doing Fedora rawhide builds with gcc-9 since early Jan. Not everything builds with -Wall -Werror, but lots of packages do. I've only seen one maybe-uninit warning cause problems -- it was spurious and for a memory object. I didn't dig into it at all. In contrast things like the missing NUL termination warnings, buffer overflow warnings, NULL strings to *printf* warnings, etc all caught numerous (dozens) of real bugs. I mention it because it's one of the ways I know packages are building with -Wall -Werror :-) Jeff
>>>>> "Marc" == Marc Glisse <marc.glisse@inria.fr> writes: >> Lastly, in the case of uninitialized variables, the usual solution >> of initializing them is trivial and always safe (some coding styles >> even require it). Marc> Here it shows that we don't work with the same type of code at all. If Marc> I am using a boost::optional, i.e. a class with a buffer and a boolean Marc> that says if the buffer is initialized, how do I initialize the Marc> (private) buffer? Or should boost itself zero out the buffer whenever Marc> the boolean is set to false? This is https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635 (I know you know, but maybe others on the thread don't). I think in this specific case (std::optional and similar classes), GCC should provide a way for the class to indicate that -Wmaybe-uninitialized should not apply to the payload. >> A shared definition of a false positive should be one of the very >> first steps to coming closer to a consensus. Real world (as opposed >> to anecdotal) data on the rates of actual rates of false positives >> and negatives vs true positives would be also most helpful, as would >> some consensus of the severity of the bugs the true positives >> expose, as well as some objective measure of the ease of >> suppression. There probably are others but these would be a start. Marc> This data is going to be super hard to get. Most projects have been Marc> compiling for years and tweaking their code to avoid some warnings. We Marc> do not get to see the code that people originally write, we can only Marc> see what they commit. gdb has gone through this over the years -- it turns on many warnings and sometimes false positives show up. Most of the time there's a comment, for -Wmaybe-uninitialized grep for "init.*gcc" in the source. Unfortunately the comment isn't standardized; but I only get ~20 hits for this in gdb, so it isn't really so bad in practice. Tom
On 2/14/19 7:23 AM, Tom Tromey wrote: >>>>>> "Marc" == Marc Glisse <marc.glisse@inria.fr> writes: > >>> Lastly, in the case of uninitialized variables, the usual solution >>> of initializing them is trivial and always safe (some coding styles >>> even require it). > > Marc> Here it shows that we don't work with the same type of code at all. If > Marc> I am using a boost::optional, i.e. a class with a buffer and a boolean > Marc> that says if the buffer is initialized, how do I initialize the > Marc> (private) buffer? Or should boost itself zero out the buffer whenever > Marc> the boolean is set to false? > > This is https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635 (I know you > know, but maybe others on the thread don't). > > I think in this specific case (std::optional and similar classes), GCC > should provide a way for the class to indicate that > -Wmaybe-uninitialized should not apply to the payload. > >>> A shared definition of a false positive should be one of the very >>> first steps to coming closer to a consensus. Real world (as opposed >>> to anecdotal) data on the rates of actual rates of false positives >>> and negatives vs true positives would be also most helpful, as would >>> some consensus of the severity of the bugs the true positives >>> expose, as well as some objective measure of the ease of >>> suppression. There probably are others but these would be a start. > > Marc> This data is going to be super hard to get. Most projects have been > Marc> compiling for years and tweaking their code to avoid some warnings. We > Marc> do not get to see the code that people originally write, we can only > Marc> see what they commit. > > gdb has gone through this over the years -- it turns on many warnings > and sometimes false positives show up. Most of the time there's a > comment, for -Wmaybe-uninitialized grep for "init.*gcc" in the source. > Unfortunately the comment isn't standardized; but I only get ~20 hits > for this in gdb, so it isn't really so bad in practice. Yea, in retrospect we should have had a consistent marker for GCC as well. I suspect a goodly number of those initializations that went in early in the process are no longer needed. Jeff
On 2/4/19 3:52 PM, Martin Jambor wrote: > Hi, > > On Mon, Feb 04 2019, Marc Glisse wrote: >> On Mon, 4 Feb 2019, Martin Sebor wrote: >>> > > ... > >>> You're right that this is hard to imagine without first hand experience >>> with the problem. If this is a common pattern with the warning in C++ >>> class templates in general, a representative test case would help get >>> a better appreciation of the problem and might also give us an idea >>> of a better solution. (If there is one in Bugzilla please point me >>> at it.) >> >> Looking for "optional" and "-Wmaybe-uninitialized" shows >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78044 >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635 >> >> Google also gives >> https://www.boost.org/doc/libs/1_69_0/libs/optional/doc/html/boost_optional/tutorial/gotchas/false_positive_with__wmaybe_uninitialized.html >> https://sourceware.org/ml/gdb-patches/2017-05/msg00130.html >> etc >> >> And that's just for using a type called 'optional' (3 implementations of >> it). > > from my very quick reading of the first googled testcase, I assume the > instance of the optional class got SRAed and a warning was generated for > what originally was a class member, which indeed is not easy to > initialize on its own in order to avoid the warning. > > Would it perhaps make sense to split the -Wmaybe-uninitialized warning > into two, one for scalars that are scalars in the original code and one > for SRA-created scalars and move only the latter to -Wextra? I could support that. It fits in with the general sense that we're not handling aggregates and addressables as well as we could. JEff
On 02/05/2019 05:07 PM, Jeff Law wrote: > FWIW, I've been doing Fedora rawhide builds with gcc-9 since early Jan. > Not everything builds with -Wall -Werror, but lots of packages do. > I've only seen one maybe-uninit warning cause problems -- it was > spurious and for a memory object. I didn't dig into it at all. Do you have insight into how many Fedora packages explicitly disable maybe-uninit? I really don't know the answer to that, but I'd at least keep that in mind as a possible reason for the warning seemingly not causing trouble often. GDB disables it (or rather, removes it from -Werror), and I wouldn't be surprised if other C++ programs do the same, when they start using std::optional more. Since boost::optional also triggers the same warnings, set may be even larger than C++11-or-later programs. I don't know how to search the Fedora codebase, but a github search for "-Wno-maybe-uninitialized" shows > 100k hits: https://github.com/search?q=-Wno-maybe-uninitialized&type=Code and a search for -Wno-error=maybe-uninitialized shows >10k: https://github.com/search?q=-Wno-error%3Dmaybe-uninitialized&type=Code That's a much higher hit rate than say -Wno-array-bounds or -Wno-format-overflow, or -Wno-stringop-overflow, which were some searches I tried. > > In contrast things like the missing NUL termination warnings, buffer > overflow warnings, NULL strings to *printf* warnings, etc all caught > numerous (dozens) of real bugs. I mention it because it's one of the > ways I know packages are building with -Wall -Werror :-) Thanks, Pedro Alves
On 2/20/19 5:36 AM, Pedro Alves wrote: > On 02/05/2019 05:07 PM, Jeff Law wrote: >> FWIW, I've been doing Fedora rawhide builds with gcc-9 since early Jan. >> Not everything builds with -Wall -Werror, but lots of packages do. >> I've only seen one maybe-uninit warning cause problems -- it was >> spurious and for a memory object. I didn't dig into it at all. > > Do you have insight into how many Fedora packages explicitly disable > maybe-uninit? I really don't know the answer to that, but I'd at least > keep that in mind as a possible reason for the warning seemingly > not causing trouble often. Unfortunately not. And my tester only saves failed log files, so I can't just grep the log files for it. > > GDB disables it (or rather, removes it from -Werror), and I wouldn't be > surprised if other C++ programs do the same, when they start using > std::optional more. Since boost::optional also triggers the same warnings, > set may be even larger than C++11-or-later programs. Well, one of the proposals is to distinguish between scalars and memory/addressables. That should (in theory) make it possible to deal with this more sanely. jeff
On 2/20/19 7:25 AM, Jeff Law wrote: > On 2/20/19 5:36 AM, Pedro Alves wrote: >> On 02/05/2019 05:07 PM, Jeff Law wrote: >>> FWIW, I've been doing Fedora rawhide builds with gcc-9 since early Jan. >>> Not everything builds with -Wall -Werror, but lots of packages do. >>> I've only seen one maybe-uninit warning cause problems -- it was >>> spurious and for a memory object. I didn't dig into it at all. >> >> Do you have insight into how many Fedora packages explicitly disable >> maybe-uninit? I really don't know the answer to that, but I'd at least >> keep that in mind as a possible reason for the warning seemingly >> not causing trouble often. > Unfortunately not. And my tester only saves failed log files, so I > can't just grep the log files for it. Would it difficult to add a pass over the build logs to scan for these sorts of things? E.g., count the number of instances of each warning in builds that don't use -Werror? Martin > > >> >> GDB disables it (or rather, removes it from -Werror), and I wouldn't be >> surprised if other C++ programs do the same, when they start using >> std::optional more. Since boost::optional also triggers the same warnings, >> set may be even larger than C++11-or-later programs. > Well, one of the proposals is to distinguish between scalars and > memory/addressables. That should (in theory) make it possible to deal > with this more sanely. > > jeff >
On 2/20/19 8:11 AM, Martin Sebor wrote: > On 2/20/19 7:25 AM, Jeff Law wrote: >> On 2/20/19 5:36 AM, Pedro Alves wrote: >>> On 02/05/2019 05:07 PM, Jeff Law wrote: >>>> FWIW, I've been doing Fedora rawhide builds with gcc-9 since early Jan. >>>> Not everything builds with -Wall -Werror, but lots of packages do. >>>> I've only seen one maybe-uninit warning cause problems -- it was >>>> spurious and for a memory object. I didn't dig into it at all. >>> >>> Do you have insight into how many Fedora packages explicitly disable >>> maybe-uninit? I really don't know the answer to that, but I'd at least >>> keep that in mind as a possible reason for the warning seemingly >>> not causing trouble often. >> Unfortunately not. And my tester only saves failed log files, so I >> can't just grep the log files for it. > > Would it difficult to add a pass over the build logs to scan for > these sorts of things? E.g., count the number of instances of > each warning in builds that don't use -Werror? It's essentially the same as saving the successful logs :-) Everything is built with fedpkg which buries the log files. For a failed build the scripts dig them out of the filesystem and dump them to stdout. Doing the same for a successful build wouldn't be hard. Once that's in place we can scan for warnings or whatever else, even in a successful build. I'll try to get that added for the next mass test which would start Sunday after the next gcc-9 snapshot. jeff
On 2/3/19 3:02 AM, Marc Glisse wrote: > On Sat, 2 Feb 2019, Martin Sebor wrote: > >> I don't feel too strongly about whether -Wmaybe-uninitialized should >> be in -Wall or in -Wextra, and I might even be somewhat more inclined >> to expect to find it in the latter. But since you sound like you are >> gearing up for proposing other changes in the same vein down the line >> where I might have stronger opinions, I should comment. >> >> [It's a bit of a long-winded response because I've been thinking about >> this topic a lot lately.] > > Thank you for taking the time to write this down. > >> In general, I think a discussion of warning groupings is worth having >> (even needed) at the right time, but stage 4 doesn't strike me as >> the most opportune occasion. >> >> Specifically for -Wmaybe-uninitialized, the option has been in -Wall >> for many releases, and no major changes to it have been made recently >> that I know. So what I'm missing in this proposal is: why now? What >> has changed to make this a pressing issue now? Has its rate of false >> positives gone up significantly? If so, by how much and why? > > I've been wanting to post this for years, but I only got motivated > enough recently after seeing negative effects of -Wmaybe-uninitialized > in two projects within a few days. I don't think the rate of false > positives has gone up significantly in gcc-9, they just randomly appear > or disappear from one release to the next. In some sense, for projects > that support multiple compiler versions, each new gcc release makes the > number of false positives (on at least one version) go up, but that > doesn't count as the warning getting worse. > > The discussion and/or the change don't need to happen now, but if I > didn't post this patch while motivated, it might be years before I > actually do it, so I ignored stage 4. I'll also note that I proposed changes to how we handled Wuninitialized many years ago which were designed to address the problem of instability in these warnings from release to release. We would have the ability to issue warnings from early in the pipeline -- before the optimizers really muck things up. This gives you much more stable warnings from release to release (at the expense of many more false positives). We'd also be able to use the results of that very early analysis to indicate to the user when the optimizers were able to ultimately prove the early uninitialized use was optimized away (or the path which lead to the flow of an uninitialized value was optimized away). I ultimately gave up, not because of technical issues, but because of the inability to get any kind of consensus on changing things in this space. It was an amazingly frustrating experience, but did certainly highlight that there is no one answer in this space. Everyone wants different defaults. > > I am mostly looking at what I see in practice. I regularly see false > positives of -Wstrict-overflow (less now that I neutered parts of it) > and -Wmaybe-uninitialized. And when I have strange crashes that later > turn out to be caused by uninitialized memory uses, it is very seldom > that -Wmaybe-uninitialized helps detect them. IIRC, last time, I had > slightly more success with -Wreturn-local-addr -O3 > -fkeep-inline-functions -fkeep-static-functions. Which is consistent with the belief that we simply don't do a good job at uninitialized warnings for addressables and aggregates. > >> "May be unitialized" doesn't preclude the possibility that the >> variable is not used uninitialized. > > But if the variable can never be used uninitialized and gcc is simply > unable to prove it, the warning is bad. It may not be a "false positive" > depending on your definition, but it is undesirable. It's certainly undesirable and we go to nearly heroic lengths to avoid them. I'm really looking forward to the new range infrastructure as it can be exploited in the backwards threader to address a significant class of these false positives. I'd also really like to see the predicate analysis we currently use to prune uninit warnings be split out as a separate analysis. THe ability to prove a path is infeasible, even if we have to leave it in the CFG has uses elsewhere. I'd also like a way to mark infeasible paths found by the threader, but which the threader decides are not worth optimizing (usually because of the amount of code it would need to duplicate). Marking them (and somehow keeping the data accurate) would also eliminate classes of false positives from the uninitialized warnings. Finally, we also still have cases where iteration of threading + dom is needed to remove infeasible paths and avoid false positives. I'm not going to suggest we return to the that world, but we do need to continue to explore ways to thread deeper in a single iteration. >\ > True. To me, one of the key documented properties of -Wall is being > "easy to avoid". -Wall -Werror should be a usable combination. Anything > not easy to avoid belongs in a different class, say -Wextra. We can > rename that to W1/W2/... if Wall is too charged with historical meaning. > > One issue is that, especially in a large, long-lived code base, people > want to know when a change (or a new compiler) introduces a new warning. > However, they do not want to have to go through hundreds or > pre-existing, already analysed warnings. The easiest way to do that is > to make sure there are no warnings at all (-Werror). For speculative > warnings, that's really not convenient. I don't think there is a good > solution to this (marking with a pragma the known false positives may be > hard when instantiating some class somewhere causes a warning in some > almost unrelated place, and you want the marking to be narrow enough > that it won't also disable other warnings), and it is independent on > whether the warning is in -Wall or -Wextra. I just have a feeling that > splitting the easy and the not-easy warnings may help developers handle > them differently. I understand this sentiment. Believe me I do. I've worked with folks that have to deal with legacy codebases and warning stability from release to release is very important to them. We don't have a real good solution for them (this class of user was the motivation behind the -Wuninitialized changes I proposed many years ago). The best right now is to identify a set of warnings that are important to those organizations and use those and only those with -Werror. Jeff ps. Yes, I know I did a lot of snipping of the discussion. I didn't feel I had much to add to the other parts. I hope that in doing so I haven't taken anything out of context or conflated any of the issues.
On 2/2/19 1:20 PM, Segher Boessenkool wrote: > On Fri, Feb 01, 2019 at 12:27:57PM -0700, Jeff Law wrote: >> On 2/1/19 7:01 AM, Marek Polacek wrote: >>> On Fri, Feb 01, 2019 at 07:19:25AM -0600, Segher Boessenkool wrote: >>>> On Fri, Feb 01, 2019 at 12:32:45PM +0100, Marc Glisse wrote: >>>>> My opinion is that -Wmaybe-uninitialized would serve its purpose better as >>>>> part of -Wextra. >>>> >>>> +1 >>> >>> +1 from me too. >> I disagree strongly. If we move it to Wextra it's going to see a lot >> less usage in real world codebases and potentially lead to the >> re-introduction of a class of bugs that we've largely helped stomp out. > > The usual workaround, especially for programs that build with multiple > (i.e. older) versions of GCC, is to initialise any such variable (to an > either or not useful value) early. This doesn't fix the actual problem > usually (which is that your control flow is too complex). > >> It's also the case that maybe uninitialized vs is uninitialized is >> really just a function of CFG shape. Give me any "maybe uninitialized" >> case and I can turn it into a "is uninitialized" with simple block >> duplication of the forms done by jump threading, path isolation, >> superblock formation, etc. > > Are you saying that -Wmaybe-uninitialized should be included in > -Wuninitialized, since it has no extra false positives? That wasn't true > at all historically: -Wuninitialized has false positives on paths that > cannot be executed because of function preconditions, but > -Wmaybe-uninitialized used to warn for things that can be locally proven to > never execute, like > if (a) > b = 42; > ... > if (a) > f(b); No, I'm saying the distinction between maybe and always uninitialized is a false distinction. Code duplication can easily take something that triggers a "maybe" warning and turn it into a "always" warning. The distinction between them is just bogus. To take your example if (a) b = 42 ... if (a) f(b); Can be rewritten as if (a) { b = 42 [ copy of ...] f (b); } else { ... f (b); } But more importantly consider when the value flows by way of a PHI argument. We consider those "maybe" warnings (and it's not just the uninitialized warning analysis). But fairly simple code duplication can eliminate the flow via PHI and suddenly we go from a "maybe" to an "always" warning. Let's consider this... x4 = PHI (x0(D), x1, x2, x3) use x4; That's a maybe uninitialized warning by way of the value flow of x0 through the PHI into x4. But we can duplicate the block creating something like this: BB: x5 = PHI (x0(D)) use x5 goto join BB: x4 = PHI (x1, x2, x3) use x4 goto join Which we then simplify into BB: use x0(D) goto join BB: x4 = PHI (x1, x2, x3) use x4 goto join That changes the warning from a maybe-uninitialized into an always uninitialized. Note that we still may or may not reach the use at runtime. THe distinction between maybe and always is just bogus. Jeff
On 2/1/19 1:31 PM, Marc Glisse wrote: > > I am not surprised, but I had to at least start the conversation. Would > you mind providing a patch that changes the definition of -Wall, since > the current one doesn't quite match reality anymore? Also, what do you > recommend people do when they hit false positives? So I'm not sure how I'd change the definition of -Wall. > >> If we move it to Wextra it's going to see a lot less usage in real >> world codebases > > I am very tempted by the strawman: should we deprecate -Wextra since > nobody uses it? (Sorry) :-) I don't see a lot of use of -Wextra and I'm always disappointed when we have to relegate useful warnings to it precisely because it gets used so rarely. But as I've indicated earlier on this thread, it may make sense to split out uninitialized warnings on aggregates/addressables and relegate those to -Wextra until we can really beef up analysis in that space. > > Ideally serious projects would use (parts of) -Wextra, at least > occasionally, and with circumspection. But some warnings like > -Wmaybe-uninitialized are dangerous tools in the hands of quickfix > developers, and I am indeed trying to keep it out of their hands... Well, that's a different problem. If the developers are doing quick, dumb hacks to avoid a warning, then the developers need to be retrained. We shouldn't cater the warning set to make that class of developer happy or less dangerous. > >> and potentially lead to the re-introduction of a class of bugs that >> we've largely helped stomp out. > > That's very optimistic. -Wmaybe-uninitialized only detects a very small > proportion of uninitialized uses. Also, my experience is that people > have stomped out the warning, not the bugs. In some cases they even > introduced bugs to stomp out false warnings, or made it harder to detect > real bugs in the future, so the warning did more harm than good. I am > mostly working on large C++ header-only template-heavy scientific > libraries, it is quite possible that people who handle different types > of code bases have a different experience, and -Wmaybe-uninitialized may > have had a more positive impact on other projects. I see things differently. Specifically we've done a good job at making the analysis good for simple objects (anything that's an SSA_NAME). False positives happen, but are relatively rare and developers have through the decades addressed most of these problems (we've had this stuff in -Wall for 3 decades). Are there cases where developers have screwed things up in the process, certainly, but I suspect that's the exception rather than the rule. The simple fact is that if we throw an uninitialized warning on a scalar you *really* have to look at the code to figure out if it's a false positive or not. And you have to really think hard about how to initialize the value if that's in fact what you end up doing -- there's no guaranteed safe value and that's why I've resisted calls to have "initialize everything to XXX" flags through the years. On the addressable/aggregate side things are totally different. We miss tons of warnings, our ability to analyze things to avoid false positives is poor at best, etc. > >> It's also the case that maybe uninitialized vs is uninitialized is >> really just a function of CFG shape. Give me any "maybe uninitialized" >> case and I can turn it into a "is uninitialized" with simple block >> duplication of the forms done by jump threading, path isolation, >> superblock formation, etc. > > Hmm, you know those things better than me, so you are probably right, > but I am not seeing it. We omit "maybe" if, starting from the entry of > the function, and barring exceptions, we know the statement will always > be executed. If you take a false positive for maybe-uninitialized, i.e. > a statement in a dead branch, I don't see how block duplication can make > it so the statement is now always executed. The natural way to remove > "maybe" is through function cloning or outlining. Then you can create > functions that are never called, and any warning about those is a false > positive. Well, that's the problem. Right now the maybe vs always is based on if the value flows through a PHI. But I can avoid having the value flow through a PHI with simple block copying and light CFG manipulation. But in both cases there's still a control dependency path to get to the point where the uninitialized use happens. And you also have the issue of proving that a particular function even executes to begin with. That's why I think the distinction simply doesn't make any sense. We should just call them all maybe-uninitialized uses. Jeff
On 2/20/19, Jeff Law <law@redhat.com> wrote: > On 2/1/19 1:31 PM, Marc Glisse wrote: >> >> I am not surprised, but I had to at least start the conversation. Would >> you mind providing a patch that changes the definition of -Wall, since >> the current one doesn't quite match reality anymore? Also, what do you >> recommend people do when they hit false positives? > So I'm not sure how I'd change the definition of -Wall. > >> >>> If we move it to Wextra it's going to see a lot less usage in real >>> world codebases >> >> I am very tempted by the strawman: should we deprecate -Wextra since >> nobody uses it? (Sorry) > :-) I don't see a lot of use of -Wextra and I'm always disappointed > when we have to relegate useful warnings to it precisely because it gets > used so rarely. Part of that could be because they're still using the old name of "-W" for it. Since the documentation says the name "-Wextra" is preferred, maybe it's time to deprecate the old "-W" alias to get people to spell it just one way; that would make it easier to count its usage. > > But as I've indicated earlier on this thread, it may make sense to split > out uninitialized warnings on aggregates/addressables and relegate those > to -Wextra until we can really beef up analysis in that space. > > > >> >> Ideally serious projects would use (parts of) -Wextra, at least >> occasionally, and with circumspection. But some warnings like >> -Wmaybe-uninitialized are dangerous tools in the hands of quickfix >> developers, and I am indeed trying to keep it out of their hands... > Well, that's a different problem. If the developers are doing quick, > dumb hacks to avoid a warning, then the developers need to be retrained. > We shouldn't cater the warning set to make that class of developer > happy or less dangerous. > >> >>> and potentially lead to the re-introduction of a class of bugs that >>> we've largely helped stomp out. >> >> That's very optimistic. -Wmaybe-uninitialized only detects a very small >> proportion of uninitialized uses. Also, my experience is that people >> have stomped out the warning, not the bugs. In some cases they even >> introduced bugs to stomp out false warnings, or made it harder to detect >> real bugs in the future, so the warning did more harm than good. I am >> mostly working on large C++ header-only template-heavy scientific >> libraries, it is quite possible that people who handle different types >> of code bases have a different experience, and -Wmaybe-uninitialized may >> have had a more positive impact on other projects. > I see things differently. Specifically we've done a good job at making > the analysis good for simple objects (anything that's an SSA_NAME). > False positives happen, but are relatively rare and developers have > through the decades addressed most of these problems (we've had this > stuff in -Wall for 3 decades). > > Are there cases where developers have screwed things up in the process, > certainly, but I suspect that's the exception rather than the rule. > > The simple fact is that if we throw an uninitialized warning on a scalar > you *really* have to look at the code to figure out if it's a false > positive or not. And you have to really think hard about how to > initialize the value if that's in fact what you end up doing -- there's > no guaranteed safe value and that's why I've resisted calls to have > "initialize everything to XXX" flags through the years. > > On the addressable/aggregate side things are totally different. We miss > tons of warnings, our ability to analyze things to avoid false positives > is poor at best, etc. > > >> >>> It's also the case that maybe uninitialized vs is uninitialized is >>> really just a function of CFG shape. Give me any "maybe uninitialized" >>> case and I can turn it into a "is uninitialized" with simple block >>> duplication of the forms done by jump threading, path isolation, >>> superblock formation, etc. >> >> Hmm, you know those things better than me, so you are probably right, >> but I am not seeing it. We omit "maybe" if, starting from the entry of >> the function, and barring exceptions, we know the statement will always >> be executed. If you take a false positive for maybe-uninitialized, i.e. >> a statement in a dead branch, I don't see how block duplication can make >> it so the statement is now always executed. The natural way to remove >> "maybe" is through function cloning or outlining. Then you can create >> functions that are never called, and any warning about those is a false >> positive. > Well, that's the problem. Right now the maybe vs always is based on if > the value flows through a PHI. But I can avoid having the value flow > through a PHI with simple block copying and light CFG manipulation. But > in both cases there's still a control dependency path to get to the > point where the uninitialized use happens. And you also have the issue > of proving that a particular function even executes to begin with. > > That's why I think the distinction simply doesn't make any sense. We > should just call them all maybe-uninitialized uses. > > Jeff >
Hi, On Wed, 20 Feb 2019, Jeff Law wrote: > No, I'm saying the distinction between maybe and always uninitialized is > a false distinction. Code duplication can easily take something that > triggers a "maybe" warning and turn it into a "always" warning. The > distinction between them is just bogus. > > To take your example > > if (a) > b = 42 > ... > if (a) > f(b); > > Can be rewritten as > > if (a) { > b = 42 > [ copy of ...] > f (b); > } else { > ... > f (b); // xxx > } No, it cannot. It would introduce a call to f(b) in a path (at xxx) where it wasn't before. Any transformation that transforms maybe-uninit into must-uninit is either wrong or introduces more-obviously-dead-than-before code, like in this variant of yours that is correct: if (a) { b = 42 [ copy of ...] f (b); } else { ... if (a) // clearly false, as dominated by if (!a) f (b); // hence more obviously dead than in original code } > x4 = PHI (x0(D), x1, x2, x3) > use x4; > > That's a maybe uninitialized warning by way of the value flow of x0 > through the PHI into x4. So clearly the use of x4 when reached via edge using x0(D) must be dead (or we have a real uninit use) but we can't differ because there's only one use of x4 statically ... nevertheless it's unobviously-dead. > Which we then simplify into > > BB: > use x0(D) > goto join > > BB: > x4 = PHI (x1, x2, x3) > use x4 > goto join And this makes the uninit use of x0(D) more obviously dead because now the problematic and non-problematic uses are separated. So, the transformation introduced more-obviously-dead code, which it just as well could have removed/not generated, getting rid of the must-uninit warning. I guess what I'm saying is that whenever a transformation is about to introduce a must-uninit from a maybe-uninit that instead all paths leading to that must-uninit should be removed instead. > That changes the warning from a maybe-uninitialized into an always > uninitialized. Note that we still may or may not reach the use at > runtime. THe distinction between maybe and always is just bogus. I think as stated that's a bit extreme :) On user code before optimization the distinction is useful. The problem stems from us generating warnings somewhen in the middle of the optimization pipeline. As long as we continue doing that we will always have these different opinions about what is or isn't rightfully a maybe-uninit or not :-/ Ciao, Michael.
The problem with false positives is correlated with the degree of
optimization; a lot of people have reported problems at only the
`-Og' optimization level (even when the code in question is
embarrassingly correct).
Therefore, the general solution is probably that the implem-
entation of `-Wmaybe-uninitialized' be customized for the given
level of optimization.
However, I get the impression that this is easier said than done.
From what I've read, `-Wmaybe-uninitialized' is essentially
customized for `-O2', which means that it will work pretty darn
well at that optimization level. So, in the interim, I propose a
simple approximation of the general solution:
At an optimization level below `-O2' (or other than `-O2'):
* `-Wmaybe-uninitialized' is moved to `-Wextra'.
If `-Wall' has been specified, then gcc emits an
informational note stating that `-Wmaybe-uninitialized' (or
any other warning that has similar problems) has been
disabled.
* Provide a command-line option to disable such a note.
* The user may always enable `-Wmaybe-uninitialized' either
explicitly or as part of `-Wextra'.
* If `-Wmaybe-uninitialized' is enabled by the user, then it
implies `-O2' computations.
That is to say, when the user specifies:
-Og -Wmaybe-uninitialized
or:
-Og -Wextra
then the user is explicitly telling the compiler to do all
the optimizations of `-O2', but ONLY for the purpose of
implementing `-Wmaybe-uninitialized' (or whichever warning
requires those optimizations to function well); all those
optimizations are to be thrown out after they have been
used to good effect by `-Wmaybe-uninitialized'. The Code
generation, etc., shall be performed at the optimization
level the user specified (namely, `-Og' in this case).
In other words, save the user from gcc's foibles, but let the
user pay for the extra computation if so desired!
What do you think?
Sincerely,
Michael Witten
PS
All of this trouble indicates that C (and other languages)
are just not expressive enough with regard to initialization.
Initialization semantics are basically a matter of API contract
specification; the programmer needs the tools to write it down.
Surely, gcc could provide `__builtin_assume_initialized(x);' and
parameter attributes to help inform the reader (i.e., to help
inform the compiler) about the code; a function parameter
attribute could specify whether the given argument can be
considered initialized after a call, and maybe specify further
constraints, such as whether the guarantee is made only when the
function return value is nonzero (or a certain value), etc.
We need the language to write our thoughts down!
On Tue, 2019-11-26 at 19:33 +0000, Michael Witten wrote: > The problem with false positives is correlated with the degree of > optimization; a lot of people have reported problems at only the > `-Og' optimization level (even when the code in question is > embarrassingly correct). > > Therefore, the general solution is probably that the implem- > entation of `-Wmaybe-uninitialized' be customized for the given > level of optimization. > > However, I get the impression that this is easier said than done. > > From what I've read, `-Wmaybe-uninitialized' is essentially > customized for `-O2', which means that it will work pretty darn > well at that optimization level. So, in the interim, I propose a > simple approximation of the general solution: > > At an optimization level below `-O2' (or other than `-O2'): > > * `-Wmaybe-uninitialized' is moved to `-Wextra'. > > If `-Wall' has been specified, then gcc emits an > informational note stating that `-Wmaybe-uninitialized' (or > any other warning that has similar problems) has been > disabled. > > * Provide a command-line option to disable such a note. > > * The user may always enable `-Wmaybe-uninitialized' either > explicitly or as part of `-Wextra'. > > * If `-Wmaybe-uninitialized' is enabled by the user, then it > implies `-O2' computations. > > That is to say, when the user specifies: > > -Og -Wmaybe-uninitialized > > or: > > -Og -Wextra > > then the user is explicitly telling the compiler to do all > the optimizations of `-O2', but ONLY for the purpose of > implementing `-Wmaybe-uninitialized' (or whichever warning > requires those optimizations to function well); all those > optimizations are to be thrown out after they have been > used to good effect by `-Wmaybe-uninitialized'. The Code > generation, etc., shall be performed at the optimization > level the user specified (namely, `-Og' in this case). > > In other words, save the user from gcc's foibles, but let the > user pay for the extra computation if so desired! > > What do you think? I think this would be a terrible idea. Yes, it's absolutely true that changing options can get you different warning results. Yes it's absolutely true that there are false positives. But, no the warning is not customized to -O2. It's just that O2 enables more optimizations that are particularly good at weeding out false positives. The whole point behind the uninitialized warning is to capture cases where objects may not be properly initialized. For modern code the simple cases typically "just work". What is by far the most interesting cases are those with complex flow control, often interacting with inline functions, address-of stripping, etc. These are precisely the cases that humans aren't particularly good at catching and having the compiler analyze those paths and issue warnings that humans fix ultimately results in better quality code. Experience has shown that if you put something in -Wall, people will pay attention to it, and that is good for the long term quality of code bases. If the diagnostic is outside of -Wall, it's largely ignored. I think the pain of dealing with the Wuninitialized warts is smaller than the pain of allowing these errors to persist. jeff
Hi, On Tue, Nov 26 2019, Michael Witten wrote: > [...] > From what I've read, `-Wmaybe-uninitialized' is essentially > customized for `-O2', I don't think that is true. It can be perfectly useful -O1 and there are many nasty false positives at -O2 too. > [...] > * If `-Wmaybe-uninitialized' is enabled by the user, then it > implies `-O2' computations. > > That is to say, when the user specifies: > > -Og -Wmaybe-uninitialized > > or: > > -Og -Wextra > > then the user is explicitly telling the compiler to do all > the optimizations of `-O2', but ONLY for the purpose of > implementing `-Wmaybe-uninitialized' If you try to implement this proposal, you'll quickly find that there is no such thing as like "do all optimizations only for purposes of a warning." You either perform a transformation or not and subsequent passes, such as pass_late_warn_uninitialized, start work where the previous ones left off. Martin
Hi Jeff, On Sat, Dec 07 2019, Jeff Law wrote: > [...] > The whole point behind the uninitialized warning is to capture cases > where objects may not be properly initialized. For modern code the > simple cases typically "just work". What is by far the most > interesting cases are those with complex flow control, often > interacting with inline functions, address-of stripping, etc. These > are precisely the cases that humans aren't particularly good at > catching and having the compiler analyze those paths and issue warnings > that humans fix ultimately results in better quality code. Or "fixes" like: https://gcc.gnu.org/viewcvs/gcc/trunk/gcc/expmed.c?r1=277864&r2=277863&pathrev=277864 if even we cannot deal with the false positive in our own compiler, how can we expect our users to do so? > > Experience has shown that if you put something in -Wall, people will > pay attention to it, and that is good for the long term quality of code > bases. If the diagnostic is outside of -Wall, it's largely ignored. I > think the pain of dealing with the Wuninitialized warts is smaller than > the pain of allowing these errors to persist. I'm afraid I that -Wmaybe-uninitialized is getting out of hand. I bet that some users regularly get these warnings coming from c++ header "libraries" (like they sometimes come out our vec.h which recently "broke" bootstrap) which they sometimes even cannot change and they then conclude that our -Wall is "unusable" and stop paying attention to all warnings. Martin
On 12/16/19 2:45 PM, Martin Jambor wrote: > On Sat, Dec 07 2019, Jeff Law wrote: >> [...] > I'm afraid I that -Wmaybe-uninitialized is getting out of hand. I bet > that some users regularly get these warnings coming from c++ header > "libraries" (like they sometimes come out our vec.h which recently > "broke" bootstrap) which they sometimes even cannot change and they then > conclude that our -Wall is "unusable" and stop paying attention to all > warnings. -Wmaybe-uninitialized that trigger in std::optional (and clones) (PR80635 [1]) are particularly annoying, and there's no sane workaround the user can apply. You'll find quite a number of those just by googling for it: https://www.google.com/search?q=std+optional+"-Wmaybe-uninitialized" We have a few of those in GDB, and because GDB uses -Wall + -Werror, GDB nowadays builds with -Wno-error=maybe-uninitialized so that we see the warnings but the build continues without error. People still occasionally get confused and waste time with those warnings, though. Here, just this week, point 5: https://sourceware.org/ml/gdb-patches/2019-12/msg00706.html FWIW, I've considered completely disabling -Wmaybe-uninitialized in GDB instead of downgrading it from -Werror to a warning with -Wno-error. [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635
On Mon, 2019-12-16 at 15:45 +0100, Martin Jambor wrote: > Hi Jeff, > > On Sat, Dec 07 2019, Jeff Law wrote: > > [...] > > The whole point behind the uninitialized warning is to capture cases > > where objects may not be properly initialized. For modern code the > > simple cases typically "just work". What is by far the most > > interesting cases are those with complex flow control, often > > interacting with inline functions, address-of stripping, etc. These > > are precisely the cases that humans aren't particularly good at > > catching and having the compiler analyze those paths and issue warnings > > that humans fix ultimately results in better quality code. > > Or "fixes" like: > > https://gcc.gnu.org/viewcvs/gcc/trunk/gcc/expmed.c?r1=277864&r2=277863&pathrev=277864 :( > > if even we cannot deal with the false positive in our own compiler, how > can we expect our users to do so? Fair point. And I've certainly seen painful-to-fix fallout in my Fedora builds -- by far the most painful cases have been addressables where we're able to strip the address-of due to inlining. > > > Experience has shown that if you put something in -Wall, people will > > pay attention to it, and that is good for the long term quality of code > > bases. If the diagnostic is outside of -Wall, it's largely ignored. I > > think the pain of dealing with the Wuninitialized warts is smaller than > > the pain of allowing these errors to persist. > > I'm afraid I that -Wmaybe-uninitialized is getting out of hand. I bet > that some users regularly get these warnings coming from c++ header > "libraries" (like they sometimes come out our vec.h which recently > "broke" bootstrap) which they sometimes even cannot change and they then > conclude that our -Wall is "unusable" and stop paying attention to all > warnings. If it's coming from our headers, then we make a huge effort to fix the issues :-) jeff
Index: gcc/c-family/c.opt =================================================================== --- gcc/c-family/c.opt (revision 268425) +++ gcc/c-family/c.opt (working copy) @@ -1136,21 +1136,21 @@ Warn about @selector()s without previous Wundef C ObjC C++ ObjC++ CPP(warn_undef) CppReason(CPP_W_UNDEF) Var(cpp_warn_undef) Init(0) Warning Warn if an undefined macro is used in an #if directive. Wuninitialized C ObjC C++ ObjC++ LTO LangEnabledBy(C ObjC C++ ObjC++ LTO,Wall) ; Wmaybe-uninitialized -C ObjC C++ ObjC++ LTO LangEnabledBy(C ObjC C++ ObjC++ LTO,Wall) +C ObjC C++ ObjC++ LTO LangEnabledBy(C ObjC C++ ObjC++ LTO,Wextra) ; Wunknown-pragmas C ObjC C++ ObjC++ Warning Var(warn_unknown_pragmas) LangEnabledBy(C ObjC C++ ObjC++,Wall, 1, 0) Warn about unrecognized pragmas. Wunsuffixed-float-constants C ObjC Var(warn_unsuffixed_float_constants) Warning Warn about unsuffixed float constants. Index: gcc/common.opt =================================================================== --- gcc/common.opt (revision 268425) +++ gcc/common.opt (working copy) @@ -769,25 +769,25 @@ Do not suppress warnings from system hea Wtrampolines Common Var(warn_trampolines) Warning Warn whenever a trampoline is generated. Wtype-limits Common Var(warn_type_limits) Warning EnabledBy(Wextra) Warn if a comparison is always true or always false due to the limited range of the data type. Wuninitialized -Common Var(warn_uninitialized) Warning EnabledBy(Wextra) +Common Var(warn_uninitialized) Warning EnabledBy(Wmaybe-uninitialized) Warn about uninitialized automatic variables. Wmaybe-uninitialized -Common Var(warn_maybe_uninitialized) Warning EnabledBy(Wuninitialized) +Common Var(warn_maybe_uninitialized) Warning EnabledBy(Wextra) Warn about maybe uninitialized automatic variables. Wunreachable-code Common Ignore Warning Does nothing. Preserved for backward compatibility. Wunused Common Var(warn_unused) Init(0) Warning Enable all -Wunused- warnings. Index: gcc/doc/invoke.texi =================================================================== --- gcc/doc/invoke.texi (revision 268425) +++ gcc/doc/invoke.texi (working copy) @@ -4376,21 +4376,20 @@ Options} and @ref{Objective-C and Object -Wduplicate-decl-specifier @r{(C and Objective-C only)} @gol -Wenum-compare @r{(in C/ObjC; this is on by default in C++)} @gol -Wformat @gol -Wint-in-bool-context @gol -Wimplicit @r{(C and Objective-C only)} @gol -Wimplicit-int @r{(C and Objective-C only)} @gol -Wimplicit-function-declaration @r{(C and Objective-C only)} @gol -Winit-self @r{(only for C++)} @gol -Wlogical-not-parentheses @gol -Wmain @r{(only for C/ObjC and unless} @option{-ffreestanding}@r{)} @gol --Wmaybe-uninitialized @gol -Wmemset-elt-size @gol -Wmemset-transposed-args @gol -Wmisleading-indentation @r{(only for C/C++)} @gol -Wmissing-attributes @gol -Wmissing-braces @r{(only for C/ObjC)} @gol -Wmultistatement-macros @gol -Wnarrowing @r{(only for C++)} @gol -Wnonnull @gol -Wnonnull-compare @gol -Wopenmp-simd @gol @@ -4432,28 +4431,28 @@ them must be enabled individually. This enables some extra warning flags that are not enabled by @option{-Wall}. (This option used to be called @option{-W}. The older name is still supported, but the newer name is more descriptive.) @gccoptlist{-Wclobbered @gol -Wcast-function-type @gol -Wdeprecated-copy @r{(C++ only)} @gol -Wempty-body @gol -Wignored-qualifiers @gol -Wimplicit-fallthrough=3 @gol +-Wmaybe-uninitialized @gol -Wmissing-field-initializers @gol -Wmissing-parameter-type @r{(C only)} @gol -Wold-style-declaration @r{(C only)} @gol -Woverride-init @gol -Wsign-compare @r{(C only)} @gol -Wredundant-move @r{(only for C++)} @gol -Wtype-limits @gol --Wuninitialized @gol -Wshift-negative-value @r{(in C++03 and in C99 and newer)} @gol -Wunused-parameter @r{(only with} @option{-Wunused} @r{or} @option{-Wall}@r{)} @gol -Wunused-but-set-parameter @r{(only with} @option{-Wunused} @r{or} @option{-Wall}@r{)}} The option @option{-Wextra} also prints warning messages for the following cases: @itemize @bullet @@ -5527,20 +5526,22 @@ variables that are uninitialized or clob not occur for variables or elements declared @code{volatile}. Because these warnings depend on optimization, the exact variables or elements for which there are warnings depends on the precise optimization options and version of GCC used. Note that there may be no warning about a variable that is used only to compute a value that itself is never used, because such computations may be deleted by data flow analysis before the warnings are printed. +This warning is enabled by @option{-Wmaybe-uninitialized}. + @item -Winvalid-memory-model @opindex Winvalid-memory-model @opindex Wno-invalid-memory-model Warn for invocations of @ref{__atomic Builtins}, @ref{__sync Builtins}, and the C11 atomic generic functions with a memory consistency argument that is either invalid for the operation or outside the range of values of the @code{memory_order} enumeration. For example, since the @code{__atomic_store} and @code{__atomic_store_n} built-ins are only defined for the relaxed, release, and sequentially consistent memory orders the following code is diagnosed: @@ -5599,21 +5600,21 @@ changed by a call to @code{longjmp}. The compiler sees only the calls to @code{setjmp}. It cannot know where @code{longjmp} will be called; in fact, a signal handler could call it at any point in the code. As a result, you may get a warning even when there is in fact no problem because @code{longjmp} cannot in fact be called at the place that would cause a problem. Some spurious warnings can be avoided if you declare all the functions you use that never return as @code{noreturn}. @xref{Function Attributes}. -This warning is enabled by @option{-Wall} or @option{-Wextra}. +This warning is enabled by @option{-Wextra}. @item -Wunknown-pragmas @opindex Wunknown-pragmas @opindex Wno-unknown-pragmas @cindex warning for unknown pragmas @cindex unknown pragmas, warning @cindex pragmas, warning of unknown Warn when a @code{#pragma} directive is encountered that is not understood by GCC@. If this command-line option is used, warnings are even issued for unknown pragmas in system header files. This is not the case if Index: gcc/testsuite/c-c++-common/pr69543-1.c =================================================================== --- gcc/testsuite/c-c++-common/pr69543-1.c (revision 268425) +++ gcc/testsuite/c-c++-common/pr69543-1.c (working copy) @@ -1,11 +1,11 @@ -/* { dg-options "-Wuninitialized" } */ +/* { dg-options "-Wmaybe-uninitialized -Wuninitialized" } */ /* Verify disabling a warning, where the _Pragma is within a macro, but the affected code is *not* in a macro. */ # define YY_IGNORE_MAYBE_UNINITIALIZED_BEGIN \ _Pragma ("GCC diagnostic push") \ _Pragma ("GCC diagnostic ignored \"-Wuninitialized\"")\ _Pragma ("GCC diagnostic ignored \"-Wmaybe-uninitialized\"") # define YY_IGNORE_MAYBE_UNINITIALIZED_END \ _Pragma ("GCC diagnostic pop") Index: gcc/testsuite/c-c++-common/pr69543-2.c =================================================================== --- gcc/testsuite/c-c++-common/pr69543-2.c (revision 268425) +++ gcc/testsuite/c-c++-common/pr69543-2.c (working copy) @@ -1,11 +1,11 @@ -/* { dg-options "-Wuninitialized" } */ +/* { dg-options "-Wmaybe-uninitialized -Wuninitialized" } */ /* Verify disabling a warning, where both the _Pragma and the affected code are *not* in a macro. */ void test (char yylval) { char *yyvsp; _Pragma ("GCC diagnostic push") _Pragma ("GCC diagnostic ignored \"-Wuninitialized\"") _Pragma ("GCC diagnostic ignored \"-Wmaybe-uninitialized\"") Index: gcc/testsuite/c-c++-common/pr69543-3.c =================================================================== --- gcc/testsuite/c-c++-common/pr69543-3.c (revision 268425) +++ gcc/testsuite/c-c++-common/pr69543-3.c (working copy) @@ -1,11 +1,11 @@ -/* { dg-options "-Wuninitialized" } */ +/* { dg-options "-Wmaybe-uninitialized -Wuninitialized" } */ /* Verify disabling a warning, where the _Pragma is in regular code, but the affected code is within a macro. */ /* TODO: XFAIL: both C and C++ erroneously fail to suppress the warning The warning is reported at the macro definition location, rather than the macro expansion location. */ #define WARNABLE_CODE *++yyvsp = yylval; /* { dg-bogus "used uninitialized" "" { xfail *-*-* } } */ Index: gcc/testsuite/c-c++-common/pr69543-4.c =================================================================== --- gcc/testsuite/c-c++-common/pr69543-4.c (revision 268425) +++ gcc/testsuite/c-c++-common/pr69543-4.c (working copy) @@ -1,11 +1,11 @@ -/* { dg-options "-Wuninitialized" } */ +/* { dg-options "-Wmaybe-uninitialized -Wuninitialized" } */ /* Verify disabling a warning, where both the _Pragma and the affected code are within (different) macros. */ /* TODO: XFAIL: both C and C++ erroneously fail to suppress the warning The warning is reported at the macro definition location, rather than the macro expansion location. */ # define YY_IGNORE_MAYBE_UNINITIALIZED_BEGIN \ _Pragma ("GCC diagnostic push") \ Index: gcc/testsuite/c-c++-common/uninit-17.c =================================================================== --- gcc/testsuite/c-c++-common/uninit-17.c (revision 268425) +++ gcc/testsuite/c-c++-common/uninit-17.c (working copy) @@ -1,12 +1,12 @@ /* { dg-do compile } */ -/* { dg-options "-O2 -Wuninitialized -fno-ivopts" } */ +/* { dg-options "-O2 -Wmaybe-uninitialized -fno-ivopts" } */ inline int foo(int x) { return x; } static void bar(int a, int *ptr) { do { int b; /* { dg-message "declared" } */ Index: gcc/testsuite/g++.dg/pr48484.C =================================================================== --- gcc/testsuite/g++.dg/pr48484.C (revision 268425) +++ gcc/testsuite/g++.dg/pr48484.C (working copy) @@ -1,12 +1,12 @@ /* { dg-do compile } */ -/* { dg-options "-O -finline-functions -finline-small-functions -Wuninitialized" } */ +/* { dg-options "-O -finline-functions -finline-small-functions -Wmaybe-uninitialized" } */ /* { dg-add-options bind_pic_locally } */ struct SQObjectPtr { int _type; SQObjectPtr operator = (long); }; struct SQObjectPtrVec { SQObjectPtr fff (unsigned); Index: gcc/testsuite/g++.dg/uninit-pred-1_b.C =================================================================== --- gcc/testsuite/g++.dg/uninit-pred-1_b.C (revision 268425) +++ gcc/testsuite/g++.dg/uninit-pred-1_b.C (working copy) @@ -1,12 +1,12 @@ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -O2" } */ +/* { dg-options "-Wmaybe-uninitialized -O2" } */ typedef long long int64; void incr (); bool is_valid (int); int get_time (); class A { public: A (); Index: gcc/testsuite/g++.dg/uninit-pred-2_b.C =================================================================== --- gcc/testsuite/g++.dg/uninit-pred-2_b.C (revision 268425) +++ gcc/testsuite/g++.dg/uninit-pred-2_b.C (working copy) @@ -1,12 +1,12 @@ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -O2" } */ +/* { dg-options "-Wmaybe-uninitialized -O2" } */ typedef long long int64; void incr (); bool is_valid (int); int get_time (); class A { public: A (); Index: gcc/testsuite/g++.dg/uninit-pred-3_b.C =================================================================== --- gcc/testsuite/g++.dg/uninit-pred-3_b.C (revision 268425) +++ gcc/testsuite/g++.dg/uninit-pred-3_b.C (working copy) @@ -1,12 +1,12 @@ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -O2" } */ +/* { dg-options "-Wmaybe-uninitialized -O2" } */ /* Multiple initialization paths. */ typedef long long int64; void incr (); bool is_valid (int); int get_time (); class A { Index: gcc/testsuite/g++.dg/warn/Wuninitialized-5.C =================================================================== --- gcc/testsuite/g++.dg/warn/Wuninitialized-5.C (revision 268425) +++ gcc/testsuite/g++.dg/warn/Wuninitialized-5.C (working copy) @@ -1,13 +1,13 @@ // PR middle-end/39666 // { dg-do compile } -// { dg-options "-O2 -Wuninitialized" } +// { dg-options "-O2 -Wmaybe-uninitialized" } int foo (int i) { int j; switch (i) { case -__INT_MAX__ - 1 ... -1: j = 6; break; Index: gcc/testsuite/g++.dg/warn/Wuninitialized-6.C =================================================================== --- gcc/testsuite/g++.dg/warn/Wuninitialized-6.C (revision 268425) +++ gcc/testsuite/g++.dg/warn/Wuninitialized-6.C (working copy) @@ -1,12 +1,12 @@ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -O2" } */ +/* { dg-options "-Wmaybe-uninitialized -Wuninitialized -O2" } */ namespace std { typedef __SIZE_TYPE__ size_t; } extern "C++" { inline void* operator new(std::size_t, void* __p) throw() { return __p; } } namespace boost{ Index: gcc/testsuite/g++.dg/warn/Wuninitialized-9.C =================================================================== --- gcc/testsuite/g++.dg/warn/Wuninitialized-9.C (revision 268425) +++ gcc/testsuite/g++.dg/warn/Wuninitialized-9.C (working copy) @@ -1,13 +1,13 @@ // PR c++/80119 // { dg-do compile { target c++11 } } -// { dg-options "-Wuninitialized" } +// { dg-options "-Wmaybe-uninitialized" } #include <type_traits> template <bool b> void failing_function(std::integral_constant<bool, b>) { int i; if (b && (i = 4)) { ++i; // { dg-bogus "may be used uninitialized" } } Index: gcc/testsuite/gcc.dg/gomp/pr72781.c =================================================================== --- gcc/testsuite/gcc.dg/gomp/pr72781.c (revision 268425) +++ gcc/testsuite/gcc.dg/gomp/pr72781.c (working copy) @@ -1,13 +1,13 @@ /* PR middle-end/72781 */ /* { dg-do compile } */ -/* { dg-additional-options "-O2 -Wuninitialized" } */ +/* { dg-additional-options "-O2 -Wmaybe-uninitialized" } */ int u; void foo (int *p) { int i; #pragma omp for simd lastprivate(u) schedule (static, 32) /* { dg-bogus "may be used uninitialized in this function" } */ for (i = 0; i < 1024; i++) u = p[i]; Index: gcc/testsuite/gcc.dg/pr18501.c =================================================================== --- gcc/testsuite/gcc.dg/pr18501.c (revision 268425) +++ gcc/testsuite/gcc.dg/pr18501.c (working copy) @@ -1,14 +1,14 @@ /* Expected uninitialized variable warning. */ /* { dg-do compile } */ -/* { dg-options "-O -Wuninitialized" } */ +/* { dg-options "-O -Wmaybe-uninitialized" } */ unsigned bmp_iter_set (); int something (void); void bitmap_print_value_set (void) { unsigned first; /* { dg-warning "may be used" "conditional in loop" { xfail *-*-* } } */ for (; bmp_iter_set (); ) Index: gcc/testsuite/gcc.dg/pr39666-2.c =================================================================== --- gcc/testsuite/gcc.dg/pr39666-2.c (revision 268425) +++ gcc/testsuite/gcc.dg/pr39666-2.c (working copy) @@ -1,13 +1,13 @@ /* PR middle-end/39666 */ /* { dg-do compile } */ -/* { dg-options "-O2 -Wuninitialized" } */ +/* { dg-options "-O2 -Wmaybe-uninitialized" } */ int foo (int i) { int j; switch (i) { case -__INT_MAX__ - 1 ... -1: j = 6; break; Index: gcc/testsuite/gcc.dg/pr45083.c =================================================================== --- gcc/testsuite/gcc.dg/pr45083.c (revision 268425) +++ gcc/testsuite/gcc.dg/pr45083.c (working copy) @@ -1,13 +1,13 @@ /* PR tree-optimization/45083 */ /* { dg-do compile } */ -/* { dg-options "-O2 -Wuninitialized" } */ +/* { dg-options "-O2 -Wuninitialized -Wmaybe-uninitialized" } */ struct S { char *a; unsigned b; unsigned c; }; extern int foo (const char *); extern void bar (int, int); static void baz (void) { struct S cs[1]; /* { dg-message "was declared here" } */ switch (cs->b) /* { dg-warning "cs\[^\n\r\]*\\.b\[^\n\r\]*is used uninitialized" } */ Index: gcc/testsuite/gcc.dg/pr57149.c =================================================================== --- gcc/testsuite/gcc.dg/pr57149.c (revision 268425) +++ gcc/testsuite/gcc.dg/pr57149.c (working copy) @@ -1,13 +1,13 @@ /* PR tree-optimization/57149 */ /* { dg-do compile } */ -/* { dg-options "-Os -Wuninitialized" } */ +/* { dg-options "-Os -Wmaybe-uninitialized" } */ struct A { struct A *a, *b; }; struct D { struct A e; }; struct E { unsigned char f; struct { struct A e; } g; }; struct F { struct E i[32]; }; extern int fn0 (void); extern int fn1 (struct E *, struct D *); static inline __attribute__ ((always_inline)) int Index: gcc/testsuite/gcc.dg/pr59924.c =================================================================== --- gcc/testsuite/gcc.dg/pr59924.c (revision 268425) +++ gcc/testsuite/gcc.dg/pr59924.c (working copy) @@ -1,13 +1,13 @@ /* PR tree-optimization/59924 */ /* { dg-do compile } */ -/* { dg-options "-O1 -Wall" } */ +/* { dg-options "-O1 -Wmaybe-uninitialized" } */ struct S { struct T *a; double b; struct S *c; }; struct T { struct S *d; }; extern void bar (double); void foo (struct S * x, int y, int z, int w) { int e; struct S *f; Index: gcc/testsuite/gcc.dg/pr69543.c =================================================================== --- gcc/testsuite/gcc.dg/pr69543.c (revision 268425) +++ gcc/testsuite/gcc.dg/pr69543.c (working copy) @@ -1,13 +1,13 @@ /* PR preprocessor/69543 */ /* { dg-do compile } */ -/* { dg-options "-O2 -Wuninitialized" } */ +/* { dg-options "-O2 -Wuninitialized -Wmaybe-uninitialized" } */ # define YY_IGNORE_MAYBE_UNINITIALIZED_BEGIN \ _Pragma ("GCC diagnostic push") \ _Pragma ("GCC diagnostic ignored \"-Wuninitialized\"")\ _Pragma ("GCC diagnostic ignored \"-Wmaybe-uninitialized\"") # define YY_IGNORE_MAYBE_UNINITIALIZED_END \ _Pragma ("GCC diagnostic pop") void test (char yylval) { Index: gcc/testsuite/gcc.dg/uninit-11-O0.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-11-O0.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-11-O0.c (working copy) @@ -1,13 +1,13 @@ /* Positive test for uninitialized variables. */ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized" } */ +/* { dg-options "-Wmaybe-uninitialized -Wuninitialized" } */ int sink; void f1(int parm) /* { dg-bogus "uninitialized" "parameter" } */ { sink = parm; /* { dg-bogus "uninitialized" "parameter" } */ } void f2(void) { Index: gcc/testsuite/gcc.dg/uninit-11.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-11.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-11.c (working copy) @@ -1,13 +1,13 @@ /* Positive test for uninitialized variables. */ /* { dg-do compile } */ -/* { dg-options "-O -Wuninitialized" } */ +/* { dg-options "-O -Wmaybe-uninitialized -Wuninitialized" } */ int sink; void f1(int parm) /* { dg-bogus "uninitialized" "parameter" } */ { sink = parm; /* { dg-bogus "uninitialized" "parameter" } */ } void f2(void) { Index: gcc/testsuite/gcc.dg/uninit-16.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-16.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-16.c (working copy) @@ -1,12 +1,12 @@ /* { dg-do compile } */ -/* { dg-options "-O2 -Wuninitialized" } */ +/* { dg-options "-O2 -Wmaybe-uninitialized" } */ int foo, bar; static void decode_reloc(int reloc, int *is_alt) { if (reloc >= 20) *is_alt = 1; else if (reloc >= 10) *is_alt = 0; Index: gcc/testsuite/gcc.dg/uninit-17.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-17.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-17.c (working copy) @@ -1,12 +1,12 @@ /* { dg-do compile } */ -/* { dg-options "-O -Wuninitialized" } */ +/* { dg-options "-O -Wmaybe-uninitialized" } */ typedef _Complex float C; C foo(int cond) { C f; __imag__ f = 0; if (cond) { __real__ f = 1; return f; Index: gcc/testsuite/gcc.dg/uninit-18.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-18.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-18.c (working copy) @@ -1,12 +1,12 @@ /* { dg-do compile } */ -/* { dg-options "-O -Wuninitialized" } */ +/* { dg-options "-O -Wmaybe-uninitialized" } */ char *foo(int bar, char *baz) { char *tmp; if (bar & 3) tmp = baz; switch (bar) { case 1: Index: gcc/testsuite/gcc.dg/uninit-19.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-19.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-19.c (working copy) @@ -1,12 +1,12 @@ /* { dg-do compile } */ -/* { dg-options "-O -Wuninitialized" } */ +/* { dg-options "-O -Wmaybe-uninitialized" } */ /* { dg-additional-options "-finline-small-functions" { target avr-*-* } } */ int a, l, m; float *b; float c, d, e, g, h; unsigned char i, k; void fn1 (int p1, float *f1, float *f2, float *f3, unsigned char *c1, float *f4, unsigned char *c2, float *p10) { Index: gcc/testsuite/gcc.dg/uninit-6-O0.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-6-O0.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-6-O0.c (working copy) @@ -1,15 +1,15 @@ /* Spurious uninitialized variable warnings. This one inspired by java/class.c:build_utf8_ref. */ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -ftrack-macro-expansion=2" } */ +/* { dg-options "-Wmaybe-uninitialized -ftrack-macro-expansion=2" } */ #include <stddef.h> struct tree { struct tree *car; struct tree *cdr; int type, data; }; Index: gcc/testsuite/gcc.dg/uninit-pr19430-2.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-pr19430-2.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-pr19430-2.c (working copy) @@ -1,12 +1,12 @@ /* { dg-do compile } */ -/* { dg-options "-O -Wuninitialized" } */ +/* { dg-options "-O -Wmaybe-uninitialized" } */ int *p, *q; int foo (int b) { int i, j = 0; int *x; p = &i; q = &j; if (b) Index: gcc/testsuite/gcc.dg/uninit-pr19430-O0.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-pr19430-O0.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-pr19430-O0.c (working copy) @@ -1,12 +1,12 @@ /* { dg-do compile } */ -/* { dg-options "-O0 -Wuninitialized" } */ +/* { dg-options "-O0 -Wuninitialized -Wmaybe-uninitialized" } */ extern int bar (int); extern void baz (int *); int foo (int i) { int j; /* { dg-warning "'j' may be used uninitialized in this function" "uninitialized" { xfail *-*-* } } */ if (bar (i)) { baz (&j); Index: gcc/testsuite/gcc.dg/uninit-pr19430.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-pr19430.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-pr19430.c (working copy) @@ -1,12 +1,12 @@ /* { dg-do compile } */ -/* { dg-options "-O -Wuninitialized" } */ +/* { dg-options "-O -Wmaybe-uninitialized -Wuninitialized" } */ extern int bar (int); extern void baz (int *); int foo (int i) { int j; /* { dg-warning "'j' may be used uninitialized in this function" "uninitialized" { xfail *-*-* } } */ if (bar (i)) { baz (&j); } else { Index: gcc/testsuite/gcc.dg/uninit-pr20644-O0.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-pr20644-O0.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-pr20644-O0.c (working copy) @@ -1,13 +1,13 @@ /* PR 20644 */ /* { dg-do compile } */ -/* { dg-options "-O0 -Wuninitialized" } */ +/* { dg-options "-O0 -Wmaybe-uninitialized" } */ int foo () { int i = 0; int j; if (1 == i) return j; /* { dg-bogus "uninitialized" "uninitialized" { xfail *-*-* } } */ return 0; } Index: gcc/testsuite/gcc.dg/uninit-pred-2_a.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-pred-2_a.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-pred-2_a.c (working copy) @@ -1,12 +1,12 @@ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -O2" } */ +/* { dg-options "-Wmaybe-uninitialized -O2" } */ int g; void bar (void); void blah (int); int foo (int n, int m, int r) { int flag = 0; int v; Index: gcc/testsuite/gcc.dg/uninit-pred-2_b.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-pred-2_b.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-pred-2_b.c (working copy) @@ -1,12 +1,12 @@ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -O2" } */ +/* { dg-options "-Wmaybe-uninitialized -O2" } */ int g; void bar (void); void blah (int); int foo (int n, int m, int r) { int flag = 0; int v; Index: gcc/testsuite/gcc.dg/uninit-pred-2_c.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-pred-2_c.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-pred-2_c.c (working copy) @@ -1,12 +1,12 @@ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -O2 -fno-tree-tail-merge" } */ +/* { dg-options "-Wmaybe-uninitialized -O2 -fno-tree-tail-merge" } */ int g; void bar (void); void blah (int); int foo (int n, int m, int r) { int flag = 0; int v; Index: gcc/testsuite/gcc.dg/uninit-pred-3_a.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-pred-3_a.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-pred-3_a.c (working copy) @@ -1,12 +1,12 @@ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -O2" } */ +/* { dg-options "-Wmaybe-uninitialized -O2" } */ int g; void bar(); void blah(int); int foo (int n, int m, int r) { int flag = 0; int v; Index: gcc/testsuite/gcc.dg/uninit-pred-3_b.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-pred-3_b.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-pred-3_b.c (working copy) @@ -1,12 +1,12 @@ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -O2" } */ +/* { dg-options "-Wmaybe-uninitialized -O2" } */ int g; void bar(); void blah(int); int foo (int n, int m, int r) { int flag = 0; int v; Index: gcc/testsuite/gcc.dg/uninit-pred-3_c.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-pred-3_c.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-pred-3_c.c (working copy) @@ -1,12 +1,12 @@ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -O2" } */ +/* { dg-options "-Wmaybe-uninitialized -O2" } */ int g; void bar(); void blah(int); int foo (int n, int m, int r) { int flag = 0; int v; Index: gcc/testsuite/gcc.dg/uninit-pred-3_d.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-pred-3_d.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-pred-3_d.c (working copy) @@ -1,12 +1,12 @@ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -O2" } */ +/* { dg-options "-Wmaybe-uninitialized -O2" } */ int g; void bar(); void blah(int); int foo (int n, int m, int r) { int flag = 0; int v; Index: gcc/testsuite/gcc.dg/uninit-pred-3_e.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-pred-3_e.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-pred-3_e.c (working copy) @@ -1,12 +1,12 @@ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -O2" } */ +/* { dg-options "-Wmaybe-uninitialized -O2" } */ int g; void bar(); void blah(int); int foo (int n, int m, int r) { int flag = 0; int v; Index: gcc/testsuite/gcc.dg/uninit-pred-4_a.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-pred-4_a.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-pred-4_a.c (working copy) @@ -1,12 +1,12 @@ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -O2" } */ +/* { dg-options "-Wmaybe-uninitialized -O2" } */ int g; void bar(); void blah(int); int foo (int n, int m, int r, int t) { int flag = 0; int v; if (t) Index: gcc/testsuite/gcc.dg/uninit-pred-4_b.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-pred-4_b.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-pred-4_b.c (working copy) @@ -1,12 +1,12 @@ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -O2" } */ +/* { dg-options "-Wmaybe-uninitialized -O2" } */ int g; void bar(); void blah(int); int foo (int n, int m, int r, int t) { int flag = 0; int v; if (t) Index: gcc/testsuite/gcc.dg/uninit-pred-5_a.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-pred-5_a.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-pred-5_a.c (working copy) @@ -1,12 +1,12 @@ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -Wno-attributes -O2" } */ +/* { dg-options "-Wmaybe-uninitialized -Wno-attributes -O2" } */ int g; int bar(); int blah(int); void t(int); static int __attribute__((always_inline)) foo (int n, int* v, int r) { Index: gcc/testsuite/gcc.dg/uninit-pred-5_b.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-pred-5_b.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-pred-5_b.c (working copy) @@ -1,12 +1,12 @@ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -Wno-attributes -O2" } */ +/* { dg-options "-Wmaybe-uninitialized -Wno-attributes -O2" } */ int g; int bar(); int blah(int); void t(int); static int __attribute__((always_inline)) foo (int n, int* v, int r) { Index: gcc/testsuite/gcc.dg/uninit-pred-6_a.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-pred-6_a.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-pred-6_a.c (working copy) @@ -1,13 +1,13 @@ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -O2" } */ +/* { dg-options "-Wmaybe-uninitialized -O2" } */ int g; void bar(); void blah(int); int foo (int n, int l, int m, int r) { int v; if (n && l) Index: gcc/testsuite/gcc.dg/uninit-pred-6_b.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-pred-6_b.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-pred-6_b.c (working copy) @@ -1,13 +1,13 @@ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -O2" } */ +/* { dg-options "-Wmaybe-uninitialized -O2" } */ int g; void bar(); void blah(int); int foo (int n, int l, int m, int r) { int v; if (n) Index: gcc/testsuite/gcc.dg/uninit-pred-6_c.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-pred-6_c.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-pred-6_c.c (working copy) @@ -1,13 +1,13 @@ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -O2" } */ +/* { dg-options "-Wmaybe-uninitialized -O2" } */ int g; void bar(); void blah(int); int foo (int n, int l, int m, int r) { int v; if (n > 10) Index: gcc/testsuite/gcc.dg/uninit-pred-6_d.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-pred-6_d.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-pred-6_d.c (working copy) @@ -1,13 +1,13 @@ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -O2" } */ +/* { dg-options "-Wmaybe-uninitialized -O2" } */ int g; void bar(); void blah(int); int foo (int n, int l, int m, int r) { int v; if (n) Index: gcc/testsuite/gcc.dg/uninit-pred-6_e.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-pred-6_e.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-pred-6_e.c (working copy) @@ -1,13 +1,13 @@ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -O2" } */ +/* { dg-options "-Wmaybe-uninitialized -O2" } */ int g; void bar(); void blah(int); int foo (int n, int l, int m, int r) { int v; if (n > 10) Index: gcc/testsuite/gcc.dg/uninit-pred-7_a.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-pred-7_a.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-pred-7_a.c (working copy) @@ -1,13 +1,13 @@ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -O2" } */ +/* { dg-options "-Wmaybe-uninitialized -O2" } */ int g; void bar(); void blah(int); int foo (int n, int l, int m, int r) { int v; if (n || l) Index: gcc/testsuite/gcc.dg/uninit-pred-7_b.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-pred-7_b.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-pred-7_b.c (working copy) @@ -1,13 +1,13 @@ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -O2" } */ +/* { dg-options "-Wmaybe-uninitialized -O2" } */ int g; void bar(); void blah(int); int foo (int n, int l, int m, int r) { int v; if (n > 10) Index: gcc/testsuite/gcc.dg/uninit-pred-7_c.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-pred-7_c.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-pred-7_c.c (working copy) @@ -1,13 +1,13 @@ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -O2" } */ +/* { dg-options "-Wmaybe-uninitialized -O2" } */ int g; void bar(); void blah(int); int foo (int n, int l, int m, int r) { int v; if (n) Index: gcc/testsuite/gcc.dg/uninit-pred-7_d.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-pred-7_d.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-pred-7_d.c (working copy) @@ -1,13 +1,13 @@ /* { dg-do compile { target i?86-*-* x86_64-*-* } } */ -/* { dg-options "-Wuninitialized -O2 -mbranch-cost=0" } */ +/* { dg-options "-Wmaybe-uninitialized -O2 -mbranch-cost=0" } */ int g; void bar(); void blah(int); int foo (int n, int l, int m, int r) { int v; if (n || l) Index: gcc/testsuite/gcc.dg/uninit-pred-8_a.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-pred-8_a.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-pred-8_a.c (working copy) @@ -1,13 +1,13 @@ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -O2" } */ +/* { dg-options "-Wmaybe-uninitialized -O2" } */ /* Pick a particular tuning to pin down BRANCH_COST. */ /* { dg-additional-options "-mtune=cortex-a15" { target arm*-*-* } } */ int g; void bar(); void blah(int); int foo (int n, int l, int m, int r) { int v; Index: gcc/testsuite/gcc.dg/uninit-pred-8_b.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-pred-8_b.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-pred-8_b.c (working copy) @@ -1,13 +1,13 @@ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -O2" } */ +/* { dg-options "-Wmaybe-uninitialized -O2" } */ int g; void bar(); void blah(int); int foo (int n, int l, int m, int r) { int v; if (n < 10 || m > 100 || r < 20 || l) Index: gcc/testsuite/gcc.dg/uninit-pred-8_c.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-pred-8_c.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-pred-8_c.c (working copy) @@ -1,13 +1,13 @@ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -O2" } */ +/* { dg-options "-Wmaybe-uninitialized -O2" } */ int g; void bar(); void blah(int); int foo (int n, int l, int m, int r) { int v; if (n < 10 && m > 100 && r < 20 ) Index: gcc/testsuite/gcc.dg/uninit-pred-8_d.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-pred-8_d.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-pred-8_d.c (working copy) @@ -1,13 +1,13 @@ /* { dg-do compile { target i?86-*-* x86_64-*-* } } */ -/* { dg-options "-Wuninitialized -O2 -mbranch-cost=0" } */ +/* { dg-options "-Wmaybe-uninitialized -O2 -mbranch-cost=0" } */ int g; void bar(); void blah(int); int foo (int n, int l, int m, int r) { int v; if (n || m || r || l) Index: gcc/testsuite/gcc.dg/uninit-pred-9_a.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-pred-9_a.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-pred-9_a.c (working copy) @@ -1,13 +1,13 @@ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -O2" } */ +/* { dg-options "-Wmaybe-uninitialized -O2" } */ int g; void bar(); void blah(int); int foo (int n, int l, int m, int r) { int v; if ( (n < 10) && (m == l) && (r < 20) ) Index: gcc/testsuite/gcc.dg/uninit-pred-9_b.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-pred-9_b.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-pred-9_b.c (working copy) @@ -1,13 +1,13 @@ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -O2" } */ +/* { dg-options "-Wmaybe-uninitialized -O2" } */ int g; void bar(); void blah(int); int foo (int n, int l, int m, int r) { int v; if ( (n < 10) && (m != 100) && (r < 20) ) Index: gcc/testsuite/gcc.dg/uninit-suppress_2.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-suppress_2.c (revision 268425) +++ gcc/testsuite/gcc.dg/uninit-suppress_2.c (working copy) @@ -1,12 +1,12 @@ /* { dg-do compile } */ -/* { dg-options "-fno-tree-dominator-opts -fno-tree-ccp -fno-tree-vrp -fno-tree-fre -fno-tree-pre -fno-code-hoisting -O2 -Wuninitialized -Werror=uninitialized -Wno-error=maybe-uninitialized" } */ +/* { dg-options "-fno-tree-dominator-opts -fno-tree-ccp -fno-tree-vrp -fno-tree-fre -fno-tree-pre -fno-code-hoisting -O2 -Wuninitialized -Wmaybe-uninitialized -Werror=uninitialized -Wno-error=maybe-uninitialized" } */ void blah(); void bar (int); int gflag; void foo() { int v; if (gflag) v = 10; Index: gcc/testsuite/gfortran.dg/pr25923.f90 =================================================================== --- gcc/testsuite/gfortran.dg/pr25923.f90 (revision 268425) +++ gcc/testsuite/gfortran.dg/pr25923.f90 (working copy) @@ -1,12 +1,12 @@ ! { dg-do compile } -! { dg-options "-O -Wuninitialized" } +! { dg-options "-O -Wmaybe-uninitialized" } module foo implicit none type bar integer :: yr end type contains Index: gcc/testsuite/gfortran.dg/pr39666-2.f90 =================================================================== --- gcc/testsuite/gfortran.dg/pr39666-2.f90 (revision 268425) +++ gcc/testsuite/gfortran.dg/pr39666-2.f90 (working copy) @@ -1,13 +1,13 @@ ! PR middle-end/39666 ! { dg-do compile } -! { dg-options "-O2 -Wuninitialized" } +! { dg-options "-O2 -Wmaybe-uninitialized" } FUNCTION f(n) INTEGER, INTENT(in) :: n REAL :: f SELECT CASE (n) CASE (:-1); f = -1.0 CASE (0); f = 0.0 CASE (2:); f = 1.0 END SELECT