Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:87911
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.218.45 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <93061F8216C14196896DAC28DEE55982@pc1>
References: <02a601d0dbed$2c828df0$8587a9d0$@belski.net>
	<A6C1D55B475340ACB6D42434C270DEEE@pc1>
	<01b801d0df28$fccbe090$f663a1b0$@belski.net>
	<93061F8216C14196896DAC28DEE55982@pc1>
Date: Tue, 25 Aug 2015 20:41:37 +0700
Message-ID: <CAEZPtU6Gymt3Mwy4CsMfSCx8a0rVae77DTYDV_d-vFiih8SS5A@mail.gmail.com>
To: Matt W <php_lists@realplain.com>
Cc: Nikita Popov <nikita.ppv@gmail.com>, Anatol Belski <anatol.php@belski.net>, 
	Dmitry Stogov <dmitry@php.net>, Jakub Zelenka <bukka@php.net>, Bob Weinand <bobwei9@hotmail.com>, 
	internals@lists.php.net, Xinchen Hui <xinchen.h@zend.com>
Content-Type: multipart/alternative; boundary=001a113d6c5ebcd2e1051e22e499
Subject: Re: [PHP-DEV] Overflow checks and integral vars comparison
From: pierre.php@gmail.com (Pierre Joye)

--001a113d6c5ebcd2e1051e22e499
Content-Type: text/plain; charset=UTF-8

On Aug 25, 2015 7:19 PM, "Matt Wilmas" <php_lists@realplain.com> wrote:

>
> I didn't reply in last week's thread about the overflow checks in
OpenSSL...
>
> But it is *definitely* fine to be optimistic and rely on compiler to do
basic, basic stuff like this.  No reason to make things more complicated
just to think one is helping the compiler.
>
> Proof?  Have you seen the whole VM in the last, like, 10 years?  Using
your logic, we have major, major, MAJOR problems literally everywhere there!
>
> Now can anyone point to problems compiling the default, specialized VM?
I rest my case. :-)  There's conditional stuff ALL over the place (not
macros), and propagated to inline functions, etc. where the compiler is
"counted on" to remove the constant conditions and all that stuff.
>
> The current FAST_ZPP parameter parsing, any unnecessary parts are
optimized out (it'd be bad if that wasn't the case).  My new proposal
(coming soon...) has a lot of more complicated-looking source code, but
almost all deleted by the compiler.

> Transforming zpp's type spec string at compile time, which MSVC can't do
because it does indeed suck for some reason, involves *dozens* of KB of
source, many inline functions, etc. but nearly 100% of it is deleted,
correctly, by GCC and Clang.
>
>
> Anyway, that's why I say if "some" compiler can't do basic optimizations,
it *sucks*, and has much bigger problems in the rest of the source, so
there's no reason to worry about it...  At all.

Let me disagree on this comment. Most of it and  its wording.

>> Also it is probably a question of taste. SIZEOF_INT == SIZEOF_ZEND_LONG
is
>> something with zero calculation and guaranteed to exclude any possible
side
>> effect, while sizeof(int) == sizeof(zend_long) depends on the
optimization
>> choosen. Not disputing that your suggestion will do nearly the same job
in
>> the usual case, ofc.
>>
>> With the casts - probably yes, many cases will go by the standard type
>> promotion rules, but having the casts only ensures it does what one
actually
>> intended to do and has no effect in the generated code. For example one
>> would need a cast when comparing with INT_MAX + 1. Worse is  with
unsigned,
>> because the result depends on the platform and concrete types and values.
>>
>> Hiding behind the macros  makes full sense for two reasons at least
>>
>> - there are people with various skills that can just rely on what the API
>> provides without going too deep into the detail
>> - doing an internal change won't affect anything in the existing usages
>>
>>> > 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...
>>>
>> Not sure what you mean here, 32-bit?
>
>
> I mean, PHP's strlen() on any platform, why is it possible to have larger
strings (size_t length) than can be returned in zend_long.  That's a bug.

It is wrong to use long for portability. It is also wrong (very) to use
int* for length of a buffer. It is a very well known good practice (correct
way) to use size_t, even more when interacting with external sources.

I have to disagree about your statement here too.




> So?  Why does it matter if libraries are using size_t?  The same
libraries always have been and passed an int (from string length) before
PHP 7.  Pass them zend_long instead now...

And we have issues because of this exact case. It also helps to use the
right type for a given usage. In this case size_t.

>
>
>> Before the 64-bit RFC my idea was actually to port the dependency libs
for
>> the full 64-bit support, but it's insane. So better to fix the side
effects,
>> as those are almost the same as in PHP5 on most of 64-bit.
>>
>> Regards
>>
>> Anatol
>
>
> - Matt

--001a113d6c5ebcd2e1051e22e499--