[Bug c++/109720] New: -Wmaybe-uninitialized triggering when I can see no path that would allow it

[Bug c++/109720] New: -Wmaybe-uninitialized triggering when I can see no path that would allow it

[psmith at gnu dot org]

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

            Bug ID: 109720
           Summary: -Wmaybe-uninitialized triggering when I can see no
                    path that would allow it
           Product: gcc
           Version: 13.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: psmith at gnu dot org
  Target Milestone: ---

I recently upgraded to GCC 13.1 and when building some code that uses
boost::dynamic_bitset I'm seeing the -Wmaybe-ininitialized warning triggering
when it's impossible for any object of the class to be created without the
m_num_bits field being initialized.

I've tried to reduce this without luck.  Also if I do other, seemingly
irrelevant things then the warning goes away.  I wonder if it's somehow related
to the placement new that we use for the union...?

I'll attach the postprocessed output.

Code:

DynamicBitSet set(200);

size_t setup(size_t ln)
{
    size_t count = 0;
    DynamicBitSet nbits(set);
    for (auto bit: nbits) {
        count += bit;
    }
    return count;
}

I checked and this also failed in GCC 11.3; maybe -Wall didn't used to include
this warning and now it does which is why I didn't notice it before?

Results:

$ /data/src/build/x86_64-linux/bin/x86_64-rl84-linux-gnu-g++
-I/data/src/build/common/boost/include -std=gnu++20 -Wmaybe-uninitialized -O2
-c -o /tmp/bitset.o /tmp/bitset.i
In member function 'boost::dynamic_bitset<Block, Allocator>::size_type
boost::dynamic_bitset<Block, Allocator>::size() const [with Block = long
unsigned int; Allocator = std::allocator<long unsigned int>]',
    inlined from 'boost::dynamic_bitset<Block, Allocator>::size_type
boost::dynamic_bitset<Block, Allocator>::find_next(size_type) const [with Block
= long unsigned int; Allocator = std::allocator<long unsigned int>]' at
/tmp/bitset.i:70473:30,
    inlined from 'DynamicBitSet::size_type DynamicBitSet::find_next(size_type)
const' at /tmp/bitset.i:77301:44,
    inlined from 'DynamicBitSet::Iterator&
DynamicBitSet::Iterator::operator++()' at /tmp/bitset.i:77325:38,
    inlined from 'size_t setup(size_t)' at /tmp/bitset.i:77340:20:
/tmp/bitset.i:70355:12: warning: '*(const boost::dynamic_bitset<long unsigned
int, std::allocator<long unsigned int> >*)((char*)&nbits +
offsetof(DynamicBitSet,
DynamicBitSet::<unnamed>)).boost::dynamic_bitset<>::m_num_bits' may be used
uninitialized [-Wmaybe-uninitialized]
70355 |     return m_num_bits;
      |            ^~~~~~~~~~
/tmp/bitset.i: In function 'size_t setup(size_t)':
/tmp/bitset.i:77339:19: note: 'nbits' declared here
77339 |     DynamicBitSet nbits(set);
      |                   ^~~~~

[Bug tree-optimization/109720] -Wmaybe-uninitialized triggering when I can see no path that would allow it

[psmith at gnu dot org]

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

--- Comment #1 from Paul Smith <psmith at gnu dot org> ---
Hm, maybe it's due to the union.  Maybe GCC can't grok that we ensure that we
only use the dynamic_bitset leg of the union after we've initialized it?

I wonder if this warning could just assume that if the code uses one leg of the
union, that the other legs should be avoided.  Or something like that.  Else I
don't see how this warning can ever be used in code which uses unions.

[Bug tree-optimization/109720] -Wmaybe-uninitialized triggering when I can see no path that would allow it

[pinskia at gcc dot gnu.org]

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

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Paul Smith from comment #1)
> Hm, maybe it's due to the union.  Maybe GCC can't grok that we ensure that
> we only use the dynamic_bitset leg of the union after we've initialized it?

Once we get the preprocessed source, it should be easy to figure out why the
warning is showing up and if it is related to unions or something else.

