Bug 59932 - spurious undefined behavior warning on valid code
Summary: spurious undefined behavior warning on valid code
Status: RESOLVED WONTFIX
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.9.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-01-24 03:09 UTC by Zhendong Su
Modified: 2014-01-30 01:40 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Zhendong Su 2014-01-24 03:09:09 UTC
The current gcc trunk issues a spurious undefined behavior warning when compiling the following valid code at -Os and above in both 32-bit and 64-bit mode. 

It does not affect gcc 4.8 though, so it is a regression from 4.8.x. 

Interestingly, the code also causes clang (from 3.2 to its current trunk) to hang at -Os and above. 


$ gcc-trunk -v
Using built-in specs.
COLLECT_GCC=gcc-trunk
COLLECT_LTO_WRAPPER=/usr/local/gcc-trunk/libexec/gcc/x86_64-unknown-linux-gnu/4.9.0/lto-wrapper
Target: x86_64-unknown-linux-gnu
Configured with: ../gcc-trunk/configure --prefix=/usr/local/gcc-trunk --enable-languages=c,c++ --disable-werror --enable-multilib
Thread model: posix
gcc version 4.9.0 20140123 (experimental) [trunk revision 206958] (GCC) 
$ 
$ gcc-trunk -O1 small.c; a.out
$ gcc-4.8.2 -Os small.c; a.out
$ 
$ gcc-trunk -Os small.c
small.c: In function ‘main’:
small.c:27:16: warning: iteration 2147483646u invokes undefined behavior [-Waggressive-loop-optimizations]
   for (; p1; p1++)
                ^
small.c:27:3: note: containing loop
   for (; p1; p1++)
   ^
$ 
$ timeout -s 9 30 clang-trunk -Os small.c
Killed
$ 
$ 


---------------------------------------


int a, b, c, e[1], f, g, h, i, k, m, n;
short j;

struct
{
  short f1;
} d;

static int
fn1 (int p1)
{
  int l = 64696;
  for (f = 0; f != 1; f--)
    {
      if (d.f1 > l)
	{
	  h = p1;
	  if (!h)
	    l = 0;
	}
      else
	return b;
      for (; a; a++)
	if (e[a])
	  break;
    }
  for (; p1; p1++)
    {
      if (h)
	continue;
      if (l)
	i = j = k = g && d.f1 != p1;
      for (; h != 1; h++)
	;
    }
  return 0;
}

