Bug 28651

Summary: [4.0 Regression] signed compare incorrectly false for (int)(U+4)<(int)U where U is unsigned INT_MAX (for optimized x86)
Product: gcc Reporter: per.mildner
Component: middle-endAssignee: Not yet assigned to anyone <unassigned>
Status: RESOLVED FIXED    
Severity: normal CC: debian-gcc, fang, gcc-bugs, janis, matz, rguenth
Priority: P3 Keywords: wrong-code
Version: 4.1.2   
Target Milestone: 4.0.4   
Host: Target:
Build: Known to work: 4.2.0 4.1.2 4.0.4
Known to fail: 2.95.4 3.3.6 3.4.6 4.0.3 4.1.1 Last reconfirmed: 2006-08-09 07:31:44

Description per.mildner 2006-08-08 10:57:35 UTC
This happens with 4.1.1 release too but not with gcc-4.2-20060805. Also happens for gcc 4.1.1 on i386 Solaris 10.

bash-3.00$ uname -a
Linux bonk.sics.se 2.6.16-1.2111_FC4smp #1 SMP Sat May 20 20:16:24 EDT 2006 i686 unknown unknown GNU/Linux
bash-3.00$ cat bug.c
#include <stdlib.h>
#include <stdio.h>

int main(int argc, char *argv[])
{
  unsigned u;

  /* u = INT_MAX but gcc cannot know that */
  if (argc != 2 || sscanf(argv[1], "%x", &u) != 1) exit(2);

  if (((int)(u + 4)) < (int)u)
    {
      exit(0);
    }
  exit(1);
}
bash-3.00$ /src/sicstus/install/gcc-4.1-20060804/x86-linux-glibc2.3/bin/gcc -v
Using built-in specs.
Target: i686-pc-linux-gnu
Configured with: /src/sicstus/install/src/gcc-4.1-20060804/configure --prefix=/src/sicstus/install/gcc-4.1-20060804/x86-linux-glibc2.3 --enable-languages=c,c++
Thread model: posix
gcc version 4.1.2 20060804 (prerelease)
## Uses sscanf to obtain INT_MAX to keep gcc in the dark
## without optimization the comparison is true which is correct
bash-3.00$ /src/sicstus/install/gcc-4.1-20060804/x86-linux-glibc2.3/bin/gcc /home/perm/sicstus/sicstus4slon/bug.c && ./a.out 0x7fffffff && echo "success" || echo "exited with $?"
success
## with optimization the comparison is false which is correct
bash-3.00$ /src/sicstus/install/gcc-4.1-20060804/x86-linux-glibc2.3/bin/gcc -O1 /home/perm/sicstus/sicstus4slon/bug.c && ./a.out 0x7fffffff && echo "success" || echo "exited with $?"
exited with 1
bash-3.00$
Comment 1 Richard Biener 2006-08-08 16:06:27 UTC
Confirmed.  After combine it's broked.
Comment 2 Richard Biener 2006-08-08 16:13:54 UTC
ppc also broken.  But not a regression, so technically FIXED.  Still this looks like something we want to fix - but maybe someone can convince me it is undefined ;)

Shorter testcase ("folds" to return 0):

int foo(unsigned u)
{
  return (int)(u + 4) < (int)u;
}
Comment 3 Richard Biener 2006-08-08 16:26:20 UTC
Janis, can you look what fixed this on the mainline?
Comment 4 Richard Biener 2006-08-08 17:17:35 UTC
Breakpoint 4, simplify_const_relational_operation (code=LT, mode=SImode, 
    op0=0xb7cc9f60, op1=0xb7d3ef60)
    at /space/rguenther/src/svn/gcc-4_1-branch/gcc/simplify-rtx.c:3040
