[Bug tree-optimization/38984] New: [4.2/4.3/4.4 Regression] NULL pointers always considered distinct by PTA, even with -fno-delete-null-pointer-checks

[Bug tree-optimization/38984] New: [4.2/4.3/4.4 Regression] NULL pointers always considered distinct by PTA, even with -fno-delete-null-pointer-checks

[bonzini at gnu dot org]

This testcase fails:

/* { dg-do compile } */
/* { dg-options "-O2 -fno-delete-null-pointer-checks -fdump-tree-optimized" }
*/

int f(int *p)
{
  int a = *p;
  int *null = 0;
  *null = 5;
  return *p == a;
}

/* { dg-final { scan-tree-dump-times " = \\\*p" 2 "optimized" } } */
/* { dg-final { scan-tree-dump-not "return 1" "optimized" } } */
/* { dg-final { cleanup-tree-dump "optimized" } } */

This testcase is important because targets that are always MMU-less (like AVR)
will always have -fdelete-null-pointer-checks disabled, so the wrong code bug
will happen with -O2.  IMO it is a P2.


-- 
           Summary: [4.2/4.3/4.4 Regression] NULL pointers always considered
                    distinct by PTA, even with -fno-delete-null-pointer-
                    checks
           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: bonzini at gnu dot org


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

[Bug tree-optimization/38984] [4.2/4.3/4.4 Regression] NULL pointers always considered distinct by PTA, even with -fno-delete-null-pointer-checks

[bonzini at gnu dot org]

------- Comment #1 from bonzini at gnu dot org  2009-01-27 11:02 -------
This simple patch is not enough:

