Bug 78394 (Og_x_Wmaybe-uninitialized) - False positives of maybe-uninitialized with -Og
Summary: False positives of maybe-uninitialized with -Og
Status: NEW
Alias: Og_x_Wmaybe-uninitialized
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 6.1.1
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
Depends on: 89723 90710 100047 108379 65244 100845
Blocks: Wuninitialized ohgee
  Show dependency treegraph
 
Reported: 2016-11-17 13:27 UTC by Allan Jensen
Modified: 2023-10-23 22:49 UTC (History)
6 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail: 10.2.0, 11.0, 6.3.0, 7.0.1, 8.3.0, 9.3.0
Last reconfirmed: 2021-04-01 00:00:00


Attachments
maybe_uninitialized.cpp (157 bytes, text/plain)
2016-11-17 13:27 UTC, Allan Jensen
Details
maybe_uninitialized.cpp (252 bytes, text/plain)
2016-11-17 13:56 UTC, Allan Jensen
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Allan Jensen 2016-11-17 13:27:28 UTC
Created attachment 40064 [details]
maybe_uninitialized.cpp

Compiling with -Og produces a number of unique false positives for the maybe-unintialized warnings. The warnings are only emited for -Og and not for -O0, -O1, -O2 or -O3.
Comment 1 Allan Jensen 2016-11-17 13:56:12 UTC
Created attachment 40065 [details]
maybe_uninitialized.cpp

Added another example
Comment 2 Eric Gallager 2016-11-17 16:31:00 UTC
I can confirm it only happens with the -Og optimization level; I also tested -Ofast and -Os, and neither of those triggered it either.

gcc_bugs root$ /usr/local/bin/g++ -Wall -Wextra -pedantic -Wmaybe-uninitialized -Weffc++ -Winline -Wfloat-conversion -c -O0 maybe_uninit_00.cpp
gcc_bugs root$ /usr/local/bin/g++ -Wall -Wextra -pedantic -Wmaybe-uninitialized -Weffc++ -Winline -Wfloat-conversion -c -Og maybe_uninit_00.cpp
maybe_uninit_00.cpp: In function ‘float foo()’:
maybe_uninit_00.cpp:19:17: warning: ‘vy’ may be used uninitialized in this function [-Wmaybe-uninitialized]
     return vx + vy;
                 ^~
maybe_uninit_00.cpp:19:17: warning: ‘vx’ may be used uninitialized in this function [-Wmaybe-uninitialized]
gcc_bugs root$ /usr/local/bin/g++ -Wall -Wextra -pedantic -Wmaybe-uninitialized -Weffc++ -Winline -Wfloat-conversion -c -O1 maybe_uninit_00.cpp
gcc_bugs root$ /usr/local/bin/g++ -Wall -Wextra -pedantic -Wmaybe-uninitialized -Weffc++ -Winline -Wfloat-conversion -c -O2 maybe_uninit_00.cpp
gcc_bugs root$ /usr/local/bin/g++ -Wall -Wextra -pedantic -Wmaybe-uninitialized -Weffc++ -Winline -Wfloat-conversion -c -O3 maybe_uninit_00.cpp
gcc_bugs root$ /usr/local/bin/g++ -Wall -Wextra -pedantic -Wmaybe-uninitialized -Weffc++ -Winline -Wfloat-conversion -c -Ofast maybe_uninit_00.cpp
gcc_bugs root$ /usr/local/bin/g++ -Wall -Wextra -pedantic -Wmaybe-uninitialized -Weffc++ -Winline -Wfloat-conversion -c -Os maybe_uninit_00.cpp
gcc_bugs root$ /usr/local/bin/g++ -Wall -Wextra -pedantic -Wmaybe-uninitialized -Weffc++ -Winline -Wfloat-conversion -c -O0 maybe_uninit_01.cpp
gcc_bugs root$ /usr/local/bin/g++ -Wall -Wextra -pedantic -Wmaybe-uninitialized -Weffc++ -Winline -Wfloat-conversion -c -O0 -g maybe_uninit_01.cpp
gcc_bugs root$ /usr/local/bin/g++ -Wall -Wextra -pedantic -Wmaybe-uninitialized -Weffc++ -Winline -Wfloat-conversion -c -Og maybe_uninit_01.cpp
maybe_uninit_01.cpp: In function ‘float foo()’:
maybe_uninit_01.cpp:19:17: warning: ‘vy’ may be used uninitialized in this function [-Wmaybe-uninitialized]
     return vx + vy;
                 ^~