3040      gcc_assert (mode != VOIDmode
(gdb) call debug_rtx (op0)
(plus:SI (reg/v:SI 59 [ u ])
    (const_int 4 [0x4]))
(gdb) call debug_rtx (op1)
(reg/v:SI 59 [ u ])

via

Breakpoint 4, simplify_const_relational_operation (code=LT, mode=SImode, 
    op0=0xb7cc6230, op1=0xb7cc6210)
3040      gcc_assert (mode != VOIDmode
(gdb) call debug_rtx (op0)
(const_int 4 [0x4])
(gdb) call debug_rtx (op1)
(const_int 0 [0x0])

returns 0.  So, here's the bug:

  /* For integer comparisons of A and B maybe we can simplify A - B and can
     then simplify a comparison of that with zero.  If A and B are both either
     a register or a CONST_INT, this can't help; testing for these cases will
     prevent infinite recursion here and speed things up.
    
     If CODE is an unsigned comparison, then we can never do this optimization,
     because it gives an incorrect result if the subtraction wraps around zero.
     ANSI C defines unsigned operations such that they never overflow, and
     thus such cases can not be ignored; but we cannot do it even for
     signed comparisons for languages such as Java, so test flag_wrapv.  */

  if (!flag_wrapv && INTEGRAL_MODE_P (mode) && trueop1 != const0_rtx
      && ! ((REG_P (op0) || GET_CODE (trueop0) == CONST_INT)
            && (REG_P (op1) || GET_CODE (trueop1) == CONST_INT)) 
      && 0 != (tem = simplify_binary_operation (MINUS, mode, op0, op1))
      /* We cannot do this for == or != if tem is a nonzero address.  */
      && ((code != EQ && code != NE) || ! nonzero_address_p (tem))
      && code != GTU && code != GEU && code != LTU && code != LEU)
    return simplify_const_relational_operation (signed_condition (code),
                                                mode, tem, const0_rtx);

actually, simplify_binary_operation (MINUS, mode, op0, op1) should not have
returned 4 (it's a signed MINUS which changes overflow because of the
unsigned PLUS, which is not ok as we are relying on undefined overflow).

For mainline, simplify_plus_minus (code=MINUS, mode=SImode, op0=0xb7e0a93c, op1=0xb7e15f10) returns NULL_RTX while on the branch it simplifies.
Comment 5 Richard Biener 2006-08-08 17:18:51 UTC
Which means it was probably "fixed" by

2005-11-30  Paolo Bonzini  <bonzini@gnu.org>

        * simplify-rtx.c (simplify_plus_minus): Remove final parameter.
        Always produce an output if we can remove NEGs or canonicalize
        (minus (minus ...)) expressions.  Provide a fast path for the
        two-operand case.
        (simplify_gen_binary): Do not call simplify_plus_minus.
        (simplify_binary_operation_1): Reassociate at the end of the
        function.

or more like worked around.
Comment 6 Janis Johnson 2006-08-08 20:59:44 UTC
A regression hunt on powerpc-linux confirmed that this patch caused the change in behavior:

    http://gcc.gnu.org/viewcvs?view=rev&rev=107702

    r107702 | bonzini | 2005-11-30 08:20:23 +0000 (Wed, 30 Nov 2005)

Here's the test I used:

------------------------
extern void abort (void);
int
foo (unsigned int u)
{
  return (int)(u + 4) < (int)u;
}
                                                                                
int
main (int argc, char *argv[])
{
  unsigned int u;
                                                                                
  /* Run with no arguments so u will be MAX_INT and the optimizers
     won't know its value.  */
  if (argc > 1)
    u = 1;
  else
    u = 0x7fffffff;
                                                                                
  if (foo (u) == 0)
    abort();
  return 0;
}
------------------------
Comment 7 Richard Biener 2006-08-09 07:31:44 UTC
So, I have a fix as the problem is latent on mainline, too.
Comment 8 patchapp@dberlin.org 2006-08-09 09:50:23 UTC
Subject: Bug number PR28651

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-08/msg00247.html
Comment 9 Andrew Pinski 2006-08-10 16:44:38 UTC
Here is a testcase which makes this a regression:
extern void abort (void);
int
foo (unsigned int u)
{
  unsigned t;
  for (t=0;t!=u;t++) ;
  return (int)(t + 4) < (int)u;
}

int
main (int argc, char *argv[])
{
  unsigned int u;

  /* Run with no arguments so u will be MAX_INT and the optimizers
     won't know its value.  */
  if (argc > 1)
    u = 1;
  else
    u = 0x7fffffff;

  if (foo (u) == 0)
    abort();
  return 0;
}
Comment 10 Andrew Pinski 2006-08-10 16:53:09 UTC
(In reply to comment #9)
> Here is a testcase which makes this a regression:
I forgot to say this testcase worked in 3.4.0 but failed in 4.0.0.
Comment 11 Richard Biener 2006-08-11 07:44:55 UTC
Subject: Bug 28651

Author: rguenth
Date: Fri Aug 11 07:44:45 2006
New Revision: 116079

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

	PR middle-end/28651
	* simplify-rtx.c (simplify_const_relational_operation):
	Simplify A CMP B to A - B CMP 0 only for EQ and NE comparison
	codes.

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

Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/pr28651.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/simplify-rtx.c
    trunk/gcc/testsuite/ChangeLog

Comment 12 Richard Biener 2006-08-11 07:52:05 UTC
Fixed for 4.1.2 and 4.2.0.  No longer blocks PR26847.  Unassigning.
Comment 13 Richard Biener 2006-08-11 07:52:11 UTC
Subject: Bug 28651

Author: rguenth
Date: Fri Aug 11 07:52:01 2006
New Revision: 116080

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

	PR middle-end/28651
	* simplify-rtx.c (simplify_const_relational_operation):
	Simplify A CMP B to A - B CMP 0 only for EQ and NE comparison
	codes.

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

Added:
    branches/gcc-4_1-branch/gcc/testsuite/gcc.c-torture/execute/pr28651.c
      - copied unchanged from r116079, trunk/gcc/testsuite/gcc.c-torture/execute/pr28651.c
Modified:
    branches/gcc-4_1-branch/gcc/ChangeLog
    branches/gcc-4_1-branch/gcc/simplify-rtx.c
    branches/gcc-4_1-branch/gcc/testsuite/ChangeLog

Comment 14 Debian GCC Maintainers 2006-11-11 02:11:41 UTC
the PR28651 testcase fails on the gcc-4.1-branch 20061110 on powerpc-linux and amd64-linux.


FAIL: gcc.c-torture/execute/pr28651.c execution,  -O0 
FAIL: gcc.c-torture/execute/pr28651.c execution,  -O1 
FAIL: gcc.c-torture/execute/pr28651.c execution,  -O2 
FAIL: gcc.c-torture/execute/pr28651.c execution,  -O3 -fomit-frame-pointer 
FAIL: gcc.c-torture/execute/pr28651.c execution,  -O3 -g 
FAIL: gcc.c-torture/execute/pr28651.c execution,  -Os 
Comment 15 Debian GCC Maintainers 2006-11-11 02:12:38 UTC
a regression compared to 4.1 20061028

  Matthias
Comment 16 Janis Johnson 2006-11-13 17:54:28 UTC
I a saw a failure for this when testing backported testsuite changes, but it passed when I ran it alone (with execute.exp=pr28651.c in RUNTESTFLAGS).  I'm testing it again now to see if the failure is intermittent, or if my testsuite changes somehow broke the (argc > 1) check in the test.
Comment 17 Janis Johnson 2006-11-13 18:01:16 UTC
The version of the test in mainline was modified to not check argc; I'll backport Richard's test fix to 4.1.
Comment 18 Janis Johnson 2006-11-13 23:03:52 UTC
Richard's testsuite change is now on the 4.1 branch, so the test passes again there.
Comment 19 Richard Biener 2007-01-25 19:05:34 UTC
Subject: Bug 28651

Author: rguenth
Date: Thu Jan 25 19:05:19 2007
New Revision: 121181

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=121181
Log:
2007-01-25  Richard Guenther  <rguenther@suse.de>

	Backport from mainline:
	2006-08-11  Richard Guenther  <rguenther@suse.de>

	PR middle-end/28651
	* simplify-rtx.c (simplify_const_relational_operation):
	Simplify A CMP B to A - B CMP 0 only for EQ and NE comparison
	codes.

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

	2006-08-14  Richard Guenther  <rguenther@suse.de>

	PR testsuite/28703
	* gcc.c-torture/execute/pr28651.c: Do not use argc
	to avoid optimization, instead forbid inlining.

Added:
    branches/gcc-4_0-branch/gcc/testsuite/gcc.c-torture/execute/pr28651.c
      - copied, changed from r116079, trunk/gcc/testsuite/gcc.c-torture/execute/pr28651.c
Modified:
    branches/gcc-4_0-branch/gcc/ChangeLog
    branches/gcc-4_0-branch/gcc/simplify-rtx.c
    branches/gcc-4_0-branch/gcc/testsuite/ChangeLog

Comment 20 Richard Biener 2007-01-25 20:02:41 UTC
Fixed.