Skip to content

Commit dc5acb7

Browse files
yfeldblummeta-codesync[bot]
authored andcommitted
Fix F14Table UBSAN insufficient-object-size error
Summary: UBSAN fires `insufficient-object-size` when F14Table's single-chunk optimization allocates less than `sizeof(F14Chunk)` bytes, then calls member functions on the `Chunk*`. For example, `F14Chunk<unsigned int>` had `sizeof` = 64 bytes, but a 2-element table allocates only 24 bytes (16-byte header + 2×4-byte items). Fix by removing the `rawItems_` member array from `F14Chunk` so that `sizeof(F14Chunk)` equals 16 (just the header: `tags_[14]` + `control_` + `outboundOverflowCount_`). Items are accessed via pointer arithmetic from `this + kItemsOffset` instead of through the member array. Multi-chunk tables use an explicit stride constant (`kChunkStride`, equal to the old `sizeof`) for chunk-to-chunk navigation. This preserves the single-chunk memory optimization while making all member calls on `Chunk*` UBSAN-clean: - Minimum real allocation = 16 + 2*sizeof(Item) >= 24 > 16 = sizeof(Chunk) - Empty instance (F14EmptyTagVector) = 16 bytes = sizeof(Chunk) - Multi-chunk allocations use kChunkStride * N (same total size) Reviewed By: ilvokhin Differential Revision: D94696039 fbshipit-source-id: 06ef5480be9a2fdf3f0beb1e2c87916667e72abe
1 parent 3fe2357 commit dc5acb7

2 files changed

Lines changed: 160 additions & 65 deletions

File tree

third-party/folly/src/folly/container/detail/F14Policy.h

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,8 @@ struct FOLLY_MSVC_DECLSPEC(empty_bases) BasePolicy
187187
typename AllocTraits::pointer>::template rebind<Chunk>;
188188
using ItemIter = F14ItemIter<ChunkPtr>;
189189

190+
static constexpr std::size_t kChunkAllocAlignment = alignof(Chunk);
191+
190192
static constexpr bool kIsMap = !std::is_same<Key, Value>::value;
191193
static_assert(
192194
kIsMap == !std::is_void<MappedTypeOrVoid>::value,
@@ -333,11 +335,11 @@ struct FOLLY_MSVC_DECLSPEC(empty_bases) BasePolicy
333335
P&& /*rhs*/) {}
334336

335337
std::size_t alignedAllocSize(std::size_t n) const {
336-
if (kRequiredVectorAlignment <= alignof(max_align_t) ||
338+
if (kChunkAllocAlignment <= alignof(max_align_t) ||
337339
std::is_same<ByteAlloc, std::allocator<uint8_t>>::value) {
338340
return n;
339341
} else {
340-
return n + kRequiredVectorAlignment;
342+
return n + kChunkAllocAlignment;
341343
}
342344
}
343345

@@ -347,9 +349,8 @@ struct FOLLY_MSVC_DECLSPEC(empty_bases) BasePolicy
347349
std::size_t /*newCapacity*/,
348350
std::size_t chunkAllocSize,
349351
BytePtr& outChunkAllocation) {
350-
outChunkAllocation =
351-
allocateOverAligned<ByteAlloc, kRequiredVectorAlignment>(
352-
ByteAlloc{alloc()}, chunkAllocSize);
352+
outChunkAllocation = allocateOverAligned<ByteAlloc, kChunkAllocAlignment>(
353+
ByteAlloc{alloc()}, chunkAllocSize);
353354
return false;
354355
}
355356

