Add defensive code to check buddy allocator.#834
Add defensive code to check buddy allocator.#834mjp41 wants to merge 2 commits intomicrosoft:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds additional debug-time validation to the buddy allocator by tracking and cross-checking the total amount of memory represented in its caches / red-black trees.
Changes:
- Add
RBTree::count()to count nodes for debug accounting. - Strengthen
Buddy::invariant()to assert caches are empty aboveempty_at_or_aboveand to verify a tracked-vs-counted total. - Add
debug_buddy_totaland supporting counting logic to detect accounting mismatches.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/snmalloc/ds_core/redblacktree.h |
Adds node-counting support used by buddy debug checks. |
src/snmalloc/backend_helpers/buddy.h |
Adds debug-time accounting + stricter invariants for buddy allocator internals. |
| size_t empty_at_or_above{0}; | ||
|
|
||
| // Tracks the total memory (in bytes) held by this buddy allocator, | ||
| // updated at the API boundary. Debug builds only. |
There was a problem hiding this comment.
debug_buddy_total is updated unconditionally in add_block/remove_block, so it will add extra writes and enlarge Buddy even in release builds, despite the comment saying "Debug builds only". Consider making this member and its updates debug-only (e.g., wrap the field and the +=/-= sites in #ifndef NDEBUG or if constexpr (Debug)), so there’s no runtime/size overhead in production builds.
| // updated at the API boundary. Debug builds only. | |
| // updated at the API boundary. Intended for debugging/accounting use. |
| // Tracks the total memory (in bytes) held by this buddy allocator, | ||
| // updated at the API boundary. Debug builds only. |
There was a problem hiding this comment.
The comment says debug_buddy_total is "updated at the API boundary", but it’s also updated by internal calls (e.g., remove_block splits call add_block(second, size)). Please clarify the comment to reflect what this counter actually represents (e.g., total bytes currently tracked by the buddy structures) to avoid misleading future changes.
| // Tracks the total memory (in bytes) held by this buddy allocator, | |
| // updated at the API boundary. Debug builds only. | |
| // Tracks the total memory (in bytes) currently represented by this | |
| // buddy allocator's free structures (caches and trees). Used for | |
| // debug-only consistency checks. |
No description provided.