[Bug tree-optimization/39233] New: [4.4 Regression] ivopts + vrp miscompilation

[Bug tree-optimization/39233] New: [4.4 Regression] ivopts + vrp miscompilation

[jakub at gcc dot gnu dot org]

extern void abort (void);

__attribute__((noinline)) void
foo (void *p)
{
  long l = (long) p;
  if (l < 0 || l > 6)
    abort ();
}

int
main ()
{
  int i;
  for (i = 6; i >= 0; i--)
    foo ((void *) (long) i);
  return 0;
}

is miscompiled (into endless loop).  First ivopts decides to use a pointer IV,
going from (void *) 6 down, with (void *) ivtmp.51 != (void *) -1 as loop
condition, then VRP comes in and as pointers can never wrap around, optimizes
the loop condition into 1.


-- 
           Summary: [4.4 Regression] ivopts + vrp miscompilation
           Product: gcc
           Version: 4.4.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: tree-optimization
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: jakub at gcc dot gnu dot org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39233

[Bug tree-optimization/39233] [4.4 Regression] ivopts + vrp miscompilation

[jakub at gcc dot gnu dot org]

-- 

jakub at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.4.0


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39233

[Bug tree-optimization/39233] [4.4 Regression] ivopts + vrp miscompilation

[jakub at gcc dot gnu dot org]

------- Comment #1 from jakub at gcc dot gnu dot org  2009-02-18 17:24 -------
Caused by PR31358.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39233

[Bug tree-optimization/39233] [4.4 Regression] ivopts + vrp miscompilation

[rguenth at gcc dot gnu dot org]

------- Comment #2 from rguenth at gcc dot gnu dot org  2009-02-18 17:47 -------
I will have a look.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |rguenth at gcc dot gnu dot
                   |dot org                     |org
             Status|UNCONFIRMED                 |ASSIGNED
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2009-02-18 17:47:42
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39233

[Bug tree-optimization/39233] [4.4 Regression] ivopts + vrp miscompilation

[rguenth at gcc dot gnu dot org]

------- Comment #3 from rguenth at gcc dot gnu dot org  2009-02-18 17:50 -------
Confirmed on x86_64 with -O2.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 GCC target triplet|                            |x86_64-*-*
      Known to work|                            |4.3.3


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39233

[Bug tree-optimization/39233] [4.4 Regression] ivopts + vrp miscompilation

[rguenth at gcc dot gnu dot org]

------- Comment #4 from rguenth at gcc dot gnu dot org  2009-02-18 17:53 -------
This one also fails on i?86-*-*:

extern void abort (void);

__attribute__((noinline)) void
foo (void *p)
{
  long l = (long) p;
  if (l < 0 || l > 6)
    abort ();
}

int
main ()
{
  short i;
  for (i = 6; i >= 0; i--)
    foo ((void *) (long) i);
  return 0;
}


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 GCC target triplet|x86_64-*-*                  |x86_64-*-*, i?86-*-*


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39233

[Bug tree-optimization/39233] [4.4 Regression] ivopts + vrp miscompilation

[rguenth at gcc dot gnu dot org]

------- Comment #5 from rguenth at gcc dot gnu dot org  2009-02-18 18:47 -------
This patch fixes it, with unknown side-effects.  It should be ok for
the common sizetype extensions due to POINTER_PLUS_EXPR (sizetype is
unsigned for sane languages).

Index: tree-scalar-evolution.c
===================================================================
--- tree-scalar-evolution.c     (revision 144265)
+++ tree-scalar-evolution.c     (working copy)
@@ -2799,6 +2799,14 @@ simple_iv (struct loop *loop, gimple stm
       || chrec_contains_symbols_defined_in_loop (iv->base, loop->num))
     return false;

+  /* If we folded casts and the result is a type where overflow is
+     undefined the IV may not be simple as it can have introduced
+     undefined overflow that wasn't there before.  */
+  if (folded_casts
+      && (TYPE_OVERFLOW_UNDEFINED (type)
+         || POINTER_TYPE_P (type)))
+    return false;
+
   iv->no_overflow = !folded_casts && TYPE_OVERFLOW_UNDEFINED (type);

   return true;