@@ -363,7 +364,7 @@ struct FOLLY_MSVC_DECLSPEC(empty_bases) BasePolicy
363364
std::size_t chunkAllocSize) {
364365
// on success, this will be the old allocation, on failure the new one
365366
if (chunkAllocation != nullptr) {
366-
deallocateOverAligned<ByteAlloc, kRequiredVectorAlignment>(
367+
deallocateOverAligned<ByteAlloc, kChunkAllocAlignment>(
367368
ByteAlloc{alloc()}, chunkAllocation, chunkAllocSize);
368369
}
369370
}
@@ -379,7 +380,7 @@ struct FOLLY_MSVC_DECLSPEC(empty_bases) BasePolicy
379380
std::size_t /*capacity*/,
380381
BytePtr chunkAllocation,
381382
std::size_t chunkAllocSize) {
382-
deallocateOverAligned<ByteAlloc, kRequiredVectorAlignment>(
383+
deallocateOverAligned<ByteAlloc, kChunkAllocAlignment>(
383384
ByteAlloc{alloc()}, chunkAllocation, chunkAllocSize);
384385
}
385386

@@ -528,6 +529,7 @@ class ValueContainerPolicy
528529
using Hasher = typename Super::Hasher;
529530
using Mapped = typename Super::Mapped;
530531

532+
using Super::kChunkAllocAlignment;
531533
static constexpr bool kDefaultConstructIsNoexcept =
532534
Super::kDefaultConstructIsNoexcept;
533535
static constexpr bool kAllocIsAlwaysEqual = Super::kAllocIsAlwaysEqual;
@@ -650,7 +652,7 @@ class ValueContainerPolicy
650652
V&& visitor) const {
651653
if (chunkAllocSize > 0) {
652654
visitor(
653-
allocationBytesForOverAligned<ByteAlloc, kRequiredVectorAlignment>(
655+
allocationBytesForOverAligned<ByteAlloc, kChunkAllocAlignment>(
654656
chunkAllocSize),
655657
1);
656658
}
@@ -781,6 +783,7 @@ class NodeContainerPolicy
781783
private:
782784
using ByteAlloc = typename Super::ByteAlloc;
783785

786+
using Super::kChunkAllocAlignment;
784787
using Super::kIsMap;
785788

786789
public:
@@ -885,7 +888,7 @@ class NodeContainerPolicy
885888
V&& visitor) const {
886889
if (chunkAllocSize > 0) {
887890
visitor(
888-
allocationBytesForOverAligned<ByteAlloc, kRequiredVectorAlignment>(
891+
allocationBytesForOverAligned<ByteAlloc, kChunkAllocAlignment>(
889892
chunkAllocSize),
890893
1);
891894
}
@@ -1032,6 +1035,11 @@ class VectorContainerPolicy
10321035
private:
10331036
using Super::kIsMap;
10341037

1038+
// Allocation alignment must satisfy both chunk alignment (for SIMD tag
1039+
// loads) and value alignment (values are placed after chunks).
1040+
static constexpr std::size_t kAllocAlignment =
1041+
constexpr_max(Super::kChunkAllocAlignment, alignof(Value));
1042+
10351043
public:
10361044
static constexpr bool kEnableItemIteration = false;
10371045

@@ -1350,9 +1358,8 @@ class VectorContainerPolicy
13501358
newCapacity <= (std::numeric_limits<Item>::max)(),
13511359
"");
13521360

1353-
outChunkAllocation =
1354-
allocateOverAligned<ByteAlloc, kRequiredVectorAlignment>(
1355-
ByteAlloc{Super::alloc()}, allocSize(chunkAllocSize, newCapacity));
1361+
outChunkAllocation = allocateOverAligned<ByteAlloc, kAllocAlignment>(
1362+
ByteAlloc{Super::alloc()}, allocSize(chunkAllocSize, newCapacity));
13561363

13571364
ValuePtr before = values_;
13581365
ValuePtr after = std::pointer_traits<ValuePtr>::pointer_to(
@@ -1392,7 +1399,7 @@ class VectorContainerPolicy
13921399
// on success, chunkAllocation is the old allocation, on failure it is the
13931400
// new one
13941401
if (chunkAllocation != nullptr) {
1395-
deallocateOverAligned<ByteAlloc, kRequiredVectorAlignment>(
1402+
deallocateOverAligned<ByteAlloc, kAllocAlignment>(
13961403
ByteAlloc{Super::alloc()},
13971404
chunkAllocation,
13981405
allocSize(chunkAllocSize, (success ? oldCapacity : newCapacity)));
@@ -1418,7 +1425,7 @@ class VectorContainerPolicy
14181425
BytePtr chunkAllocation,
14191426
std::size_t chunkAllocSize) {
14201427
if (chunkAllocation != nullptr) {
1421-
deallocateOverAligned<ByteAlloc, kRequiredVectorAlignment>(
1428+
deallocateOverAligned<ByteAlloc, kAllocAlignment>(
14221429
ByteAlloc{Super::alloc()},
14231430
chunkAllocation,
14241431
allocSize(chunkAllocSize, capacity));
@@ -1435,7 +1442,7 @@ class VectorContainerPolicy
14351442
FOLLY_SAFE_DCHECK((chunkAllocSize == 0) == (capacity == 0), "");
14361443
if (chunkAllocSize > 0) {
14371444
visitor(
1438-
allocationBytesForOverAligned<ByteAlloc, kRequiredVectorAlignment>(
1445+
allocationBytesForOverAligned<ByteAlloc, kAllocAlignment>(
14391446
allocSize(chunkAllocSize, capacity)),
14401447
1);
14411448
}

0 commit comments

Comments
 (0)