int
main ()
{
  m = c = 1;
  n = fn1 (c);
  return 0;
}
Comment 1 Andrew Pinski 2014-01-24 03:12:39 UTC
I don't see why you think this is not undefined behavior.
If p1 starts at 1, it cannot turn into 0 as p1++ overflows during the 2147483646th iteration.
Comment 2 Zhendong Su 2014-01-24 03:20:52 UTC
(In reply to Andrew Pinski from comment #1)
> I don't see why you think this is not undefined behavior.
> If p1 starts at 1, it cannot turn into 0 as p1++ overflows during the
> 2147483646th iteration.

Andrew, because "d.f1 > l" is false, so the code simply returns ("return b;"). 

I also always double-check with CompCert's reference interpreter and Frama-C if possible.
Comment 3 Andrew Pinski 2014-01-24 04:16:46 UTC
(In reply to Zhendong Su from comment #2)
> (In reply to Andrew Pinski from comment #1)
> > I don't see why you think this is not undefined behavior.
> > If p1 starts at 1, it cannot turn into 0 as p1++ overflows during the
> > 2147483646th iteration.
> 
> Andrew, because "d.f1 > l" is false, so the code simply returns ("return
> b;"). 
> 
> I also always double-check with CompCert's reference interpreter and Frama-C
> if possible.

I see what is happening.  It is a true warning that happens due to optimizing order differences.  The place we warn does not know that f is zero the first time through the loop.  Since -Os disables copy headers, we don't get a different copy of the header.  So the code does not optimize away the header.

This is where I am going to say there is a false positive due to optimizing.  I want to close it as won't fix because if we change the value of l to be 0xfe, then we always warn.
Comment 4 Zhendong Su 2014-01-24 20:49:24 UTC
(In reply to Andrew Pinski from comment #3)
> (In reply to Zhendong Su from comment #2)
> > (In reply to Andrew Pinski from comment #1)
> > > I don't see why you think this is not undefined behavior.
> > > If p1 starts at 1, it cannot turn into 0 as p1++ overflows during the
> > > 2147483646th iteration.
> > 
> > Andrew, because "d.f1 > l" is false, so the code simply returns ("return
> > b;"). 
> > 
> > I also always double-check with CompCert's reference interpreter and Frama-C
> > if possible.
> 
> I see what is happening.  It is a true warning that happens due to
> optimizing order differences.  The place we warn does not know that f is
> zero the first time through the loop.  Since -Os disables copy headers, we
> don't get a different copy of the header.  So the code does not optimize
> away the header.
> 
> This is where I am going to say there is a false positive due to optimizing.
> I want to close it as won't fix because if we change the value of l to be
> 0xfe, then we always warn.

Andrew, sorry, I'm baffled by your comments above. 

Please note: 
1) The issue isn't only triggered at -Os, but also at -O2 and -O3. 
2) It doesn't affect GCC 4.8.
3) I don't see how changing l to 0xfe has changed anything. 
4) Also optimizations shouldn't really change the warnings issued. 

Perhaps I have some misunderstandings, so could you clarify?  Thanks.
Comment 5 Jakub Jelinek 2014-01-28 16:20:33 UTC
(In reply to Zhendong Su from comment #2)
> (In reply to Andrew Pinski from comment #1)
> > I don't see why you think this is not undefined behavior.
> > If p1 starts at 1, it cannot turn into 0 as p1++ overflows during the
> > 2147483646th iteration.
> 
> Andrew, because "d.f1 > l" is false, so the code simply returns ("return
> b;"). 
> 
> I also always double-check with CompCert's reference interpreter and Frama-C
> if possible.

There clearly is a loop with undefined behavior if it is every entered (several of them), but you just never happen to enter it.  Whether to warn about bugs in obviously dead code is a heated debate, some people ask for it, others don't want it, but in this case whether the compiler knows it is dead code or not depends on optimizations.  GCC doesn't warn about the other loops because they have multiple exits and the -Waggressive-loop-optimization warning is, in order to have as few false positives as possible, only used for the most simple loops.

Anyway, in this case IMHO it is very well worth the false positive in this case, rather than never warning because we can't be 100% sure if it isn't in dead code.
After all, we couldn't then warn about
int a[3];
void foo (void)
{
  for (int i = 0; i < 4; i++)
    a[i]++;
}
just because main might not call foo at all and thus you'd never invoke the undefined behavior.
Comment 6 Zhendong Su 2014-01-28 18:15:36 UTC
Thanks for your explanation Jakub. It's more clear now, but I still don't fully understand the difference in behavior from 4.8 to the current trunk. 

Is it because 4.8's support for warning undefined behaviors is weaker than 4.9's, and with that enhanced support, 4.9 sometimes gives false warnings like the one reported here?
Comment 7 Jakub Jelinek 2014-01-28 21:45:18 UTC
(In reply to Zhendong Su from comment #6)
> Thanks for your explanation Jakub. It's more clear now, but I still don't
> fully understand the difference in behavior from 4.8 to the current trunk. 
> 
> Is it because 4.8's support for warning undefined behaviors is weaker than
> 4.9's, and with that enhanced support, 4.9 sometimes gives false warnings
> like the one reported here?

4.8 only warned about this in later passes when the loops have been already constructed and preserved, so it wouldn't warn e.g. if it was cunrolli (as in this case) that found the undefined behavior.  GCC 4.9 creates loops immediately after cfg is created and the warning is thus enabled much earlier.  This means we warn in more cases when it is desirable to warn, but as this testcase shows also sometimes means there can be false positives.  The loop with the undefined behavior is there for many passes, from cunrolli where it warns another 20 passes until dom1 is able to find out the code is dead.
Comment 8 Zhendong Su 2014-01-30 01:38:20 UTC
(In reply to Jakub Jelinek from comment #7)
> (In reply to Zhendong Su from comment #6)
> > Thanks for your explanation Jakub. It's more clear now, but I still don't
> > fully understand the difference in behavior from 4.8 to the current trunk. 
> > 
> > Is it because 4.8's support for warning undefined behaviors is weaker than
> > 4.9's, and with that enhanced support, 4.9 sometimes gives false warnings
> > like the one reported here?
> 
> 4.8 only warned about this in later passes when the loops have been already
> constructed and preserved, so it wouldn't warn e.g. if it was cunrolli (as
> in this case) that found the undefined behavior.  GCC 4.9 creates loops
> immediately after cfg is created and the warning is thus enabled much
> earlier.  This means we warn in more cases when it is desirable to warn, but
> as this testcase shows also sometimes means there can be false positives. 
> The loop with the undefined behavior is there for many passes, from cunrolli
> where it warns another 20 passes until dom1 is able to find out the code is
> dead.

Thanks
Comment 9 Zhendong Su 2014-01-30 01:40:47 UTC
Thanks Jakub; I follow now. 

Let me mark it as RESOLVED/WONTFIX.