Bug 26763 - [4.1/4.2 Regression] wrong final value of induction variable calculated
Summary: [4.1/4.2 Regression] wrong final value of induction variable calculated
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.1.0
: P3 normal
Target Milestone: 4.1.1
Assignee: Zdenek Dvorak
URL:
Keywords: wrong-code
: 27176 27180 (view as bug list)
Depends on:
Blocks:
 
Reported: 2006-03-19 23:03 UTC by Debian GCC Maintainers
Modified: 2014-02-16 13:12 UTC (History)
6 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2006-03-27 15:43:12


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Debian GCC Maintainers 2006-03-19 23:03:02 UTC
[ 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
Comment 1 Richard Biener 2006-03-20 10:15:36 UTC
Confirmed.  -fno-tree-loop-optimize makes it work.
Comment 2 Richard Biener 2006-03-25 22:14:33 UTC
Zdenek, can you have a look?
Comment 3 Zdenek Dvorak 2006-03-27 22:33:10 UTC
(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.
Comment 4 Zdenek Dvorak 2006-03-28 12:08:25 UTC
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;
}
Comment 5 Richard Biener 2006-03-28 13:33:04 UTC
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 ;)
Comment 6 Richard Biener 2006-03-31 21:12:07 UTC
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.
Comment 7 patchapp@dberlin.org 2006-04-03 16:45:12 UTC
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
Comment 8 Zdenek Dvorak 2006-04-03 16:52:58 UTC
(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?
Comment 9 rguenther@suse.de 2006-04-03 16:59:25 UTC
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.
Comment 10 Zdenek Dvorak 2006-04-03 17:22:34 UTC
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.
Comment 11 patchapp@dberlin.org 2006-04-04 08:15:24 UTC
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
Comment 12 Richard Biener 2006-04-05 08:16:42 UTC
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

Comment 13 Richard Biener 2006-04-05 08:20:23 UTC
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

Comment 14 Richard Biener 2006-04-05 08:22:04 UTC
Fixed.
Comment 15 Andrew Pinski 2006-04-17 02:27:34 UTC
*** Bug 27180 has been marked as a duplicate of this bug. ***
Comment 16 Richard Biener 2006-04-17 10:10:32 UTC
*** Bug 27176 has been marked as a duplicate of this bug. ***
Comment 17 Jackie Rosen 2014-02-16 13:12:17 UTC Comment hidden (spam)