Repository navigation
Fix object pool size reporting under ASan - #3739
Open
chenBright wants to merge 1 commit into
Open
chenBright wants to merge 1 commit into
chenBright wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The review found no active issues, and the change is supported by targeted tests covering allocation, cleanup, and ASan versus non-ASan size calculations.
No open issues 🎉
What changed in this PR
This PR fixes how ObjectPoolInfo::total_size is calculated in bRPC's object pool. The old formula used sizeof(T), which left out the alignment padding that AddressSanitizer (ASan) adds to each slot. As a result, the reported size was too small under ASan. For example, one char with the default settings reported 256 bytes, but the block's slot array actually takes 2048 bytes. The fix uses sizeof(BlockItem), the real slot size, so the reported size matches the memory in use.
Changes:
total_sizeinObjectPool::describe_objectsnow multiplies bysizeof(BlockItem)instead ofsizeof(T).- Unit tests check
total_sizeafter allocating, returning, and clearing objects. They also check the expected size for acharunder ASan (2048 bytes) and without ASan (256 bytes).
| File | Description |
|---|---|
| src/butil/object_pool_inl.h | Changes the total_size formula to use sizeof(BlockItem), which includes slot alignment padding. |
| test/object_pool_unittest.cpp | Adds checks for total_size across allocation and cleanup, and a test for the char case with and without ASan. |
Review effort: Lite
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.
What problem does this PR solve?
Issue Number:
Related PR: #3737
Problem Summary:
ObjectPoolInfo::total_sizeusessizeof(T), omitting alignment padding in ASan object-pool slots.For example, allocating one
charwith the default configuration reports 256 bytes, although the block's slot array occupies 2048 bytes.What is changed and the side effects?
Changed:
Calculate
total_sizeusingsizeof(BlockItem)to include slot alignment padding.Side effects:
Performance effects:
Breaking backward compatibility:
Check List: