Re: [RFC] Fix div/mul crash and more undefined behavior

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Xi Wang <xi(dot)wang(at)gmail(dot)com>
Cc: Bruce Momjian <bruce(at)momjian(dot)us>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: [RFC] Fix div/mul crash and more undefined behavior
Date: 2012-11-19 16:04:31
Message-ID: 2498.1353341071@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Xi Wang <xi(dot)wang(at)gmail(dot)com> writes:
> On 11/18/12 6:47 PM, Tom Lane wrote:
>> I was against this style of coding before, and I still am.
>> For one thing, it's just about certain to introduce conflicts
>> against system headers.

> I totally agree.

> I would be happy to rewrite the integer overflow checks without
> using these explicit constants, but it seems extremely tricky to
> do so.

I thought about this some more and realized that we can handle it
by realizing that division by -1 is the same as negation, and so
we can copy the method used in int4um. So the code would look like

if (arg2 == -1)
{
result = -arg1;
if (arg1 != 0 && SAMESIGN(result, arg1))
ereport(ERROR, ...);
PG_RETURN_INT32(result);
}

(with rather more comments than this, of course). This looks faster
than what's there now, as well as removing the need for use of
explicit INT_MIN constants.

> Compared to (arg1 == INTn_MIN && arg2 == -1), the above check is
> not only more confusing and difficult to understand, but it also
> invokes undefined behavior (-INT_MIN overflow), which is dangerous:
> many C compilers will optimize away the check.

They'd better not, else they'll break many of our overflow checks.
This is why we use -fwrapv with gcc, for example. Any other compiler
with similar optimizations needs to be invoked with a similar switch.

> Since INTn_MIN and INTn_MAX are standard macros from the C library,
> can we assume that every C compiler should provide them in stdint.h?

Not every C compiler provides stdint.h, unfortunately --- otherwise
I'd not be so resistant to depending on this.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dimitri Fontaine 2012-11-19 16:12:57 Re: gset updated patch
Previous Message Dimitri Fontaine 2012-11-19 15:39:27 Re: ALTER command reworks