Skip to content

Commit

Permalink
Use base::IsAligned() in more places.
Browse files Browse the repository at this point in the history
Avoid manually doing bitwise operation, now that base::IsAligned()
exists. Also fix lint errors and some nits in modified files.

Change-Id: I6836367a9af03295c61b6273752fb51bef46bd33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2245690
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: James Darpinian <jdarpinian@chromium.org>
Reviewed-by: Peng Huang <penghuang@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#780133}
  • Loading branch information
leizleiz authored and Commit Bot committed Jun 19, 2020
1 parent b522e9f commit 25c9ff6
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 52 deletions.
20 changes: 9 additions & 11 deletions base/containers/stack_container_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include <algorithm>

#include "base/memory/aligned_memory.h"
#include "base/memory/ref_counted.h"
#include "build/build_config.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand All @@ -16,14 +17,14 @@ namespace base {

namespace {

class Dummy : public base::RefCounted<Dummy> {
class Dummy : public RefCounted<Dummy> {
public:
explicit Dummy(int* alive) : alive_(alive) {
++*alive_;
}

private:
friend class base::RefCounted<Dummy>;
friend class RefCounted<Dummy>;

~Dummy() {
--*alive_;
Expand Down Expand Up @@ -110,31 +111,28 @@ class AlignedData {
alignas(alignment) char data_[alignment];
};

} // anonymous namespace

#define EXPECT_ALIGNED(ptr, align) \
EXPECT_EQ(0u, reinterpret_cast<uintptr_t>(ptr) & (align - 1))
} // namespace

TEST(StackContainer, BufferAlignment) {
StackVector<wchar_t, 16> text;
text->push_back(L'A');
EXPECT_ALIGNED(&text[0], alignof(wchar_t));
EXPECT_TRUE(IsAligned(&text[0], alignof(wchar_t)));

StackVector<double, 1> doubles;
doubles->push_back(0.0);
EXPECT_ALIGNED(&doubles[0], alignof(double));
EXPECT_TRUE(IsAligned(&doubles[0], alignof(double)));

StackVector<AlignedData<16>, 1> aligned16;
aligned16->push_back(AlignedData<16>());
EXPECT_ALIGNED(&aligned16[0], 16);
EXPECT_TRUE(IsAligned(&aligned16[0], 16));

#if !defined(__GNUC__) || defined(ARCH_CPU_X86_FAMILY)
// It seems that non-X86 gcc doesn't respect greater than 16 byte alignment.
// See http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33721 for details.
// TODO(sbc):re-enable this if GCC starts respecting higher alignments.
// TODO(sbc): Re-enable this if GCC starts respecting higher alignments.
StackVector<AlignedData<256>, 1> aligned256;
aligned256->push_back(AlignedData<256>());
EXPECT_ALIGNED(&aligned256[0], 256);
EXPECT_TRUE(IsAligned(&aligned256[0], 256));
#endif
}

Expand Down
11 changes: 5 additions & 6 deletions base/lazy_instance_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <stddef.h>

#include <memory>
#include <utility>
#include <vector>

#include "base/at_exit.h"
Expand All @@ -13,6 +14,7 @@
#include "base/barrier_closure.h"
#include "base/bind.h"
#include "base/lazy_instance.h"
#include "base/memory/aligned_memory.h"
#include "base/system/sys_info.h"
#include "base/threading/platform_thread.h"
#include "base/threading/simple_thread.h"
Expand Down Expand Up @@ -178,9 +180,6 @@ class AlignedData {

} // namespace

#define EXPECT_ALIGNED(ptr, align) \
EXPECT_EQ(0u, reinterpret_cast<uintptr_t>(ptr) & (align - 1))

TEST(LazyInstanceTest, Alignment) {
using base::LazyInstance;

Expand All @@ -194,9 +193,9 @@ TEST(LazyInstanceTest, Alignment) {
static LazyInstance<AlignedData<4096>>::DestructorAtExit align4096 =
LAZY_INSTANCE_INITIALIZER;

EXPECT_ALIGNED(align4.Pointer(), 4);
EXPECT_ALIGNED(align32.Pointer(), 32);
EXPECT_ALIGNED(align4096.Pointer(), 4096);
EXPECT_TRUE(base::IsAligned(align4.Pointer(), 4));
EXPECT_TRUE(base::IsAligned(align32.Pointer(), 32));
EXPECT_TRUE(base::IsAligned(align4096.Pointer(), 4096));
}

namespace {
Expand Down
10 changes: 5 additions & 5 deletions base/memory/platform_shared_memory_region.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "base/memory/platform_shared_memory_region.h"

#include "base/memory/aligned_memory.h"
#include "base/memory/shared_memory_mapping.h"
#include "base/memory/shared_memory_security_policy.h"
#include "base/metrics/histogram_functions.h"
Expand All @@ -15,7 +16,7 @@ namespace subtle {
namespace {

void RecordMappingWasBlockedHistogram(bool blocked) {
base::UmaHistogramBoolean("SharedMemory.MapBlockedForSecurity", blocked);
UmaHistogramBoolean("SharedMemory.MapBlockedForSecurity", blocked);
}

} // namespace
Expand Down Expand Up @@ -62,14 +63,13 @@ bool PlatformSharedMemoryRegion::MapAt(off_t offset,
if (!SharedMemorySecurityPolicy::AcquireReservationForMapping(size)) {
RecordMappingWasBlockedHistogram(/*blocked=*/true);
return false;
} else {
RecordMappingWasBlockedHistogram(/*blocked=*/false);
}

RecordMappingWasBlockedHistogram(/*blocked=*/false);

bool success = MapAtInternal(offset, size, memory, mapped_size);
if (success) {
DCHECK_EQ(
0U, reinterpret_cast<uintptr_t>(*memory) & (kMapMinimumAlignment - 1));
DCHECK(IsAligned(*memory, kMapMinimumAlignment));
} else {
SharedMemorySecurityPolicy::ReleaseReservationForMapping(size);
}
Expand Down
12 changes: 5 additions & 7 deletions base/memory/singleton_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <stdint.h>

#include "base/at_exit.h"
#include "base/memory/aligned_memory.h"
#include "base/memory/singleton.h"
#include "testing/gtest/include/gtest/gtest.h"

Expand Down Expand Up @@ -278,9 +279,6 @@ TEST_F(SingletonTest, Basic) {
VerifiesCallbacksNotCalled();
}

#define EXPECT_ALIGNED(ptr, align) \
EXPECT_EQ(0u, reinterpret_cast<uintptr_t>(ptr) & (align - 1))

TEST_F(SingletonTest, Alignment) {
// Create some static singletons with increasing sizes and alignment
// requirements. By ordering this way, the linker will need to do some work to
Expand All @@ -294,10 +292,10 @@ TEST_F(SingletonTest, Alignment) {
AlignedTestSingleton<AlignedData<4096>>* align4096 =
AlignedTestSingleton<AlignedData<4096>>::GetInstance();

EXPECT_ALIGNED(align4, 4);
EXPECT_ALIGNED(align32, 32);
EXPECT_ALIGNED(align128, 128);
EXPECT_ALIGNED(align4096, 4096);
EXPECT_TRUE(IsAligned(align4, 4));
EXPECT_TRUE(IsAligned(align32, 32));
EXPECT_TRUE(IsAligned(align128, 128));
EXPECT_TRUE(IsAligned(align4096, 4096));
}

} // namespace
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@

#include <algorithm>
#include <memory>
#include <string>
#include <utility>

#include "base/format_macros.h"
#include "base/memory/aligned_memory.h"
#include "base/memory/discardable_shared_memory.h"
#include "base/memory/ptr_util.h"
#include "base/stl_util.h"
Expand Down Expand Up @@ -116,10 +118,8 @@ DiscardableSharedMemoryHeap::Grow(
int32_t id,
base::OnceClosure deleted_callback) {
// Memory must be aligned to block size.
DCHECK_EQ(
reinterpret_cast<size_t>(shared_memory->memory()) & (block_size_ - 1),
0u);
DCHECK_EQ(size & (block_size_ - 1), 0u);
DCHECK(base::IsAligned(shared_memory->memory(), block_size_));
DCHECK(base::IsAligned(size, block_size_));

std::unique_ptr<Span> span(
new Span(shared_memory.get(),
Expand Down
34 changes: 17 additions & 17 deletions gpu/command_buffer/client/fenced_allocator_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ TEST_F(FencedAllocatorTest, TestOutOfMemory) {

const unsigned int kSize = 16;
const unsigned int kAllocCount = kBufferSize / kSize;
CHECK(kAllocCount * kSize == kBufferSize);
CHECK_EQ(kAllocCount * kSize, kBufferSize);

// Allocate several buffers to fill in the memory.
FencedAllocator::Offset offsets[kAllocCount];
Expand Down Expand Up @@ -161,7 +161,7 @@ TEST_F(FencedAllocatorTest, TestFreePendingToken) {

const unsigned int kSize = 16;
const unsigned int kAllocCount = kBufferSize / kSize;
CHECK(kAllocCount * kSize == kBufferSize);
CHECK_EQ(kAllocCount * kSize, kBufferSize);

// Allocate several buffers to fill in the memory.
FencedAllocator::Offset offsets[kAllocCount];
Expand Down Expand Up @@ -209,7 +209,7 @@ TEST_F(FencedAllocatorTest, FreeUnused) {

const unsigned int kSize = 16;
const unsigned int kAllocCount = kBufferSize / kSize;
CHECK(kAllocCount * kSize == kBufferSize);
CHECK_EQ(kAllocCount * kSize, kBufferSize);

// Allocate several buffers to fill in the memory.
FencedAllocator::Offset offsets[kAllocCount];
Expand Down Expand Up @@ -406,7 +406,7 @@ TEST_F(FencedAllocatorWrapperTest, TestBasic) {
allocator_->CheckConsistency();

const unsigned int kSize = 16;
void *pointer = allocator_->Alloc(kSize);
void* pointer = allocator_->Alloc(kSize);
ASSERT_TRUE(pointer);
EXPECT_LE(buffer_.get(), static_cast<char *>(pointer));
EXPECT_GE(kBufferSize, static_cast<char *>(pointer) - buffer_.get() + kSize);
Expand All @@ -415,14 +415,14 @@ TEST_F(FencedAllocatorWrapperTest, TestBasic) {
allocator_->Free(pointer);
EXPECT_TRUE(allocator_->CheckConsistency());

char *pointer_char = allocator_->AllocTyped<char>(kSize);
char* pointer_char = allocator_->AllocTyped<char>(kSize);
ASSERT_TRUE(pointer_char);
EXPECT_LE(buffer_.get(), pointer_char);
EXPECT_GE(buffer_.get() + kBufferSize, pointer_char + kSize);
allocator_->Free(pointer_char);
EXPECT_TRUE(allocator_->CheckConsistency());

unsigned int *pointer_uint = allocator_->AllocTyped<unsigned int>(kSize);
unsigned int* pointer_uint = allocator_->AllocTyped<unsigned int>(kSize);
ASSERT_TRUE(pointer_uint);
EXPECT_LE(buffer_.get(), reinterpret_cast<char *>(pointer_uint));
EXPECT_GE(buffer_.get() + kBufferSize,
Expand All @@ -439,7 +439,7 @@ TEST_F(FencedAllocatorWrapperTest, TestBasic) {
TEST_F(FencedAllocatorWrapperTest, TestAllocZero) {
allocator_->CheckConsistency();

void *pointer = allocator_->Alloc(0);
void* pointer = allocator_->Alloc(0);
ASSERT_FALSE(pointer);
EXPECT_TRUE(allocator_->CheckConsistency());
}
Expand All @@ -449,15 +449,15 @@ TEST_F(FencedAllocatorWrapperTest, TestAlignment) {
allocator_->CheckConsistency();

const unsigned int kSize1 = 75;
void *pointer1 = allocator_->Alloc(kSize1);
void* pointer1 = allocator_->Alloc(kSize1);
ASSERT_TRUE(pointer1);
EXPECT_EQ(reinterpret_cast<intptr_t>(pointer1) & (kAllocAlignment - 1), 0);
EXPECT_TRUE(base::IsAligned(pointer1, kAllocAlignment));
EXPECT_TRUE(allocator_->CheckConsistency());

const unsigned int kSize2 = 43;
void *pointer2 = allocator_->Alloc(kSize2);
void* pointer2 = allocator_->Alloc(kSize2);
ASSERT_TRUE(pointer2);
EXPECT_EQ(reinterpret_cast<intptr_t>(pointer2) & (kAllocAlignment - 1), 0);
EXPECT_TRUE(base::IsAligned(pointer2, kAllocAlignment));
EXPECT_TRUE(allocator_->CheckConsistency());

allocator_->Free(pointer2);
Expand All @@ -473,18 +473,18 @@ TEST_F(FencedAllocatorWrapperTest, TestOutOfMemory) {

const unsigned int kSize = 16;
const unsigned int kAllocCount = kBufferSize / kSize;
CHECK(kAllocCount * kSize == kBufferSize);
CHECK_EQ(kAllocCount * kSize, kBufferSize);

// Allocate several buffers to fill in the memory.
void *pointers[kAllocCount];
void* pointers[kAllocCount];
for (unsigned int i = 0; i < kAllocCount; ++i) {
pointers[i] = allocator_->Alloc(kSize);
EXPECT_TRUE(pointers[i]);
EXPECT_TRUE(allocator_->CheckConsistency());
}

// This allocation should fail.
void *pointer_failed = allocator_->Alloc(kSize);
void* pointer_failed = allocator_->Alloc(kSize);
EXPECT_FALSE(pointer_failed);
EXPECT_TRUE(allocator_->CheckConsistency());

Expand Down Expand Up @@ -513,18 +513,18 @@ TEST_F(FencedAllocatorWrapperTest, TestFreePendingToken) {

const unsigned int kSize = 16;
const unsigned int kAllocCount = kBufferSize / kSize;
CHECK(kAllocCount * kSize == kBufferSize);
CHECK_EQ(kAllocCount * kSize, kBufferSize);

// Allocate several buffers to fill in the memory.
void *pointers[kAllocCount];
void* pointers[kAllocCount];
for (unsigned int i = 0; i < kAllocCount; ++i) {
pointers[i] = allocator_->Alloc(kSize);
EXPECT_TRUE(pointers[i]);
EXPECT_TRUE(allocator_->CheckConsistency());
}

// This allocation should fail.
void *pointer_failed = allocator_->Alloc(kSize);
void* pointer_failed = allocator_->Alloc(kSize);
EXPECT_FALSE(pointer_failed);
EXPECT_TRUE(allocator_->CheckConsistency());

Expand Down
5 changes: 3 additions & 2 deletions gpu/command_buffer/client/transfer_buffer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <memory>

#include "base/compiler_specific.h"
#include "base/memory/aligned_memory.h"
#include "gpu/command_buffer/client/client_test_helper.h"
#include "gpu/command_buffer/client/cmd_buffer_helper.h"
#include "gpu/command_buffer/common/command_buffer.h"
Expand Down Expand Up @@ -218,11 +219,11 @@ TEST_F(TransferBufferTest, TooLargeAllocation) {
TEST_F(TransferBufferTest, MemoryAlignmentAfterZeroAllocation) {
Initialize();
void* ptr = transfer_buffer_->Alloc(0);
EXPECT_EQ((reinterpret_cast<uintptr_t>(ptr) & (kAlignment - 1)), 0u);
EXPECT_TRUE(base::IsAligned(ptr, kAlignment));
transfer_buffer_->FreePendingToken(ptr, helper_->InsertToken());
// Check that the pointer is aligned on the following allocation.
ptr = transfer_buffer_->Alloc(4);
EXPECT_EQ((reinterpret_cast<uintptr_t>(ptr) & (kAlignment - 1)), 0u);
EXPECT_TRUE(base::IsAligned(ptr, kAlignment));
transfer_buffer_->FreePendingToken(ptr, helper_->InsertToken());
}

Expand Down

0 comments on commit 25c9ff6

Please sign in to comment.