[Bug tree-optimization/109720] -Wmaybe-uninitialized triggering when I can see no path that would allow it

[psmith at gnu dot org]

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

--- Comment #3 from Paul Smith <psmith at gnu dot org> ---
Created attachment 54986
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54986&action=edit
bitset.i.gz compressed preprocessor output

Hm, I did attach it when I filed the bug; I guess I forgot to click some final
"OK" button because it's not there now!  Sorry about that attaching now.

[Bug tree-optimization/109720] -Wmaybe-uninitialized triggering when I can see no path that would allow it

[pinskia at gcc dot gnu.org]

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

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Some internal GCC Notes:
PRE introduces the read from uninitialized memory though it is not used in some
pathes. The IR/CFG is very has some interesting twists and turns to it which
looks like it confuses both PRE and the uninitialized warning code.

[Bug tree-optimization/109720] -Wmaybe-uninitialized triggering when I can see no path that would allow it

[rguenth at gcc dot gnu.org]

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

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
We complain about the m_num_bits read.  While PRE isolates it to BB3 even
before PRE that happens unconditionally:

  <bb 2> [local count: 118111600]:
  nbits ={v} {CLOBBER};
  _25 = MEM[(const struct DynamicBitSet &)&set]._size;
  nbits._size = _25;
  if (_25 <= 64)
    goto <bb 3>; [67.00%]
  else
    goto <bb 4>; [33.00%]

  <bb 3> [local count: 79134772]:
  _162 = MEM[(const struct DynamicBitSet &)&set].D.151332._fixedSizeBitSet;
  nbits.D.151332._fixedSizeBitSet = _162;
  goto <bb 13>; [100.00%]

...
  <bb 12> [local count: 56691384]:
  _46 = (long unsigned int) _40;
  _47 = iftmp.2_49 + _46;
  MEM[(struct vector *)&nbits + 8B].D.150855._M_impl.D.150172._M_finish = _47;
  _29 = MEM[(const struct dynamic_bitset &)&set + 8].m_num_bits;
  MEM[(struct dynamic_bitset *)&nbits + 8B].m_num_bits = _29;
  _60 = boost::dynamic_bitset<>::m_do_find_from
(&nbits.D.151332._dynamicBitSet, 0);
  if (_60 != 18446744073709551615)
    goto <bb 13>; [66.92%]
  else
    goto <bb 33>; [33.08%]

  <bb 13> [local count: 103222856]:
  # _63 = PHI <_60(12), _25(3)>
  _68 = MEM[(const struct dynamic_bitset *)&nbits + 8B].m_num_bits;

PRE just sees the partial redundancy on the edge from BB12.

The read is from dynamic_bitset<Block, Allocator>::to_ulong().

It _looks_ like that maybe the range-for handling code exposes this in a way
that doesn't properly short-cut this read (or that GCC short-circuits a
test there).  Difficult to identify in the preprocessed sources.

[Bug tree-optimization/109720] -Wmaybe-uninitialized triggering when I can see no path that would allow it

[psmith at gnu dot org]

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

--- Comment #6 from Paul Smith <psmith at gnu dot org> ---
I'm happy to provide the source for DynamicBitSet (it's basically a union of a
uint64_t and a boost::dynamic_bitset so that if you have <=64 bits you use the
uint64_t and if you have >64 bits you use boost::dynamic_bitset).  I have a
hacked-up version of the original that removes all the unnecessary methods and
also throws away some of the complexity for handling the small bitset leg of
the union, which is not used in this example.

Providing the Boost stuff is harder because, as I'm sure you're aware if you've
used Boost, it's a LOT of headers and they all include others, etc. :).  But I
can try to get the necessary headers, or Boost can be downloaded separately.

[Bug tree-optimization/109720] -Wmaybe-uninitialized triggering when I can see no path that would allow it

[psmith at gnu dot org]

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

--- Comment #7 from Paul Smith <psmith at gnu dot org> ---
Just to note this code also throws this warning in GCC 12.3 but it doesn't
complain in GCC 11.3 which is what I was using before.