Bug 47679

Summary: [4.8/4.9/5 Regression] Strange uninitialized warning after SRA
Product: gcc Reporter: Jakub Jelinek <jakub>
Component: tree-optimizationAssignee: Not yet assigned to anyone <unassigned>
Status: NEW ---    
Severity: normal CC: andysem, caolanm, dtardon, gafnium, Laurent.Rineau__gcc, law, mika.fischer, rmansfield, rutsky.vladimir, Woebbeking
Priority: P2 Keywords: diagnostic
Version: 4.6.0   
Target Milestone: 4.8.5   
Host: Target:
Build: Known to work:
Known to fail: Last reconfirmed: 2011-03-03 11:26:57
Attachments: 676361.C
rh676361.C
Another warning appearance example

Description Jakub Jelinek 2011-02-10 14:44:42 UTC
On the attached testcase (simplified from #include <boost/optional> and then just getitem prototype and following lines) g++ starting with
http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=164136
issues a weird warning with -O2 -m64 -Wall:
warning: ‘*((void*)& aNewItem +8)’ may be used uninitialized in this function [-Wuninitialized]
(well, for some time it was an error about unhandled mem_ref in diagnostics). Before that commit no warnings were reported.

For more details see http://bugzilla.redhat.com/676361
Comment 1 Jakub Jelinek 2011-02-10 14:48:38 UTC
Created attachment 23294 [details]
676361.C
Comment 2 Jakub Jelinek 2011-02-10 15:43:53 UTC
Created attachment 23296 [details]
rh676361.C

Actually, that was delta reduced perhaps too much, by having operator! return false always it is certainly correct that it warns, as when the original is not initialized dummy_ will be used uninitialized.
Comment 3 Richard Biener 2011-02-10 16:19:05 UTC
As for the message for diagnostics (if we know that ...) we can generalize
it to warning: 'aNewItem' may be used uninitialized.  That's less precise
but does not contain funny pointer arith.  It would of course be bad to
do that for the generic tree dumpers.

But anyway, what's so strange about ‘*((void*)& aNewItem +8)’?  It was
invented exactly to avoid 'MEM[&aNewItem+8]'.
Comment 4 Jakub Jelinek 2011-02-10 16:20:13 UTC
So, before esra we have:
<bb 2>:
  aOldItem = getitem (); [return slot optimization]
  MEM[(struct optional_base *)&aNewItem].m_initialized = 0;
  D.3621_11 = MEM[(const struct optional_base *)this_2(D)];
  if (D.3621_11 != 0)
    goto <bb 3>;
  else
    goto <bb 4>;

<bb 3>:
  D.3623_13 = MEM[(const long unsigned int &)this_2(D) + 8];
  MEM[(internal_type *)&aNewItem + 8B] = D.3623_13;
  MEM[(struct optional_base *)&aNewItem].m_initialized = 1;

<bb 4>:
  D.3628_14 = MEM[(const struct optional_base *)&aOldItem];
  D.3629_15 = !D.3628_14;
  D.3630_16 = MEM[(const struct optional_base *)&aNewItem];
  D.3631_17 = !D.3630_16;
  if (D.3629_15 == D.3631_17)
    goto <bb 5>;
  else
    goto <bb 7>;

<bb 5>:
  D.3632_18 = MEM[(const struct optional_base *)&aOldItem];
  if (D.3632_18 == 0)
    goto <bb 7>;
  else
    goto <bb 6>;

<bb 6>:
  D.3634_19 = MEM[(const long unsigned int &)&aOldItem + 8];
  D.3633_20 = MEM[(const long unsigned int &)&aNewItem + 8];
  D.3635_21 = D.3634_19 == D.3633_20;

<bb 7>:
  # D.3635_22 = PHI <0(4), 1(5), D.3635_21(6)>

which has aNewItem+8 uninitialized if aNewItem.m_initialized is false, but then
aNewItem+8 isn't used either, just the condition is not very simple.
Then eSRA comes in and puts in an uninitialized SSA_NAME on one side of the PHI, but as the condition is too complicated we don't figure it out and warn anyway.

Not sure what can be done about it though,
return (!x) != (!y) ? false : ( !x ? true : (*x) == (*y) ) ;
is simply too complicated even for the current predicate aware uninitialized variable analysis (http://gcc.gnu.org/ml/gcc-patches/2010-04/msg00706.html), particularly figuring out that (*x) == (*y) is only invoked if !!x.

Another thing is why the MEM_REF in the diagnostic isn't printed as aNewItem.m_storage.dummy_ or aNewItem.m_storage.dummy_.data or aNewItem.m_storage.
Comment 5 Jakub Jelinek 2011-02-10 16:29:18 UTC
Changing boost slightly:
    template<class OptionalPointee> inline bool equal_pointees2 (OptionalPointee const& x, OptionalPointee const& y ) {
-    return (!x) != (!y) ? false : ( !x ? true : (*x) == (*y) ) ;
+    if (!x && !y) return true;
+    if (!x || !y) return false;
+    return (*x) == (*y);
   }
makes the warning go away, then the predicate aware uninit analysis figures things out.  But the generated code for this testcase is then 3 bytes longer (though, same number of insns).
Comment 6 Richard Biener 2011-03-03 11:26:57 UTC
> Another thing is why the MEM_REF in the diagnostic isn't printed as
> aNewItem.m_storage.dummy_ or aNewItem.m_storage.dummy_.data or
> aNewItem.m_storage.

this is because it doesn't usually match the source and in some cases
is ambiguous.  It's by design.
Comment 7 Jakub Jelinek 2011-03-25 19:53:00 UTC
GCC 4.6.0 is being released, adjusting target milestone.
Comment 8 Jakub Jelinek 2011-06-27 12:33:07 UTC
GCC 4.6.1 is being released.
Comment 9 Jakub Jelinek 2011-10-26 17:13:51 UTC
GCC 4.6.2 is being released.
Comment 10 Jakub Jelinek 2012-03-01 14:39:05 UTC
GCC 4.6.3 is being released.
Comment 11 Jakub Jelinek 2013-04-12 15:16:20 UTC
GCC 4.6.4 has been released and the branch has been closed.
Comment 12 André Wöbbeking 2013-08-30 13:04:10 UTC
Does anyone look into this? The warnings are really annoying. 

FYI, Clang 3.3 doesn't warn about this.
Comment 13 Jackie Rosen 2014-02-16 13:12:47 UTC
*** 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.
Comment 14 Richard Biener 2014-06-12 13:43:31 UTC
The 4.7 branch is being closed, moving target milestone to 4.8.4.
Comment 15 Jakub Jelinek 2014-12-19 13:26:36 UTC
GCC 4.8.4 has been released.
Comment 16 Sergey Eliseev 2015-01-12 17:10:59 UTC
Created attachment 34425 [details]
Another warning appearance example

Found similar warning in optional value comparison. 
GCC 4.8.3, boost::optional 1.40 and 1.53 

Compile with -O2 -Wall -Werror
Comment 17 Jeffrey A. Law 2015-02-16 23:39:46 UTC
Couldn't this be seen as a failure to thread jumps?  In the rs676361 testcase we have the following relevant blocks after DOM2:

;;   basic block 2, loop depth 0
;;    pred:       ENTRY
  aOldItem = getitem (); [return slot optimization]
  aNewItem_19 = 0;
  _9 = MEM[(bool *)this_4(D)];
  if (_9 != 0)
    goto <bb 3>;
  else
    goto <bb 4>;
;;    succ:       3
;;                4

;;   basic block 3, loop depth 0
;;    pred:       2
  aNewItem$8_22 = MEM[(const long unsigned int &)this_4(D) + 8];
  aNewItem$8_20 = aNewItem$8_22;
  aNewItem_21 = 1;
;;    succ:       4

;;   basic block 4, loop depth 0
;;    pred:       3
;;                2
  # aNewItem_24 = PHI <1(3), 0(2)>
  # aNewItem$8_23 = PHI <aNewItem$8_22(3), aNewItem$8_6(D)(2)>
  _5 = MEM[(bool *)&aOldItem];
  _10 = ~_5;
  _11 = VIEW_CONVERT_EXPR<const bool>(aNewItem_24);
  _12 = ~_11;
  if (_10 == _12)
    goto <bb 5>;
  else
    goto <bb 7>;
;;    succ:       5
;;                7

;;   basic block 5, loop depth 0
;;    pred:       4
  if (_10 != 0)
    goto <bb 7>;
  else
    goto <bb 6>;
;;    succ:       7
;;                6

;;   basic block 6, loop depth 0
;;    pred:       5
  _13 = MEM[(const long unsigned int &)&aOldItem + 8];
  aNewItem$8_14 = aNewItem$8_23;
  iftmp.4_15 = _13 == aNewItem$8_23;
  _16 = ~iftmp.4_15;
  _17 = (int) _16;
;;    succ:       7


The key here is that we know the value of _12 depending on how we reach block #4.  If BB4 then transfers control to block #5 we'll also know the value of _10 which would allow us to thread jumps and I believe simplifies things enough to avoid the false positive, at least on the reduced case.  I haven't tried case from Sergey yet.

This appears to be caused by a few deficiencies in jump threading.  None appear to be major issues though.