Fixed bug #61998 (Crash when declare trait after class if user define the same name as aliased one) by reeze · Pull Request #83 · php/php-src

reeze · 2012-05-15T04:30:46Z

Thanks ron for your test script. I've make a minimal reproducible one below:

In class Class1:

newFunc was referred to T::func
func was itself (by overriding);

In trait T

func was referred by Class1 and T itself;

--- since class was destroyed by reverse order --

Destroy T： will not release the function name defined in trait. since
the Class1 referred to this function.
Destroy Class1：it will destroy the alias name since the aliased
function name was referred to it.(this let riginal function name
in trait unreleased and leak).
after destroy function table it will destroy alias info. but alias name was
already destroyed in function table releasing phrase. This cause double free(crash).

Solutions:

Copy the whole function will solve the problem. but it was too heavy.
Don't change the aliases function's name, since function call are always lookup by hash key name.
but it will make reflection unhappy and can't throw right error message for function.
Make a reference in function table if trait function was overrided to avoid releasing problem.
This need to change reflection ignore it.get_defined_functions() & get_delcared_clesses()
use this trick to filter special entry. so we need to change ReflectionClass::getMethods().

in summary I prefer option 3. What do you think?

<?php
class Class1 {
    use T {
        func as newFunc;
    }

public function func() { // <------------ if this override trait method and the method get aliased will lead crash
echo "From Class1::func\n";
}
}

class Class2 {
use T;
}

trait T { // <------------------------------ declare after the Class1 and it will be destroy before Class1
public function func() {
echo "From trait T\n";
}
}

reeze · 2012-05-15T05:58:03Z

@laruence thanks I will update that later.

It used to add a null to the start of the key to get a special key can't be accessed by user. the tmp_fn_name was emalloced with an additional byte to hold it.
please refer: Zend/zend_builtin_functions.c: ZEND_FUNCTION(create_function), I do it like that:)

…d the same method as aliased one)

php-pulls · 2012-05-16T06:24:13Z

On Tue, May 15, 2012 at 12:30 PM, Reeze Xia
[email protected]
wrote:

Thanks ron for your test script. I've make a minimal reproducible one below:

In class Class1:
newFunc was referred to T::func
func was itself (by overriding);

In trait T
func was referred by T and itself;

--- since class was destroyed by reverse order --

Destroy T： will not release the function name defined in trait. since
the Class1 referred to this function.

Destroy Class1：it will destroy the alias name since the aliased
function name was referred to it.(this let riginal function name
in trait unreleased and leak).
after destroy function table it will destroy alias info. but alias name was
already destroyed in function table releasing phrase. This cause double free(crash).

Solutions:

Copy the whole function will solve the problem. but it was too heavy.

Don't change the aliases function's name, since function call are always lookup by hash key name.
but it will make reflection unhappy and can't throw right error message for function.

Make a reference in function table if trait function was overrided to avoid releasing problem.
This need to change reflection ignore it.get_defined_functions() & get_delcared_clesses()
use this trick to filter special entry. so we need to change ReflectionClass::getMethods().
Hi:

thanks for your working on it. it is appreciated.

However, the trick way should not be encouraged, and after deeply look
into it, I got another option. I will work it out this weekend.

thanks

in summary I prefer option 3. What do you think?

Laruence Xinchen Hui
http://www.laruence.com/

laruence · 2012-05-19T06:19:55Z

Hi:

my patch was attached here:
https://bugs.php.net/patch-display.php?bug_id=61998&patch=bug61998.patch&revision=latest

thanks

On Wed, May 16, 2012 at 2:24 PM, Account for PHP Pull Requests
[email protected]
wrote:

On Tue, May 15, 2012 at 12:30 PM, Reeze Xia
[email protected]
wrote:

Thanks ron for your test script. I've make a minimal reproducible one below:

In class Class1:
newFunc was referred to T::func
func was itself (by overriding);

In trait T
func was referred by T and itself;

--- since class was destroyed by reverse order --

Destroy T： will not release the function name defined in trait. since
the Class1 referred to this function.

Destroy Class1：it will destroy the alias name since the aliased
function name was referred to it.(this let riginal function name
in trait unreleased and leak).
after destroy function table it will destroy alias info. but alias name was
already destroyed in function table releasing phrase. This cause double free(crash).

Solutions:

Copy the whole function will solve the problem. but it was too heavy.

Don't change the aliases function's name, since function call are always lookup by hash key name.
but it will make reflection unhappy and can't throw right error message for function.

Make a reference in function table if trait function was overrided to avoid releasing problem.
This need to change reflection ignore it.get_defined_functions() & get_delcared_clesses()
use this trick to filter special entry. so we need to change ReflectionClass::getMethods().
Hi:

thanks for your working on it. it is appreciated.

However, the trick way should not be encouraged, and after deeply look
into it, I got another option. I will work it out this weekend.

thanks

in summary I prefer option 3. What do you think?

Laruence Xinchen Hui
http://www.laruence.com/

Reply to this email directly or view it on GitHub:
#83 (comment)

惠新宸 laruence
Senior PHP Engineer
http://www.laruence.com

reeze · 2012-05-19T08:50:39Z

Add extra function to function_table is not that perfect.@laruence 's patch looks better.
closed

- Added ZEND_ASYNC_IO_TYPE_TCP and ZEND_ASYNC_IO_TYPE_UDP types - Modified zend_async_io_s to use union for fd/socket descriptor - Added zend_async_udp_req_s structure for UDP operations - Added socket options enum (broadcast, multicast, TCP options) - Extended zend_async_io_register() with UDP and socket option functions - Added includes for socket types (sys/socket.h, winsock2.h)

reeze added 2 commits May 15, 2012 17:34

Fixed bug #61998 (Crash when declare trait after class if user define…

3e80b98

…d the same method as aliased one)

Make ReflectionClass::getMethods() handle trait method

1096fb2

reeze closed this May 19, 2012

dunglas mentioned this pull request Sep 17, 2023

Segmentation fault when building Symfony cache on Alpine #12234

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed bug #61998 (Crash when declare trait after class if user define the same name as aliased one)#83

Fixed bug #61998 (Crash when declare trait after class if user define the same name as aliased one)#83
reeze wants to merge 2 commits intophp:PHP-5.4from
reeze:fix-trait-double-free-and-leak

reeze commented May 15, 2012

Uh oh!

reeze commented May 15, 2012

Uh oh!

php-pulls commented May 16, 2012

Uh oh!

laruence commented May 19, 2012

Uh oh!

reeze commented May 19, 2012

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

reeze commented May 15, 2012

Uh oh!

reeze commented May 15, 2012

Uh oh!

php-pulls commented May 16, 2012

Uh oh!

laruence commented May 19, 2012

Uh oh!

reeze commented May 19, 2012

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants