Fix heap corruption from repeated bypass_printr() rename - #32
Merged
andrewdalpino merged 1 commit intoSep 1, 2026
Merged
Conversation
EG(function_table) is process-wide and persists across requests, so bypass_printr() was re-finding and re-renaming the same print_r entry on every request after the first. On request 2+, releasing the already-renamed (persistent, malloc-backed) function_name string via the per-request memory manager corrupted the heap. Guard the rename with a static flag so it only runs once per process. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Member
|
Thank you @foppelfb! |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a long-running-process crash caused by re-entering bypass_printr() on subsequent requests and re-releasing a persistent zend_string via the request allocator, corrupting the Zend heap. This PR hardens the print_r interception so the function-table mutation is intended to happen only once per process.
Changes:
- Adds a per-process
doneguard inbypass_printr()to prevent repeated mutation across requests. - Expands the in-code rationale documenting why repeated renaming corrupts the heap (and notes the related once-per-process leak behavior).
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+1352
to
+1356
| static int done = 0; | ||
| if (done) { | ||
| return; | ||
| } | ||
| done = 1; |
Member
There was a problem hiding this comment.
Will address in a followup
Member
|
Merging as this is a partial fix, will follow up with improved concurrency guard. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
EG(function_table) is process-wide and persists across requests, so bypass_printr() was re-finding and re-renaming the same print_r entry on every request after the first. On request 2+, releasing the already-renamed (persistent, malloc-backed) function_name string via the per-request memory manager corrupted the heap. Guard the rename with a static flag so it only runs once per process.
This happened with long-running processes or the php builtin server. It resulted in an segfault with zend_mm_heap corrupted, even if no method of numpower was executed. The issue lay in the overloading of the print_r function, which could happen a second time. I assume this could happen in a FrankenPHP environment as well.
The Patch was partially done with the help of AI.
Submission Checklist:
Due to the inherent complexity of this library, we created this checklist to remind everyone of the essential steps to have an MR approved depending on the type of change that is made. You can delete this.
export USE_ZEND_ALLOC=1 && make testexport USE_ZEND_ALLOC=0 && make testChange to methods and operations
NDARRAY_VCHECKoption enabled and no VRAM memory leaks were displayed?export NDARRAY_VCHECK=1 && make testChanges to Core Components:
This include changes to:
buffer.c,gpu_alloc.c,ndarray.c,iterators.cand their associated header files.