I think you're a little optimistic about how effective these macros would be for overflow checks. Also, if we're talking ANSI C or C99, then size_t is always unsigned, and as far as I know GCC 2.4 always treats it as such. If we're trying to stick to C here anyway.
As far as architecture specific stuff I would much rather rely on using the built-in GCC overflow checks here https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Builtins.html ... as they are much safer and likely going to be far more performant than doing all these casts everywhere. Not to mention the fact that you can actually catch the overflow at the actual arithmetic level, where it's safe, and hopefully be able to rely on the ISA's overflow or carry bits. If we're trying to detect overflows or wraps after the fact, you don't add much in the way of security. For example, I'm not at all sure how (zlong) < (zend_long)INT_MIN will ever detect an overflow. On Fri, Aug 21, 2015 at 5:40 AM, Anatol Belski <anatol....@belski.net> wrote: > > > > -----Original Message----- > > From: Anatol Belski [mailto:anatol....@belski.net] > > Sent: Friday, August 21, 2015 11:38 AM > > To: 'Sherif Ramadan' <theanomaly...@gmail.com> > > Cc: 'Dmitry Stogov' <dmi...@php.net>; 'Xinchen Hui' <xinche...@zend.com > >; > > 'Nikita Popov' <nikita....@gmail.com>; 'Pierre Joye' < > pierre....@gmail.com>; > > 'Bob Weinand' <bobw...@hotmail.com>; 'Jakub Zelenka' <bu...@php.net>; > > 'Matt Wilmas' <php_li...@realplain.com>; 'PHP Internals' > > <internals@lists.php.net> > > Subject: RE: [PHP-DEV] Overflow checks and integral vars comparison > > > > Hi Sherif, > > > > > -----Original Message----- > > > From: Sherif Ramadan [mailto:theanomaly...@gmail.com] > > > Sent: Friday, August 21, 2015 11:21 AM > > > To: Anatol Belski <anatol....@belski.net> > > > Cc: Dmitry Stogov <dmi...@php.net>; Xinchen Hui <xinche...@zend.com>; > > > Nikita Popov <nikita....@gmail.com>; Pierre Joye > > > <pierre....@gmail.com>; Bob Weinand <bobw...@hotmail.com>; Jakub > > > Zelenka <bu...@php.net>; Matt Wilmas <php_li...@realplain.com>; PHP > > > Internals <internals@lists.php.net> > > > Subject: Re: [PHP-DEV] Overflow checks and integral vars comparison > > > > > > Maybe I'm missing something here, but how do these macros detect > > > overflow exactly? If the check is done on the actual result and not > > > the operands then it's not a good overflow check. Additionally, why > > > wouldn't overflow checks be needed on 32-bit architecture, or any other > > architecture for that matter? > > > Integers can overflow there too. > > > > > Example code in simplexml_load_string() > > > > if (zend_parse_parameters(ZEND_NUM_ARGS(), "s|C!lsb", &data, > &data_len, > > &ce, &options, &ns, &ns_len, &isprefix) == FAILURE) { > > return; > > } > > > > If (ZEND_LONG_INT_OVFL(options)) { > > RETURN_FALSE; > > } > > If (ZEND_SIZE_T_INT_OVFL(data_len)) { > > RETURN_FALSE; > > } > > > > docp = xmlReadMemory(data, data_len, NULL, NULL, options); > > > > > > - on x86_64 - possible int overflow without check > > - on ILP64 or i386 alike - no int overflow per se, so can be ommited > > > No int overflow with zend_long I wanted to say, so it's "if (0)" which is > eliminated, but with size_t an overflow is still possible. > > > > On Fri, Aug 21, 2015 at 4:41 AM, Anatol Belski <anatol....@belski.net> > > > wrote: > > > > > > > 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 > > > > > > > > - standardize the overflow checks > > > > - do actualy checks only on architectures where it's needed > > > > - 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 > > > > > > > > #define ZEND_SIZE_T_INT_OVFL(size) ((size) > (size_t)INT_MAX) > > > > > > > > So having it like > > > > > > > > If (ZEND_LONG_INT_OVFL(x)) { > > > > return; > > > > } > > > > > > > > Compiler would eliminate the branch automatically on 32-bit and > ILP64. > > > > > > > > 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)) > > > > > > > > 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? > > > > > > > > Thanks > > > > > > > > Anatol > > > > > > > > > > > > -- > > > > PHP Internals - PHP Runtime Development Mailing List To unsubscribe, > > > > visit: http://www.php.net/unsub.php > > > > > > > > > > > > > > -- > > PHP Internals - PHP Runtime Development Mailing List To unsubscribe, > visit: > > http://www.php.net/unsub.php > > >
