Welcome! Log In Create A New Profile

Advanced

[PHP-DEV] Z_CACHE_SLOT changed meaning in PHP 7.3?

Posted by Christoph M. Becker 
Christoph M. Becker
[PHP-DEV] Z_CACHE_SLOT changed meaning in PHP 7.3?
July 06, 2018 03:40PM
Hi!

While trying to put together a patch to make PECL/uopz compatible with
PHP 7.3[1], I've stumbled upon commit ca035f2[2] and commit 1a63fa6[3].
The former removed Z_CACHE_SLOT and related stuff, but the latter
reintroduced it with an apparently somewhat different meaning. If that
is correct, this change should be documented in UPGRADING.INTERNALS, and
perhaps it might even be better to rename the RECV_INIT cache and the
related macros, so any extension (probably few, if any) would already
fail at compile time.

Thanks!

[1] https://github.com/krakjoe/uopz/issues/79
[2]
http://git.php.net/?p=php-src.git;a=commit;h=ca035f26aa296acf553f289e2d459fd052367db2
[3]
http://git.php.net/?p=php-src.git;a=commit;h=1a63fa6ec9b0bacbb726e60c3c212e7d97b518c6

--
Christoph M. Becker

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Nikita Popov
Re: [PHP-DEV] Z_CACHE_SLOT changed meaning in PHP 7.3?
July 06, 2018 04:00PM
On Fri, Jul 6, 2018 at 3:38 PM, Christoph M. Becker <[email protected]>
wrote:

> Hi!
>
> While trying to put together a patch to make PECL/uopz compatible with
> PHP 7.3[1], I've stumbled upon commit ca035f2[2] and commit 1a63fa6[3].
> The former removed Z_CACHE_SLOT and related stuff, but the latter
> reintroduced it with an apparently somewhat different meaning. If that
> is correct, this change should be documented in UPGRADING.INTERNALS, and
> perhaps it might even be better to rename the RECV_INIT cache and the
> related macros, so any extension (probably few, if any) would already
> fail at compile time.
>
> Thanks!
>
> [1] https://github.com/krakjoe/uopz/issues/79
> [2]
> <http://git.php.net/?p=php-src.git;a=commit;h=
> ca035f26aa296acf553f289e2d459fd052367db2>
> [3]
> <http://git.php.net/?p=php-src.git;a=commit;h=
> 1a63fa6ec9b0bacbb726e60c3c212e7d97b518c6>
>

The meaning of Z_CACHE_SLOT is still the same (cache slot stored in u2 of
literal). The thing that changed is that we now prefer storing the cache
slot directly on the opline, if possible. RECV_INIT is one of the cases
where two independent cache slots are needed, one is stored on the opline
and the other in Z_CACHE_SLOT of the initializer expression literal.

Nikita
Christoph M. Becker
Re: [PHP-DEV] Z_CACHE_SLOT changed meaning in PHP 7.3?
July 06, 2018 05:30PM
On 06.07.2018 at 15:55, Nikita Popov wrote:

> On Fri, Jul 6, 2018 at 3:38 PM, Christoph M. Becker <[email protected]>
> wrote:
>
>> While trying to put together a patch to make PECL/uopz compatible with
>> PHP 7.3[1], I've stumbled upon commit ca035f2[2] and commit 1a63fa6[3].
>> The former removed Z_CACHE_SLOT and related stuff, but the latter
>> reintroduced it with an apparently somewhat different meaning. If that
>> is correct, this change should be documented in UPGRADING.INTERNALS, and
>> perhaps it might even be better to rename the RECV_INIT cache and the
>> related macros, so any extension (probably few, if any) would already
>> fail at compile time.
>>
>> [1] https://github.com/krakjoe/uopz/issues/79
>> [2]
>> <http://git.php.net/?p=php-src.git;a=commit;h=
>> ca035f26aa296acf553f289e2d459fd052367db2>
>> [3]
>> <http://git.php.net/?p=php-src.git;a=commit;h=
>> 1a63fa6ec9b0bacbb726e60c3c212e7d97b518c6>
>
> The meaning of Z_CACHE_SLOT is still the same (cache slot stored in u2 of
> literal). The thing that changed is that we now prefer storing the cache
> slot directly on the opline, if possible. RECV_INIT is one of the cases
> where two independent cache slots are needed, one is stored on the opline
> and the other in Z_CACHE_SLOT of the initializer expression literal.

Thanks! However, the fact that the cache slot is now preferably stored
on the opline (extended_value) is an issue for PECL/uopz. For instance,
running the following script with uopz enabled

<?php
function foo() {}
foo();

makes valgrind report an “Use of uninitialised value of size 8”[1], and
obviously this code doesn't do anymore what it is supposed to do:

CACHE_PTR(Z_CACHE_SLOT_P(function_name), NULL);

[1]
<https://gist.github.com/cmb69/3060a552fc825497d066262ab31f3998#file-0001-update-for-php-7-3-patch-L43>;

--
Christoph M. Becker

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Sorry, only registered users may post in this forum.

Click here to login