| Message ID | 20190522204005.GA30214@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | PR libstdc++/77691 fix resource_adaptor failures due to max_align_t bugs | expand |
On 22/05/19 21:40 +0100, Jonathan Wakely wrote: >Remove the hardcoded whitelist of allocators expected to return memory >aligned to alignof(max_align_t), because that doesn't work when the >platform's malloc() and GCC's max_align_t do not agree what the largest >fundamental alignment is. It's also sub-optimal for user-defined >allocators that return memory suitable for any fundamental alignment. > >Instead use a hardcoded list of alignments that are definitely supported >by the platform malloc, and use a copy of the allocator rebound to a POD >type with the requested alignment. Only allocate an oversized >buffer to use with std::align for alignments larger than any of the >hardcoded values. > >For 32-bit Solaris x86 do not include alignof(max_align_t) in the >hardcoded values. > > PR libstdc++/77691 > * include/experimental/memory_resource: Add system header pragma. > (__resource_adaptor_common::__guaranteed_alignment): Remove. > (__resource_adaptor_common::_Types) > (__resource_adaptor_common::__new_list) > (__resource_adaptor_common::_New_list) > (__resource_adaptor_common::_Alignments) > (__resource_adaptor_common::_Fund_align_types): New utilities for > creating a list of types with fundamental alignments. > (__resource_adaptor_imp::do_allocate): Call new _M_allocate function. > (__resource_adaptor_imp::do_deallocate): Call new _M_deallocate > function. > (__resource_adaptor_imp::_M_allocate): New function that first tries > to use an allocator rebound to a type with a fundamental alignment. > (__resource_adaptor_imp::_M_deallocate): Likewise for deallocation. > * testsuite/experimental/memory_resource/new_delete_resource.cc: > Adjust expected allocation sizes. > * testsuite/experimental/memory_resource/resource_adaptor.cc: Remove > xfail. Gah, that's the wrong ChangeLog, from an earlier version of the patch. The right one is in the ChangeLog file, but the svn commit has the wrong info, sorry. The correct one is: PR libstdc++/77691 * include/experimental/memory_resource: Add system header pragma and do not define anything unless compiled as C++14 or later. (__resource_adaptor_common::__guaranteed_alignment): Remove. (__resource_adaptor_imp::do_allocate): If the requested alignment is a fundamental alignment then either allocate directly from _M_alloc or call the new _M_allocate function. (__resource_adaptor_imp::do_deallocate): Likewise for deallocation. (__resource_adaptor_imp::_M_allocate): New function that uses a copy of the allocator rebound to a POD type with the specified alignment. (__resource_adaptor_imp::_M_deallocate): Likewise for deallocation. * testsuite/experimental/memory_resource/new_delete_resource.cc: Adjust expected allocation sizes. * testsuite/experimental/memory_resource/resource_adaptor.cc: Remove xfail for Solaris x86.
On 22/05/19 23:17 +0100, Jonathan Wakely wrote: >On 22/05/19 21:40 +0100, Jonathan Wakely wrote: >>Remove the hardcoded whitelist of allocators expected to return memory >>aligned to alignof(max_align_t), because that doesn't work when the >>platform's malloc() and GCC's max_align_t do not agree what the largest >>fundamental alignment is. It's also sub-optimal for user-defined >>allocators that return memory suitable for any fundamental alignment. >> >>Instead use a hardcoded list of alignments that are definitely supported >>by the platform malloc, and use a copy of the allocator rebound to a POD >>type with the requested alignment. Only allocate an oversized >>buffer to use with std::align for alignments larger than any of the >>hardcoded values. >> >>For 32-bit Solaris x86 do not include alignof(max_align_t) in the >>hardcoded values. >> >> PR libstdc++/77691 >> * include/experimental/memory_resource: Add system header pragma. >> (__resource_adaptor_common::__guaranteed_alignment): Remove. >> (__resource_adaptor_common::_Types) >> (__resource_adaptor_common::__new_list) >> (__resource_adaptor_common::_New_list) >> (__resource_adaptor_common::_Alignments) >> (__resource_adaptor_common::_Fund_align_types): New utilities for >> creating a list of types with fundamental alignments. >> (__resource_adaptor_imp::do_allocate): Call new _M_allocate function. >> (__resource_adaptor_imp::do_deallocate): Call new _M_deallocate >> function. >> (__resource_adaptor_imp::_M_allocate): New function that first tries >> to use an allocator rebound to a type with a fundamental alignment. >> (__resource_adaptor_imp::_M_deallocate): Likewise for deallocation. >> * testsuite/experimental/memory_resource/new_delete_resource.cc: >> Adjust expected allocation sizes. >> * testsuite/experimental/memory_resource/resource_adaptor.cc: Remove >> xfail. > >Gah, that's the wrong ChangeLog, from an earlier version of the patch. >The right one is in the ChangeLog file, but the svn commit has the >wrong info, sorry. > >The correct one is: > > PR libstdc++/77691 > * include/experimental/memory_resource: Add system header pragma and > do not define anything unless compiled as C++14 or later. > (__resource_adaptor_common::__guaranteed_alignment): Remove. > (__resource_adaptor_imp::do_allocate): If the requested alignment > is a fundamental alignment then either allocate directly from _M_alloc > or call the new _M_allocate function. > (__resource_adaptor_imp::do_deallocate): Likewise for deallocation. > (__resource_adaptor_imp::_M_allocate): New function that uses a copy > of the allocator rebound to a POD type with the specified alignment. > (__resource_adaptor_imp::_M_deallocate): Likewise for deallocation. > * testsuite/experimental/memory_resource/new_delete_resource.cc: > Adjust expected allocation sizes. > * testsuite/experimental/memory_resource/resource_adaptor.cc: Remove > xfail for Solaris x86. Double-gah, the tests are failing for 32-bit x86-linux, but not on my machine. Presumably because glibc malloc only used to guarantee 8-byte alignment until fairly recently.
diff --git a/libstdc++-v3/include/experimental/memory_resource b/libstdc++-v3/include/experimental/memory_resource index a0b30632154..dde3753fab7 100644 --- a/libstdc++-v3/include/experimental/memory_resource +++ b/libstdc++-v3/include/experimental/memory_resource @@ -30,6 +30,10 @@ #ifndef _GLIBCXX_EXPERIMENTAL_MEMORY_RESOURCE #define _GLIBCXX_EXPERIMENTAL_MEMORY_RESOURCE 1 +#pragma GCC system_header + +#if __cplusplus >= 201402L + #include <memory> // align, uses_allocator, __uses_alloc #include <experimental/utility> // pair, experimental::erased_type #include <atomic> // atomic @@ -358,23 +362,6 @@ namespace pmr { return __val; } }; - - template<typename _Alloc> - struct __guaranteed_alignment : std::integral_constant<size_t, 1> { }; - - template<typename _Tp> - struct __guaranteed_alignment<__gnu_cxx::new_allocator<_Tp>> - : std::alignment_of<std::max_align_t>::type { }; - - template<typename _Tp> - struct __guaranteed_alignment<__gnu_cxx::malloc_allocator<_Tp>> - : std::alignment_of<std::max_align_t>::type { }; - -#if _GLIBCXX_USE_ALLOCATOR_NEW - template<typename _Tp> - struct __guaranteed_alignment<std::allocator<_Tp>> - : std::alignment_of<std::max_align_t>::type { }; -#endif }; /// @endcond @@ -425,14 +412,22 @@ namespace pmr { virtual void* do_allocate(size_t __bytes, size_t __alignment) override { - if (__alignment <= __guaranteed_alignment<_Alloc>::value) + // Cannot use max_align_t on 32-bit Solaris x86, see PR libstdc++/77691 +#if ! (defined __sun__ && defined __i386__) + if (__alignment == alignof(max_align_t)) + return _M_allocate<alignof(max_align_t)>(__bytes); +#endif + switch (__alignment) { - if (__bytes < __alignment) - __bytes = __alignment; + case 1: return _M_alloc.allocate(__bytes); + case 2: + return _M_allocate<2>(__bytes); + case 4: + return _M_allocate<4>(__bytes); + case 8: + return _M_allocate<8>(__bytes); } - - const _AlignMgr __mgr(__bytes, __alignment); // Assume _M_alloc returns 1-byte aligned memory, so allocate enough // space to fit a block of the right size and alignment, plus some @@ -441,21 +436,28 @@ namespace pmr { } virtual void - do_deallocate(void* __p, size_t __bytes, size_t __alignment) noexcept + do_deallocate(void* __ptr, size_t __bytes, size_t __alignment) noexcept override { - auto __ptr = static_cast<char*>(__p); - if (__alignment <= __guaranteed_alignment<_Alloc>::value) +#if ! (defined __sun__ && defined __i386__) + if (__alignment == alignof(max_align_t)) + return (void) _M_deallocate<alignof(max_align_t)>(__ptr, __bytes); +#endif + switch (__alignment) { - if (__bytes < __alignment) - __bytes = __alignment; - _M_alloc.deallocate(__ptr, __bytes); - return; + case 1: + return (void) _M_alloc.deallocate((char*)__ptr, __bytes); + case 2: + return (void) _M_deallocate<2>(__ptr, __bytes); + case 4: + return (void) _M_deallocate<4>(__ptr, __bytes); + case 8: + return (void) _M_deallocate<8>(__ptr, __bytes); } - const _AlignMgr __mgr(__bytes, __alignment); - // Use the stored token to retrieve the original pointer to deallocate. - _M_alloc.deallocate(__mgr._M_unadjust(__ptr), __mgr._M_alloc_size()); + // Use the stored token to retrieve the original pointer. + _M_alloc.deallocate(__mgr._M_unadjust((char*)__ptr), + __mgr._M_alloc_size()); } virtual bool @@ -467,6 +469,31 @@ namespace pmr { } private: + template<size_t _Num> + struct _Aligned_type { alignas(_Num) char __c[_Num]; }; + + // Rebind the allocator to the specified type and use it to allocate. + template<size_t _Num, typename _Tp = _Aligned_type<_Num>> + void* + _M_allocate(size_t __bytes) + { + typename allocator_traits<_Alloc>::template + rebind_alloc<_Tp> __a2(_M_alloc); + const size_t __n = (__bytes + _Num - 1) / _Num; + return __a2.allocate(__n); + } + + // Rebind the allocator to the specified type and use it to deallocate. + template<size_t _Num, typename _Tp = _Aligned_type<_Num>> + void + _M_deallocate(void* __ptr, size_t __bytes) noexcept + { + typename allocator_traits<_Alloc>::template + rebind_alloc<_Tp> __a2(_M_alloc); + const size_t __n = (__bytes + _Num - 1) / _Num; + __a2.deallocate((_Tp*)__ptr, __n); + } + _Alloc _M_alloc{}; }; @@ -537,4 +564,5 @@ namespace pmr { _GLIBCXX_END_NAMESPACE_VERSION } // namespace std +#endif // C++14 #endif // _GLIBCXX_EXPERIMENTAL_MEMORY_RESOURCE diff --git a/libstdc++-v3/testsuite/experimental/memory_resource/new_delete_resource.cc b/libstdc++-v3/testsuite/experimental/memory_resource/new_delete_resource.cc index 41232b6fc67..7dcb408f3f7 100644 --- a/libstdc++-v3/testsuite/experimental/memory_resource/new_delete_resource.cc +++ b/libstdc++-v3/testsuite/experimental/memory_resource/new_delete_resource.cc @@ -23,6 +23,11 @@ #include <cstdlib> #include <testsuite_hooks.h> +#if defined __sun__ && defined __i386__ +// See PR libstdc++/77691 +# define BAD_MAX_ALIGN_T 1 +#endif + bool new_called = false; bool delete_called = false; std::size_t bytes_allocated = 0; @@ -38,14 +43,14 @@ void* operator new(std::size_t n) throw std::bad_alloc(); } -void operator delete(void* p) +void operator delete(void* p) noexcept { delete_called = true; std::free(p); bytes_allocated = 0; // assume everything getting deleted } -void operator delete(void* p, std::size_t n) +void operator delete(void* p, std::size_t n) noexcept { delete_called = true; std::free(p); @@ -55,9 +60,7 @@ void operator delete(void* p, std::size_t n) template<std::size_t A> bool aligned(void* p) - { - return (reinterpret_cast<std::uintptr_t>(p) % A) == 0; - } + { return (reinterpret_cast<std::uintptr_t>(p) % A) == 0; } template<typename T> bool aligned(void* p) @@ -108,13 +111,17 @@ test03() using std::size_t; void* p = nullptr; - auto max = [](int n, int a) { return n > a ? n : a; }; + auto round = [](size_t n, size_t a) { return n % a ? n + a - (n % a) : n; }; bytes_allocated = 0; memory_resource* r1 = new_delete_resource(); p = r1->allocate(1); // uses alignment = alignof(max_align_t) - VERIFY( bytes_allocated <= alignof(max_align_t) ); +#ifdef BAD_MAX_ALIGN_T + VERIFY( bytes_allocated != 0 ); +#else + VERIFY( bytes_allocated == alignof(max_align_t) ); +#endif VERIFY( aligned<max_align_t>(p) ); r1->deallocate(p, 1); VERIFY( bytes_allocated == 0 ); @@ -123,17 +130,16 @@ test03() VERIFY( bytes_allocated == 2 ); VERIFY( aligned<max_align_t>(p) ); r1->deallocate(p, 2, alignof(char)); - __builtin_printf("%d\n", (int)bytes_allocated); VERIFY( bytes_allocated == 0 ); p = r1->allocate(3, alignof(short)); - VERIFY( bytes_allocated == max(3, alignof(short)) ); + VERIFY( bytes_allocated == round(3, alignof(short)) ); VERIFY( aligned<short>(p) ); r1->deallocate(p, 3, alignof(short)); VERIFY( bytes_allocated == 0 ); p = r1->allocate(4, alignof(long)); - VERIFY( bytes_allocated == max(4, alignof(long)) ); + VERIFY( bytes_allocated == round(4, alignof(long)) ); VERIFY( aligned<long>(p) ); r1->deallocate(p, 4, alignof(long)); VERIFY( bytes_allocated == 0 ); diff --git a/libstdc++-v3/testsuite/experimental/memory_resource/resource_adaptor.cc b/libstdc++-v3/testsuite/experimental/memory_resource/resource_adaptor.cc index 27ee1078dd4..56c8ffa2803 100644 --- a/libstdc++-v3/testsuite/experimental/memory_resource/resource_adaptor.cc +++ b/libstdc++-v3/testsuite/experimental/memory_resource/resource_adaptor.cc @@ -1,6 +1,5 @@ // { dg-do run { target c++14 } } // { dg-require-cstdint "" } -// { dg-xfail-run-if "PR libstdc++/77691" { { i?86-*-solaris2.* x86_64-*-solaris2.* } && ilp32 } } // Copyright (C) 2016-2019 Free Software Foundation, Inc. //
Remove the hardcoded whitelist of allocators expected to return memory aligned to alignof(max_align_t), because that doesn't work when the platform's malloc() and GCC's max_align_t do not agree what the largest fundamental alignment is. It's also sub-optimal for user-defined allocators that return memory suitable for any fundamental alignment. Instead use a hardcoded list of alignments that are definitely supported by the platform malloc, and use a copy of the allocator rebound to a POD type with the requested alignment. Only allocate an oversized buffer to use with std::align for alignments larger than any of the hardcoded values. For 32-bit Solaris x86 do not include alignof(max_align_t) in the hardcoded values. PR libstdc++/77691 * include/experimental/memory_resource: Add system header pragma. (__resource_adaptor_common::__guaranteed_alignment): Remove. (__resource_adaptor_common::_Types) (__resource_adaptor_common::__new_list) (__resource_adaptor_common::_New_list) (__resource_adaptor_common::_Alignments) (__resource_adaptor_common::_Fund_align_types): New utilities for creating a list of types with fundamental alignments. (__resource_adaptor_imp::do_allocate): Call new _M_allocate function. (__resource_adaptor_imp::do_deallocate): Call new _M_deallocate function. (__resource_adaptor_imp::_M_allocate): New function that first tries to use an allocator rebound to a type with a fundamental alignment. (__resource_adaptor_imp::_M_deallocate): Likewise for deallocation. * testsuite/experimental/memory_resource/new_delete_resource.cc: Adjust expected allocation sizes. * testsuite/experimental/memory_resource/resource_adaptor.cc: Remove xfail. Tested x86_64-linux, committed to trunk. commit 5a14390cba4480685ef98c97a88a1ea965679c6a Author: Jonathan Wakely <jwakely@redhat.com> Date: Wed May 22 21:06:25 2019 +0100 PR libstdc++/77691 fix resource_adaptor failures due to max_align_t bugs Remove the hardcoded whitelist of allocators expected to return memory aligned to alignof(max_align_t), because that doesn't work when the platform's malloc() and GCC's max_align_t do not agree what the largest fundamental alignment is. It's also sub-optimal for user-defined allocators that return memory suitable for any fundamental alignment. Instead use a hardcoded list of alignments that are definitely supported by the platform malloc, and use a copy of the allocator rebound to a POD type with the requested alignment. Only allocate an oversized buffer to use with std::align for alignments larger than any of the hardcoded values. For 32-bit Solaris x86 do not include alignof(max_align_t) in the hardcoded values. PR libstdc++/77691 * include/experimental/memory_resource: Add system header pragma. (__resource_adaptor_common::__guaranteed_alignment): Remove. (__resource_adaptor_common::_Types) (__resource_adaptor_common::__new_list) (__resource_adaptor_common::_New_list) (__resource_adaptor_common::_Alignments) (__resource_adaptor_common::_Fund_align_types): New utilities for creating a list of types with fundamental alignments. (__resource_adaptor_imp::do_allocate): Call new _M_allocate function. (__resource_adaptor_imp::do_deallocate): Call new _M_deallocate function. (__resource_adaptor_imp::_M_allocate): New function that first tries to use an allocator rebound to a type with a fundamental alignment. (__resource_adaptor_imp::_M_deallocate): Likewise for deallocation. * testsuite/experimental/memory_resource/new_delete_resource.cc: Adjust expected allocation sizes. * testsuite/experimental/memory_resource/resource_adaptor.cc: Remove xfail.