maybe_uninit_01.cpp:19:17: warning: ‘vx’ may be used uninitialized in this function [-Wmaybe-uninitialized]
gcc_bugs root$ /usr/local/bin/g++ -Wall -Wextra -pedantic -Wmaybe-uninitialized -Weffc++ -Winline -Wfloat-conversion -c -Ofast maybe_uninit_01.cpp
gcc_bugs root$ /usr/local/bin/g++ -Wall -Wextra -pedantic -Wmaybe-uninitialized -Weffc++ -Winline -Wfloat-conversion -c -Ofast -g maybe_uninit_01.cpp
gcc_bugs root$ /usr/local/bin/g++ -Wall -Wextra -pedantic -Wmaybe-uninitialized -Weffc++ -Winline -Wfloat-conversion -c -Os maybe_uninit_01.cpp
gcc_bugs root$ /usr/local/bin/g++ -Wall -Wextra -pedantic -Wmaybe-uninitialized -Weffc++ -Winline -Wfloat-conversion -c -Os -g maybe_uninit_01.cpp
gcc_bugs root$ /usr/local/bin/g++ -v
Using built-in specs.
COLLECT_GCC=/usr/local/bin/g++
COLLECT_LTO_WRAPPER=/usr/local/libexec/gcc/i386-apple-darwin9.8.0/7.0.0/lto-wrapper
Target: i386-apple-darwin9.8.0
Configured with: ../configure --disable-werror --disable-werror-always --enable-languages=c,c++,lto,objc,obj-c++ --enable-stage1-checking=release -C --with-system-libunwind --enable-secureplt --enable-frame-pointer --enable-debug --with-isl --enable-objc-gc --disable-host-shared --enable-maintainer-mode --disable-default-pie --with-ld64 --without-pic CC=/usr/local/bin/gcc CXX=/usr/local/bin/g++ AUTOCONF=/usr/local/bin/autoconf AUTOHEADER=/usr/local/bin/autoheader AUTORECONF=/usr/local/bin/autoreconf AUTOM4TE=/usr/local/bin/autom4te AUTOSCAN=/usr/local/bin/autoscan AUTOUPDATE=/usr/local/bin/autoupdate IFNAMES=/usr/local/bin/ifnames
Thread model: posix
gcc version 7.0.0 20161027 (experimental) (GCC)
Comment 3 Andrew Pinski 2016-11-18 04:22:49 UTC
There might be a dup of this bug already.  See the linked PR if there is one.
Comment 4 Eric Gallager 2017-07-20 20:05:48 UTC
(In reply to Andrew Pinski from comment #3)
> There might be a dup of this bug already.  See the linked PR if there is one.

If it is a duplicate, I can't tell, so I'm just going to confirm this in the meantime (still happens with trunk)
Comment 5 Jeffrey A. Law 2017-12-16 00:24:14 UTC
Well, -Og doesn't do any of the detailed analysis and optimization that would give the compiler the opportunity to eliminate the paths that result in the false positive.  So it's no great surprise.
Comment 6 Eric Gallager 2017-12-16 02:41:52 UTC
(In reply to Jeffrey A. Law from comment #5)
> Well, -Og doesn't do any of the detailed analysis and optimization that
> would give the compiler the opportunity to eliminate the paths that result
> in the false positive.  So it's no great surprise.

-O0 doesn't do any of that detailed analysis or optimization either, but it doesn't have the same issue of warning like -Og
Comment 7 Jeffrey A. Law 2017-12-16 02:50:16 UTC
-O0 has none of the analysis necessary and I believe you get no warnings at all.

A minimum of -Og is needed, but -Og is inherently going to give many false positives.
Comment 8 Eric Gallager 2018-12-17 03:45:59 UTC
(In reply to Jeffrey A. Law from comment #7)
> -O0 has none of the analysis necessary and I believe you get no warnings at
> all.
> 
> A minimum of -Og is needed, but -Og is inherently going to give many false
> positives.

So is it worth trying to fix this then?
Comment 9 Allan Jensen 2018-12-17 08:35:46 UTC
I see two other level effort ways to possibly fix the issue. Disable the warning like for -O0 as it is buggy, or if we believe it still has some value in -Og even with the false positivies, just removing it from -Wall or -Wextra, so it at least doesn't get enabled unless explicitly asked for.
Comment 10 Marc Glisse 2018-12-17 09:10:09 UTC
IMO -Wmaybe-uninitialized should not be enabled by -Wall, whatever the optimization level (even at -O3), it has too many false positives that are all but impossible to work around (thus violating the definition of -Wall), -Wextra is good enough for it. We could keep -Wuninitialized though (it requires changing the dependency between these 2 warnings).
It is possible that even -Wextra should require at least -O1 (considering -Og as -O0.5) to enable it...
Comment 11 Eric Gallager 2018-12-17 15:11:06 UTC
(In reply to Marc Glisse from comment #10)
> IMO -Wmaybe-uninitialized should not be enabled by -Wall, whatever the
> optimization level (even at -O3), it has too many false positives that are
> all but impossible to work around (thus violating the definition of -Wall),
> -Wextra is good enough for it. We could keep -Wuninitialized though (it
> requires changing the dependency between these 2 warnings).

I think I remember at least 1 other bug where we came to this conclusion; I forget its number right now though...
Comment 12 Jeffrey A. Law 2018-12-19 00:08:53 UTC
Whether or not to fix as well as whether or not to warn at -O0 are a topic of debate.  I'm not sure I'm up for re-opening that can of worms right now.

I strongly believe -Wmaybe-uninitialized should continue to be enabled by -Wall.   They tend to either point out obscure ways objects are uninitialized or they point out missed optimizations.  Both are critical in my mind.
Comment 13 Eric Gallager 2018-12-19 03:27:55 UTC
(In reply to Eric Gallager from comment #11)
> (In reply to Marc Glisse from comment #10)
> > IMO -Wmaybe-uninitialized should not be enabled by -Wall, whatever the
> > optimization level (even at -O3), it has too many false positives that are
> > all but impossible to work around (thus violating the definition of -Wall),
> > -Wextra is good enough for it. We could keep -Wuninitialized though (it
> > requires changing the dependency between these 2 warnings).
> 
> I think I remember at least 1 other bug where we came to this conclusion; I
> forget its number right now though...

Right, it was bug 80635
Comment 14 Marc Glisse 2018-12-19 11:14:09 UTC
(In reply to Jeffrey A. Law from comment #12)
> Whether or not to fix as well as whether or not to warn at -O0 are a topic
> of debate.  I'm not sure I'm up for re-opening that can of worms right now.

I think we can both work on reducing false positives and move it out of -Wall, it isn't exclusive.

> I strongly believe -Wmaybe-uninitialized should continue to be enabled by
> -Wall.   They tend to either point out obscure ways objects are
> uninitialized or they point out missed optimizations.  Both are critical in
> my mind.

-Wall
           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.

I don't see how you can ever satisfy the "easy to avoid" part. In my experience with several code bases, having this warning in -Wall (as opposed to -Wextra) does more harm than good, with people doing random bad code changes to try and get the compiler to shut up.

I still believe this warning is a very useful static analysis tool (I contributed to make it appear more often in the past), but by definition it will never avoid false positives.
Comment 15 Eric Gallager 2019-03-19 07:13:28 UTC
(In reply to Marc Glisse from comment #14)
> (In reply to Jeffrey A. Law from comment #12)
> > Whether or not to fix as well as whether or not to warn at -O0 are a topic
> > of debate.  I'm not sure I'm up for re-opening that can of worms right now.
> 
> I think we can both work on reducing false positives and move it out of
> -Wall, it isn't exclusive.
> 
> > I strongly believe -Wmaybe-uninitialized should continue to be enabled by
> > -Wall.   They tend to either point out obscure ways objects are
> > uninitialized or they point out missed optimizations.  Both are critical in
> > my mind.
> 
> -Wall
>            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.
> 
> I don't see how you can ever satisfy the "easy to avoid" part. In my
> experience with several code bases, having this warning in -Wall (as opposed
> to -Wextra) does more harm than good, with people doing random bad code
> changes to try and get the compiler to shut up.
> 
> I still believe this warning is a very useful static analysis tool (I
> contributed to make it appear more often in the past), but by definition it
> will never avoid false positives.

For reference, this conversation moved to gcc-patches here: 
https://gcc.gnu.org/ml/gcc-patches/2019-02/msg00020.html
Comment 16 Martin Sebor 2021-04-01 20:42:41 UTC
Reconfirming with GCC 11.  Both instances of the warning go back as far as support  for -Og so it's not a regression.

With my patched GCC the output for just the first function is:

pr78394.C: In function ‘float foo()’:
pr78394.C:19:17: warning: ‘vy’ may be used uninitialized in this function [-Wmaybe-uninitialized]
   19 |     return vx + vy;
      |                 ^~
pr78394.C:13:15: note: used when ‘(a <= i)’
   13 |     float vx, vy;
      |               ^~
pr78394.C:13:15: note: ‘vy’ was declared here
pr78394.C:19:17: warning: ‘vx’ may be used uninitialized in this function [-Wmaybe-uninitialized]
   19 |     return vx + vy;
      |                 ^~
pr78394.C:13:11: note: used when ‘(a <= i)’
   13 |     float vx, vy;
      |           ^~
pr78394.C:13:11: note: ‘vx’ was declared here


The dump the warning sees is below as as noted the root cause is that the IL satisfies the conditions under which it's designed to trigger.  So the only ways to avoid it would seem to be to either a) extend the warning to figure out that the condition it uses cannot happen (basically implement some of the optimizations disabled at -Og) or b) turn off -Wmaybe-uninitialized at -Og (i.e,. remove it from -Wall).  I'm not in favor of (a) but (b) makes sense to me.  I'd like to try to improve it for GCC 12 so unless I fail at that I'm not for disabling it at other optimization levels.

float foo ()
{
  int i;
  float vy;
  float vx;
  int a;
  float _6;
  float _8;

  <bb 2> [local count: 118111600]:
  # VUSE <.MEM_4(D)>
  a_5 = b;
  if (a_5 <= 3)
    goto <bb 3>; [50.00%]
  else
    goto <bb 8>; [50.00%]

  <bb 8> [local count: 59055800]:
  goto <bb 4>; [100.00%]

  <bb 3> [local count: 59055800]:

  <bb 4> [local count: 118111600]:
  # a_1 = PHI <a_5(8), 4(3)>
  goto <bb 6>; [100.00%]

  <bb 5> [local count: 955630225]:
  _8 = (float) i_2;
  i_7 = i_2 + 1;

  <bb 6> [local count: 1073741824]:
  # i_2 = PHI <1(4), i_7(5)>
  # .MEM_3 = PHI <.MEM_4(D)(4), .MEM_3(5)>
  # vx_9 = PHI <vx_11(D)(4), _8(5)>
  # vy_10 = PHI <vy_12(D)(4), _8(5)>
  if (a_1 > i_2)
    goto <bb 5>; [89.00%]
  else
    goto <bb 7>; [11.00%]

  <bb 7> [local count: 118111600]:
  _6 = vx_9 + vy_10;
  # VUSE <.MEM_3>
  return _6;

}
Comment 17 Allan Jensen 2021-04-02 08:02:49 UTC
Yes, if you can figure out exactly what optimization passes it needs, then we could disable the warning when those passes are disabled.