forked from OSchip/llvm-project
bebdab63e8
Summary: There was an issue in `releaseToOSMaybe`: one of the criteria to decide if we should proceed with the release was wrong. Namely: ``` const uptr N = Sci->Stats.PoppedBlocks - Sci->Stats.PushedBlocks; if (N * BlockSize < PageSize) return; // No chance to release anything. ``` I meant to check if the amount of bytes in the free list was lower than a page, but this actually checks if the amount of **in use** bytes was lower than a page. The correct code is: ``` const uptr BytesInFreeList = Region->AllocatedUser - (Region->Stats.PoppedBlocks - Region->Stats.PushedBlocks) * BlockSize; if (BytesInFreeList < PageSize) return 0; // No chance to release anything. ``` Consequences of the bug: - if a class size has less than a page worth of in-use bytes (allocated or in a cache), reclaiming would not occur, whatever the amount of blocks in the free list; in real world scenarios this is unlikely to happen and be impactful; - if a class size had less than a page worth of free bytes (and enough in-use bytes, etc), then reclaiming would be attempted, with likely no result. This means the reclaiming was overzealous at times. I didn't have a good way to test for this, so I changed the prototype of the function to return the number of bytes released, allowing to get the information needed. The test added fails with the initial criteria. Another issue is that `ReleaseToOsInterval` can actually be 0, meaning we always try to release (side note: it's terrible for performances). so change a `> 0` check to `>= 0`. Additionally, decrease the `CanRelease` threshold to `PageSize / 32`. I still have to make that configurable but I will do it at another time. Finally, rename some variables in `printStats`: I feel like "available" was too ambiguous, so change it to "total". Reviewers: morehouse, hctim, eugenis, vitalybuka, cferris Reviewed By: morehouse Subscribers: delcypher, #sanitizers, llvm-commits Tags: #llvm, #sanitizers Differential Revision: https://reviews.llvm.org/D68471 llvm-svn: 373930 |
||
|---|---|---|
| .. | ||
| standalone | ||
| CMakeLists.txt | ||
| scudo_allocator.cpp | ||
| scudo_allocator.h | ||
| scudo_allocator_combined.h | ||
| scudo_allocator_secondary.h | ||
| scudo_crc32.cpp | ||
| scudo_crc32.h | ||
| scudo_errors.cpp | ||
| scudo_errors.h | ||
| scudo_flags.cpp | ||
| scudo_flags.h | ||
| scudo_flags.inc | ||
| scudo_interface_internal.h | ||
| scudo_malloc.cpp | ||
| scudo_new_delete.cpp | ||
| scudo_platform.h | ||
| scudo_termination.cpp | ||
| scudo_tsd.h | ||
| scudo_tsd_exclusive.cpp | ||
| scudo_tsd_exclusive.inc | ||
| scudo_tsd_shared.cpp | ||
| scudo_tsd_shared.inc | ||
| scudo_utils.cpp | ||
| scudo_utils.h | ||