Bug 42721 - possible integer wrong code bug
Summary: possible integer wrong code bug
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: unknown
: P3 normal
Target Milestone: 4.4.4
Assignee: Jakub Jelinek
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2010-01-13 03:54 UTC by John Regehr
Modified: 2010-03-13 03:10 UTC (History)
5 users (show)

See Also:
Host: i686-pc-linux-gnu
Target: i686-pc-linux-gnu
Build: i686-pc-linux-gnu
Known to work:
Known to fail:
Last reconfirmed: 2010-01-13 12:39:52


Attachments
gcc45-pr42721.patch (636 bytes, patch)
2010-01-13 16:09 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Regehr 2010-01-13 03:54:25 UTC
Seen on Ubuntu 9.10.  I think "1" is the right answer.

regehr@john-home:~/volatile/bugs/tmp254$ current-gcc -O1 small.c -o small
regehr@john-home:~/volatile/bugs/tmp254$ ./small
checksum = 1
regehr@john-home:~/volatile/bugs/tmp254$ current-gcc -O2 small.c -o small
regehr@john-home:~/volatile/bugs/tmp254$ ./small
checksum = 0
regehr@john-home:~/volatile/bugs/tmp254$ current-gcc -v
Using built-in specs.
COLLECT_GCC=current-gcc
COLLECT_LTO_WRAPPER=/home/regehr/z/tmp/gcc-r155838-install/libexec/gcc/i686-pc-linux-gnu/4.5.0/lto-wrapper
Target: i686-pc-linux-gnu
Configured with: ../configure --with-libelf=/usr/local --enable-lto --prefix=/home/regehr/z/tmp/gcc-r155838-install --program-prefix=r155838- --enable-languages=c,c++
Thread model: posix
gcc version 4.5.0 20100112 (experimental) (GCC) 
regehr@john-home:~/volatile/bugs/tmp254$ cat small.c
#include <stdint.h>
#include <stdio.h>

static uint64_t safe_div_uint64_t (uint64_t ui1, uint64_t ui2)
{
  return (ui2 == 0) ? ui1 : (ui1 / ui2);
}

static int8_t safe_mod_int8_t (int8_t si1, int8_t si2)
{
  return 
    ((si2 == 0) || ((si1 == INT8_MIN) && (si2 == (-1)))) ? 
    si1 : 
    (si1 % si2);
}

static int32_t g_5;
static int32_t g_11;

int main (void)
{
  uint64_t l_7 = 0x509CB0BEFCDF11BBLL;
  g_11 ^= l_7 && ((safe_div_uint64_t ((safe_mod_int8_t (g_5, 0)), -1L)) != 1L);
  printf ("checksum = %d\n", g_11);
  return 0;
}
Comment 1 Richard Biener 2010-01-13 10:15:14 UTC
HWI32 issue?  It doesn't reproduce for me on x86_64 with -m32.
Comment 2 Mikael Pettersson 2010-01-13 12:14:22 UTC
With a recent gcc-4.4 I see this -O1/-O2 difference on i686 but not powerpc64.
On i686 gcc-4.3 also seems affected (-O0 vs -O1), but 4.2 and 4.1 seem Ok.
Comment 3 Jakub Jelinek 2010-01-13 12:39:52 UTC
Indeed, looks like HWI32 issue.
Smaller testcase:
static unsigned long long
foo (unsigned long long x, unsigned long long y)
{
  return x / y;
}

static int a, b;

int
main (void)
{
  unsigned long long c = 1;
  b ^= c && (foo (a, -1ULL) != 1L);
  if (b != 1)
    __builtin_abort ();
  return 0;
}
Comment 4 Ozkan Sezer 2010-01-13 12:56:21 UTC
gcc-3.4.6, 4.3.2 and 4.4.3 always print 1 with or without -m32 for both -O1 and -O2 on x86_64 (fedora 10).  On i686 (fedora 9), gcc-3.3.6 and 3.4.6 always prints 1, gcc-4.3.0 (as shipped by fedora) always prints 0, and gcc-4.4.3 prints 1 at -O1 but prints 0 at -O2.
Comment 5 Jakub Jelinek 2010-01-13 15:05:00 UTC
The bug is in add_double_with_sign it seems.
319int
320add_double_with_sign (unsigned HOST_WIDE_INT l1, HOST_WIDE_INT h1,
321      unsigned HOST_WIDE_INT l2, HOST_WIDE_INT h2,
322      unsigned HOST_WIDE_INT *lv, HOST_WIDE_INT *hv,
323      bool unsigned_p)
324{
325  unsigned HOST_WIDE_INT l;
326  HOST_WIDE_INT h;
327
328  l = l1 + l2;
329  h = h1 + h2 + (l < l1);
330
331  *lv = l;
332  *hv = h;
333
334  if (unsigned_p)
335    return (unsigned HOST_WIDE_INT) h < (unsigned HOST_WIDE_INT) h1;
336  else
337    return OVERFLOW_SUM_SIGN (h1, h2, h);
338}
When called with say -1U, -1, -2U, -1, &x, &y, true it pretends no overflow
happened.  That is because we don't add just 2 numbers together, but 3 (also (l < l1)), and so -1U + -1U + 1 gives -1U, so h == h1.  I believe it is wrong not just for unsigned_p, but also for !unsigned_p.  The question is how to fix this as efficiently as possible.
Comment 6 rguenther@suse.de 2010-01-13 15:08:18 UTC
Subject: Re:  possible integer wrong code bug

