From 25c9ff6476962164df3965a17834be76641a5c4b Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Fri, 19 Jun 2020 02:30:43 +0000 Subject: [PATCH] Use base::IsAligned() in more places. 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 Reviewed-by: James Darpinian Reviewed-by: Peng Huang Commit-Queue: Lei Zhang Cr-Commit-Position: refs/heads/master@{#780133} --- base/containers/stack_container_unittest.cc | 20 +++++------ base/lazy_instance_unittest.cc | 11 +++--- base/memory/platform_shared_memory_region.cc | 10 +++--- base/memory/singleton_unittest.cc | 12 +++---- .../common/discardable_shared_memory_heap.cc | 8 ++--- .../client/fenced_allocator_test.cc | 34 +++++++++---------- .../client/transfer_buffer_unittest.cc | 5 +-- 7 files changed, 48 insertions(+), 52 deletions(-) diff --git a/base/containers/stack_container_unittest.cc b/base/containers/stack_container_unittest.cc index 7fa609556a7436..e0f5dc5baebcb6 100644 --- a/base/containers/stack_container_unittest.cc +++ b/base/containers/stack_container_unittest.cc @@ -8,6 +8,7 @@ #include +#include "base/memory/aligned_memory.h" #include "base/memory/ref_counted.h" #include "build/build_config.h" #include "testing/gtest/include/gtest/gtest.h" @@ -16,14 +17,14 @@ namespace base { namespace { -class Dummy : public base::RefCounted { +class Dummy : public RefCounted { public: explicit Dummy(int* alive) : alive_(alive) { ++*alive_; } private: - friend class base::RefCounted; + friend class RefCounted; ~Dummy() { --*alive_; @@ -110,31 +111,28 @@ class AlignedData { alignas(alignment) char data_[alignment]; }; -} // anonymous namespace - -#define EXPECT_ALIGNED(ptr, align) \ - EXPECT_EQ(0u, reinterpret_cast(ptr) & (align - 1)) +} // namespace TEST(StackContainer, BufferAlignment) { StackVector text; text->push_back(L'A'); - EXPECT_ALIGNED(&text[0], alignof(wchar_t)); + EXPECT_TRUE(IsAligned(&text[0], alignof(wchar_t))); StackVector doubles; doubles->push_back(0.0); - EXPECT_ALIGNED(&doubles[0], alignof(double)); + EXPECT_TRUE(IsAligned(&doubles[0], alignof(double))); StackVector, 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, 1> aligned256; aligned256->push_back(AlignedData<256>()); - EXPECT_ALIGNED(&aligned256[0], 256); + EXPECT_TRUE(IsAligned(&aligned256[0], 256)); #endif } diff --git a/base/lazy_instance_unittest.cc b/base/lazy_instance_unittest.cc index d1012ef6984813..ad1f0ebf44c114 100644 --- a/base/lazy_instance_unittest.cc +++ b/base/lazy_instance_unittest.cc @@ -5,6 +5,7 @@ #include #include +#include #include #include "base/at_exit.h" @@ -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" @@ -178,9 +180,6 @@ class AlignedData { } // namespace -#define EXPECT_ALIGNED(ptr, align) \ - EXPECT_EQ(0u, reinterpret_cast(ptr) & (align - 1)) - TEST(LazyInstanceTest, Alignment) { using base::LazyInstance; @@ -194,9 +193,9 @@ TEST(LazyInstanceTest, Alignment) { static LazyInstance>::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 { diff --git a/base/memory/platform_shared_memory_region.cc b/base/memory/platform_shared_memory_region.cc index 944b12cb297a86..964844adff691e 100644 --- a/base/memory/platform_shared_memory_region.cc +++ b/base/memory/platform_shared_memory_region.cc @@ -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" @@ -15,7 +16,7 @@ namespace subtle { namespace { void RecordMappingWasBlockedHistogram(bool blocked) { - base::UmaHistogramBoolean("SharedMemory.MapBlockedForSecurity", blocked); + UmaHistogramBoolean("SharedMemory.MapBlockedForSecurity", blocked); } } // namespace @@ -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(*memory) & (kMapMinimumAlignment - 1)); + DCHECK(IsAligned(*memory, kMapMinimumAlignment)); } else { SharedMemorySecurityPolicy::ReleaseReservationForMapping(size); } diff --git a/base/memory/singleton_unittest.cc b/base/memory/singleton_unittest.cc index 815d6f2b404c88..be2253f27f0c85 100644 --- a/base/memory/singleton_unittest.cc +++ b/base/memory/singleton_unittest.cc @@ -5,6 +5,7 @@ #include #include "base/at_exit.h" +#include "base/memory/aligned_memory.h" #include "base/memory/singleton.h" #include "testing/gtest/include/gtest/gtest.h" @@ -278,9 +279,6 @@ TEST_F(SingletonTest, Basic) { VerifiesCallbacksNotCalled(); } -#define EXPECT_ALIGNED(ptr, align) \ - EXPECT_EQ(0u, reinterpret_cast(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 @@ -294,10 +292,10 @@ TEST_F(SingletonTest, Alignment) { AlignedTestSingleton>* align4096 = AlignedTestSingleton>::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 diff --git a/components/discardable_memory/common/discardable_shared_memory_heap.cc b/components/discardable_memory/common/discardable_shared_memory_heap.cc index 86a0a8805a0727..2e06fa83549725 100644 --- a/components/discardable_memory/common/discardable_shared_memory_heap.cc +++ b/components/discardable_memory/common/discardable_shared_memory_heap.cc @@ -6,9 +6,11 @@ #include #include +#include #include #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" @@ -116,10 +118,8 @@ DiscardableSharedMemoryHeap::Grow( int32_t id, base::OnceClosure deleted_callback) { // Memory must be aligned to block size. - DCHECK_EQ( - reinterpret_cast(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( new Span(shared_memory.get(), diff --git a/gpu/command_buffer/client/fenced_allocator_test.cc b/gpu/command_buffer/client/fenced_allocator_test.cc index ed09dd197f6a26..f68bf95f8b3def 100644 --- a/gpu/command_buffer/client/fenced_allocator_test.cc +++ b/gpu/command_buffer/client/fenced_allocator_test.cc @@ -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]; @@ -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]; @@ -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]; @@ -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(pointer)); EXPECT_GE(kBufferSize, static_cast(pointer) - buffer_.get() + kSize); @@ -415,14 +415,14 @@ TEST_F(FencedAllocatorWrapperTest, TestBasic) { allocator_->Free(pointer); EXPECT_TRUE(allocator_->CheckConsistency()); - char *pointer_char = allocator_->AllocTyped(kSize); + char* pointer_char = allocator_->AllocTyped(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(kSize); + unsigned int* pointer_uint = allocator_->AllocTyped(kSize); ASSERT_TRUE(pointer_uint); EXPECT_LE(buffer_.get(), reinterpret_cast(pointer_uint)); EXPECT_GE(buffer_.get() + kBufferSize, @@ -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()); } @@ -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(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(pointer2) & (kAllocAlignment - 1), 0); + EXPECT_TRUE(base::IsAligned(pointer2, kAllocAlignment)); EXPECT_TRUE(allocator_->CheckConsistency()); allocator_->Free(pointer2); @@ -473,10 +473,10 @@ 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]); @@ -484,7 +484,7 @@ TEST_F(FencedAllocatorWrapperTest, TestOutOfMemory) { } // This allocation should fail. - void *pointer_failed = allocator_->Alloc(kSize); + void* pointer_failed = allocator_->Alloc(kSize); EXPECT_FALSE(pointer_failed); EXPECT_TRUE(allocator_->CheckConsistency()); @@ -513,10 +513,10 @@ 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]); @@ -524,7 +524,7 @@ TEST_F(FencedAllocatorWrapperTest, TestFreePendingToken) { } // This allocation should fail. - void *pointer_failed = allocator_->Alloc(kSize); + void* pointer_failed = allocator_->Alloc(kSize); EXPECT_FALSE(pointer_failed); EXPECT_TRUE(allocator_->CheckConsistency()); diff --git a/gpu/command_buffer/client/transfer_buffer_unittest.cc b/gpu/command_buffer/client/transfer_buffer_unittest.cc index 8ca8609db9e7b0..6bd739d0cc6d98 100644 --- a/gpu/command_buffer/client/transfer_buffer_unittest.cc +++ b/gpu/command_buffer/client/transfer_buffer_unittest.cc @@ -12,6 +12,7 @@ #include #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" @@ -218,11 +219,11 @@ TEST_F(TransferBufferTest, TooLargeAllocation) { TEST_F(TransferBufferTest, MemoryAlignmentAfterZeroAllocation) { Initialize(); void* ptr = transfer_buffer_->Alloc(0); - EXPECT_EQ((reinterpret_cast(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(ptr) & (kAlignment - 1)), 0u); + EXPECT_TRUE(base::IsAligned(ptr, kAlignment)); transfer_buffer_->FreePendingToken(ptr, helper_->InsertToken()); }