Re: [RFC] overflow checks optimized away

On Wed, Nov 27, 2013 at 10:48 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>
> On Mon, Sep 9, 2013 at 12:21:56PM -0400, Robert Haas wrote:
> > On Sat, Sep 7, 2013 at 6:55 PM, Greg Stark <stark(at)mit(dot)edu> wrote:
> > >> Should these patches be applied?
> > >
> > > I have a copy of the program and was going to take care of this.
> >
> > When?
>
> 2.5 months later, status report?

Attached is what I have so far. I have to say I'm starting to come
around to Tom's point of view. This is a lot of hassle for not much
gain. i've noticed a number of other overflow checks that the llvm
optimizer is not picking up on so even after this patch it's not clear
that all the signed overflow checks that depend on -fwrapv will be
gone.

This patch still isn't quite finished though.

a) It triggers a spurious gcc warning about overflows on constant
expressions. These value of these expressions aren't actually being
used as they're used in the other branch of the overflow test. I think
I see how to work around it for gcc using __builtin_choose_expr but it
might be pretty grotty.

b) I'm concerned these checks depend on INT_MIN/MAX and SHRT_MIN/MAX
which may not be exactly the right length. I'm kind of confused why
c.h assumes long is 32 bits and short is 16 bits though so I don't
think I'm making it any worse. It may be better for us to just define
our own limits since we know exactly how large we expect these data
types to be.

c) I want to add regression tests that will ensure that the overflow
checks are all working. So far I haven't been able to catch any being
optimized away even with -fno-wrapv and -fstrict-overflow. I think I
just didn't build with the right options though and it should be
possible.

The goal here imho is to allow building with -fno-wrapv
-fstrict-overflow safely. Whether we actually do or not is another
question but it means we would be able to use new compilers like clang
without worrying about finding the equivalent of -fwrapv for them.

--
greg