Prevent func_hash_counters underflow #91
Open
Conversation
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
- The 1 (third parameter) means "destroy the original zval after copying" - This transfers ownership completely to the return value - XHPROF_G(stats_count) becomes IS_UNDEF - During cleanup, the check if (Z_TYPE(XHPROF_G(stats_count)) != IS_UNDEF) fails - No double-free occurs
1. Ignored functions in PHP 8.0+ create entries with is_trace = 0 2. These entries skip hp_mode_common_beginfn() so the hash counter is never incremented 3. But hp_mode_hier_endfn_cb() was always decrementing the hash counter 4. This caused hash counter underflow, which could lead to memory corruption and segfaults
If a callback is found and executed, it calls zend_string_release(function_name) and returns trace_name. However, if no callback is found, it returns function_name directly without releasing it.
This reverts commit 7e06de7.
Fixes segmentation fault when the first function call in a request is an ignored function. Previously, the code assumed *(entries) was always non-NULL when creating entries for ignored functions, but this caused a NULL pointer dereference when profiling started with an ignored function. The fix adds a NULL check and uses the actual function name when entries is NULL, ensuring proper initialization of the entry stack even when the first call is ignored.
Sets p->name_hprof to NULL after releasing the string in hp_fast_free_hprof_entry() to prevent potential double-free errors if the same entry is processed multiple times or reused from the free list with stale pointer data
…emory" This reverts commit 5a6c4e6.
kasparsd
commented
Jun 28, 2025
| callback = (hp_trace_callback*)zend_hash_find_ptr(XHPROF_G(trace_callbacks), function_name); | ||
| if (callback) { | ||
| trace_name = (*callback)(function_name, data); | ||
| } else { |
Author
There was a problem hiding this comment.
Previously:
- If a callback is found, release function_name and return trace_name
- If no callback is found, return function_name directly without releasing it
| #if PHP_VERSION_ID >= 80000 | ||
| hp_entry_t *cur_entry = hp_fast_alloc_hprof_entry(); | ||
| (cur_entry)->name_hprof = zend_string_copy((*(entries))->name_hprof); | ||
| /* Check if entries is not NULL before dereferencing */ |
Author
There was a problem hiding this comment.
Previously assumed *(entries) is always non-NULL when creating entries for ignored functions.
| #if PHP_VERSION_ID >= 80000 | ||
| if (top->is_trace == 0) { | ||
| XHPROF_G(func_hash_counters[top->hash_code])--; | ||
| /* For ignored functions, don't decrement hash counter since it was never incremented */ |
Author
There was a problem hiding this comment.
Previously:
hp_mode_common_beginfn()was never called -> hash counter never incrementedhp_mode_hier_endfn_cb()was always decrementing the hash counter -> counter underflow
Owner
There was a problem hiding this comment.
Hello, this should only affect PHP 8.4?
kasparsd
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Fixes #84, #90.