Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:72205
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain belski.net from 85.214.73.107 cause and error)
Message-ID: <647013700aff31d909f00714fde3c8d8.squirrel@webmail.klapt.com>
In-Reply-To: <52F10D11.1090703@hristov.com>
References: <e077612e6dd6e128cc0b8042db8d6b1c.squirrel@webmail.klapt.com>
    <52F0E8CF.7090501@hristov.com>
    <0681aaa573a34b3e07e246e7f88efbab.squirrel@webmail.klapt.com>
    <52F10D11.1090703@hristov.com>
Date: Tue, 4 Feb 2014 17:58:18 +0100
To: "Andrey Hristov" <php@hristov.com>
Cc: "Anatol Belski" <anatol.php@belski.net>,
 internals@lists.php.net
User-Agent: SquirrelMail/1.5.2 [SVN]
MIME-Version: 1.0
Content-Type: text/plain;charset=UTF-8
Content-Transfer-Encoding: 8bit
Subject: Re: [PHP-DEV] 64 bit RFC, #2 and #3 yes
From: anatol.php@belski.net ("Anatol Belski")

Andrey,

On Tue, February 4, 2014 16:53, Andrey Hristov wrote:
> Anatol,
>
>
> On  4.02.2014 16:39, Anatol Belski wrote:
>
>> Hi Andrey,
>>
>>
>> On Tue, February 4, 2014 14:19, Andrey Hristov wrote:
>>
>>> On  1.02.2014 14:30, Anatol Belski wrote:
>>>
>>>
>>>> Hi,
>>>>
>>>>
>>>>
>>>> as the concerns on the BC breach by zpp and macros changes are
>>>> huge, we've invented the below to make the essential change only
>>>> visible.
>>>>
>>>> This branches have zpp and macros change reverted (like #2 and #3
>>>> had resulted yes), only the necessary 64 improvement is in place.
>>>>
>>>> PHP core, zpp and macros reverted to the state of mainstream
>>>> https://github.com/weltling/php-src/tree/str_size_and_int64_old_name
>>>> s
>>>>
>>>>
>>>> Diff of the branch with old names to master
>>>> http://belski.net/phpz/str_size_and_int64_old_names.diff
>>>>
>>>>
>>>>
>>>>
>>>> An extension ported and fully worky with 5.3/4/5 and
>>>> str_size_and_int64 branch, diff
>>>> https://github.com/weltling/phurple/compare/str_size_and_int64
>>>>
>>>>
>>>>
>>>> The same ext how it looks now (not using the new zpp placeholders)
>>>> https://github.com/weltling/phurple
>>>>
>>>>
>>>>
>>>>
>>>> A sample extension generated with ext_skel from str_size_and_int64
>>>> branch with several usage examples, worky also wit h5.3/4/5
>>>> https://github.com/weltling/str_size_and_int64_example/blob/master/
>>>> str_ size_and_int64.c#L47
>>>>
>>>> The sample ext diff to the current mainstream base
>>>> https://github.com/weltling/str_size_and_int64_example/compare/str_s
>>>> ize_ and_int64_revert
>>>>
>>>>
>>>> Were this an acceptable tradeoff for this RFC to make it, one could
>>>>  still decide it in favor of 5.6. The worries and wishes are not
>>>> reasonless, which is clear. However I think to strike a compromise
>>>> is important to keep balance.
>>>>
>>>> Regards
>>>>
>>>>
>>>>
>>>> Anatol
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>> I saw 2 problems with the patch, in mysqlnd. After 2 instances I
>>> stopped reading but a parameter of type uint is being declared as
>>> php_int_t, a flag variable.
>>
>> I appreciate you could take a look at this. Could you point to the
>> exact place where you see an issue? With the replacement of uint there
>> are multiple reasons (and we was talking about it while being on the
>> discussion about the mysqli_poll), two main of those
>>
>> - php_size_t should be used for string lengths
>> - even flags coming from userland may cause overflow in 64 bit mode
>>
>>
>> So uint to php_uint_t might be because of that, depending on what
>> you're talking about.
>>
>> Thanks
>>
>>
>> Anatol
>>
>>
> -typedef enum_func_status
> (*func_mysqlnd_conn_data__tx_commit_or_rollback)(MYSQLND_CONN_DATA *
> conn, const zend_bool commit, const unsigned int flags, const char * const
> name TSRMLS_DC);
>
> +typedef enum_func_status
> (*func_mysqlnd_conn_data__tx_commit_or_rollback)(MYSQLND_CONN_DATA *
> conn, const zend_bool commit, const php_int_t flags, const char * const
> name TSRMLS_DC);
My bad, this one has to be php_uint_t.

> The API needs unsigned integer, not signed one. mysql, mysqli and pdo
> should take care when they get signed integers and pass them
> appropriately.
>
> Here is more:
>
>
> -	unsigned int	char_minlen;
> -	unsigned int	char_maxlen;
> +	php_size_t	char_minlen;
> +	php_size_t	char_maxlen;
>
>
> even unsigned int is too much for this data but still, going size_t just
> is completely unnecessary, as the value will be between 1 and 4. However,
> as you don't know the inner workings it is excuseable. Still my standpoint
> that we inflate memory usage where it is not necessary at all.
>
Homogenous 64 bit variables allow the compiler to construct a better
binary. That regards especially variables for loops, pointers and string
length comparsion. Avoiding conversions from 32 vs 64 bit enables
exhaustion of the architecture. Also, in many cases, one gets int or zval
from userspace, and that value (say number or string length) has to bubble
through various internal functions which often use different datatypes for
the same, so an overflow is possible. Thus, the security aspect. I'd be
glad if you could accomplish support on this matter.

> -			  Check bug #52891 : Wrong data inserted with mysqli/mysqlnd when
> using bind_param, value > LONG_MAX +			  Check bug #52891 : Wrong data
> inserted with mysqli/mysqlnd when using bind_param, value > PHP_INT_MAX
>
> Here this is a line in a comment. The bug has specific title and it
> hasn't changed, however the constant mentioned is changed. A bit annoying.
>
We can fix it.

> There might be more, I gave a glance look at it. I'm curious to see how
> much memory real scripts will use with the branch.
>
In fact, while 64 bit programs take more memory that 32 bit ones, the
exact advantage of the 64 bit systems is that the amount of that memory is
bigger. So comparing a program on 32 bit system with 2G (without
extensions) and the same program on 64 bit system with 8G memory - while
the program size itself increases, the relative increase opposing to the
system memory is smaller. Significant memory increase is only to expect
when using huge amounts of data split into small pieces, say you have
millions of string buffers of 3 byte length - in that case it'll need the
same amount of pointers which are 4 vs 8 bytes big on 64 bit. Stack
variables are of less importance for that.

Regards

Anatol