From f55d18ade6877134e87515d9e2fc332dd40dea19 Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Tue, 8 Dec 2015 15:38:17 -0500 Subject: [PATCH] Add AlignedVector and use it for vector MEMORY_BASIC_INFORMATION64 specifies an alignment of 16, but the standard allocator used by containers doesn't honor this. Although 16 is the default alignment size used on Windows for x86_64, it's not for 32-bit x86. clang assumed that the alignment of the structure was as declared, and used an SSE load sequence that required this alignment. AlignedAllocator is a replacement for std::allocator that allows the alignment to be specified. AlignedVector is an std::vector<> that uses AlignedAllocator instead of std::allocator. BUG=chromium:564691 R=scottmg@chromium.org Review URL: https://codereview.chromium.org/1498133002 . --- util/stdlib/aligned_allocator.cc | 78 +++++++++++++++ util/stdlib/aligned_allocator.h | 131 ++++++++++++++++++++++++++ util/stdlib/aligned_allocator_test.cc | 117 +++++++++++++++++++++++ util/util.gyp | 2 + util/util_test.gyp | 1 + util/win/process_info.cc | 7 +- util/win/process_info.h | 23 ++++- util/win/process_info_test.cc | 24 ++--- 8 files changed, 365 insertions(+), 18 deletions(-) create mode 100644 util/stdlib/aligned_allocator.cc create mode 100644 util/stdlib/aligned_allocator.h create mode 100644 util/stdlib/aligned_allocator_test.cc diff --git a/util/stdlib/aligned_allocator.cc b/util/stdlib/aligned_allocator.cc new file mode 100644 index 00000000..363022c8 --- /dev/null +++ b/util/stdlib/aligned_allocator.cc @@ -0,0 +1,78 @@ +// Copyright 2015 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "util/stdlib/aligned_allocator.h" + +#include + +#include "build/build_config.h" + +#if defined(OS_POSIX) +#include +#elif defined(OS_WIN) +#include +#include +#endif // OS_POSIX + +namespace { + +// Throws std::bad_alloc() by calling an internal function provided by the C++ +// library to do so. This works even if C++ exceptions are disabled, causing +// program termination if uncaught. +void ThrowBadAlloc() { +#if defined(OS_POSIX) + // This works with both libc++ and libstdc++. + std::__throw_bad_alloc(); +#elif defined(OS_WIN) + std::_Xbad_alloc(); +#endif // OS_POSIX +} + +} // namespace + +namespace crashpad { +namespace internal { + +void* AlignedAllocate(size_t alignment, size_t size) { +#if defined(OS_POSIX) + // posix_memalign() requires that alignment be at least sizeof(void*), so the + // power-of-2 check needs to happen before potentially changing the alignment. + if (alignment == 0 || alignment & (alignment - 1)) { + ThrowBadAlloc(); + } + + void* pointer; + if (posix_memalign(&pointer, std::max(alignment, sizeof(void*)), size) != 0) { + ThrowBadAlloc(); + } +#elif defined(OS_WIN) + void* pointer = _aligned_malloc(size, alignment); + if (pointer == nullptr) { + ThrowBadAlloc(); + } +#endif // OS_POSIX + + return pointer; +} + +void AlignedFree(void* pointer) { +#if defined(OS_POSIX) + free(pointer); +#elif defined(OS_WIN) + _aligned_free(pointer); +#endif // OS_POSIX +} + +} // namespace internal +} // namespace crashpad diff --git a/util/stdlib/aligned_allocator.h b/util/stdlib/aligned_allocator.h new file mode 100644 index 00000000..38ecf617 --- /dev/null +++ b/util/stdlib/aligned_allocator.h @@ -0,0 +1,131 @@ +// Copyright 2015 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef CRASHPAD_UTIL_STDLIB_ALIGNED_ALLOCATOR_H_ +#define CRASHPAD_UTIL_STDLIB_ALIGNED_ALLOCATOR_H_ + +#include + +#include +#include +#include +#include +#include + +#include "base/compiler_specific.h" +#include "build/build_config.h" + +#if defined(COMPILER_MSVC) && _MSC_VER < 1900 +#define CRASHPAD_NOEXCEPT _NOEXCEPT +#else +#define CRASHPAD_NOEXCEPT noexcept +#endif + +namespace crashpad { +namespace internal { + +//! \brief Allocates memory with the specified alignment constraint. +//! +//! This function wraps `posix_memalign()` or `_aligned_malloc()`. Memory +//! allocated by this function must be released by AlignFree(). +void* AlignedAllocate(size_t alignment, size_t size); + +//! \brief Frees memory allocated by AlignedAllocate(). +//! +//! This function wraps `free()` or `_aligned_free()`. +void AlignedFree(void* pointer); + +} // namespace internal + +//! \brief A standard allocator that aligns its allocations as requested, +//! suitable for use as an allocator in standard containers. +//! +//! This is similar to `std::allocator`, with the addition of an alignment +//! guarantee. \a Alignment must be a power of 2. If \a Alignment is not +//! specified, the default alignment for type \a T is used. +template +struct AlignedAllocator { + public: + using value_type = T; + using pointer = T*; + using const_pointer = const T*; + using reference = T&; + using const_reference = const T&; + using size_type = size_t; + using difference_type = ptrdiff_t; + + template + struct rebind { + using other = AlignedAllocator; + }; + + AlignedAllocator() CRASHPAD_NOEXCEPT {} + AlignedAllocator(const AlignedAllocator& other) CRASHPAD_NOEXCEPT {} + + template + AlignedAllocator(const AlignedAllocator& other) + CRASHPAD_NOEXCEPT {} + + ~AlignedAllocator() {} + + pointer address(reference x) CRASHPAD_NOEXCEPT { return &x; } + const_pointer address(const_reference x) CRASHPAD_NOEXCEPT { return &x; } + + pointer allocate(size_type n, std::allocator::const_pointer hint = 0) { + return reinterpret_cast( + internal::AlignedAllocate(Alignment, sizeof(value_type) * n)); + } + + void deallocate(pointer p, size_type n) { internal::AlignedFree(p); } + + size_type max_size() const CRASHPAD_NOEXCEPT { + return std::numeric_limits::max() / sizeof(value_type); + } + + template + void construct(U* p, Args&&... args) { + new (reinterpret_cast(p)) U(std::forward(args)...); + } + + template + void destroy(U* p) { + p->~U(); + } +}; + +template +bool operator==(const AlignedAllocator& lhs, + const AlignedAllocator& rhs) CRASHPAD_NOEXCEPT { + return true; +} + +template +bool operator!=(const AlignedAllocator& lhs, + const AlignedAllocator& rhs) CRASHPAD_NOEXCEPT { + return false; +} + +//! \brief A `std::vector` using AlignedAllocator. +//! +//! This is similar to `std::vector`, with the addition of an alignment +//! guarantee. \a Alignment must be a power of 2. If \a Alignment is not +//! specified, the default alignment for type \a T is used. +template +using AlignedVector = std::vector>; + +} // namespace crashpad + +#undef CRASHPAD_NOEXCEPT + +#endif // CRASHPAD_UTIL_STDLIB_ALIGNED_ALLOCATOR_H_ diff --git a/util/stdlib/aligned_allocator_test.cc b/util/stdlib/aligned_allocator_test.cc new file mode 100644 index 00000000..fe3b9e65 --- /dev/null +++ b/util/stdlib/aligned_allocator_test.cc @@ -0,0 +1,117 @@ +// Copyright 2015 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "util/stdlib/aligned_allocator.h" + +#include + +#include "gtest/gtest.h" + +#if defined(OS_WIN) +#include +#endif + +namespace crashpad { +namespace test { +namespace { + +bool IsAligned(void* pointer, size_t alignment) { + uintptr_t address = reinterpret_cast(pointer); + return (address & (alignment - 1)) == 0; +} + +TEST(AlignedAllocator, AlignedVector) { + // Test a structure with natural alignment. + struct NaturalAlignedStruct { + int i; + }; + + AlignedVector natural_aligned_vector; + natural_aligned_vector.push_back(NaturalAlignedStruct()); + EXPECT_TRUE( + IsAligned(&natural_aligned_vector[0], ALIGNOF(NaturalAlignedStruct))); + + natural_aligned_vector.resize(3); + EXPECT_TRUE( + IsAligned(&natural_aligned_vector[0], ALIGNOF(NaturalAlignedStruct))); + EXPECT_TRUE( + IsAligned(&natural_aligned_vector[1], ALIGNOF(NaturalAlignedStruct))); + EXPECT_TRUE( + IsAligned(&natural_aligned_vector[2], ALIGNOF(NaturalAlignedStruct))); + + // Test a structure that declares its own alignment. + struct ALIGNAS(32) AlignedStruct { + int i; + }; + ASSERT_EQ(32u, ALIGNOF(AlignedStruct)); + + AlignedVector aligned_vector; + aligned_vector.push_back(AlignedStruct()); + EXPECT_TRUE(IsAligned(&aligned_vector[0], ALIGNOF(AlignedStruct))); + + aligned_vector.resize(3); + EXPECT_TRUE(IsAligned(&aligned_vector[0], ALIGNOF(AlignedStruct))); + EXPECT_TRUE(IsAligned(&aligned_vector[1], ALIGNOF(AlignedStruct))); + EXPECT_TRUE(IsAligned(&aligned_vector[2], ALIGNOF(AlignedStruct))); + + // Try a custom alignment. Since the structure itself doesn’t specify an + // alignment constraint, only the base address will be aligned to the + // requested boundary. + AlignedVector custom_aligned_vector; + custom_aligned_vector.push_back(NaturalAlignedStruct()); + EXPECT_TRUE(IsAligned(&custom_aligned_vector[0], 64)); + + // Try a structure with a pretty big alignment request. + struct ALIGNAS(1024) BigAlignedStruct { + int i; + }; + ASSERT_EQ(1024u, ALIGNOF(BigAlignedStruct)); + + AlignedVector big_aligned_vector; + big_aligned_vector.push_back(BigAlignedStruct()); + EXPECT_TRUE(IsAligned(&big_aligned_vector[0], ALIGNOF(BigAlignedStruct))); + + big_aligned_vector.resize(3); + EXPECT_TRUE(IsAligned(&big_aligned_vector[0], ALIGNOF(BigAlignedStruct))); + EXPECT_TRUE(IsAligned(&big_aligned_vector[1], ALIGNOF(BigAlignedStruct))); + EXPECT_TRUE(IsAligned(&big_aligned_vector[2], ALIGNOF(BigAlignedStruct))); +} + +void BadAlignmentTest() { +#if defined(OS_WIN) + // Suppress the assertion MessageBox() normally displayed by the CRT in debug + // mode. + int previous = _CrtSetReportMode(_CRT_ASSERT, _CRTDBG_MODE_DEBUG); + + // In release mode, _CrtSetReportMode() is #defined to ((int)0), so |previous| + // would appear unused. + ALLOW_UNUSED_LOCAL(previous); +#endif + + // Alignment constraints must be powers of 2. 7 is not valid. + AlignedVector bad_aligned_vector; + bad_aligned_vector.push_back(0); + +#if defined(OS_WIN) + _CrtSetReportMode(_CRT_ASSERT, previous); +#endif +} + +TEST(AlignedAllocatorDeathTest, BadAlignment) { + ASSERT_DEATH(BadAlignmentTest(), ""); +} + +} // namespace +} // namespace test +} // namespace crashpad diff --git a/util/util.gyp b/util/util.gyp index eb5f898f..5e89ec09 100644 --- a/util/util.gyp +++ b/util/util.gyp @@ -128,6 +128,8 @@ 'posix/process_info_mac.cc', 'posix/symbolic_constants_posix.cc', 'posix/symbolic_constants_posix.h', + 'stdlib/aligned_allocator.cc', + 'stdlib/aligned_allocator.h', 'stdlib/cxx.h', 'stdlib/map_insert.h', 'stdlib/move.h', diff --git a/util/util_test.gyp b/util/util_test.gyp index 2d94ea71..f5adc039 100644 --- a/util/util_test.gyp +++ b/util/util_test.gyp @@ -72,6 +72,7 @@ 'numeric/int128_test.cc', 'posix/process_info_test.cc', 'posix/symbolic_constants_posix_test.cc', + 'stdlib/aligned_allocator_test.cc', 'stdlib/map_insert_test.cc', 'stdlib/string_number_conversion_test.cc', 'stdlib/strlcpy_test.cc', diff --git a/util/win/process_info.cc b/util/win/process_info.cc index 1a07df1d..5628e04b 100644 --- a/util/win/process_info.cc +++ b/util/win/process_info.cc @@ -589,7 +589,8 @@ bool ProcessInfo::Modules(std::vector* modules) const { return true; } -const std::vector& ProcessInfo::MemoryInfo() const { +const ProcessInfo::MemoryBasicInformation64Vector& ProcessInfo::MemoryInfo() + const { INITIALIZATION_STATE_DCHECK_VALID(initialized_); return memory_info_; } @@ -629,11 +630,11 @@ const std::vector& ProcessInfo::Handles() const { std::vector> GetReadableRangesOfMemoryMap( const CheckedRange& range, - const std::vector& memory_info) { + const ProcessInfo::MemoryBasicInformation64Vector& memory_info) { using Range = CheckedRange; // Find all the ranges that overlap the target range, maintaining their order. - std::vector overlapping; + ProcessInfo::MemoryBasicInformation64Vector overlapping; for (const auto& mi : memory_info) { static_assert(base::is_same::value, "expected range address to be WinVMAddress"); diff --git a/util/win/process_info.h b/util/win/process_info.h index fb1b8b3e..03624451 100644 --- a/util/win/process_info.h +++ b/util/win/process_info.h @@ -24,6 +24,7 @@ #include "base/basictypes.h" #include "util/misc/initialization_state_dcheck.h" #include "util/numeric/checked_range.h" +#include "util/stdlib/aligned_allocator.h" #include "util/win/address_types.h" namespace crashpad { @@ -32,6 +33,10 @@ namespace crashpad { //! primarily of information stored in the Process Environment Block. class ProcessInfo { public: + //! \brief The return type of MemoryInfo(), for convenience. + using MemoryBasicInformation64Vector = + AlignedVector; + //! \brief Contains information about a module loaded into a process. struct Module { Module(); @@ -124,7 +129,7 @@ class ProcessInfo { bool Modules(std::vector* modules) const; //! \brief Retrieves information about all pages mapped into the process. - const std::vector& MemoryInfo() const; + const MemoryBasicInformation64Vector& MemoryInfo() const; //! \brief Given a range to be read from the target process, returns a vector //! of ranges, representing the readable portions of the original range. @@ -174,7 +179,19 @@ class ProcessInfo { WinVMAddress peb_address_; WinVMSize peb_size_; std::vector modules_; - std::vector memory_info_; + + // memory_info_ is a MemoryBasicInformation64Vector instead of a + // std::vector because MEMORY_BASIC_INFORMATION64 + // is declared with __declspec(align(16)), but std::vector<> does not maintain + // this alignment on 32-bit x86. clang-cl (but not MSVC cl) takes advantage of + // the presumed alignment and emits SSE instructions that require aligned + // storage. clang-cl should relax (unfortunately), but in the mean time, this + // provides aligned storage. See https://crbug.com/564691 and + // http://llvm.org/PR25779. + // + // TODO(mark): Remove this workaround when http://llvm.org/PR25779 is fixed + // and the fix is present in the clang-cl that compiles this code. + MemoryBasicInformation64Vector memory_info_; // Handles() is logically const, but updates this member on first retrieval. // See https://crashpad.chromium.org/bug/9. @@ -195,7 +212,7 @@ class ProcessInfo { //! ProcessInfo::GetReadableRanges(). std::vector> GetReadableRangesOfMemoryMap( const CheckedRange& range, - const std::vector& memory_info); + const ProcessInfo::MemoryBasicInformation64Vector& memory_info); } // namespace crashpad diff --git a/util/win/process_info_test.cc b/util/win/process_info_test.cc index e5bde2f6..6917ed9e 100644 --- a/util/win/process_info_test.cc +++ b/util/win/process_info_test.cc @@ -59,7 +59,7 @@ void VerifyAddressInInCodePage(const ProcessInfo& process_info, WinVMAddress code_address) { // Make sure the child code address is an code page address with the right // information. - const std::vector& memory_info = + const ProcessInfo::MemoryBasicInformation64Vector& memory_info = process_info.MemoryInfo(); bool found_region = false; for (const auto& mi : memory_info) { @@ -199,7 +199,7 @@ TEST(ProcessInfo, OtherProcessWOW64) { #endif // ARCH_CPU_64_BITS TEST(ProcessInfo, AccessibleRangesNone) { - std::vector memory_info; + ProcessInfo::MemoryBasicInformation64Vector memory_info; MEMORY_BASIC_INFORMATION64 mbi = {0}; mbi.BaseAddress = 0; @@ -215,7 +215,7 @@ TEST(ProcessInfo, AccessibleRangesNone) { } TEST(ProcessInfo, AccessibleRangesOneInside) { - std::vector memory_info; + ProcessInfo::MemoryBasicInformation64Vector memory_info; MEMORY_BASIC_INFORMATION64 mbi = {0}; mbi.BaseAddress = 0; @@ -233,7 +233,7 @@ TEST(ProcessInfo, AccessibleRangesOneInside) { } TEST(ProcessInfo, AccessibleRangesOneTruncatedSize) { - std::vector memory_info; + ProcessInfo::MemoryBasicInformation64Vector memory_info; MEMORY_BASIC_INFORMATION64 mbi = {0}; mbi.BaseAddress = 0; @@ -256,7 +256,7 @@ TEST(ProcessInfo, AccessibleRangesOneTruncatedSize) { } TEST(ProcessInfo, AccessibleRangesOneMovedStart) { - std::vector memory_info; + ProcessInfo::MemoryBasicInformation64Vector memory_info; MEMORY_BASIC_INFORMATION64 mbi = {0}; mbi.BaseAddress = 0; @@ -279,7 +279,7 @@ TEST(ProcessInfo, AccessibleRangesOneMovedStart) { } TEST(ProcessInfo, ReserveIsInaccessible) { - std::vector memory_info; + ProcessInfo::MemoryBasicInformation64Vector memory_info; MEMORY_BASIC_INFORMATION64 mbi = {0}; mbi.BaseAddress = 0; @@ -302,7 +302,7 @@ TEST(ProcessInfo, ReserveIsInaccessible) { } TEST(ProcessInfo, PageGuardIsInaccessible) { - std::vector memory_info; + ProcessInfo::MemoryBasicInformation64Vector memory_info; MEMORY_BASIC_INFORMATION64 mbi = {0}; mbi.BaseAddress = 0; @@ -327,7 +327,7 @@ TEST(ProcessInfo, PageGuardIsInaccessible) { } TEST(ProcessInfo, PageNoAccessIsInaccessible) { - std::vector memory_info; + ProcessInfo::MemoryBasicInformation64Vector memory_info; MEMORY_BASIC_INFORMATION64 mbi = {0}; mbi.BaseAddress = 0; @@ -352,7 +352,7 @@ TEST(ProcessInfo, PageNoAccessIsInaccessible) { } TEST(ProcessInfo, AccessibleRangesCoalesced) { - std::vector memory_info; + ProcessInfo::MemoryBasicInformation64Vector memory_info; MEMORY_BASIC_INFORMATION64 mbi = {0}; mbi.BaseAddress = 0; @@ -380,7 +380,7 @@ TEST(ProcessInfo, AccessibleRangesCoalesced) { } TEST(ProcessInfo, AccessibleRangesMiddleUnavailable) { - std::vector memory_info; + ProcessInfo::MemoryBasicInformation64Vector memory_info; MEMORY_BASIC_INFORMATION64 mbi = {0}; mbi.BaseAddress = 0; @@ -410,7 +410,7 @@ TEST(ProcessInfo, AccessibleRangesMiddleUnavailable) { } TEST(ProcessInfo, RequestedBeforeMap) { - std::vector memory_info; + ProcessInfo::MemoryBasicInformation64Vector memory_info; MEMORY_BASIC_INFORMATION64 mbi = {0}; mbi.BaseAddress = 10; @@ -428,7 +428,7 @@ TEST(ProcessInfo, RequestedBeforeMap) { } TEST(ProcessInfo, RequestedAfterMap) { - std::vector memory_info; + ProcessInfo::MemoryBasicInformation64Vector memory_info; MEMORY_BASIC_INFORMATION64 mbi = {0}; mbi.BaseAddress = 10;