Commit 25c28325 authored by Jan Wilken Dörrie's avatar Jan Wilken Dörrie Committed by Commit Bot

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

This reverts commit 5416e508.

Reason for revert: Likely culprit of Cast Audio Linux breakage: https://ci.chromium.org/p/chromium/builders/ci/Cast%20Audio%20Linux/75925

Original change's description:
> 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: Kentaro Hara <haraken@chromium.org>
> Reviewed-by: Takashi Sakamoto <tasak@google.com>
> Cr-Commit-Position: refs/heads/master@{#772662}

TBR=tasak@google.com,haraken@chromium.org,lizeb@chromium.org

Change-Id: Ic3266b40748d22cb25d91ab041db79da42851f4f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 787153, 998048
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2220085Reviewed-by: default avatarJan Wilken Dörrie <jdoerrie@chromium.org>
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#772669}
parent 2f4e4788
......@@ -62,8 +62,7 @@ static_assert(kGenericMaxDirectMapped <=
(1UL << 31) + kPageAllocationGranularity,
"maximum direct mapped allocation");
// Check that some of our zanier calculations worked out as expected.
static_assert(kGenericSmallestBucket == alignof(std::max_align_t),
"generic smallest bucket");
static_assert(kGenericSmallestBucket == 8, "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,7 +6,6 @@
#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"
......@@ -137,18 +136,7 @@ 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_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.
static const size_t kGenericMinBucketedOrder = 4; // 8 bytes.
// 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,7 +8,6 @@
#include <string.h>
#include <algorithm>
#include <cstddef>
#include <limits>
#include <memory>
#include <vector>
......@@ -2341,34 +2340,6 @@ 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