GCC Bugzilla Bug 26763

[ forwarded from http://bugs.debian.org/356896 ]

void abort(void);

__attribute__((noinline))
int *foo(int *start) {
    int *tmp;
    for (tmp = start + 100; tmp > start; --tmp) ;
    return tmp;
}

int main() {
  int x[100];

  if (foo(x) != x)
      abort();

  return 0;
}

aborts with 4.1.0 20051124 at -O1 or higher
does not abort at -O0, or with 4.0.3 or 4.2.0 20060304

Confirmed.  -fno-tree-loop-optimize makes it work.

Zdenek, can you have a look?

(gdb) call debug_generic_stmt (ret)
startD.1278_2 + -3B > startD.1278_2 + 396B;

(gdb) call debug_generic_stmt (fold (ret))
1

I guess the reasoning of fold is: it is pointer arithmetics, so it
does not wrap.  (-3B) = (0xfff...7) > 396B, so the result is always true.

4.0 does not have final value replacement, and 4.2 has different # of
iterations analysis; but most likely some similar problem is latent in both
versions.

With this testcase, problem reproduces both in 4.1 and in mainline:

int try (int *a)
{
  return a + -1 > a;
}

int main(void)
{
  int bla[100];

  if (try (bla + 50))
    abort ();

  return 0;
}

We had this some time ago, but the discussion stopped at the point where people
said that pointers do not wrap.  I believe that a + -1 is done using unsigned
arithmetic, so this may be the bug.  I also know where the transformation is
done
as I invented it ;)

I believe c-common.c:pointer_int_sum is wrong in relying on pointer overflow
during conversion of the integer offset to an unsigned pointer.  I'm sending
a patch that fixes this for comments.

Subject: Bug number PR26763

A patch for this bug has been added to the patch tracker.
The mailing list url for the patch is
http://gcc.gnu.org/ml/gcc-patches/2006-04/msg00082.html

(In reply to comment #6)
> I believe c-common.c:pointer_int_sum is wrong in relying on pointer overflow
> during conversion of the integer offset to an unsigned pointer.  I'm sending
> a patch that fixes this for comments.

The patch seems a bit too conservative to me; perhaps just always comparing the
offsets as signed could work?

Subject: Re:  [4.1 Regression] wrong final value
 of induction variable calculated

On Mon, 3 Apr 2006, rakdver at gcc dot gnu dot org wrote:

> (In reply to comment #6)
> > I believe c-common.c:pointer_int_sum is wrong in relying on pointer overflow
> > during conversion of the integer offset to an unsigned pointer.  I'm sending
> > a patch that fixes this for comments.
> 
> The patch seems a bit too conservative to me; perhaps just always comparing the
> offsets as signed could work?

I'm not a language lawyer here - and as this is the second (or third) 
patch to this folding to correct problems I'd rather be safe than sorry 
this time.  I'm sure jsm can construct a testcase where comparing offsets
as signed leads to wrong code.  Maybe

char *memory = 0;

int foo(void)
{
  return memory + 0x80000000 < memory;
}

int main()
{
  if (foo())
    abort ();
}

i.e. have a mapping >2Gb on a 32bit machine.  A corner case, but valid I 
guess.

Subject: Re:  [4.1 Regression] wrong final value of induction variable
calculated

> > (In reply to comment #6)
> > > I believe c-common.c:pointer_int_sum is wrong in relying on pointer overflow
> > > during conversion of the integer offset to an unsigned pointer.  I'm sending
> > > a patch that fixes this for comments.
> > 
> > The patch seems a bit too conservative to me; perhaps just always comparing the
> > offsets as signed could work?
> 
> I'm not a language lawyer here - and as this is the second (or third) 
> patch to this folding to correct problems I'd rather be safe than sorry 
> this time.  I'm sure jsm can construct a testcase where comparing offsets
> as signed leads to wrong code.  Maybe
> 
> char *memory = 0;
> 
> int foo(void)
> {
>   return memory + 0x80000000 < memory;
> }
> 
> int main()
> {
>   if (foo())
>     abort ();
> }
> 
> i.e. have a mapping >2Gb on a 32bit machine.  A corner case, but valid I 
> guess.

no -- the result in this example is undefined.  The comparisons are only
defined for pointers in the same object.  I guess nothing really
prevents having an object whose size is more than half of the address
space, though.

Subject: Bug number PR26763

A patch for this bug has been added to the patch tracker.
The mailing list url for the patch is
http://gcc.gnu.org/ml/gcc-patches/2006-04/msg00126.html

Subject: Bug 26763

Author: rguenth
Date: Wed Apr  5 08:16:38 2006
New Revision: 112697

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=112697
Log:
2006-04-05  Richard Guenther  <rguenther@suse.de>

        PR tree-optimization/26763
        * fold-const.c (fold_comparison): Move folding of
        PTR + CST CMP PTR + CST ...
        (fold_binary): ... here.  Fold only for EQ_EXPR and NE_EXPR.

        * gcc.dg/torture/pr26763-1.c: New testcase.
        * gcc.dg/torture/pr26763-2.c: Likewise.

Added:
    trunk/gcc/testsuite/gcc.dg/torture/pr26763-1.c
    trunk/gcc/testsuite/gcc.dg/torture/pr26763-2.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/fold-const.c
    trunk/gcc/testsuite/ChangeLog

Subject: Bug 26763

Author: rguenth
Date: Wed Apr  5 08:20:21 2006
New Revision: 112698

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=112698
Log:
2006-04-05  Richard Guenther  <rguenther@suse.de>

        PR tree-optimization/26763
        * fold-const.c (fold_binary): Fold PTR + CST CMP PTR + CST
        only for EQ_EXPR and NE_EXPR.

        * gcc.dg/torture/pr26763-1.c: New testcase.
        * gcc.dg/torture/pr26763-2.c: Likewise.

Added:
    branches/gcc-4_1-branch/gcc/testsuite/gcc.dg/torture/pr26763-1.c
    branches/gcc-4_1-branch/gcc/testsuite/gcc.dg/torture/pr26763-2.c
Modified:
    branches/gcc-4_1-branch/gcc/ChangeLog
    branches/gcc-4_1-branch/gcc/fold-const.c
    branches/gcc-4_1-branch/gcc/testsuite/ChangeLog

Fixed.

*** Bug 27180 has been marked as a duplicate of this bug. ***

*** Bug 27176 has been marked as a duplicate of this bug. ***