Re: BUG #5416: int4inc() is wrong

From: John Regehr <regehr(at)cs(dot)utah(dot)edu>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-bugs(at)postgresql(dot)org
Subject: Re: BUG #5416: int4inc() is wrong
Date: 2010-04-14 19:25:47
Message-ID: 4BC616BB.4010403@cs.utah.edu
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

Hi Tom,

> Note that we recommend using -fwrapv with gcc, so that it doesn't break
> code that depends on this type of test. (If int4inc isn't working then
> there are probably a lot of other places that are broken too.) I imagine
> LLVM has the same or similar switch.

llvm-gcc has the -fwrapv flag, but Clang does not, nor does Intel CC.

>> There are several easy ways to fix this code. One would be to test arg
>> against INT_MAX before incrementing. Another would be to cast arg to
>> unsigned, increment it, then do the check.
>
> None of these proposals are improvements over what's there. The
> fundamental problem is that if the compiler chooses to believe that
> overflow doesn't exist, it can optimize away *any* test that could only
> succeed in overflow cases. Finding a form of the test that fails to be
> optimized away by today's version of gcc doesn't protect you against
> tomorrow's version.

Your assessment is not quite correct. The existing function -- when passed
INT_MAX as the argument -- executes an operation with undefined behavior,
and a correct C compiler can optimize away the check.

The alternate tests do not execute operations with undefined behavior for
any argument value. Therefore, any compiler which removes the test is
wrong. Both the GCC and LLVM groups will be happy to fix a bug of that
kind if it exists.

Thanks,

John Regehr

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Tom Lane 2010-04-14 19:30:10 Re: [BUGS] BUG #5412: test case produced, possible race condition.
Previous Message Tom Lane 2010-04-14 19:18:51 Re: BUG #5421: pg_attribute broken