Fix bug #69227 and #65967

[vektah]

See https://bugs.php.net/bug.php?id=69227

The old:
SplObjectStorage had a get_gc method that returns SplObjectStorage->properties. A hidden \x00gcdata property is added as an array so the gc can iterate over the contents of the collection. This was added to address https://bugs.php.net/bug.php?id=53071.

When a SplObjectStorage has a reference to itself in its collection (directly or indirectly) the gc will fetch a copy of gcdata, then going depth first into its children it hits itself a second time at which point it truncates the existing gcdata hash and fills it with the same data. This frees the pointer being held by the first pass of gcdata. Use after free ensues.

The new:
This patch removes the hidden \x00gcdata property from SplObjectStorage, replacing it with a list of zval*'s. Unlike a linked list updating the contents of a flat array does not invalidate the callers reference. Also the values never really change as the underlying collection does not change during gc

This fixes the free after use, and other bugs resulting from gcdata being a property (eg https://bugs.php.net/bug.php?id=65967)

This is broken in 5.* and master, but master will require a different patch as the gc has changed a little.

[laruence]

from my first glance, I don't think this could fixed bug 69227 , because you still erealloc (free previously allocated zval*[]) while doing recursively call to get_gc..

[laruence]

but maybe your are right, it only do realloc while elements are changed...

[vektah]

@laruence the realloc should only ever get triggered on the first call, or after modification of the collection, never during a gc run.

[laruence]

@vektah Anyway, this break ABI BC... and SPL is used by some exts, so maybe could only goes into PHP 7 even if it is works..

that means, we still needs some ugly fix for 5.x

[laruence]

hmm, after a second thought , I think it's okey, only appending... thanks... this seems much better than previouse ugly hack

[laruence]

why efree here, you use erealloc blow anyway.

[vektah]

Initially I was doing free/emalloc. Missed it when I switched to erealloc. Oops!

[reeze]

Do you mean count? length seems not a good name for this.

[laruence]

merged after a little improvement, thanks

[vektah]

@laruence Should this be if (intern->storage.nNumOfElements * 2 > ...?

    if (intern->storage.nNumOfElements > intern->gcdata_num) {
        intern->gcdata_num = intern->storage.nNumOfElements * 2;    
        intern->gcdata = (zval**)erealloc(intern->gcdata, sizeof(zval*) * intern->gcdata_num);
    }

[laruence]

@vektah yeah, it was a typo, fixed, thanks