Bug 65686 - [5/6/7 regression] inconsistent warning maybe-uninitialized: warn about 'unsigned', not warn about 'int'
Summary: [5/6/7 regression] inconsistent warning maybe-uninitialized: warn about 'unsi...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 5.0
: P2 normal
Target Milestone: 7.0
Assignee: Richard Biener
URL:
Keywords: diagnostic, missed-optimization
Depends on: 13962
Blocks:
  Show dependency treegraph
 
Reported: 2015-04-07 12:25 UTC by Dmitry G. Dyachenko
Modified: 2016-04-29 08:37 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2015-04-15 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dmitry G. Dyachenko 2015-04-07 12:25:28 UTC
gcc version 5.0.0 20150404 (experimental) [trunk revision 221866] (GCC)

$ cat x.i
typedef unsigned mytype; // 'int' fixes issue

struct S {
  mytype *pu;
};

mytype f(struct S *e)
{
  mytype x;
  if(&x != e->pu)
    __builtin_memcpy(&x, e->pu, sizeof(unsigned));
  return x;
}

$ gcc -fpreprocessed -Werror -Wall -O2 -c x.i
x.i: In function ‘f’:
x.i:9:10: error: ‘x’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
   mytype x;
          ^
cc1: all warnings being treated as error
Comment 1 Dmitry G. Dyachenko 2015-04-12 20:03:41 UTC
gcc version 4.9.3 20150412 (prerelease) [gcc-4_9-branch revision 222021] (GCC)
PASS
Comment 2 Dmitry G. Dyachenko 2015-04-14 11:14:15 UTC
r212420 PASS
r212492 FAIL

gcc version 4.10.0 20140710 (experimental) [trunk revision 212420] (GCC)
gcc version 4.10.0 20140713 (experimental) [trunk revision 212492] (GCC)
Comment 3 Dmitry G. Dyachenko 2015-04-14 18:36:47 UTC
start FAIL r212452 (fix for PR middle-end/61473)
Comment 4 Richard Biener 2015-04-15 07:46:36 UTC
It's (good)

  <bb 2>:
  _4 = e_3(D)->pu;
  if (&x != _4)
    goto <bb 3>;
  else
    goto <bb 5>;

  <bb 5>:
  goto <bb 4>;

  <bb 3>:
  _5 = MEM[(char * {ref-all})_4];
  MEM[(char * {ref-all})&x] = _5;

  <bb 4>:
  _7 = x;
  x ={v} {CLOBBER};
  return _7;

vs. (bad)

  <bb 2>:
  _4 = e_3(D)->pu;
  if (&x != _4)
    goto <bb 4>;
  else
    goto <bb 3>;

  <bb 3>:
  pretmp_9 = x;
  goto <bb 5>;

  <bb 4>:
  _5 = MEM[(char * {ref-all})_4];

  <bb 5>:
  # prephitmp_10 = PHI <pretmp_9(3), _5(4)>
  x ={v} {CLOBBER};
  return prephitmp_10;

that's a missed PRE opportunity in the good case (not sure why we use
unsigned int for the memcpy inlining - ah, because it goes the new
single load/store case in gimple_fold_builtin_memory_op).  PRE should
still be able to handle.

Only after PRE we hit the very special case where uninit warns about memory
reads (no store before the reads).  For this case inconsistencies are
very much expected...

But the testcase also shows a missed optimization opportunity because
&x can never be equal to e->pu (PR13962).
Comment 5 Richard Biener 2015-06-18 11:50:03 UTC
Note that you are basically relying on optimization to remove the uninitialized load.

Where does this obfuscated 'return *(e->pu)' come from?

Related to PR13962 (to simplify the compare to false)
Comment 6 Dmitry G. Dyachenko 2015-06-18 12:09:10 UTC
(In reply to Richard Biener from comment #5)
> 
> Where does this obfuscated 'return *(e->pu)' come from?

If I remove aggregate then there are no warning.


i.e. this code produce no warning

mytype f(mytype *pu)
{
  mytype x;
  if(&x != pu)
    __builtin_memcpy(&x, pu, sizeof(unsigned));
  return x;
}
Comment 7 Richard Biener 2015-07-16 09:10:47 UTC
GCC 5.2 is being released, adjusting target milestone to 5.3.
Comment 8 Richard Biener 2015-12-04 10:43:19 UTC
GCC 5.3 is being released, adjusting target milestone.
Comment 9 Jakub Jelinek 2016-01-27 16:04:42 UTC
So, does the https://gcc.gnu.org/bugzilla/show_bug.cgi?id=13962#c9 patch help here?
Comment 10 Richard Biener 2016-01-28 10:59:52 UTC
(In reply to Jakub Jelinek from comment #9)
> So, does the https://gcc.gnu.org/bugzilla/show_bug.cgi?id=13962#c9 patch
> help here?

Yes.  With it we simplify the function during early FRE to

f (struct S * e)
{
  mytype x;
  mytype * _4;
  unsigned int _6;
  mytype _8;

  <bb 2>:
  _4 = e_3(D)->pu;
  _6 = MEM[(char * {ref-all})_4];
  MEM[(char * {ref-all})&x] = _6;
  _8 = x;
  x ={v} {CLOBBER};
  return _8;

(missed optimization - alias stmt walking doesn't honor edge non-executablility as figured out by FRE).  Later optimization turns this into

  <bb 2>:
  _3 = e_2(D)->pu;
  _4 = MEM[(char * {ref-all})_3];
  return _4;
Comment 11 Richard Biener 2016-01-28 11:03:42 UTC
So we can simplify the fix for PR13962 by only considering pointer with decl compares which points-to should handle well enough (and not fall into the issue
of bogus ptr1 vs. ptr2 simplifications because we don't properly track whether
ptr1 or ptr2 may be NULL).  ISTR such checks also happen in libstdc++ code.

Not sure if appropriate at this stage, I would tend to a no here.  I'll queue
re-visiting PR13962 during next stage1.
Comment 12 Jakub Jelinek 2016-01-28 11:05:05 UTC
(In reply to Richard Biener from comment #11)
> So we can simplify the fix for PR13962 by only considering pointer with decl
> compares which points-to should handle well enough (and not fall into the
> issue
> of bogus ptr1 vs. ptr2 simplifications because we don't properly track
> whether
> ptr1 or ptr2 may be NULL).  ISTR such checks also happen in libstdc++ code.
> 
> Not sure if appropriate at this stage, I would tend to a no here.  I'll queue
> re-visiting PR13962 during next stage1.

I agree it is too risky now.  Let's retarget this at GCC 7 then.
Comment 13 Richard Biener 2016-04-29 08:26:56 UTC
Fixed for GCC 7.
Comment 14 Richard Biener 2016-04-29 08:37:21 UTC
Author: rguenth
Date: Fri Apr 29 08:36:49 2016
New Revision: 235622

URL: https://gcc.gnu.org/viewcvs?rev=235622&root=gcc&view=rev
Log:
2016-04-29  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/13962
	PR tree-optimization/65686
	* tree-ssa-alias.h (ptrs_compare_unequal): Declare.
	* tree-ssa-alias.c (ptrs_compare_unequal): New function
	using PTA to compare pointers.
	* match.pd: Add pattern for pointer equality compare simplification
	using ptrs_compare_unequal.

	* gcc.dg/uninit-pr65686.c: New testcase.

Added:
    trunk/gcc/testsuite/gcc.dg/uninit-pr65686.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/match.pd
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-alias.c
    trunk/gcc/tree-ssa-alias.h