Re: [PATCH 0/3] Work around icc miscompilation - Mailing list pgsql-hackers

From Xi Wang
Subject Re: [PATCH 0/3] Work around icc miscompilation
Date
Msg-id 51016409.2020808@gmail.com
Whole thread Raw
In response to Re: [PATCH 0/3] Work around icc miscompilation  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: [PATCH 0/3] Work around icc miscompilation
List pgsql-hackers
On 1/24/13 10:48 AM, Tom Lane wrote:
> The fundamental problem here is that the compiler, unless told otherwise
> by a compilation switch, believes it is entitled to assume that no
> integer overflow will happen anywhere in the program.  Therefore, any
> error check that is looking for overflow *should* get optimized away.
> The only reason the compiler would fail to do that is if its optimizer
> isn't quite smart enough to prove that the code is testing for an
> overflow condition.  So what you are proposing here is merely the next
> move in an arms race with the compiler writers, and it will surely
> break again in a future generation of compilers.  Or even if these
> particular trouble spots don't break, something else will.  The only
> reliable solution is to not let the compiler make that type of
> assumption.

What I am proposing here is the opposite: _not_ to enter an arm race
with the compiler writers.  Instead, make the code conform to the C
standard, something both sides can agree on.

Particularly, avoid using (signed) overflowed results to detect
overflows, which the C standard clearly specifies as undefined
behavior and many compilers are actively exploiting.

We could use either unsigned overflows (which is well defined) or
precondition testing (like `x > INT_MAX - y' in the patches).

> So I think we should just reject all of these, and instead fix configure
> to make sure it turns on icc's equivalent of -fwrapv.

While I agree it's better to turn on icc's -fno-strict-overflow as a
workaround, the fundamental problem here is that we are _not_
programming in the C language.  Rather, we are programming in some
C-with-signed-wrapraround dialect.

The worst part of this C dialect is that it has never been specified
clearly what it means by "signed wraparound":

gcc's -fwrapv assumes signed wraparound for add/sub/mul, but not for
div (e.g., INT_MIN/-1 traps on x86) nor shift (e.g., 1<<32 produces
undef with clang).

clang's -fwrapv also assumes workaround for pointer arithmetic, while
gcc's does not.

I have no idea what icc and pathscale's -fno-strict-overflow option
does (probably the closest thing to -fwrapv).  Sometimes it prevents
such checks from being optimized away, sometimes it doesn't.

Anyway, it would not be surprising if future C compilers break this
dialect again.  But they shouldn't break standard-conforming code.

- xi



pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]
Next
From: Phil Sorber
Date:
Subject: Re: [PATCH] pg_isready (was: [WIP] pg_ping utility)