diff --git a/minidump/minidump_context_writer.cc b/minidump/minidump_context_writer.cc index c6c29d96..218d7775 100644 --- a/minidump/minidump_context_writer.cc +++ b/minidump/minidump_context_writer.cc @@ -23,6 +23,7 @@ #include "base/logging.h" #include "snapshot/cpu_context.h" #include "util/file/file_writer.h" +#include "util/stdlib/aligned_allocator.h" namespace crashpad { @@ -65,10 +66,8 @@ MinidumpContextWriter::CreateFromSnapshot(const CPUContext* context_snapshot) { } case kCPUArchitectureX86_64: { - MSVC_PUSH_DISABLE_WARNING(4316); // Object on heap may not be aligned. MinidumpContextAMD64Writer* context_amd64 = new MinidumpContextAMD64Writer(); - MSVC_POP_WARNING(); // C4316 context.reset(context_amd64); context_amd64->InitializeFromSnapshot(context_snapshot->x86_64); break; @@ -152,6 +151,12 @@ size_t MinidumpContextX86Writer::ContextSize() const { return sizeof(context_); } +static_assert(alignof(MinidumpContextAMD64) >= 16, + "MinidumpContextAMD64 alignment"); +static_assert(alignof(MinidumpContextAMD64Writer) >= + alignof(MinidumpContextAMD64), + "MinidumpContextAMD64Writer alignment"); + MinidumpContextAMD64Writer::MinidumpContextAMD64Writer() : MinidumpContextWriter(), context_() { context_.context_flags = kMinidumpContextAMD64; @@ -160,6 +165,20 @@ MinidumpContextAMD64Writer::MinidumpContextAMD64Writer() MinidumpContextAMD64Writer::~MinidumpContextAMD64Writer() { } +// static +void* MinidumpContextAMD64Writer::operator new(size_t size) { + // MinidumpContextAMD64 requests an alignment of 16, which can be larger than + // what standard new provides. This may trigger MSVC warning C4316. As a + // workaround to this language deficiency, provide a custom allocation + // function to allocate a block meeting the alignment requirement. + return AlignedAllocate(alignof(MinidumpContextAMD64Writer), size); +} + +// static +void MinidumpContextAMD64Writer::operator delete(void* pointer) { + return AlignedFree(pointer); +} + void MinidumpContextAMD64Writer::InitializeFromSnapshot( const CPUContextX86_64* context_snapshot) { DCHECK_EQ(state(), kStateMutable); diff --git a/minidump/minidump_context_writer.h b/minidump/minidump_context_writer.h index 29bc3ee6..25d717e5 100644 --- a/minidump/minidump_context_writer.h +++ b/minidump/minidump_context_writer.h @@ -110,6 +110,16 @@ class MinidumpContextAMD64Writer final : public MinidumpContextWriter { MinidumpContextAMD64Writer(); ~MinidumpContextAMD64Writer() override; + // Ensure proper alignment of heap-allocated objects. This should not be + // necessary in C++17. + static void* operator new(size_t size); + static void operator delete(void* ptr); + + // Prevent unaligned heap-allocated arrays. Provisions could be made to allow + // these if necessary, but there is currently no use for them. + static void* operator new[](size_t size) = delete; + static void operator delete[](void* ptr) = delete; + //! \brief Initializes the MinidumpContextAMD64 based on \a context_snapshot. //! //! \param[in] context_snapshot The context snapshot to use as source data. diff --git a/minidump/minidump_context_writer_test.cc b/minidump/minidump_context_writer_test.cc index 0c2b5ea2..82b4db75 100644 --- a/minidump/minidump_context_writer_test.cc +++ b/minidump/minidump_context_writer_test.cc @@ -69,6 +69,15 @@ TEST(MinidumpContextWriter, MinidumpContextX86Writer) { } TEST(MinidumpContextWriter, MinidumpContextAMD64Writer) { + { + // Make sure that a heap-allocated context writer has the proper alignment, + // because it may be nonstandard. + auto context_writer = std::make_unique(); + EXPECT_EQ(reinterpret_cast(context_writer.get()) & + (alignof(MinidumpContextAMD64Writer) - 1), + 0u); + } + StringFile string_file; { diff --git a/minidump/minidump_thread_writer_test.cc b/minidump/minidump_thread_writer_test.cc index 3e8e7de7..60173da2 100644 --- a/minidump/minidump_thread_writer_test.cc +++ b/minidump/minidump_thread_writer_test.cc @@ -220,13 +220,7 @@ TEST(MinidumpThreadWriter, OneThread_AMD64_Stack) { kMemoryBase, kMemorySize, kMemoryValue); thread_writer->SetStack(std::move(memory_writer)); - // Object allocated on heap may not be aligned. - MSVC_PUSH_DISABLE_WARNING(4316); - // This would use std::make_unique, but since the “new” would be in - // and not here, MSVC_PUSH_DISABLE_WARNING wouldn’t have the intended effect. - std::unique_ptr context_amd64_writer( - new MinidumpContextAMD64Writer()); - MSVC_POP_WARNING(); // C4316. + auto context_amd64_writer = std::make_unique(); InitializeMinidumpContextAMD64(context_amd64_writer->context(), kSeed); thread_writer->SetContext(std::move(context_amd64_writer)); diff --git a/util/stdlib/aligned_allocator.cc b/util/stdlib/aligned_allocator.cc index 363022c8..9aedb299 100644 --- a/util/stdlib/aligned_allocator.cc +++ b/util/stdlib/aligned_allocator.cc @@ -42,7 +42,6 @@ void ThrowBadAlloc() { } // namespace namespace crashpad { -namespace internal { void* AlignedAllocate(size_t alignment, size_t size) { #if defined(OS_POSIX) @@ -74,5 +73,4 @@ void AlignedFree(void* pointer) { #endif // OS_POSIX } -} // namespace internal } // namespace crashpad diff --git a/util/stdlib/aligned_allocator.h b/util/stdlib/aligned_allocator.h index c3beb741..97be27a2 100644 --- a/util/stdlib/aligned_allocator.h +++ b/util/stdlib/aligned_allocator.h @@ -24,7 +24,6 @@ #include namespace crashpad { -namespace internal { //! \brief Allocates memory with the specified alignment constraint. //! @@ -37,8 +36,6 @@ void* AlignedAllocate(size_t alignment, size_t size); //! 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. //! @@ -74,10 +71,10 @@ struct AlignedAllocator { pointer allocate(size_type n, std::allocator::const_pointer hint = 0) { return reinterpret_cast( - internal::AlignedAllocate(Alignment, sizeof(value_type) * n)); + AlignedAllocate(Alignment, sizeof(value_type) * n)); } - void deallocate(pointer p, size_type n) { internal::AlignedFree(p); } + void deallocate(pointer p, size_type n) { AlignedFree(p); } size_type max_size() const noexcept { return std::numeric_limits::max() / sizeof(value_type);