Bug 67828

Summary: [6 Regression] wrong code at -O3 on x86_64-linux-gnu
Product: gcc Reporter: Zhendong Su <su>
Component: rtl-optimizationAssignee: Alexandre Oliva <aoliva>
Severity: normal CC: aoliva
Priority: P3    
Version: 6.0   
Target Milestone: 6.0   
Host: Target:
Build: Known to work: 5.2.0
Known to fail: Last reconfirmed: 2015-10-05 00:00:00
Attachments: Patch I'm testing to fix the bug

Description Zhendong Su 2015-10-03 01:05:52 UTC
The current gcc trunk miscompiles the following code on x86_64-linux-gnu at -O3 in both 32-bit and 64-bit modes. 

This is a regression from 5.2.x.

$ gcc-trunk -v
Using built-in specs.
Target: x86_64-pc-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 6.0.0 20151002 (experimental) [trunk revision 228389] (GCC) 
$ gcc-trunk -O2 small.c; ./a.out
$ gcc-5.2 -O3 small.c; ./a.out
$ gcc-trunk -O3 small.c
$ ./a.out


int printf (const char *, ...);

int a, b;
short c;

main ()
  int j, d = 1;
  for (; c >= 0; c++)
      a = d;
      d = 0;
      if (b)
	  printf ("%d", 0);
	  if (j)
	    printf ("%d", 0);
  printf ("%d\n", d);
  return 0; 
Comment 1 Richard Biener 2015-10-05 08:44:49 UTC
Comment 2 Marek Polacek 2015-10-05 10:07:10 UTC
Started with r226901.
Comment 3 Alexandre Oliva 2015-10-06 07:40:44 UTC
Created attachment 36448 [details]
Patch I'm testing to fix the bug

This testcase invokes undefined behavior because of the overflow on the iterator, but the cause of the problem is undefined behavior we introduce ourselves by unswitching the loop on the uninitialized variable j.  Things go down the hill from there, as we uncprop j's default def in the j==0 branch into one of D's phi nodes in that version of the loop, then we coalesce j's default def with some versions of d (we only detect conflicts for parms' and results' default defs), including the one passed to printf.  init-regs adds zero initialization for j, the loop-entry initialization of d with 1 overrides it, and the assignment to zero within the loop, that uncprop had turned into a copy from j (known to be zero in that branch), is optimized out because of the coalescing.  So the initialization of d to 1 prevails.  Preventing coalescing by detecting conflicts with non-params would also avoid the symptom, but since accessing uninitialized variables is undefined behavior, we shouldn't have to worry about that.  This is only relevant in this case because we introduce the undefined access in the first place, so that's what the patch fixes.
Comment 4 Richard Biener 2015-10-06 08:48:52 UTC
Use ssa_undefined_value_p (..., true), otherwise looks sensible.  I suppose
we could unswitch on the value if the condition is always executed in the loop
but then that's hardly a useful optimization (on undefined values).
Comment 5 Zhendong Su 2015-10-07 15:21:59 UTC
> This testcase invokes undefined behavior because of the overflow on the
> iterator, ... 

Just a quick comment that the testcase doesn't have undefined behaviors. As the variable c is of type short, there shouldn't be signed overflow --- the operations will be performed on its signed extended integer value and then truncated back to a short.  

Thanks for looking into and fixing the issue.
Comment 6 Zhendong Su 2015-10-07 15:28:43 UTC
Below is another testcase that I believe exposes the same issue: 


int a = 2, b = 1, c = 1;

fn1 ()
  int d;
  for (; a; a--)
      for (d = 0; d < 4; d++)
          int k;
          if (c < 1)
            if (k)
              c = 0;
          if (b)
          return 0;
      b = !1;
  return 0;

main ()
  fn1 ();

  if (a != 1) 
    __builtin_abort (); 

  return 0;
Comment 7 Alexandre Oliva 2015-10-09 12:18:56 UTC
Author: aoliva
Date: Fri Oct  9 12:18:24 2015
New Revision: 228650

URL: https://gcc.gnu.org/viewcvs?rev=228650&root=gcc&view=rev
[PR67828] don't unswitch on default defs of non-parms

for  gcc/ChangeLog

	PR rtl-optimizatoin/67828
	* tree-ssa-loop-unswitch.c: Include tree-ssa.h.
	(tree_may_unswitch_on): Don't unswitch on expressions
	involving undefined values.

for  gcc/testsuite/ChangeLog

	PR rtl-optimization/67828
	* gcc.dg/torture/pr67828.c: New.

Comment 8 Alexandre Oliva 2015-10-09 12:20:47 UTC