On Wed, 13 Jan 2010, jakub at gcc dot gnu dot org wrote:

> 
> 
> ------- Comment #5 from jakub at gcc dot gnu dot org  2010-01-13 15:05 -------
> The bug is in add_double_with_sign it seems.
> 319int
> 320add_double_with_sign (unsigned HOST_WIDE_INT l1, HOST_WIDE_INT h1,
> 321      unsigned HOST_WIDE_INT l2, HOST_WIDE_INT h2,
> 322      unsigned HOST_WIDE_INT *lv, HOST_WIDE_INT *hv,
> 323      bool unsigned_p)
> 324{
> 325  unsigned HOST_WIDE_INT l;
> 326  HOST_WIDE_INT h;
> 327
> 328  l = l1 + l2;
> 329  h = h1 + h2 + (l < l1);
> 330
> 331  *lv = l;
> 332  *hv = h;
> 333
> 334  if (unsigned_p)
> 335    return (unsigned HOST_WIDE_INT) h < (unsigned HOST_WIDE_INT) h1;
> 336  else
> 337    return OVERFLOW_SUM_SIGN (h1, h2, h);
> 338}
> When called with say -1U, -1, -2U, -1, &x, &y, true it pretends no overflow
> happened.  That is because we don't add just 2 numbers together, but 3 (also (l
> < l1)), and so -1U + -1U + 1 gives -1U, so h == h1.  I believe it is wrong not
> just for unsigned_p, but also for !unsigned_p.  The question is how to fix this
> as efficiently as possible.

No-undefined-overflow branch has

{
  unsigned HOST_WIDE_INT l;
  HOST_WIDE_INT h;

  l = l1 + l2;
  h = (HOST_WIDE_INT)((unsigned HOST_WIDE_INT)h1
                      + (unsigned HOST_WIDE_INT)h2
                      + (l < l1));

  *lv = l;
  *hv = h;

  if (unsigned_p)
    return ((unsigned HOST_WIDE_INT) h < (unsigned HOST_WIDE_INT) h1
            || (h == h1
                && l < l1));
  else
    return OVERFLOW_SUM_SIGN (h1, h2, h);
}


but I think that caused fallout somewhere else... (extra TREE_OVERFLOW
stuff, etc)

Richard.
Comment 7 Richard Biener 2010-01-13 15:10:57 UTC
http://gcc.gnu.org/ml/gcc-cvs/2009-03/msg00226.html
Comment 8 Jakub Jelinek 2010-01-13 16:09:08 UTC
Created attachment 19574 [details]
gcc45-pr42721.patch

This is what I'm going to bootstrap/regtest now.
Regarding fallouts, I believe this particular one shouldn't have any, as add_double_with_sign ... true is only called in 2 places, fold_div_comparison where it fixes this bug and pointer_may_wrap_p, where it shouldn't make a difference (h2 is 0, so if l < l1 is 1, h will be certainly != h1).
Comment 9 Jakub Jelinek 2010-01-14 09:47:30 UTC
Subject: Bug 42721

Author: jakub
Date: Thu Jan 14 09:47:09 2010
New Revision: 155887

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=155887
Log:
	PR c/42721
	Port from no-undefined-overflow branch
	2009-03-09  Richard Guenther  <rguenther@suse.de>

	* fold-const.c (add_double_with_sign): Fix unsigned overflow
	detection.

	* gcc.c-torture/execute/pr42721.c: New test.

Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/pr42721.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/fold-const.c
    trunk/gcc/testsuite/ChangeLog

Comment 10 Jakub Jelinek 2010-01-14 09:48:16 UTC
Subject: Bug 42721

Author: jakub
Date: Thu Jan 14 09:48:01 2010
New Revision: 155888

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=155888
Log:
	PR c/42721
	Port from no-undefined-overflow branch
	2009-03-09  Richard Guenther  <rguenther@suse.de>

	* fold-const.c (add_double_with_sign): Fix unsigned overflow
	detection.

	* gcc.c-torture/execute/pr42721.c: New test.

Added:
    branches/gcc-4_4-branch/gcc/testsuite/gcc.c-torture/execute/pr42721.c
Modified:
    branches/gcc-4_4-branch/gcc/ChangeLog
    branches/gcc-4_4-branch/gcc/fold-const.c
    branches/gcc-4_4-branch/gcc/testsuite/ChangeLog

Comment 11 Andrew Pinski 2010-03-13 03:10:36 UTC
Fixed in 4.4.