[Bug c++/55288] New: Improve handling/suppression of maybe-uninitialized warnings

[Bug c++/55288] New: Improve handling/suppression of maybe-uninitialized warnings

[scovich at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55288

             Bug #: 55288
           Summary: Improve handling/suppression of maybe-uninitialized
                    warnings
    Classification: Unclassified
           Product: gcc
           Version: 4.7.1
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: P3
         Component: c++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: scovich@gmail.com


Created attachment 28669
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=28669
maybe-uninitialized false positive

Enabling -Wmaybe-unused (part of -Wall) can result in false positives, which is
fine (the warning is still quite useful). However, there is currently no way to
disable such warnings on a per-variable basis. 

It is possible, but ineffective, to push a diagnostic pragma to ignore such
warnings: Warnings are generated where the uninitialized value (!= variable) is
eventually consumed, and that can easily happen outside the range covered by
the pragma. Inlining makes the problem much worse [1]. 

The attached test case (reduced from actual code) illustrates the problem
clearly, failing to compile with `-O2 -Wall -Werror' even though (a) the value
*is* always written before being read and (b) even though the containing
function has maybe-uninitialized warnings disabled. Adding -DWORKS allows it to
compile by disabling the warning for the call site, even though the offending
variable is not in scope at any part of the source code where the pragma is in
effect.

Since the compiler can clearly track which variable was the problem, I would
instead propose a new variable attribute, ((maybe_uninitialized)), to suppress
all maybe-uninitialized warnings the marked variable might trigger for its
consumers. That way, known false positives can be whitelisted without disabling
a useful warning for large swaths of unrelated code [2].

[1] First, it can vastly expand the number of problematice end points that lie
outside the pragma (they may even reside in different files). Second, the
resulting error message is extremely unhelpful, because it names the variable
that was originally uninitialized, rather than the variable that ended up
holding the "poisoned" value at the point of use (the former might not even be
in the same file, let alone be in scope, and there's no easy way to figure out
which of its uses causes the problem). It would be much better in this case if
the diagnostic listed the call site(s) and/or assignments that led to the
identified line of code depending on the potentially-uninitialized value,
similar to how template substitution failures or errors in included headers are
handled today.

[2] Another potential solution would be to propagate the pragma to inlined call
sites, but that seems like a horrifically hacky and error prone solution.

[Bug c++/55288] Improve handling/suppression of maybe-uninitialized warnings

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55288

Manuel López-Ibáñez <manu at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-11-12
                 CC|                            |manu at gcc dot gnu.org
     Ever Confirmed|0                           |1

--- Comment #1 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-11-12 20:07:31 UTC ---
Why don't just initialize the variable? It seems simpler than implementing yet
another special attribute in GCC.

That said, I find strange that the warning points to somewhere within a
function without telling the user where from that function was called. For more
complex testcases, this could turn out to be very confusing. Also, the location
is not actually pointing to the variable but to ';'.

Clang says:

pr55288.cc:40:9: warning: variable 'q' may be uninitialized when used here
[-Wconditional-uninitialized]
    foo(q);
        ^
pr55288.cc:22:8: note: initialize the variable 'q' to silence this warning
  int q;
       ^
        = 0

[Bug c++/55288] Improve handling/suppression of maybe-uninitialized warnings

[scovich at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55288

--- Comment #2 from Ryan Johnson <scovich at gmail dot com> 2012-11-12 21:11:43 UTC ---
(In reply to comment #1)
> Why don't just initialize the variable? It seems simpler than implementing yet
> another special attribute in GCC.

In the original program, the "variable" is a largish struct, the function is
hot, and the 'valid' execution path is not the most common one. Avoiding
unnecessary initialization there has a measurable impact on performance. 

Note that, in other parts of the code that gcc understands better, the
initialization is unnecessary (no warning) and gets optimized away even if I do
have it in place... much to my chagrin once, after I did a lot of work to
refactor a complex function, only to realize that gcc emitted *exactly* the
same machine code afterward, because it had already noticed and eliminated the
dead stores. 

There's also a philosophical argument to be made... if we agree that all
warnings subject to false positives should be supressible, the current
mechanism for maybe-uninitialized is inadequate, and a variable attribute would
resolve the issue very nicely. There's precedent for this: you *could* use
#ifndef NDEBUG (or even pragma diagnostic) to avoid unused-variable warnings
for helper variables used by multiple assertions scattered over a region of
code, but setting ((unused)) on the offending variable is much easier to read
and maintain, while still allowing other unused variables to be flagged
properly.

[Bug tree-optimization/55288] Improve handling/suppression of maybe-uninitialized warnings

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55288

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.9.0
                 CC|                            |msebor at gcc dot gnu.org

--- Comment #3 from Martin Sebor <msebor at gcc dot gnu.org> ---
The false positive was fixed in r202496.  Let me add the test.  

Clang implements attribute uninitialized (with slightly different meaning than
what's being requested here) so adding support for it to GCC to help control
warnings might make sense:
https://clang.llvm.org/docs/AttributeReference.html#uninitialized

[Bug tree-optimization/55288] Improve handling/suppression of maybe-uninitialized warnings

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55288

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Martin Sebor <msebor@gcc.gnu.org>:

https://gcc.gnu.org/g:b04093adb28bd6ee8b0390e840219fd2bba134db

commit r11-8099-gb04093adb28bd6ee8b0390e840219fd2bba134db
Author: Martin Sebor <msebor@redhat.com>
Date:   Fri Apr 9 11:40:48 2021 -0600

    PR middle-end/55288 - Improve handling/suppression of maybe-uninitialized
warnings

    gcc/testsuite/ChangeLog:
            PR middle-end/55288
            * g++.dg/warn/uninit-pr55288.C: New test.

[Bug tree-optimization/55288] Improve handling/suppression of maybe-uninitialized warnings

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55288

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.9.0                       |---

[Bug tree-optimization/55288] Improve handling/suppression of maybe-uninitialized warnings

[egallager at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55288

--- Comment #5 from Eric Gallager <egallager at gcc dot gnu.org> ---
(In reply to CVS Commits from comment #4)
> The master branch has been updated by Martin Sebor <msebor@gcc.gnu.org>:
> 
> https://gcc.gnu.org/g:b04093adb28bd6ee8b0390e840219fd2bba134db
> 
> commit r11-8099-gb04093adb28bd6ee8b0390e840219fd2bba134db
> Author: Martin Sebor <msebor@redhat.com>
> Date:   Fri Apr 9 11:40:48 2021 -0600
> 
>     PR middle-end/55288 - Improve handling/suppression of
> maybe-uninitialized warnings
>     
>     gcc/testsuite/ChangeLog:
>             PR middle-end/55288
>             * g++.dg/warn/uninit-pr55288.C: New test.

so... is the intent to backport this?

[Bug tree-optimization/55288] Improve handling/suppression of maybe-uninitialized warnings

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55288

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED
      Known to work|                            |4.8.2

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.