Bug 39100 - [4.4 Regression] -fstrict-aliasing miscompilation
Summary: [4.4 Regression] -fstrict-aliasing miscompilation
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.4.0
: P1 normal
Target Milestone: 4.4.0
Assignee: Richard Biener
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2009-02-04 18:59 UTC by Jakub Jelinek
Modified: 2009-02-05 12:00 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.3.4 4.4.0
Known to fail:
Last reconfirmed: 2009-02-04 20:04:52


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jakub Jelinek 2009-02-04 18:59:54 UTC
typedef struct E
{
  int p;
  struct E *n;
} *EP;   

typedef struct C
{
  EP x;
  short cn, cp; 
} *CP;

__attribute__((noinline)) CP
foo (CP h, EP x)
{
  EP pl = 0, *pa = &pl;
  EP nl = 0, *na = &nl;
  EP n;

  while (x)
    {
      n = x->n;   
      if ((x->p & 1) == 1) 
        {
          h->cp++;
          *pa = x;
          pa = &((*pa)->n);
        }
      else
        {
          h->cn++;
          *na = x;
          na = &((*na)->n);
        }    
      x = n;
    }
  *pa = nl;
  *na = 0;
  h->x = pl;
  return h;
}

int
main (void)
{  
  struct C c = { 0, 0, 0 };
  struct E e[2] = { { 0, &e[1] }, { 1, 0 } };
  EP p;

  foo (&c, &e[0]);
  if (c.cn != 1 || c.cp != 1)
    __builtin_abort ();
  if (c.x != &e[1])
    __builtin_abort ();
  if (e[1].n != &e[0])
    __builtin_abort ();
  if (e[0].n)
    __builtin_abort ();
  return 0;  
}

is miscompiled on x86_64-linux with -O2, works with -O2 -fno-strict-aliasing
and with 4.3.2.  When miscompiled, the *na = 0 store is optimized away.
Comment 1 Jakub Jelinek 2009-02-04 20:04:08 UTC
Caused by http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=136657
Comment 2 Richard Biener 2009-02-04 20:04:52 UTC
I will have a looksee.
Comment 3 Richard Biener 2009-02-04 20:05:48 UTC
Note that the patch only enables us to use PTA results which are likely wrong here.
Comment 4 H.J. Lu 2009-02-04 20:13:41 UTC
It is caused by revision 136657:

http://gcc.gnu.org/ml/gcc-cvs/2008-06/msg00414.html
Comment 5 Richard Biener 2009-02-04 20:59:03 UTC
On the alias-improvements branch the testcase also fails with
-O -fdelete-null-pointer-checks, so this is definitely a PTA bug.
PTA thinks the points-to set for na_5 is

na_5, is dereferenced, points-to NULL, points-to vars: { nl }

which would definitely enable deleting the store, as nl is a local variable.

The constraints that should make na_5 point to NONLOCAL as well are

h = &NONLOCAL
x = &NONLOCAL
x_1 = x
*na_5 = x_1
D.1262_20 = *na_5
na_21 = D.1262_20 + 32
na_4 = na_21
na_5 = na_4

relevant unifications are

Unifying x to h
Unifying na_4 to na_5


The PTA bug can be seen with the following simplified testcase:

typedef struct E
{
  struct E *n;
} *EP;

void __attribute__((noinline))
foo (EP x)
{
  EP nl = 0, *na = &nl;
  EP n;

  while (x)
    {
      n = x->n;
      *na = x;
      na = &((*na)->n);
      x = n;
    }
  *na = 0;
}
Comment 6 Daniel Berlin 2009-02-04 21:16:56 UTC
Subject: Re:  [4.4 Regression] -fstrict-aliasing 
	miscompilation

On Wed, Feb 4, 2009 at 3:59 PM, rguenth at gcc dot gnu dot org
<gcc-bugzilla@gcc.gnu.org> wrote:
>
>
> ------- Comment #5 from rguenth at gcc dot gnu dot org  2009-02-04 20:59 -------
> On the alias-improvements branch the testcase also fails with
> -O -fdelete-null-pointer-checks, so this is definitely a PTA bug.
> PTA thinks the points-to set for na_5 is
>
> na_5, is dereferenced, points-to NULL, points-to vars: { nl }
>
> which would definitely enable deleting the store, as nl is a local variable.
>
> The constraints that should make na_5 point to NONLOCAL as well are
>
> h = &NONLOCAL
> x = &NONLOCAL
> x_1 = x
> *na_5 = x_1
> D.1262_20 = *na_5
> na_21 = D.1262_20 + 32
> na_4 = na_21
> na_5 = na_4
>
> relevant unifications are
>
> Unifying x to h
> Unifying na_4 to na_5
>
Given those constraints, there is no way it will or should point to NONLOCAL.
na_5 doesn't actually point to anything given these constraints (IE
there is no na_5 = <something>), unless D.1262_20 + 32 points to
something, which it doesn't give the above constraints.