Index: tree-ssa-structalias.c
===================================================================
--- tree-ssa-structalias.c      (revision 142960)
+++ tree-ssa-structalias.c      (working copy)
@@ -3030,8 +3030,14 @@ get_constraint_for_1 (tree t, VEC (ce_s,
      happens below, since it will fall into the default case. The only
      case we know something about an integer treated like a pointer is
      when it is the NULL pointer, and then we just say it points to
-     NULL.  */
-  if (TREE_CODE (t) == INTEGER_CST
+     NULL.
+
+     Do not do that if -fno-delete-null-pointer-checks though, because
+     in that case *NULL does not fail, so it _should_ alias *anything.
+     It is not worth adding a new option or renaming the existing one,
+     since this case is relatively obscure.  */
+  if (flag_delete_null_pointer_checks
+      && TREE_CODE (t) == INTEGER_CST
       && integer_zerop (t))
     {
       temp.var = nothing_id;

We get:

ANYTHING = &ANYTHING
ESCAPED = *ESCAPED
NONLOCAL = &ESCAPED
INTEGER = &ANYTHING
derefaddrtmp.8 = &NONLOCAL
*ESCAPED = derefaddrtmp.8
p = &NONLOCAL

...

NULL = { }
ANYTHING = { ANYTHING }
READONLY = { READONLY }
ESCAPED = { }
NONLOCAL = { ESCAPED }
CALLUSED = { }
INTEGER = { ANYTHING }
derefaddrtmp.7 = { ESCAPED }
derefaddrtmp.8 = { NONLOCAL }
p = same as derefaddrtmp.8

...

Updating SSA information for statement a_2 = *p_1(D);

Updating SSA information for statement D.1236_4 = *p_1(D);

...

VUSE operands                             2          8b
VDEF operands                             0          0b

...

  # VUSE <SMT.9D.1248_6(D)> { SMT.9D.1248 }
  aD.1233_2 = *pD.1230_1(D);
  *0B ={v} 5;
  # VUSE <SMT.9D.1248_6(D)> { SMT.9D.1248 }
  D.1236_4 = *pD.1230_1(D);
  D.1235_5 = D.1236_4 == aD.1233_2;
  return D.1235_5;

Note there is no vdef, so FRE comes and removes the second load.


-- 

bonzini at gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2009-01-27 11:02:25
               date|                            |


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

[Bug tree-optimization/38984] [4.2/4.3/4.4 Regression] NULL pointers always considered distinct by PTA, even with -fno-delete-null-pointer-checks

[rguenth at gcc dot gnu dot org]

------- Comment #2 from rguenth at gcc dot gnu dot org  2009-01-27 11:06 -------
Can you fill in a known-to-work version?

ANYTHING = &ANYTHING
ESCAPED = *ESCAPED
NONLOCAL = &ESCAPED
INTEGER = &ANYTHING
derefaddrtmp.8 = &NONLOCAL
*ESCAPED = derefaddrtmp.8
p = &NONLOCAL

misses a constraint for null.  It should say

null = &ANYTHING


-- 


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

[Bug tree-optimization/38984] [4.2/4.3/4.4 Regression] NULL pointers always considered distinct by PTA, even with -fno-delete-null-pointer-checks

[rguenth at gcc dot gnu dot org]

------- Comment #3 from rguenth at gcc dot gnu dot org  2009-01-27 11:06 -------
hm, or rather, we CCP into null ... which makes this an operand scanner issue.


-- 


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

[Bug tree-optimization/38984] [4.2/4.3/4.4 Regression] NULL pointers always considered distinct by PTA, even with -fno-delete-null-pointer-checks

[pinskia at gmail dot com]

------- Comment #4 from pinskia at gmail dot com  2009-01-27 11:08 -------
Subject: Re:  [4.2/4.3/4.4 Regression] NULL pointers always considered distinct
by PTA, even with -fno-delete-null-pointer-checks



Sent from my iPhone

On Jan 27, 2009, at 3:02 AM, "bonzini at gnu dot org" <gcc-bugzilla@gcc.gnu.org 
 > wrote:

>
>
> ------- Comment #1 from bonzini at gnu dot org  2009-01-27 11:02  
> -------
> This simple patch is not enough:
>
> Index: tree-ssa-structalias.c
> ===================================================================
> --- tree-ssa-structalias.c      (revision 142960)
> +++ tree-ssa-structalias.c      (working copy)
> @@ -3030,8 +3030,14 @@ get_constraint_for_1 (tree t, VEC (ce_s,
>      happens below, since it will fall into the default case. The only
>      case we know something about an integer treated like a pointer is
>      when it is the NULL pointer, and then we just say it points to
> -     NULL.  */
> -  if (TREE_CODE (t) == INTEGER_CST
> +     NULL.
> +
> +     Do not do that if -fno-delete-null-pointer-checks though,  
> because
> +     in that case *NULL does not fail, so it _should_ alias  
> *anything.
> +     It is not worth adding a new option or renaming the existing  
> one,
> +     since this case is relatively obscure.  */
> +  if (flag_delete_null_pointer_checks
> +      && TREE_CODE (t) == INTEGER_CST
>       && integer_zerop (t))
>     {
>       temp.var = nothing_id;
>
> We get:
>
> ANYTHING = &ANYTHING
> ESCAPED = *ESCAPED
> NONLOCAL = &ESCAPED
> INTEGER = &ANYTHING
> derefaddrtmp.8 = &NONLOCAL
> *ESCAPED = derefaddrtmp.8
> p = &NONLOCAL
>
> ...
>
> NULL = { }
> ANYTHING = { ANYTHING }
> READONLY = { READONLY }
> ESCAPED = { }
> NONLOCAL = { ESCAPED }
> CALLUSED = { }
> INTEGER = { ANYTHING }
> derefaddrtmp.7 = { ESCAPED }
> derefaddrtmp.8 = { NONLOCAL }
> p = same as derefaddrtmp.8
>
> ...
>
> Updating SSA information for statement a_2 = *p_1(D);
>
> Updating SSA information for statement D.1236_4 = *p_1(D);
>
> ...
>
> VUSE operands                             2          8b
> VDEF operands                             0          0b
>
> ...
>
>  # VUSE <SMT.9D.1248_6(D)> { SMT.9D.1248 }
>  aD.1233_2 = *pD.1230_1(D);
>  *0B ={v} 5;
>  # VUSE <SMT.9D.1248_6(D)> { SMT.9D.1248 }
>  D.1236_4 = *pD.1230_1(D);
>  D.1235_5 = D.1236_4 == aD.1233_2;
>  return D.1235_5;
>
> Note there is no vdef, so FRE comes and removes the second load.

If that patch is not enough and the above is happening we are going to  
have issues wit volatiles also.


>
>
>
> -- 
>
> bonzini at gnu dot org changed:
>
>           What    |Removed                     |Added
> --- 
> --- 
> ----------------------------------------------------------------------
>             Status|UNCONFIRMED                 |NEW
>     Ever Confirmed|0                           |1
>   Last reconfirmed|0000-00-00 00:00:00         |2009-01-27 11:02:25
>               date|                            |
>
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38984
>


-- 


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

Re: [Bug tree-optimization/38984] [4.2/4.3/4.4 Regression] NULL pointers always considered distinct by PTA, even with -fno-delete-null-pointer-checks

[Andrew Thomas Pinski]

Sent from my iPhone

On Jan 27, 2009, at 3:02 AM, "bonzini at gnu dot org" <gcc-bugzilla@gcc.gnu.org 
 > wrote:

>
>
> ------- Comment #1 from bonzini at gnu dot org  2009-01-27 11:02  
> -------
> This simple patch is not enough:
>
> Index: tree-ssa-structalias.c
> ===================================================================
> --- tree-ssa-structalias.c      (revision 142960)
> +++ tree-ssa-structalias.c      (working copy)
> @@ -3030,8 +3030,14 @@ get_constraint_for_1 (tree t, VEC (ce_s,
>      happens below, since it will fall into the default case. The only
>      case we know something about an integer treated like a pointer is
>      when it is the NULL pointer, and then we just say it points to
> -     NULL.  */
> -  if (TREE_CODE (t) == INTEGER_CST
> +     NULL.
> +
> +     Do not do that if -fno-delete-null-pointer-checks though,  
> because
> +     in that case *NULL does not fail, so it _should_ alias  
> *anything.
> +     It is not worth adding a new option or renaming the existing  
> one,
> +     since this case is relatively obscure.  */
> +  if (flag_delete_null_pointer_checks
> +      && TREE_CODE (t) == INTEGER_CST
>       && integer_zerop (t))
>     {
>       temp.var = nothing_id;
>
> We get:
>
> ANYTHING = &ANYTHING
> ESCAPED = *ESCAPED
> NONLOCAL = &ESCAPED
> INTEGER = &ANYTHING
> derefaddrtmp.8 = &NONLOCAL
> *ESCAPED = derefaddrtmp.8
> p = &NONLOCAL
>
> ...
>
> NULL = { }
> ANYTHING = { ANYTHING }
> READONLY = { READONLY }
> ESCAPED = { }
> NONLOCAL = { ESCAPED }
> CALLUSED = { }
> INTEGER = { ANYTHING }
> derefaddrtmp.7 = { ESCAPED }
> derefaddrtmp.8 = { NONLOCAL }
> p = same as derefaddrtmp.8
>
> ...
>
> Updating SSA information for statement a_2 = *p_1(D);
>
> Updating SSA information for statement D.1236_4 = *p_1(D);
>
> ...
>
> VUSE operands                             2          8b
> VDEF operands                             0          0b
>
> ...
>
>  # VUSE <SMT.9D.1248_6(D)> { SMT.9D.1248 }
>  aD.1233_2 = *pD.1230_1(D);
>  *0B ={v} 5;
>  # VUSE <SMT.9D.1248_6(D)> { SMT.9D.1248 }
>  D.1236_4 = *pD.1230_1(D);
>  D.1235_5 = D.1236_4 == aD.1233_2;
>  return D.1235_5;
>
> Note there is no vdef, so FRE comes and removes the second load.

If that patch is not enough and the above is happening we are going to  
have issues wit volatiles also.


>
>
>
> -- 
>
> bonzini at gnu dot org changed:
>
>           What    |Removed                     |Added
> --- 
> --- 
> ----------------------------------------------------------------------
>             Status|UNCONFIRMED                 |NEW
>     Ever Confirmed|0                           |1
>   Last reconfirmed|0000-00-00 00:00:00         |2009-01-27 11:02:25
>               date|                            |
>
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38984
>

[Bug tree-optimization/38984] [4.2/4.3/4.4 Regression] NULL pointers always considered distinct by PTA, even with -fno-delete-null-pointer-checks

[bonzini at gnu dot org]

------- Comment #5 from bonzini at gnu dot org  2009-01-27 11:09 -------
I don't think CCPing of the "null" variable is a problem.  Writing it as "*(int
*)0 = 5" would not make the testcase invalid.


-- 


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

[Bug tree-optimization/38984] [4.2/4.3/4.4 Regression] NULL pointers always considered distinct by PTA, even with -fno-delete-null-pointer-checks

[bonzini at gnu dot org]

------- Comment #6 from bonzini at gnu dot org  2009-01-27 11:15 -------
The issue with the operand scanner is now PR38985


-- 


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

[Bug tree-optimization/38984] [4.2/4.3/4.4 Regression] NULL pointers always considered distinct by PTA, even with -fno-delete-null-pointer-checks

[rguenth at gcc dot gnu dot org]

------- Comment #7 from rguenth at gcc dot gnu dot org  2009-01-27 12:13 -------
Note that for *(int *)0 = 5 the gimplifier inserts a temporary.


-- 


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

[Bug tree-optimization/38984] [4.2/4.3/4.4 Regression] NULL pointers always considered distinct by PTA, even with -fno-delete-null-pointer-checks

[bonzini at gnu dot org]

------- Comment #8 from bonzini at gnu dot org  2009-01-27 12:27 -------
>From PR38985:

> These passes are able to propagate the address to the load: CCP, VRP, DOM.  If 
> we decide that it's an invalid transformation, fixing them would fix this bug.

The interesting part is that after you disable them... you find out PTA is not
actually able to use the NULL special variable because *NULL and *ANYTHING
conflict!  The pointed-to sets are okay though:

  pD.1230_1(D), is dereferenced, its value escapes, points-to anything
  nullD.1234_3, is dereferenced, points-to anything, points-to NULL

The patch in comment #1 however works (tested with -fno-tree-ccp -fno-tree-vrp
-fno-tree-dominator-opts, and looking at the constraints for null).  So my plan
would be:

1) apply the patch after regtesting, XFAILing the testcase;

2) when PR38985 is fixed, the testcase will pass and the XFAIL will be removed.

Sounds okay?


-- 


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

[Bug tree-optimization/38984] [4.2/4.3/4.4 Regression] NULL pointers always considered distinct by PTA, even with -fno-delete-null-pointer-checks

[rguenther at suse dot de]

------- Comment #9 from rguenther at suse dot de  2009-01-27 12:29 -------
Subject: Re:  [4.2/4.3/4.4 Regression] NULL
 pointers always considered distinct by PTA, even with
 -fno-delete-null-pointer-checks

On Tue, 27 Jan 2009, bonzini at gnu dot org wrote:

> ------- Comment #8 from bonzini at gnu dot org  2009-01-27 12:27 -------
> From PR38985:
> 
> > These passes are able to propagate the address to the load: CCP, VRP, DOM.  If 
> > we decide that it's an invalid transformation, fixing them would fix this bug.
> 
> The interesting part is that after you disable them... you find out PTA is not
> actually able to use the NULL special variable because *NULL and *ANYTHING
> conflict!  The pointed-to sets are okay though:
> 
>   pD.1230_1(D), is dereferenced, its value escapes, points-to anything
>   nullD.1234_3, is dereferenced, points-to anything, points-to NULL
> 
> The patch in comment #1 however works (tested with -fno-tree-ccp -fno-tree-vrp
> -fno-tree-dominator-opts, and looking at the constraints for null).  So my plan
> would be:
> 
> 1) apply the patch after regtesting, XFAILing the testcase;
> 
> 2) when PR38985 is fixed, the testcase will pass and the XFAIL will be removed.
> 
> Sounds okay?

Sounds good.

Richard.


-- 


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

[Bug tree-optimization/38984] [4.2/4.3/4.4 Regression] NULL pointers always considered distinct by PTA, even with -fno-delete-null-pointer-checks

[rguenth at gcc dot gnu dot org]

-- 

rguenth at gcc dot gnu dot org changed:

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


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

[Bug tree-optimization/38984] [4.2/4.3/4.4 Regression] NULL pointers always considered distinct by PTA, even with -fno-delete-null-pointer-checks

[bonzini at gcc dot gnu dot org]

------- Comment #10 from bonzini at gnu dot org  2009-01-28 08:02 -------
Subject: Bug 38984

Author: bonzini
Date: Wed Jan 28 08:02:31 2009
New Revision: 143721

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=143721
Log:
gcc:
2009-01-28  Paolo Bonzini  <bonzini@gnu.org>

        PR tree-optimization/38984
        * tree-ssa-structalias.c (get_constraints_for_1): Do not use
        the nothing_id variable if -fno-delete-null-pointer-checks.

gcc/testsuite:
2009-01-28  Paolo Bonzini  <bonzini@gnu.org>

        PR tree-optimization/38984
        * gcc.dg/pr38984.c: New XFAILed testcase.


Added:
    trunk/gcc/testsuite/gcc.dg/pr38984.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-structalias.c


-- 


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

[Bug tree-optimization/38984] [4.2/4.3/4.4 Regression] NULL pointers always considered distinct by PTA, even with -fno-delete-null-pointer-checks

[bonzini at gnu dot org]

------- Comment #11 from bonzini at gnu dot org  2009-01-28 08:04 -------
"fixed" though 38985 still holds.


-- 

bonzini at gnu dot org changed:

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


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

[Bug tree-optimization/38984] [4.2/4.3 Regression] NULL pointers always considered distinct by PTA, even with -fno-delete-null-pointer-checks

[bonzini at gnu dot org]

------- Comment #13 from bonzini at gnu dot org  2009-01-28 11:29 -------
WONTFIX on 4.2/4.3 since anyway the real fix depends on 38985.


-- 

bonzini at gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|                            |WONTFIX


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

[Bug tree-optimization/38984] [4.2/4.3 Regression] NULL pointers always considered distinct by PTA, even with -fno-delete-null-pointer-checks

[bonzini at gnu dot org]

------- Comment #12 from bonzini at gnu dot org  2009-01-28 11:29 -------
To be precise:


-- 

bonzini at gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
      Known to fail|4.4.0 4.1.3                 |4.1.3
      Known to work|3.4.6                       |3.4.6 4.4.0
         Resolution|FIXED                       |
            Summary|[4.2/4.3/4.4 Regression]    |[4.2/4.3 Regression] NULL
                   |NULL pointers always        |pointers always considered
                   |considered distinct by PTA, |distinct by PTA, even with -
                   |even with -fno-delete-null- |fno-delete-null-pointer-
                   |pointer-checks              |checks


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

[Bug tree-optimization/38984] [4.2/4.3 Regression] NULL pointers always considered distinct by PTA, even with -fno-delete-null-pointer-checks

[rguenth at gcc dot gnu dot org]

------- Comment #14 from rguenth at gcc dot gnu dot org  2009-02-01 21:34 -------
Hm, on the alias-improvements branch I now XPASS the not return 1 check, but -
why do you think we should have two dereferences to *p?

Hm, because:

  # VUSE <.MEM_6(D)>
  a = *p;
  # .MEM_7 = VDEF <.MEM_6(D)>
  *0B = 5;
  return *p == a;

in optimized.

  # VUSE <.MEM_6(D)>
  a_2 = *p_1(D);
  # .MEM_7 = VDEF <.MEM_6(D)>
  *0B = 5;
  # VUSE <.MEM_7>
  D.1236_4 = *p_1(D);


-- 


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

[Bug tree-optimization/38984] [4.2/4.3 Regression] NULL pointers always considered distinct by PTA, even with -fno-delete-null-pointer-checks

[rguenth at gcc dot gnu dot org]

------- Comment #15 from rguenth at gcc dot gnu dot org  2009-02-01 22:02 -------
Err, that last comment probably didn't make much sense.  I wanted to say I see

  # VUSE <.MEM_6(D)>
  a = *p;
  # .MEM_7 = VDEF <.MEM_6(D)>
  *0B = 5;
  return *p == a;

so the pattern for two = *p does not match.


-- 


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

[Bug tree-optimization/38984] [4.2/4.3 Regression] NULL pointers always considered distinct by PTA, even with -fno-delete-null-pointer-checks

[bonzini at gnu dot org]

------- Comment #16 from bonzini at gnu dot org  2009-02-02 08:26 -------
Subject: Re:  [4.2/4.3 Regression] NULL pointers
 always considered distinct by PTA, even with -fno-delete-null-pointer-checks

rguenth at gcc dot gnu dot org wrote:
> ------- Comment #15 from rguenth at gcc dot gnu dot org  2009-02-01 22:02 -------
> Err, that last comment probably didn't make much sense.  I wanted to say I see
> 
>   # VUSE <.MEM_6(D)>
>   a = *p;
>   # .MEM_7 = VDEF <.MEM_6(D)>
>   *0B = 5;
>   return *p == a;
> 
> so the pattern for two = *p does not match.

I'll check out alias-improvements-branch and fix it.  I'll also add a
testcase for PR38985 there.

Paolo


-- 


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