Skip to content

Commit d34d8b0

Browse files
wangqin0meta-codesync[bot]
authored andcommitted
Enhance SimpleAllocator with size validation
Summary: - Added size validation to `SimpleAllocator` constructor to ensure minimum allocation size of `sizeof(void*)` - Updated documentation in `CacheLocality.h` to clarify that allocations smaller than `sizeof(void*)` are rounded up - Added test case `CoreAllocator.MinimumAllocationSize` to verify correct behavior for small allocations X-link: facebook/folly#2558 Reviewed By: dmm-fb Differential Revision: D93888606 Pulled By: yfeldblum fbshipit-source-id: c67d1dd48f05f9e0319d9be29c43b40e71fac5af
1 parent 72d4196 commit d34d8b0

3 files changed

Lines changed: 38 additions & 1 deletion

File tree

third-party/folly/src/folly/concurrency/CacheLocality.cpp

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -582,7 +582,18 @@ class SimpleAllocator {
582582
// To support array aggregate initialization without an implicit constructor.
583583
struct Ctor {};
584584

585-
SimpleAllocator(Ctor, size_t sz) : sz_(sz) {}
585+
SimpleAllocator(Ctor, size_t sz) : sz_(sz) {
586+
static_assert(
587+
sizeof(void*) <= 64,
588+
"SimpleAllocator assumes sizeof(void*) fits in maximum size class");
589+
if (sz_ < sizeof(void*)) {
590+
folly::throw_exception<std::invalid_argument>(fmt::format(
591+
"SimpleAllocator size {} is too small (minimum: {})",
592+
sz_,
593+
sizeof(void*)));
594+
}
595+
}
596+
586597
~SimpleAllocator() {
587598
std::lock_guard g(m_);
588599
for (auto& block : blocks_) {

third-party/folly/src/folly/concurrency/CacheLocality.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,9 @@ class LLCAccessSpreader {
424424
* AccessSpreader can allocate memory in smaller-than cacheline increments, and
425425
* be assured that it won't cause more false sharing than it otherwise would.
426426
*
427+
* Allocations smaller than sizeof(void*) (typically 8 bytes) are automatically
428+
* rounded up to ensure correct internal bookkeeping.
429+
*
427430
* Note that allocation and deallocation takes a per-size-class lock.
428431
*
429432
* Memory allocated with coreMalloc() must be freed with coreFree().

third-party/folly/src/folly/concurrency/test/CacheLocalityTest.cpp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1237,3 +1237,26 @@ TEST(CoreAllocator, Basic) {
12371237
}
12381238
mems.clear();
12391239
}
1240+
1241+
TEST(CoreAllocator, MinimumAllocationSize) {
1242+
// coreMalloc should handle sizes smaller than sizeof(void*) by rounding up
1243+
constexpr size_t kNumStripes = 32;
1244+
1245+
// Test that small allocations (< 8 bytes) work correctly
1246+
// The Allocator class should round these up to 8 bytes
1247+
auto res1 = coreMalloc(1, kNumStripes, 0);
1248+
EXPECT_NE(nullptr, res1);
1249+
memset(res1, 0xFF, 1); // Should not crash
1250+
coreFree(res1);
1251+
1252+
auto res4 = coreMalloc(4, kNumStripes, 0);
1253+
EXPECT_NE(nullptr, res4);
1254+
memset(res4, 0xFF, 4); // Should not crash
1255+
coreFree(res4);
1256+
1257+
// Verify that 8-byte allocation works (minimum valid size)
1258+
auto res8 = coreMalloc(8, kNumStripes, 0);
1259+
EXPECT_NE(nullptr, res8);
1260+
memset(res8, 0xFF, 8);
1261+
coreFree(res8);
1262+
}

0 commit comments

Comments
 (0)