| Summary: | [11/12/13/14 Regression] Missing 'used uninitialized' warning (CCP) | ||
|---|---|---|---|
| Product: | gcc | Reporter: | Nathan Sidwell <nathan> |
| Component: | tree-optimization | Assignee: | Not yet assigned to anyone <unassigned> |
| Status: | REOPENED --- | ||
| Severity: | minor | CC: | cernekee, christoph.mallon, david.cuthbert, dberlin, dhill, dimhen, dodji, egallager, eggert, felix, fredrik, fw, gcc-bugs, gcc-bugs, gccbugs, hp, jan.smets, jasonwucj, jellegeerts, manu, mark, msebor, mueller, nszabolcs, P.Schaffnit, pinskia, ppluzhnikov, roche+gccbugs, satyam, tadhunt, thutt, torvalds, tstdenis, vincent-gcc, william, zack+srcbugz |
| Priority: | P5 | Keywords: | diagnostic |
| Version: | 4.0.0 | ||
| Target Milestone: | 11.5 | ||
| See Also: |
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=19430 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=179 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97353 |
||
| Host: | Target: | ||
| Build: | Known to work: | ||
| Known to fail: | 10.2.0, 11.0, 4.7.0, 4.8.4, 4.9.4, 5.5.0, 6.4.0, 7.2.0, 8.3.0, 9.1.0 | Last reconfirmed: | 2021-04-07 00:00:00 |
| Bug Depends on: | |||
| Bug Blocks: | 24639, 90844 | ||
| Attachments: | test case, from tree-ssa-pre.c | ||
|
Description
Nathan Sidwell
2004-11-15 13:35:42 UTC
Created attachment 7548 [details]
test case, from tree-ssa-pre.c
Confirmed, CCP is removing the use of the uninitialized variable. It has never worked on the tree-ssa branch. This is not really fixable. Analysis: http://gcc.gnu.org/ml/gcc/2004-12/msg00681.html Re-opening. A re-organization of the tree optimizers is causing us to misdiagnose this test case once again. I am going to work on a better warning implementation that doesn't get confused so easily. See http://gcc.gnu.org/ml/gcc-patches/2005-05/msg00955.html Personally, I prefer misdiagnosing uninit-5.c as a possible uninitialized use, rather than missing this pretty nasty case. Downgrading to P5; this would never be release-critical. Missing this warning does seem a shame though. And, I think that people who want this warning would rather have false positives (which can be avoid with perhaps-unncessary optimizations) than false negatives, which can result in bugs of exactly the kind they're trying to catch. I'd suggest that we consider moving this check very early in the optimization chain so that we get more consistent results across platforms, more consistent results across optimization levels, and more predictability from release to release. Jeff has a much better approach to solving this. http://gcc.gnu.org/ml/gcc/2005-11/msg00032.html Subject: Re: [4.1 Regression] Missing 'used
unintialized' warning
On Tue, 2005-11-01 at 18:29 +0000, dnovillo at gcc dot gnu dot org
wrote:
>
> ------- Comment #9 from dnovillo at gcc dot gnu dot org 2005-11-01 18:29 -------
>
> Jeff has a much better approach to solving this.
> http://gcc.gnu.org/ml/gcc/2005-11/msg00032.html
I'd rather you not assign it to me just yet -- while I think my approach
is better, I don't think we have a consensus that it's what we're going
to do yet :-)
jeff
Subject: Re: [4.1 Regression] Missing 'used unintialized' warning
On Tuesday 01 November 2005 13:50, law at redhat dot com wrote:
> I'd rather you not assign it to me just yet -- while I think my approach
> is better, I don't think we have a consensus that it's what we're going
> to do yet :-)
>
Don't worry, I haven't assigned it :) I just added a pointer to the thread
and added you to the CC list.
Will not be fixed in 4.1.1; adjust target milestone to 4.1.2. (In reply to comment #4) > This is not really fixable. > > Analysis: http://gcc.gnu.org/ml/gcc/2004-12/msg00681.html I am really ignorant on this CCP optimisaton but I am willing to learn. I guess that we merge UNDEFINED and 0 because we can assume that undefined is anything. I am not so sure that is OK in this particular case. However, even in that case, we should warn just at that moment, since we are already using an UNDEFINED value, aren't we? My point is that this testcase is fundamentally different from: int x; if (f()) x = 3; return x; where f() is optimised away because it is always true. Some users may not want to get warnings for the latter case, but any reasonable user wants to get a warning for this PR. Please, correct me when I am wrong. Thanks. *** Bug 33327 has been marked as a duplicate of this bug. *** This is CCP related. (See "Problem 2: CCP assumes a value for uninitialized variables" in http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings) Closing 4.1 branch. *** Bug 37148 has been marked as a duplicate of this bug. *** This defect has been open nearly 4 years. Any hope of actually getting a fix commited? (In reply to comment #18) > This defect has been open nearly 4 years. Any hope of > actually getting a fix commited? To be bluntly honest. No, do not expect a fix in the near/medium future. Not for GCC 4.4 for sure. CCP relies on this behaviour to optimize code, so we cannot disable it. Moving passes around (warning before CCP, warning before DCE) would just generate different false positives/negatives. This is a complex issue: http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings#problem_ccp Plus, no one is working or planning to work on this at the moment AFAIK. *** Bug 36814 has been marked as a duplicate of this bug. *** *** Bug 38945 has been marked as a duplicate of this bug. *** *** Bug 39113 has been marked as a duplicate of this bug. *** note that gcc version 3.2.3 correctly reports this warning *** Bug 30575 has been marked as a duplicate of this bug. *** *** Bug 30856 has been marked as a duplicate of this bug. *** Closing 4.2 branch. *** Bug 40469 has been marked as a duplicate of this bug. *** We are not going to fix this. *** Bug 30542 has been marked as a duplicate of this bug. *** (In reply to comment #28) > We are not going to fix this. > Why? There are many ways to alleviate this. Doing some warnings in the front-ends, such LLVM does is one. Or propagate some "uninitialized" bit, that can checked later. Or something that hasn't invented yet. It is clear that other compilers can get this right, so GCC could, if someone had the time and interest. (In reply to comment #30) > (In reply to comment #28) > > We are not going to fix this. > > > > Why? There are many ways to alleviate this. Doing some warnings in the > front-ends, such LLVM does is one. Or propagate some "uninitialized" bit, that > can checked later. Or something that hasn't invented yet. It is clear that > other compilers can get this right, so GCC could, if someone had the time and > interest. BTW, in my review of Wuninitialized problems, this is problem number 1 of missing warnings, as evidenced by the number of duplicates. So even alleviating this in simple cases would be a major improvement. We can only fix it with the chance of raising more spurious warnings. One reason why we run the "may be used uninitialized" pass very late. (In reply to comment #32) > We can only fix it with the chance of raising more spurious warnings. One > reason why we run the "may be used uninitialized" pass very late. The solution of moving the passes around has been discarded long ago. There are other possible solutions. Nonetheless, every change that warns were we previously didn't may rise more spurious warnings, per the Halting problem. In fact, your patch to enable Wuninitialized at -O0 did certainly introduce many spurious warnings. Are you going to revert it? Making use of alias info to detect unitialized uses will introduce spurious warnings, are you going to close PR 19430 and similar? If so, feel free to go through http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings#current and close all items as WONTFIX. You may all well directly close all bugs listed in PR 24639 that report a missing warning, since implementing the warning will almost certainly lead to spurious warning. The goal is to find a balance. Per Chris Lattner account, LLVM is able to detect simple cases of this PR without generating spurious warnings. So it can definitely be done. I think this PR should be kept open in case someone decides to improve the situation a bit. Please reopen. Huh. Well then. > The solution of moving the passes around has been discarded long ago. There are
> other possible solutions.
so, what are these? Once we removed the uninitialized use (CCP) we cannot
recover the information.
(In reply to comment #35) > > so, what are these? Once we removed the uninitialized use (CCP) we cannot > recover the information. We usually do not remove the uninitialized use but assume a value for the variable and propagate it. The standard testcase int x; if (f()) x = 3; return x; The code is simplified to return (3), but perhaps we could mark this constant as being possibly uninitialized. A later pass can report it if it never gets removed. Or perhaps we could do some basic (and fast) CCP in the front-end to detect may-be-uninitialized uses. In http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings#current I give more ideas. That no one has found a fix does not mean that a fix is not possible. Users are going to keep reporting this, and we have many much older bugs still open. It took 8 years to fix PR179. GCC 4.3.4 is being released, adjusting target milestone. *** Bug 42724 has been marked as a duplicate of this bug. *** GCC 4.3.5 is being released, adjusting target milestone. *** Bug 45493 has been marked as a duplicate of this bug. *** *** Bug 42884 has been marked as a duplicate of this bug. *** *** Bug 46853 has been marked as a duplicate of this bug. *** Maybe it's high time someone address this shortcoming as opposed to adding additional language front ends. If you can't even get the core ones right... Just saying... (In reply to comment #43) > Maybe it's high time someone address this shortcoming as opposed to adding > additional language front ends. If you can't even get the core ones right... Generally the people who add front-ends aren't the people working on the middle-end and optimisers, so work on one doesn't take resources away from the other. Feel free to pay someone to fix it if this is important to you. Just sayin' (In reply to comment #44) > (In reply to comment #43) > > Maybe it's high time someone address this shortcoming as opposed to adding > > additional language front ends. If you can't even get the core ones right... > > Generally the people who add front-ends aren't the people working on the > middle-end and optimisers, so work on one doesn't take resources away from the > other. > Moreover, the people that add new FEs are not the same people working on the C/C++ FEs. Anyway, in the comments above you can find the reasons why this is difficult, how Clang has solved it (or so they say, I haven't seen a comparison over many different testcases), and why you should not expect a fix in the medium term. (In reply to comment #44) > (In reply to comment #43) > > Maybe it's high time someone address this shortcoming as opposed to adding > > additional language front ends. If you can't even get the core ones right... > > Generally the people who add front-ends aren't the people working on the > middle-end and optimisers, so work on one doesn't take resources away from the > other. It still adds work to the project as a whole and serves as a distraction for new people who have the time to contribute. > Feel free to pay someone to fix it if this is important to you. Just sayin' If fixing known bugs is not a priority then of what value is this project other than being free? I thought the whole point was to also be correct. Granted this isn't a show-stopper as far as bugs go, but the laissez-faire "if you hate it fix it yourself" trend in OSS is really annoying. When I ran my own OSS projects I never told the users "nyah nyah fix it yourself!" When I ran out of time to work on the OSS projects I gave them up, but so long as I called myself a maintainer I addressed issues as best as I could. I'd almost rather they leave it as WONTFIX then just leaving it open. I'd also be happier if they documented this class of SSA fail in the man page so people don't walk into it [as I and others have]. (In reply to comment #46) > It still adds work to the project as a whole and serves as a distraction for > new people who have the time to contribute. > The same could be said of any feature or bugfix. It depends on your priorities and viewpoint. In fact, the way GCC development works, making a new feature work (like a new FE) may provide the motivation for someone to fix an old bug. > I'd almost rather they leave it as WONTFIX then just leaving it open. How is that a solution? People will keep reporting it. As said above, it took more than 8 years to solve some bugs, but someone came around and fixed them. There are much older bugs than this one that may get fixed in the next couple of releases. WONTFIX means GCC devs don't want a fix. This is not the case here. There are even ideas above about how to fix this. It is just that there is no enough motivation for anyone to deal with the probably huge amount of work it requires. (In reply to comment #47) > WONTFIX means GCC devs don't want a fix. Not quite, and saying it that way, sends the wrong message. It's more like "are not going to put resources (of their own) into fixing it". (In reply to comment #46) > > If fixing known bugs is not a priority then of what value is this project other > than being free? I thought the whole point was to also be correct. Granted > this isn't a show-stopper as far as bugs go, but the laissez-faire "if you hate > it fix it yourself" trend in OSS is really annoying. So is the trend of users who leave snarky "just saying" comments claiming things "can't be done right." GCC is not a static analysis tool, it will never be as good at some things as some other tools - that doesn't make it useless or incorrect. Imperfect, yes, and noone disputes that. Fixing bugs is a priority, just look on the mailing lists to see how many are fixed. Search for Manu's comments to see how hard he works (in his own time) trying to fix longstanding bugs such as this, or at least try to improve the situation slightly, or just identify duplicate reports. But not all bugs are equal. To generalise from the fact that one particularly difficult bug hasn't been fixed to claim that fixing known bugs is not a priority, and to question the value of the entire project, just makes you sound ridiculous. *** Bug 47623 has been marked as a duplicate of this bug. *** BTW, anyone interested in fixing this may want to take a look at the newest proposal for improving Wuninitialized in Clang: http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-February/013170.html They specifically mention the issues of GCC's implementation and how they plan to address them. Nevertheless, there are several problems for implementing Clang's approach in GCC. First, they prefer to have false positives (a false warning) than false negatives (a missed correct warning), whereas GCC tries as hard as possible to not warn when it shouldn't. Second, their proposal makes use of the static analysis checker build into clang, which GCC does not have (and probably never will) in its front-ends. 4.3 branch is being closed, moving to 4.4.7 target. (In reply to comment #52) > 4.3 branch is being closed, moving to 4.4.7 target. Richard, I would suggest to remove the regression markers. This is a regression since 4.0 that it is unlikely to be fixed in the near future. Hence, not really worth to track as a regression. One less bug to handle by the release managers. *** Bug 49971 has been marked as a duplicate of this bug. *** 4.4 branch is being closed, moving to 4.5.4 target. *** Bug 48414 has been marked as a duplicate of this bug. *** BTW, Clang has decided to implement this in the FE for simple cases. Quoting: http://clang.llvm.org/docs/ReleaseNotes.html -Wuninitialized has been taught to recognise uninitialized uses which always occur when an explicitly-written non-constant condition is either true or false. For example: int f(bool b) { int n; if (b) n = 1; return n; } sometimes-uninit.cpp:3:7: warning: variable 'n' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] if (b) ^ sometimes-uninit.cpp:5:10: note: uninitialized use occurs here return n; ^ sometimes-uninit.cpp:3:3: note: remove the 'if' if its condition is always true if (b) ^~~~~~ sometimes-uninit.cpp:2:8: note: initialize the variable 'n' to silence this warning int n; ^ = 0 *** Bug 46684 has been marked as a duplicate of this bug. *** *** Bug 48643 has been marked as a duplicate of this bug. *** GCC 4.6.4 has been released and the branch has been closed. *** Bug 56972 has been marked as a duplicate of this bug. *** *** Bug 57629 has been marked as a duplicate of this bug. *** *** Bug 57856 has been marked as a duplicate of this bug. *** (In reply to Manuel López-Ibáñez)
>
Firstly, thank you very much for keeping tracing this bug almost 10 years, and provided your suggestions as much as possible.
What you have done is valuable to developers (at least to me); at least the developers can easily and quickly know about this gcc common issue in time, and they will notice about it and think another ways to process this issue in their own projects (e.g. firstly compile with -O0 to see warnings).
And sorry for I am not quite familiar with gcc compilier implementation, so at least now, I can not fix this issue, although of cause this issue really can be fixed (just as what you said).
Thanks again.
*** Bug 58323 has been marked as a duplicate of this bug. *** *** Bug 58890 has been marked as a duplicate of this bug. *** *** Bug 59225 has been marked as a duplicate of this bug. *** (In reply to Manuel López-Ibáñez from comment #67) > *** Bug 59225 has been marked as a duplicate of this bug. *** PR 59225 gave me an idea. Wouldn't it be possible to keep a PHI node with just two operands, the constant and the undefined value? If further optimizations remove the undefined block as unexecutable, then the undefined operand will go away. Perhaps the problem is that other optimizations do not treat PHI<1,(D)> as simply 1 and this will result in worse code, although VRP seems to work just fine with PHI<1,2,(D)>. Richard, do you see it feasible? *** Bug 260998 has been marked as a duplicate of this bug. *** Seen from the domain http://volichat.com Page where seen: http://volichat.com/adult-chat-rooms Marked for reference. Resolved as fixed @bugzilla. *** Bug 60444 has been marked as a duplicate of this bug. *** Hey guys, this year will be the 10 year anniversary of this bug. We should order cake! The 4.7 branch is being closed, moving target milestone to 4.8.4. GCC 4.8.4 has been released. The gcc-4_8-branch is being closed, re-targeting regressions to 4.9.3. GCC 4.9.3 has been released. GCC 4.9 branch is being closed *** Bug 78203 has been marked as a duplicate of this bug. *** *** Bug 80152 has been marked as a duplicate of this bug. *** *** Bug 81049 has been marked as a duplicate of this bug. *** *** Bug 82203 has been marked as a duplicate of this bug. *** *** Bug 82810 has been marked as a duplicate of this bug. *** *** Bug 81329 has been marked as a duplicate of this bug. *** *** Bug 80787 has been marked as a duplicate of this bug. *** Ping.
At least -Wmaybe-uninitialized should emit warnings.
This still happens on the trunk (gcc version 9.0.0 20180723 (experimental) (GCC-Explorer-Build)) :
int f(int a)
{
int ret;
if (a) {
ret = 1;
}
return ret;
}
No warning, including with -Wmaybe-uninitialized. All other compilers warn about this (at least clang and Visual C++).
*** Bug 69578 has been marked as a duplicate of this bug. *** *** Bug 87365 has been marked as a duplicate of this bug. *** GCC 6 branch is being closed This came up again on the gcc-help mailing list here: https://gcc.gnu.org/ml/gcc-help/2018-11/msg00007.html *** Bug 89501 has been marked as a duplicate of this bug. *** At Cauldron, Vladislav Ivanishin said in his lightning talk that ISP RAS has a patch to fix this. > --- Comment #90 from Eric Gallager <egallager at gcc dot gnu.org> --- > At Cauldron, Vladislav Ivanishin said in his lightning talk that ISP > RAS has a patch to fix this. Well, I also said that this patch is not meant to go into the mainline compiler, because it trades off compiler speed for fixing this (actually, for suppressing tons of false positives that inevitably arise from doing that; see comment 32 by Richi). Adding / moving around transformation passes also affects the resulting code, and I have no data to see what the actual impact is. We did this to have as few false negatives as possible (and a sane number of false positives) - and GCC is prioritizing not having false positives a.k.a. spurious warnings. One possible solution would be to teach the CCP pass to lay off of uninitialized values and not merge them into constants (I don't know whether that's desirable and also I don't have a patch for that, though it would be interesting to try). Anyway, you are right in that this kind of effort should be documented in this bug. I've already sent the slides to Simon. They should appear shortly on the cauldron's wiki page. Thanks The GCC 7 branch is being closed, re-targeting to GCC 8.4. GCC 8.4.0 has been released, adjusting target milestone. *** Bug 96368 has been marked as a duplicate of this bug. *** Reconfirmed with GCC 11. I wonder if running CCP first, just before the early uninit pass, but only to propagate constants and without modifying the CFG, and then the "late" uninitialized pass to look for uninitialized operands in the PHIs while evaluating the predicates using the CCP lattice values, would be a way to get back the warnings without introducing false positives. *** Bug 89202 has been marked as a duplicate of this bug. *** GCC 8 branch is being closed. GCC 9.4 is being released, retargeting bugs to GCC 9.5. GCC 9 branch is being closed GCC 10.4 is being released, retargeting bugs to GCC 10.5. GCC 10 branch is being closed. |