Re: [RFC] overflow checks optimized away

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: Greg Stark <stark(at)mit(dot)edu>, Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Xi Wang <xi(dot)wang(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [RFC] overflow checks optimized away
Date: 2015-10-20 07:25:11
Message-ID: CAB7nPqSTC69XoPUObZ2jrpZGG5bOP8GFXTZUEGHi=uJU2FskNg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Oct 16, 2015 at 11:29 PM, Michael Paquier wrote:
> 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.

Hm. Looking at the options of clang
(http://clang.llvm.org/docs/UsersManual.html), there is no actual
equivalent of fno-wrapv and strict-overflow, there are a couple of
sanitizer functions though (-fsanitize=unsigned-integer-overflow
-fsanitize=signed-integer-overflow) that can be used at run time, but
that's not really useful for us.

I also looked at the definition of SHRT_MIN/MAX in a couple of places
(OSX, Linux, MinGW, Solaris, OpenBSD, FreeBSD, MSVC), and they are
always set as respectively -7fff-1 and 7fff. Hence do we really need
to worry about those two having potentially a different length, are
there opinions about having customs PG_SHRT_MIN/PG_SHRT_MAX in c.h?

Except that, attached is a result of all the hacking I have been doing
regarding this item:
- Addition of some macros to check overflows for INT16
- Addition of a couple of regression tests (Does testing int2inc &
friends make sense?)
- Addition of consistent overflow checks in more code paths, previous
patch missing a couple of places in int8.c, int.c and btree_gist (+
alpha). I have screened the code for existing "out of range" errors.
I'll add that to the next CF, perhaps this will interest somebody.
Regards,
--
Michael

Attachment Content-Type Size
20151020_overflow_checks_v2.patch application/x-patch 49.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2015-10-20 07:35:42 Re: checkpointer continuous flushing
Previous Message Amit Kapila 2015-10-20 07:04:59 Re: Parallel Seq Scan