[Bug middle-end/89501] Odd lack of warning about missing initialization
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89501 --- Comment #1 from Andrew Pinski --- I think it comes down to the same issue as PR 18501.
[Bug middle-end/89501] Odd lack of warning about missing initialization
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89501
--- Comment #2 from Linus Torvalds ---
(In reply to Andrew Pinski from comment #1)
> I think it comes down to the same issue as PR 18501.
Very possibly the same issue in just a different guise.
NOTE! I have in the meantime verified that yes, it does seem to be about the
pattern
int x;
if (somecondition) {
x = something();
if (x != XYZ)
return x;
}
return x;
where gcc seems to turn the "if (x != XYZ) return x" to mean that "x" clearly
_has_ to be XYZ elsewhere.
If I change my kernel-based test-case to do
if (ret != 1)
return ret;
instead of the original
if (ret)
return ret;
then gcc will actually generate code that ends with
movl$1, %eax
popq%rbp
popq%r12
ret
ie it will basically consider "ret" to be initialized to that value "1", even
if the basic block that assigned it was never actually executed.
Knowing how SSA works, I'm not entirely surprised, but obviously if you'd like
to see the warning about buggy source code, it's less than optimal.
Anyway, this shouldn't be a high priority, but it does strike me as a
potentially fairly common pattern that people might be missing warnings for.
Linus
[Bug middle-end/89501] Odd lack of warning about missing initialization
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89501 --- Comment #3 from Jeffrey A. Law --- Yup. It's the same as 18501. We meet UNDEFINED and [0,0] resulting in [0,0] and nothing ever causes reevaluation of the PHI. Things are working as "expected". My approach from 2005 would almost certainly address this instance since we'd see the uninitialized use in the early uninit pass, and later see it went away. It'd ultimately generate indicating there was an uninitialized use of "ret" that was optimized away. But I'm not inclined to rip the scabs of those old wounds and reopen that discussion (for the umpteenth time)
[Bug middle-end/89501] Odd lack of warning about missing initialization
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89501 Jeffrey A. Law changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED CC||law at redhat dot com Resolution|--- |DUPLICATE --- Comment #4 from Jeffrey A. Law --- Dup. 18501 is the canonical bug for this issue. *** This bug has been marked as a duplicate of bug 18501 ***
[Bug middle-end/89501] Odd lack of warning about missing initialization
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89501 --- Comment #5 from Richard Biener --- (In reply to Jeffrey A. Law from comment #3) > Yup. It's the same as 18501. We meet UNDEFINED and [0,0] resulting in > [0,0] and nothing ever causes reevaluation of the PHI. Things are working > as "expected". And this meeting helps us avoid bogus warnings for cases where GCC has difficulties proving dead code paths are actually dead ... In fact to preserve uninit warnings we avoid doing the same things for copies, thus when meeting [i, i] with UNDEFINED which in fact causes some false positives to appear... (and with my GCC-is-an-optimizing-compiler-not-a-static-analysis-tool hat on I'd wanted to change that more than once, but testsuite regressions prevented me from doing that) Uninit warnings are hard ;)
[Bug middle-end/89501] Odd lack of warning about missing initialization
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89501 --- Comment #6 from Linus Torvalds --- (In reply to Richard Biener from comment #5) > > And this meeting helps us avoid bogus warnings for cases where GCC has > difficulties proving dead code paths are actually dead ... Ack. I do see the difficulty. We already disable some of the "may-be-uninitialized" warnings in the kernel because they end up being too noisy for some configurations. NP. If you guys figure something out, it will be good. If not, we'll survive.
[Bug middle-end/89501] Odd lack of warning about missing initialization
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89501 --- Comment #7 from Jeffrey A. Law --- It's reliably the case that a false positive uninit warning also represents a failure to optimize something. So we've got significant incentives to deeply analyze and look for fixes. So feel free to pass examples of those along.
[Bug middle-end/89501] Odd lack of warning about missing initialization
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89501 --- Comment #8 from Linus Torvalds --- (In reply to Jeffrey A. Law from comment #7) > It's reliably the case that a false positive uninit warning also represents > a failure to optimize something. So we've got significant incentives to > deeply analyze and look for fixes. So feel free to pass examples of those > along. Well, most of it is due to interactions with *other* issues entirely. For example, when we enable GCOV for coverage checking, we have to disable tree-loop-im, because of excessive stack usage due to gcc bugzilla 69702. And disabling that optimization then leads to bogus "might be used uninitialized" warnings. We have a few other cases like that. Eg we can't use -Os without also disabling -Wmaybe-uninitialized etc. Some of the cases may be historical (ie maybe you've fixed the issues that cause us to do that in newer versions), but for various distro reasons we end up supporting old compilers for a _looong_ time. We not that long ago ended up increasing the minimum required gcc version for the kernel to gcc-4.6. So we have this odd "we love using new features" fight with "oops, but we end up having people using relatively ancient compiler versions".
[Bug middle-end/89501] Odd lack of warning about missing initialization
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89501 ncm at cantrip dot org changed: What|Removed |Added CC||ncm at cantrip dot org --- Comment #9 from ncm at cantrip dot org --- What I don't understand is why it doesn't optimize away the check on (somecondition), since it is assuming the code in the dependent block always runs.
[Bug middle-end/89501] Odd lack of warning about missing initialization
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89501 --- Comment #10 from Linus Torvalds --- (In reply to ncm from comment #9) > What I don't understand is why it doesn't optimize away the check on > (somecondition), since it is assuming the code in the dependent block always > runs. No, it very much doesn't assume that. The 'somecondition' really is dynamic. What happens is simply that because gcc sees only a single assignment to the variable, and that assignment is then limited by the subsequent value test to a single value, gcc will just say "ok, any other place where that variable is used, just use the known single value". And it does that whether the 'if (somecondition)' path was taken or not. It's a perfectly valid optimization. In fact, it's a lovely optimization, I'm not at all complaining about the code generation. It's just that as part of that (quite reasonable) optimization it also optimized away the whole "oops, there wasn't really a valid initialization in case the if-statement wasn't done". Obviously that's undefined behavior, and the optimization is valid regardless, but the lack of warning means that we didn't see that we had technically undefined behavior that the compiler has silently just fixed up for us. I think the cause of this all is quite reasonable and understandable, and I also see why gcc really does want to throw away the undefined case entirely (because otherwise you can get into the reverse situation where you warn unnecessarily, because gcc isn't smart enough to see that some undefined case will never ever actually happen). Plus I assume it simplifies things a lot to just not even have to track the undefined case at all. You can just track "ok, down this path we have a known value for this SSA, and we don't need to keep track of any inconvenient phi nodes etc".
[Bug middle-end/89501] Odd lack of warning about missing initialization
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89501
--- Comment #11 from Jeffrey A. Law ---
WRT c#9. Linus is right. THe condition is dynamic and we don't want to remove
it in this circumstance.
More generally we have considered whether or not we could eliminate the
control dependent path which leads to undefined behavior. But you have to be
careful because statements on the path prior to the actual undefined behavior
may have observable side effects.
So what we do in those cases is isolate the path via block copying and CFG
manipulations (usually it's just a subset of paths to a particular statement
which result in undefined behavior). Once the path is isolated we can replace
the statement which triggers the undefined behavior with a trap. That in turn
allows some CFG simplifications, const/copy propagation and usually further
dead code elimination.
Right now we do this just for NULLs that are dereferenced and division by zero.
But I can see doing it for out of bounds array indexing, bogus mem* calls and
perhaps other cases of undefined behavior. ie, if you have something like
char foo[10];
if (shouldn't happen)
indx = -1;
else
indx = whatever ();
something = foo[indx];
... more work ...
This kind of idiom occurs fairly often when -1 is used to represent a can't
happen case and you've got abstraction layers that get eliminated via inlining.
Path isolation would turn that into something like this:
if (shouldn't happen) {
indx = -1
trap;
} else {
indx = whatever ();
something = foo[indx];
}
... more work ...
Which will simplify in various useful ways.
--
If we were to apply those concepts to this example we'd conceptually turn the
code into something like this:
if (somecondition) {
ret = functioncall(x);
if (ret)
return ret;
}
... some more work ...
trap
[Bug middle-end/89501] Odd lack of warning about missing initialization
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89501 --- Comment #12 from Linus Torvalds --- (In reply to Jeffrey A. Law from comment #11) > > More generally we have considered whether or not we could eliminate the > control dependent path which leads to undefined behavior. But you have to > be careful because statements on the path prior to the actual undefined > behavior may have observable side effects. Note that for the kernel, we consider those kinds of "optimizations" completely and utterly wrong-headed, and will absolutely refuse to use a compiler that does things like that. It's basically the compiler saying "I don't care what you meant, I can do anything I want, and that means I will screw the code up on purpose". I will personally switch the kernel immediately to clang the moment we cannot turn off idiotic broken behavior like that. We currently already disable 'strict-aliasing', 'strict-overflow' and 'delete-null-pointer-checks'. Because compilers that intentionally create garbage code are evil and wrong. Compiler standards bodies that add even more of them (the C aliasing rules being the prime example) should be ashamed of themselves. And compiler people that think it's ok to say "it's undefined, so we do anything we damn well like" shouldn't be working with compilers IMNSHO. If you know something is undefined, you warn about it. You don't silently generate random code that doesn't match the source code just because some paper standard says you "can".
[Bug middle-end/89501] Odd lack of warning about missing initialization
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89501 --- Comment #13 from ncm at cantrip dot org --- What I am getting is that the compiler is leaving that permitted optimization -- eliminating the inode check -- on the table. It is doing that not so much because it would make Linus angry, but as an artifact of the particular optimization processes used in Gcc at the moment. Clang, or some later release of Gcc or Clang, or even this Gcc under different circumstances, might choose differently. But maybe there are some flavors of UB, among which returning uninitialized variables might be the poster child, that you don't ever want to use to drive some kinds of optimizations. Maybe Gcc's process has that baked in.
