Commit 5416e508 authored by Benoit Lize's avatar Benoit Lize Committed by Commit Bot

base/allocator: Respect malloc() alignment guarantees in PartitionAlloc.

Generally speaking, malloc() must return data suitably aligned for
std::max_align_t. In practice this means 16 bytes on 64 bit
architectures, whereas PartitionAlloc only provides 8 there. This is
problematic to use it as a malloc() replacement.

This CL makes PartitionAlloc respect the alignment guarantees on 64 bit
architectures.

Bug: 787153,998048
Change-Id: I860f9d0439d59f8d494306452b8cdbff515c8300
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2219498
Commit-Queue: Benoit L <lizeb@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Reviewed-by: default avatarTakashi Sakamoto <tasak@google.com>
Cr-Commit-Position: refs/heads/master@{#772662}
parent 77fd944f
......@@ -62,7 +62,8 @@ static_assert(kGenericMaxDirectMapped <=
(1UL << 31) + kPageAllocationGranularity,
"maximum direct mapped allocation");
// Check that some of our zanier calculations worked out as expected.
static_assert(kGenericSmallestBucket == 8, "generic smallest bucket");
static_assert(kGenericSmallestBucket == alignof(std::max_align_t),
"generic smallest bucket");
static_assert(kGenericMaxBucketed == 983040, "generic max bucketed");
static_assert(kMaxSystemPagesPerSlotSpan < (1 << 8),
"System pages per slot span must be less than 128.");
......
......@@ -6,6 +6,7 @@
#define BASE_ALLOCATOR_PARTITION_ALLOCATOR_PARTITION_ALLOC_CONSTANTS_H_
#include <limits.h>
#include <cstddef>
#include "base/allocator/partition_allocator/page_allocator_constants.h"
#include "base/logging.h"
......@@ -136,7 +137,18 @@ static const size_t kNumPartitionPagesPerSuperPage =
// In terms of allocation sizes, order 0 covers 0, order 1 covers 1, order 2
// covers 2->3, order 3 covers 4->7, order 4 covers 8->15.
static const size_t kGenericMinBucketedOrder = 4; // 8 bytes.
static_assert(alignof(std::max_align_t) <= 16,
"PartitionAlloc doesn't support a fundamental alignment larger "
"than 16 bytes.");
// PartitionAlloc should return memory properly aligned for any type, to behave
// properly as a generic allocator. This is not strictly required as long as
// types are explicitly allocated with PartitionAlloc, but is to use it as a
// malloc() implementation, and generally to match malloc()'s behavior.
//
// In practice, this means 8 bytes alignment on 32 bit architectures, and 16
// bytes on 64 bit ones.
static const size_t kGenericMinBucketedOrder =
alignof(std::max_align_t) == 16 ? 5 : 4; // 2^(order - 1), that is 16 or 8.
// The largest bucketed order is 1 << (20 - 1), storing [512 KiB, 1 MiB):
static const size_t kGenericMaxBucketedOrder = 20;
static const size_t kGenericNumBucketedOrders =
......
......@@ -8,6 +8,7 @@
#include <string.h>
#include <algorithm>
#include <cstddef>
#include <limits>
#include <memory>
#include <vector>
......@@ -2340,6 +2341,34 @@ TEST_F(PartitionAllocTest, Alignment) {
generic_allocator.root()->Free(ptr);
}
TEST_F(PartitionAllocTest, FundamentalAlignment) {
// See the test above for details. Essentially, checking the bucket size is
// sufficient to ensure that alignment will always be respected, as long as
// the fundamental alignment is <= 16 bytes.
size_t fundamental_alignment = alignof(std::max_align_t);
for (size_t size = 0; size < base::kSystemPageSize; size++) {
// Allocate several pointers, as the first one in use in a size class will
// be aligned on a page boundary.
void* ptr = generic_allocator.root()->Alloc(size, "");
void* ptr2 = generic_allocator.root()->Alloc(size, "");
void* ptr3 = generic_allocator.root()->Alloc(size, "");
EXPECT_EQ(reinterpret_cast<uintptr_t>(ptr) % fundamental_alignment,
static_cast<uintptr_t>(0));
EXPECT_EQ(reinterpret_cast<uintptr_t>(ptr2) % fundamental_alignment,
static_cast<uintptr_t>(0));
EXPECT_EQ(reinterpret_cast<uintptr_t>(ptr3) % fundamental_alignment,
static_cast<uintptr_t>(0));
EXPECT_EQ(PartitionAllocGetSize(ptr) % fundamental_alignment,
static_cast<uintptr_t>(0));
generic_allocator.root()->Free(ptr);
generic_allocator.root()->Free(ptr2);
generic_allocator.root()->Free(ptr3);
}
}
} // namespace internal
} // namespace base
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment