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