Basically
*a = b
c = *a

Does not imply that c = b if a is empty, which is what is happening
here, AFAICT.
Comment 7 Richard Biener 2009-02-04 21:18:21 UTC
Following do_ds_constraint shows

*na_2 = x_1
nl  <-
*na_2 = derefaddrtmp.7
nl  <- NULL
*na_2 = x_1
NULL  <- ESCAPED NONLOCAL
*na_2 = derefaddrtmp.7
NULL  <- NULL

where on the lhs of <- we show DELTA and on the rhs we show the rhs solution.
The problem is obviously that DELTA doesn't include nl anymore when the
rhs solution includes NONLOCAL.

We do have a cycle here, can we really do DELTA processing there?  Note
that the constraint graph looks funny to me (but I never looked at them
until now)
Comment 8 Richard Biener 2009-02-04 21:36:56 UTC
The following works (on the branch, with added NONLOCAL = *NONLOCAL constraint):

ANYTHING = &ANYTHING
READONLY = &READONLY
ESCAPED = *ESCAPED
ESCAPED = ESCAPED + UNKNOWN
*ESCAPED = NONLOCAL
NONLOCAL = &NONLOCAL
NONLOCAL = &ESCAPED
NONLOCAL = *NONLOCAL
CALLUSED = *CALLUSED
CALLUSED = CALLUSED + UNKNOWN
INTEGER = &ANYTHING
x = &NONLOCAL
nl = &NULL
n_5 = *x_1
*na_2 = x_1
ESCAPED = x_1
D.1242_6 = *na_2
na_7 = D.1242_6
x_1 = x
x_1 = n_5
na_2 = &nl
na_2 = na_7
derefaddrtmp.7 = &NULL
*na_2 = derefaddrtmp.7
ESCAPED = &NULL

...

NULL = { }
ANYTHING = { ANYTHING }
READONLY = { READONLY }
ESCAPED = same as n_5
NONLOCAL = same as n_5
CALLUSED = { }
INTEGER = { ANYTHING }
x = { NONLOCAL }
nl = { NULL ESCAPED NONLOCAL }
n_5 = { NULL ESCAPED NONLOCAL }
x_1 = same as n_5
na_2 = { NULL ESCAPED NONLOCAL nl }
D.1242_6 = { NULL ESCAPED NONLOCAL }
na_7 = same as D.1242_6
derefaddrtmp.7 = { NULL }
Comment 9 Daniel Berlin 2009-02-05 07:10:05 UTC
Subject: Bug 39100

Author: dberlin
Date: Thu Feb  5 07:09:44 2009
New Revision: 143951

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=143951
Log:
2009-02-05  Daniel Berlin  <dberlin@dberlin.org>
	    Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/39100
	* tree-ssa-structalias.c (do_ds_constraint): Actually do what the
	comment says and add edges.


Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/pr39100.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-structalias.c

Comment 10 Richard Biener 2009-02-05 09:42:04 UTC
Fixed.  I'm going to do a backport to the 4.3 branch.
Comment 11 Richard Biener 2009-02-05 11:10:16 UTC
Subject: Bug 39100

Author: rguenth
Date: Thu Feb  5 11:10:02 2009
New Revision: 143961

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=143961
Log:
2009-02-05  Daniel Berlin  <dberlin@dberlin.org>
            Richard Guenther  <rguenther@suse.de>

        PR tree-optimization/39100
        * tree-ssa-structalias.c (do_ds_constraint): Actually do what the
        comment says and add edges.

Added:
    branches/gcc-4_3-branch/gcc/testsuite/gcc.c-torture/execute/pr39100.c
      - copied unchanged from r143951, trunk/gcc/testsuite/gcc.c-torture/execute/pr39100.c
Modified:
    branches/gcc-4_3-branch/gcc/ChangeLog
    branches/gcc-4_3-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_3-branch/gcc/tree-ssa-structalias.c