Re: [PATCH] PR libstdc++/70940 make pmr::resource_adaptor return aligned memory

[Jonathan Wakely <jwakely at redhat dot com>]

On 21/06/18 16:22 +0100, Jonathan Wakely wrote:
> On 21/06/18 15:00 +0100, Jonathan Wakely wrote:
> >      virtual void
> > -      do_deallocate(void* __p, size_t __bytes, size_t __alignment)
> > +      do_deallocate(void* __p, size_t __bytes, size_t __alignment) noexcept
> > +      override
> >      {
> > -	using _Aligned_alloc = std::__alloc_rebind<_Alloc, char>;
> > -	size_t __new_size = _S_aligned_size(__bytes,
> > -					    _S_supported(__alignment) ?
> > -					    __alignment : _S_max_align);
> > -	using _Ptr = typename allocator_traits<_Aligned_alloc>::pointer;
> > -	_Aligned_alloc(_M_alloc).deallocate(static_cast<_Ptr>(__p),
> > -					    __new_size);
> > +	auto __ptr = static_cast<char*>(__p);
> > +	if (__alignment == 1)
> > +	  _M_alloc.deallocate(__ptr, __bytes);
>
> Oops, this is missing a return!
>
> Patch incoming ...

Fixed like so, with improved testing too.

Tested x86_64-linux, committed to trunk.


commit 3786ccc11448d536e95cbdb4ff02bb446a1d1fca
Author: redi <redi@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Thu Jun 21 14:01:11 2018 +0000

    PR libstdc++/70940 make pmr::resource_adaptor return aligned memory
    
            PR libstdc++/70940
            * include/experimental/memory_resource
            (__resource_adaptor_imp::do_deallocate): Add missing return.
            * testsuite/experimental/memory_resource/new_delete_resource.cc: New.
            * testsuite/experimental/memory_resource/resource_adaptor.cc: Test
            resource_adaptor with std::allocator, __gnu_cxx::new_allocator and
            __gnu_cxx::malloc_allocator.

diff --git a/libstdc++-v3/include/experimental/memory_resource b/libstdc++-v3/include/experimental/memory_resource
index 3d2dce19868..8f5a8df14c9 100644
--- a/libstdc++-v3/include/experimental/memory_resource
+++ b/libstdc++-v3/include/experimental/memory_resource
@@ -408,7 +408,10 @@ namespace pmr {
       {
 	auto __ptr = static_cast<char*>(__p);
 	if (__alignment == 1)
-	  _M_alloc.deallocate(__ptr, __bytes);
+	  {
+	    _M_alloc.deallocate(__ptr, __bytes);
+	    return;
+	  }
 
 	const _AlignMgr __mgr(__bytes, __alignment);
 	// Use the stored token to retrieve the original pointer to deallocate.
diff --git a/libstdc++-v3/testsuite/experimental/memory_resource/new_delete_resource.cc b/libstdc++-v3/testsuite/experimental/memory_resource/new_delete_resource.cc
new file mode 100644
index 00000000000..692e520bf9a
--- /dev/null
+++ b/libstdc++-v3/testsuite/experimental/memory_resource/new_delete_resource.cc
@@ -0,0 +1,132 @@
+// Copyright (C) 2018 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-do run { target c++14 } }
+
+#include <experimental/memory_resource>
+#include <testsuite_hooks.h>
+
+bool new_called = false;
+bool delete_called = false;
+
+void* operator new(std::size_t n)
+{
+  new_called = true;
+  if (void* p = malloc(n))
+    return p;
+  throw std::bad_alloc();
+}
+
+void operator delete(void* p)
+{
+  delete_called = true;
+  std::free(p);
+}
+
+void operator delete(void* p, std::size_t)
+{
+  ::operator delete(p);
+}
+
+template<std::size_t A>
+  bool aligned(void* p)
+  {
+    return (reinterpret_cast<std::uintptr_t>(p) % A) == 0;
+  }
+
+template<typename T>
+  bool aligned(void* p)
+  { return aligned<alignof(T)>(p); }
+
+// Extended alignments:
+constexpr std::size_t al6 = (1ul << 6), al12 = (1ul << 12), al18 = (1ul << 18);
+
+using std::experimental::pmr::memory_resource;
+using std::experimental::pmr::new_delete_resource;
+
+memory_resource* global = nullptr;
+
+void
+test01()
+{
+  memory_resource* r1 = new_delete_resource();
+  VERIFY( *r1 == *r1 );
+  memory_resource* r2 = new_delete_resource();
+  VERIFY( r1 == r2 );
+  VERIFY( *r1 == *r2 );
+  global = r1;
+}
+
+void
+test02()
+{
+  memory_resource* r1 = new_delete_resource();
+  VERIFY( r1 == global );
+  VERIFY( *r1 == *global );
+
+  new_called = false;
+  delete_called = false;
+  void* p = r1->allocate(1);
+  VERIFY( new_called );
+  VERIFY( ! delete_called );
+
+  new_called = false;
+  r1->deallocate(p, 1);
+  VERIFY( ! new_called );
+  VERIFY( delete_called );
+}
+
+void
+test03()
+{
+  using std::max_align_t;
+  using std::size_t;
+  void* p = nullptr;
+
+  memory_resource* r1 = new_delete_resource();
+  p = r1->allocate(1);
+  VERIFY( aligned<max_align_t>(p) );
+  r1->deallocate(p, 1);
+  p = r1->allocate(1, alignof(short));
+  VERIFY( aligned<short>(p) );
+  r1->deallocate(p, 1, alignof(short));
+  p = r1->allocate(1, alignof(long));
+  VERIFY( aligned<long>(p) );
+  r1->deallocate(p, 1, alignof(long));
+  constexpr size_t big_al = alignof(max_align_t) * 8;
+  p = r1->allocate(1, big_al);
+  VERIFY( aligned<big_al>(p) );
+  r1->deallocate(p, 1, big_al);
+
+  // Test extended alignments
+  p = r1->allocate(1024, al6);
+  VERIFY( aligned<al6>(p) );
+  r1->deallocate(p, 1024, al6);
+  p = r1->allocate(1024, al12);
+  VERIFY( aligned<al12>(p) );
+  r1->deallocate(p, 1024, al12);
+  p = r1->allocate(1024, al18);
+  VERIFY( aligned<al18>(p) );
+  r1->deallocate(p, 1024, al18);
+}
+
+int main()
+{
+  test01();
+  test02();
+  test03();
+}
diff --git a/libstdc++-v3/testsuite/experimental/memory_resource/resource_adaptor.cc b/libstdc++-v3/testsuite/experimental/memory_resource/resource_adaptor.cc
index 4e39e773248..46cb113da7b 100644
--- a/libstdc++-v3/testsuite/experimental/memory_resource/resource_adaptor.cc
+++ b/libstdc++-v3/testsuite/experimental/memory_resource/resource_adaptor.cc
@@ -20,6 +20,8 @@
 
 #include <experimental/memory_resource>
 #include <ext/debug_allocator.h>
+#include <ext/new_allocator.h>
+#include <ext/malloc_allocator.h>
 #include <testsuite_hooks.h>
 #include <testsuite_allocator.h>
 
@@ -91,8 +93,8 @@ test05()
   VERIFY( aligned<big_al>(p) );
   r3.deallocate(p, 1, big_al);
 
-  __gnu_cxx::debug_allocator<std::allocator<short>> a5;
-  resource_adaptor<decltype(a5)> r5(a5), r6(a5);
+  __gnu_cxx::debug_allocator<std::allocator<short>> a5, a6;
+  resource_adaptor<decltype(a5)> r5(a5), r6(a6);
   VERIFY( r5 == r5 );
   VERIFY( r5 == r6 );
   VERIFY( r5 != r1 );
@@ -121,6 +123,99 @@ test05()
   p = r5.allocate(1024, al18);
   VERIFY( aligned<al18>(p) );
   r5.deallocate(p, 1024, al18);
+
+  __gnu_cxx::new_allocator<short> a7, a8;
+  resource_adaptor<decltype(a7)> r7(a7), r8(a8);
+  VERIFY( r7 == r7 );
+  VERIFY( r7 == r8 );
+  VERIFY( r7 != r1 );
+  VERIFY( r7 != r3 );
+  VERIFY( r7 != r5 );
+  p = r7.allocate(1);
+  VERIFY( aligned<max_align_t>(p) );
+  r7.deallocate(p, 1);
+  p = r7.allocate(1, alignof(short));
+  VERIFY( aligned<short>(p) );
+  r7.deallocate(p, 1, alignof(short));
+  p = r7.allocate(1, alignof(long));
+  VERIFY( aligned<long>(p) );
+  r7.deallocate(p, 1, alignof(long));
+  p = r7.allocate(1, big_al);
+  VERIFY( aligned<big_al>(p) );
+  r7.deallocate(p, 1, big_al);
+  // Test extended alignments
+  p = r7.allocate(1024, al6);
+  VERIFY( aligned<al6>(p) );
+  r7.deallocate(p, 1024, al6);
+  p = r7.allocate(1024, al12);
+  VERIFY( aligned<al12>(p) );
+  r7.deallocate(p, 1024, al12);
+  p = r7.allocate(1024, al18);
+  VERIFY( aligned<al18>(p) );
+  r7.deallocate(p, 1024, al18);
+
+  __gnu_cxx::malloc_allocator<short> a9, a10;
+  resource_adaptor<decltype(a9)> r9(a9), r10(a10);
+  VERIFY( r9 == r9 );
+  VERIFY( r9 == r10 );
+  VERIFY( r9 != r1 );
+  VERIFY( r9 != r3 );
+  VERIFY( r9 != r5 );
+  VERIFY( r9 != r7 );
+  p = r9.allocate(1);
+  VERIFY( aligned<max_align_t>(p) );
+  r9.deallocate(p, 1);
+  p = r9.allocate(1, alignof(short));
+  VERIFY( aligned<short>(p) );
+  r9.deallocate(p, 1, alignof(short));
+  p = r9.allocate(1, alignof(long));
+  VERIFY( aligned<long>(p) );
+  r9.deallocate(p, 1, alignof(long));
+  p = r9.allocate(1, big_al);
+  VERIFY( aligned<big_al>(p) );
+  r9.deallocate(p, 1, big_al);
+  // Test extended alignments
+  p = r9.allocate(1024, al6);
+  VERIFY( aligned<al6>(p) );
+  r9.deallocate(p, 1024, al6);
+  p = r9.allocate(1024, al12);
+  VERIFY( aligned<al12>(p) );
+  r9.deallocate(p, 1024, al12);
+  p = r9.allocate(1024, al18);
+  VERIFY( aligned<al18>(p) );
+  r9.deallocate(p, 1024, al18);
+
+  std::allocator<short> a11, a12;
+  resource_adaptor<decltype(a11)> r11(a11), r12(a12);
+  VERIFY( r11 == r11 );
+  VERIFY( r11 == r12 );
+  VERIFY( r11 != r1 );
+  VERIFY( r11 != r3 );
+  VERIFY( r11 != r5 );
+  VERIFY( r11 != r7 );
+  VERIFY( r11 != r9 );
+  p = r11.allocate(1);
+  VERIFY( aligned<max_align_t>(p) );
+  r11.deallocate(p, 1);
+  p = r11.allocate(1, alignof(short));
+  VERIFY( aligned<short>(p) );
+  r11.deallocate(p, 1, alignof(short));
+  p = r11.allocate(1, alignof(long));
+  VERIFY( aligned<long>(p) );
+  r11.deallocate(p, 1, alignof(long));
+  p = r11.allocate(1, big_al);
+  VERIFY( aligned<big_al>(p) );
+  r11.deallocate(p, 1, big_al);
+  // Test extended alignments
+  p = r11.allocate(1024, al6);
+  VERIFY( aligned<al6>(p) );
+  r11.deallocate(p, 1024, al6);
+  p = r11.allocate(1024, al12);
+  VERIFY( aligned<al12>(p) );
+  r11.deallocate(p, 1024, al12);
+  p = r11.allocate(1024, al18);
+  VERIFY( aligned<al18>(p) );
+  r11.deallocate(p, 1024, al18);
 }
 
 int main()