Re: [RFC] overflow checks optimized away

On Fri, Oct 9, 2015 at 10:52 PM, Bruce Momjian <bruce@momjian.us> wrote:
>
> Any news on this item from 2013, worked on again 2014?
>
> ---------------------------------------------------------------------------
>
> On Wed, Aug  6, 2014 at 12:55:59PM -0400, Bruce Momjian wrote:
>> On Fri, Nov 29, 2013 at 02:04:10AM +0000, Greg Stark wrote:
>> > 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.
>>
>> Where are we on this?

Well, I have played a bit with this patch and rebased it as attached.
One major change is the use of the variables PG_INT* that have been
added in 62e2a8d. Some places were not updated with those new checks,
in majority a couple of routines in int.c (I haven't finished
monitoring the whole code though). Also, I haven't played yet with my
compilers to optimize away some of the checks and break them, but I'll
give it a try with clang and gcc. For now, I guess that this patch is
a good thing to begin with though, I have checked that code compiles
and regression tests pass.
Regards,
--
Michael