User account creation filtered due to spam.

Bug 34070 - [4.1 Regression] Wrong code for (int)x%4
Summary: [4.1 Regression] Wrong code for (int)x%4
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 4.2.1
: P3 major
Target Milestone: 4.2.3
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2007-11-12 10:23 UTC by Simon Marlow
Modified: 2008-07-04 16:16 UTC (History)
3 users (show)

See Also:
Host: x86_64-unknown-linux-gnu
Target: x86_64-unknown-linux-gnu
Build: x86_64-unknown-linux-gnu
Known to work: 4.0.4 4.2.3 4.3.0
Known to fail: 4.1.0 4.1.3
Last reconfirmed: 2007-11-12 12:45:59


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Marlow 2007-11-12 10:23:18 UTC
The following code generates the wrong result:

--------------------
#include <stdio.h>

int f(unsigned int x)
{
    printf("%x %d\n", x, (int)x);
    return ((int)x) % 4;
}

int main(int argc, char *argv[])
{
    printf("%d\n", f((unsigned int)(-1)));
    return 0;
}
--------------------

I expect this:

$ gcc-3.4.3 ctest33.c -Wall  && ./a.out
ffffffff -1
-1

and with gcc-4 and greater I get this:

$ gcc-4.2.1 ctest33.c -Wall  && ./a.out
ffffffff -1
3

Why do I think this is a bug?  Well, initially I thought I'd run into undefined behaviour, but on closer reading of the C spec it seems the behaviour should be implementation-defined, and gcc is not implementing the documented behaviour.  Furthermore, gcc's behaviour is not consistent, as implementation-defined behaviour should be.

The bug appears to be centered around conversion from unsigned to signed integers.  We convert from unsigned to signed in f(), and the value passed is 0xffffffff.  The result is therefore implementation-defined (C99 6.3.1.3), and gcc defines it (section 4.5 of the gcc docs) as: "For conversion to a type of width N, the value is reduced modulo 2^N to be within range of the type".  I presume this means that the value is truncated to N bits and the result interpreted as twos-complement, which in this case should mean that (int)x is -1, and the expression is (-1 % 4), which has value -1.

We can see from the printf output that (int)x has value -1.  Since this is its implementation-defined value, it should have the same value in the expression (int)x % 4.

Indeed, several minor variations of this code give the expected output.  Substituting 0xffffffffU for x in the definition of f(), for example.

Optimisation level has no effect.  Bug also observed on i686-unknown-linux.
Comment 1 Richard Biener 2007-11-12 12:41:59 UTC
The difference is that we now generate

f:
.LFB2:
        andl    $3, %edi
        movl    %edi, %eax
        ret

while previously we'd get the correct

f:
.LFB3:
        leal    3(%rdi), %eax
        cmpl    $-1, %edi
        cmovg   %edi, %eax
        andl    $-4, %eax
        subl    %eax, %edi
        movl    %edi, %eax
        ret

for

int f(unsigned int x)
{
    return ((int)x) % 4;
}

This is a problem in fold, as we get initially:

;; Function f (f)
;; enabled by -tree-original


{
  return (int) x & 3;
}

which is wrong.
Comment 2 Richard Biener 2007-11-12 12:45:58 UTC
Mine.
Comment 3 Richard Biener 2007-11-12 14:16:13 UTC
Subject: Bug 34070

Author: rguenth
Date: Mon Nov 12 14:16:05 2007
New Revision: 130098

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

	PR middle-end/34070
	* fold-const.c (fold_binary): If testing for non-negative
	operands with tree_expr_nonnegative_warnv_p make sure to
	use op0 which has all (sign) conversions retained.

	* gcc.c-torture/execute/pr34070-1.c: New testcase.
	* gcc.c-torture/execute/pr34070-2.c: Likewise.

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

Comment 4 Richard Biener 2007-11-12 14:16:53 UTC
Fixed for 4.3.0.
Comment 5 Richard Biener 2008-01-22 14:46:46 UTC
Subject: Bug 34070

Author: rguenth
Date: Tue Jan 22 14:45:56 2008
New Revision: 131723

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

	PR middle-end/34739
	Backport from mainline
	2008-01-16  Richard Guenther  <rguenther@suse.de>

	PR c/34768
	* c-typeck.c (common_pointer_type): Do not merge inconsistent
	type qualifiers for function types.

	2007-11-12  Richard Guenther  <rguenther@suse.de>

	PR middle-end/34070
	* fold-const.c (fold_binary): If testing for non-negative
	operands with tree_expr_nonnegative_warnv_p make sure to
	use op0 which has all (sign) conversions retained.

	2006-10-24  Richard Guenther  <rguenther@suse.de>

	PR middle-end/28796
	* builtins.c (fold_builtin_classify): Use HONOR_INFINITIES
	and HONOR_NANS instead of MODE_HAS_INFINITIES and MODE_HAS_NANS
	for deciding optimizations in consistency with fold-const.c
	(fold_builtin_unordered_cmp): Likewise.

Added:
    branches/gcc-4_2-branch/gcc/testsuite/gcc.c-torture/execute/pr34070-1.c
      - copied unchanged from r130098, trunk/gcc/testsuite/gcc.c-torture/execute/pr34070-1.c
    branches/gcc-4_2-branch/gcc/testsuite/gcc.c-torture/execute/pr34070-2.c
      - copied unchanged from r130098, trunk/gcc/testsuite/gcc.c-torture/execute/pr34070-2.c
    branches/gcc-4_2-branch/gcc/testsuite/gcc.c-torture/execute/pr34768-1.c
      - copied unchanged from r131568, trunk/gcc/testsuite/gcc.c-torture/execute/pr34768-1.c
    branches/gcc-4_2-branch/gcc/testsuite/gcc.c-torture/execute/pr34768-2.c
      - copied unchanged from r131568, trunk/gcc/testsuite/gcc.c-torture/execute/pr34768-2.c
    branches/gcc-4_2-branch/gcc/testsuite/gcc.dg/pr28796-1.c
      - copied unchanged from r118001, trunk/gcc/testsuite/gcc.dg/pr28796-1.c
    branches/gcc-4_2-branch/gcc/testsuite/gcc.dg/pr28796-2.c
      - copied unchanged from r118001, trunk/gcc/testsuite/gcc.dg/pr28796-2.c
Modified:
    branches/gcc-4_2-branch/gcc/ChangeLog
    branches/gcc-4_2-branch/gcc/builtins.c
    branches/gcc-4_2-branch/gcc/c-typeck.c
    branches/gcc-4_2-branch/gcc/fold-const.c
    branches/gcc-4_2-branch/gcc/testsuite/ChangeLog

Comment 6 Richard Biener 2008-01-22 14:52:52 UTC
Fixed on the 4.2 branch.
Comment 7 Joseph S. Myers 2008-07-04 16:16:01 UTC
Closing 4.1 branch.