Re: Overflow checks and integral vars comparison

Hi Anatol, Dmitry, all, ----- Original Message ----- From: "Anatol Belski" Sent: Friday, August 21, 2015

Hi, Resending this as missed internals at the start. I was lately rethinking some part of the 64-bit RFC, and also seeing now Jakub's work on catching overflows in ext/openssl and Matt Williams suggestions on it (which was going a bit more global over it). So I came up with these macros with two goals

Dang, people be gettin' my name wrong, in the same way, even "in writing." ;-P

- standardize the overflow checks - do actualy checks only on architectures where it's needed

The compiler will already do that, as I've said of course, and more importantly (for most cases), it will never get it wrong.

- simplify the checks where external libs (openssl, libxml, etc.) use firm datatypes like int #if SIZEOF_INT == SIZEOF_ZEND_LONG # define ZEND_LONG_INT_OVFL(zl) (0) # define ZEND_LONG_INT_UDFL(zl) (0) #else # define ZEND_LONG_INT_OVFL(zlong) ((zlong) > (zend_long)INT_MAX) # define ZEND_LONG_INT_UDFL(zlong) ((zlong) < (zend_long)INT_MIN) #endif

Really no point in all that...

#define ZEND_SIZE_T_INT_OVFL(size) ((size) > (size_t)INT_MAX)

What's wrong with just doing the check, "as is," instead of hiding it behind a [longer] macro? (Also, none of the casts are necessary...?)

So having it like If (ZEND_LONG_INT_OVFL(x)) { return; } Compiler would eliminate the branch automatically on 32-bit and ILP64.

Already will. :^)

Some other macros to do signed/unsigned comparison, these can be extended. #define ZEND_SIZE_T_GT_ZEND_LONG(size, zlong) ((zlong) < 0 || (size) > (size_t)(zlong)) #define ZEND_SIZE_T_GTE_ZEND_LONG(size, zlong) ((zlong) < 0 || (size) >= (size_t)(zlong)) #define ZEND_SIZE_T_LT_ZEND_LONG(size, zlong) ((zlong) >= 0 && (size) < (size_t)(zlong)) #define ZEND_SIZE_T_LTE_ZEND_LONG(size, zlong) ((zlong) >= 0 && (size) <= (size_t)(zlong))

Yes, the only case where we might want to do something special is comparing an unsigned type, like size_t (string lengths), with a signed max, since that's not impossible to the compiler. Of course, you have to KNOW that's what you want and/or it's safe to ignore the check, e.g. not dealing with strings > 2 GB on 32-bit, etc. And again, a generic way to do that for a "max" of integer type or greater (since literals are smallest type >= int that will hold the value), that should work anywhere, is: #define ZEND_SIGNED_OVFL_etc(val, max) (sizeof(val) > sizeof(max) && (val) > (max)) Simple and optimizes easily...

IMHO these and maybe more are missing after the 64-bit RFC. Do you think they would make sense? Or would make sense now, or later in master?

Also, why, exactly, has the zend_string length been defined as size_t (just because it's "supposed to" be?), which is incompatible with most everything in PHP, instead of zend_long (since we had int before)? e.g. Just look at strlen() -- oops, size_t no worky! :-/ Dmitry? We need range checking there, right? Conversion to double? No? Then there's *no point* in using size_t, and would let the compiler auto-optimize the above-referenced comparisons with size_t...

Thanks Anatol

- Matt