fixes zend_type_release not freeing nested type lists#11884
fixes zend_type_release not freeing nested type lists#11884ju1ius wants to merge 4 commits intophp:masterfrom
Conversation
Girgias
left a comment
Girgias
left a comment
There was a problem hiding this comment.
As far as I can see persistent is always true for destroying arg infos of internal classes.
Also having a test added to the zend_test extension would be helpful.
The issue I'm having right now is that the valgrind configuration used by Any particular reson why this flag is not enabled? |
ju1ius
force-pushed
the
ju1ius/fix-zend_type_release
branch
from
4d121b5 to
48f1512
Compare
Note that this test only fails if valgrind is run with the `--leak-check=full` flag.
ju1ius
force-pushed
the
ju1ius/fix-zend_type_release
branch
2 times, most recently
from
cf754a9 to
1367c2b
Compare
ju1ius
force-pushed
the
ju1ius/fix-zend_type_release
branch
from
1367c2b to
29e861b
Compare
|
I've been digging into this, as I don't think the two different functions are needed. I can trigger the double use after free even without opcache using ASAN, and I think that is an issue due for some reason the type list is not allocated on an arena when it should. In other news, I think I found a bug of using DNF types in a trait property and binding it to a class... so thank you for finding that out indirectly :') |
So, what you mean is that the changeset in 9a1c9f0 is correct, but the failing test is actually a bug in the engine, that you can reproduce even with the changes in 90ca716? In this case, should I close this PR in favour of #11961 ? |
So, I think there was some other issue as to how I was getting the leak, but now it is just with opcache. And yes basically there is a bug somewhere in the engine/opcache which changes how the inner intersection type list is allocated, and this causes only an issue for property types, that are the only ones that do not store their types in a persistent manner for userland types. However, it should be arena allocated if it is a userland type. I am wondering if this is some issue caused by delayed early binding as this is like a "second" pass of the early binding reading: https://www.npopov.com/2021/10/20/Early-binding-in-PHP.html as this happens, seemingly, only when some of the class dependencies cannot be loaded. |
|
The proper fix was 02a80c5 |
2 participants
Closes #11883