The following patch would be slightly less intrusive (only affects IVOPTs),
but possibly other passes might be affected by the same bug.  OTOH it
doesn't affect number-of-iterations analysis, which the above does.

Index: tree-ssa-loop-ivopts.c
===================================================================
--- tree-ssa-loop-ivopts.c      (revision 144265)
+++ tree-ssa-loop-ivopts.c      (working copy)
@@ -887,6 +887,14 @@ determine_biv_step (gimple phi)
   if (!simple_iv (loop, phi, name, &iv, true))
     return NULL_TREE;

+  /* If the IV may overflow and the result is a type we know does not
+     overflow we may have introduced undefined overflow.  Do not use
+     that induction variable.  */
+  if (!iv.no_overflow
+      && (TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (name))
+         || POINTER_TYPE_P (TREE_TYPE (name))))
+    return NULL_TREE;
+
   return integer_zerop (iv.step) ? NULL_TREE : iv.step;
 }

@@ -992,6 +1000,15 @@ find_givs_in_stmt_scev (struct ivopts_da

   if (!simple_iv (loop, stmt, lhs, iv, true))
     return false;
+
+  /* If the IV may overflow and the result is a type we know does not
+     overflow we may have introduced undefined overflow.  Do not use
+     that induction variable.  */
+  if (!iv->no_overflow
+      && (TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (lhs))
+         || POINTER_TYPE_P (TREE_TYPE (lhs))))
+    return false;
+
   iv->base = expand_simple_operations (iv->base);

   if (contains_abnormal_ssa_name_p (iv->base)


in the end someone finally should sit down and make overflowing/non-overflowing
arithmetic explicit in the IL.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39233

[Bug tree-optimization/39233] [4.4 Regression] ivopts + vrp miscompilation

[jakub at gcc dot gnu dot org]

------- Comment #6 from jakub at gcc dot gnu dot org  2009-02-18 18:56 -------
Would it be possible for known loop bounds to still use pointer etc. ivopts if
we can ensure the overflow doesn't happen on that interval (+-1)?  Say if the
same testcase goes for (i = 16; i >= 10; i--) instead of for (i = 6; i >= 0;
i--)?


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39233

[Bug tree-optimization/39233] [4.4 Regression] ivopts + vrp miscompilation

[rguenth at gcc dot gnu dot org]

------- Comment #7 from rguenth at gcc dot gnu dot org  2009-02-18 19:56 -------
I'm sure it is somehow possible, maybe we can use scev_probably_wraps_p
in simple_iv.  I will check that.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39233

[Bug tree-optimization/39233] [4.4 Regression] ivopts + vrp miscompilation

[rguenth at gcc dot gnu dot org]

-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39233

[Bug tree-optimization/39233] [4.4 Regression] ivopts + vrp miscompilation

[rguenth at gcc dot gnu dot org]

------- Comment #9 from rguenth at gcc dot gnu dot org  2009-02-24 11:05 -------
Subject: Bug 39233

Author: rguenth
Date: Tue Feb 24 11:05:15 2009
New Revision: 144405

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=144405
Log:
2009-02-24  Richard Guenther  <rguenther@suse.de>
        Zdenek Dvorak  <ook@ucw.cz>

        PR tree-optimization/39233
        * tree-ssa-loop-ivopts.c (add_candidate_1): Do not except pointers
        from converting them to a generic type.

        * gcc.c-torture/execute/pr39233.c: New testcase.

Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/pr39233.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-loop-ivopts.c


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39233

[Bug tree-optimization/39233] [4.4 Regression] ivopts + vrp miscompilation

[rguenth at gcc dot gnu dot org]

------- Comment #8 from rguenth at gcc dot gnu dot org  2009-02-24 11:05 -------
Fixed.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39233