-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Add ASAN support to the zend allocator #18858
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
@dstogov / @nielsdos / @iluuu1994 This is now ready for review! The implementation is finalized, and has actually revealed a shutdown memory leak in php-fpm when terminated via a SIGTERM, due to the fact that alloc_globals_dtor is not getting invoked (the leak is surprisingly revealed exclusively by the usage of the poison functions, in fact, altering the ZEND_MM_POISON/ZEND_MM_UNPOISON macros to be no-ops re-hides the issue again, i.e. it's not caused by the other code changes/reorganization). This PR also includes the changes from #18834, as the bug is detectable by ASAN now. Side note, tracked_malloc also poisons/unpoisons the heap to avoid issues with the observe proxies. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! This adds a bit of complexity but this seems worth it. I would suggest we run this in nightly. We could add a step to the existing ASAN jobs that runs the test suite without --asan
(which sets USE_ZEND_ALLOC=0
).
This looks good to me apart from a few comments.
Co-authored-by: Arnaud Le Blanc <365207+arnaud-lb@users.noreply.github.com>
Co-authored-by: Arnaud Le Blanc <365207+arnaud-lb@users.noreply.github.com>
Co-authored-by: Arnaud Le Blanc <365207+arnaud-lb@users.noreply.github.com>
Co-authored-by: Arnaud Le Blanc <365207+arnaud-lb@users.noreply.github.com>
This pull request adds ASAN support to the zend allocator, by automatically poisoning all unused pages, chunks and heap management structures before exiting the alloc, free, etc (all ZEND_API) functions.
Internally, the implementation uses the following rules:
This is what allowed me to find #18833, before I found the fast shutdown workaround.