On 05/06/2017 13:31, Jonathan Wakely wrote:
On 04/06/17 22:26 +0200, François Dumont wrote:
Hi

I have eventually adapt the test to all containers and the result is successful for map/set/unordered_map/unordered_set. It is failing for deque/list/forward_list/vector/vector<bool>.

I even try to change the test to look at the difference between an explicit call to the default constructor done through the placement new call and an implicit call done on normal declaration. I wondered if we would have the same kind of difference we have between a int i; and a int i(); I tried to set the stack to ~0 before declaring the instance. I know there is no guarantee on the content of the stack for the following declaration but do you think it is reliable enough to commit it ?

   Ok to commit the successful tests ?


No, I'm seeing failures for some of these if I add
// { dg-options "-O0" }

Franckly I don't understand the result of those tests. I would have expect map/set to fail and others to succeed. We might need help from compiler guys, no ?

I think your tests are just insufficient. With optimisation enabled
(the testsuite uses -O2 by default) the compiler can remove the memset
just before the __aligned_buffer goes out of scope, because it is
unobservable in a correct program. This is similar to the situation
described at https://gcc.gnu.org/gcc-6/porting_to.html#flifetime-dse

If I change the placement new-expressions to use default-init instead
of value-init and use -O0 then I see all four tests FAIL here:

test_type *tmp = ::new(buf._M_addr()) test_type; // not test_type()

Ok, I didn't know we could do this.

So I have added value_init.cc tests showing the problem for all containers.

The patch also contains the fix for rb_tree so that map/set are now successful.

Looks there is really a misunderstanding on what the compiler is doing. If container calls _Node_allocator() it will be transform by compiler into default initialization if container default container is being called or into value init if container is value initialized which takes place only if I call this:

test_type *tmp = ::new(buf._M_addr()) test_type {};

To force default/value init looks like gcc forces you to explicitly build an allocator instance like in the attached patch.

François

Index: include/bits/stl_tree.h
===================================================================
--- include/bits/stl_tree.h	(revision 248855)
+++ include/bits/stl_tree.h	(working copy)
@@ -687,9 +687,17 @@
 
 #if __cplusplus < 201103L
 	  _Rb_tree_impl()
+	  : _Node_allocator()
 	  { }
 #else
-	  _Rb_tree_impl() = default;
+	  _Rb_tree_impl()
+	    noexcept(
+		noexcept(_Node_allocator()) && noexcept(_Base_key_compare()) )
+	  : _Rb_tree_impl(_Key_compare(), _Node_allocator())
+	  { }
+#endif
+
+#if __cplusplus >= 201103L
 	  _Rb_tree_impl(_Rb_tree_impl&&) = default;
 #endif
 
Index: testsuite/23_containers/map/allocator/default_init.cc
===================================================================
--- testsuite/23_containers/map/allocator/default_init.cc	(nonexistent)
+++ testsuite/23_containers/map/allocator/default_init.cc	(working copy)
@@ -0,0 +1,52 @@
+// Copyright (C) 2017 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++11 } }
+// { dg-options "-O0" }
+
+#include <map>
+#include <testsuite_hooks.h>
+#include <testsuite_allocator.h>
+
+#include <ext/aligned_buffer.h>
+
+using T = int;
+
+using __gnu_test::default_init_allocator;
+
+void test01()
+{
+  typedef default_init_allocator<std::pair<const T, T>> alloc_type;
+  typedef std::map<T, T, std::less<T>, alloc_type> test_type;
+
+  __gnu_cxx::__aligned_buffer<test_type> buf;
+  __builtin_memset(buf._M_addr(), ~0, sizeof(test_type));
+
+  VERIFY( buf._M_ptr()->get_allocator().state != 0 );
+  
+  test_type *tmp = ::new(buf._M_addr()) test_type();
+
+  VERIFY( tmp->get_allocator().state == 0 );
+
+  tmp->~test_type();
+}
+
+int main()
+{
+  test01();
+  return 0;
+}
Index: testsuite/23_containers/map/allocator/value_init.cc
===================================================================
--- testsuite/23_containers/map/allocator/value_init.cc	(nonexistent)
+++ testsuite/23_containers/map/allocator/value_init.cc	(working copy)
@@ -0,0 +1,52 @@
+// Copyright (C) 2017 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++11 } }
+// { dg-options "-O0" }
+
+#include <map>
+#include <testsuite_hooks.h>
+#include <testsuite_allocator.h>
+
+#include <ext/aligned_buffer.h>
+
+using T = int;
+
+using __gnu_test::default_init_allocator;
+
+void test01()
+{
+  typedef default_init_allocator<std::pair<const T, T>> alloc_type;
+  typedef std::map<T, T, std::less<T>, alloc_type> test_type;
+
+  __gnu_cxx::__aligned_buffer<test_type> buf;
+  __builtin_memset(buf._M_addr(), ~0, sizeof(test_type));
+
+  VERIFY( buf._M_ptr()->get_allocator().state != 0 );
+  
+  test_type *tmp = ::new(buf._M_addr()) test_type;
+
+  VERIFY( tmp->get_allocator().state == 0 );
+
+  tmp->~test_type();
+}
+
+int main()
+{
+  test01();
+  return 0;
+}
Index: testsuite/23_containers/set/allocator/default_init.cc
===================================================================
--- testsuite/23_containers/set/allocator/default_init.cc	(nonexistent)
+++ testsuite/23_containers/set/allocator/default_init.cc	(working copy)
@@ -0,0 +1,52 @@
+// Copyright (C) 2017 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++11 } }
+// { dg-options "-O0" }
+
+#include <set>
+#include <testsuite_hooks.h>
+#include <testsuite_allocator.h>
+
+#include <ext/aligned_buffer.h>
+
+using T = int;
+
+using __gnu_test::default_init_allocator;
+
+void test01()
+{
+  typedef default_init_allocator<T> alloc_type;
+  typedef std::set<T, std::less<T>, alloc_type> test_type;
+
+  __gnu_cxx::__aligned_buffer<test_type> buf;
+  __builtin_memset(buf._M_addr(), ~0, sizeof(test_type));
+
+  VERIFY( buf._M_ptr()->get_allocator().state != 0 );
+  
+  test_type *tmp = ::new(buf._M_addr()) test_type();
+
+  VERIFY( tmp->get_allocator().state == 0 );
+
+  tmp->~test_type();
+}
+
+int main()
+{
+  test01();
+  return 0;
+}
Index: testsuite/23_containers/set/allocator/value_init.cc
===================================================================
--- testsuite/23_containers/set/allocator/value_init.cc	(nonexistent)
+++ testsuite/23_containers/set/allocator/value_init.cc	(working copy)
@@ -0,0 +1,52 @@
+// Copyright (C) 2017 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++11 } }
+// { dg-options "-O0" }
+
+#include <set>
+#include <testsuite_hooks.h>
+#include <testsuite_allocator.h>
+
+#include <ext/aligned_buffer.h>
+
+using T = int;
+
+using __gnu_test::default_init_allocator;
+
+void test01()
+{
+  typedef default_init_allocator<T> alloc_type;
+  typedef std::set<T, std::less<T>, alloc_type> test_type;
+
+  __gnu_cxx::__aligned_buffer<test_type> buf;
+  __builtin_memset(buf._M_addr(), ~0, sizeof(test_type));
+
+  VERIFY( buf._M_ptr()->get_allocator().state != 0 );
+  
+  test_type *tmp = ::new(buf._M_addr()) test_type;
+
+  VERIFY( tmp->get_allocator().state == 0 );
+
+  tmp->~test_type();
+}
+
+int main()
+{
+  test01();
+  return 0;
+}
Index: testsuite/23_containers/unordered_map/allocator/default_init.cc
===================================================================
--- testsuite/23_containers/unordered_map/allocator/default_init.cc	(nonexistent)
+++ testsuite/23_containers/unordered_map/allocator/default_init.cc	(working copy)
@@ -0,0 +1,53 @@
+// Copyright (C) 2017 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++11 } }
+// { dg-options "-O0" }
+
+#include <unordered_map>
+#include <testsuite_hooks.h>
+#include <testsuite_allocator.h>
+
+#include <ext/aligned_buffer.h>
+
+using T = int;
+
+using __gnu_test::default_init_allocator;
+
+void test01()
+{
+  typedef default_init_allocator<std::pair<const T, T>> alloc_type;
+  typedef std::unordered_map<T, T, std::hash<T>, std::equal_to<T>,
+			     alloc_type> test_type;
+
+  __gnu_cxx::__aligned_buffer<test_type> buf;
+  __builtin_memset(buf._M_addr(), ~0, sizeof(test_type));
+
+  VERIFY( buf._M_ptr()->get_allocator().state != 0 );
+  
+  test_type *tmp = ::new(buf._M_addr()) test_type();
+
+  VERIFY( tmp->get_allocator().state == 0 );
+
+  tmp->~test_type();
+}
+
+int main()
+{
+  test01();
+  return 0;
+}
Index: testsuite/23_containers/unordered_set/allocator/default_init.cc
===================================================================
--- testsuite/23_containers/unordered_set/allocator/default_init.cc	(nonexistent)
+++ testsuite/23_containers/unordered_set/allocator/default_init.cc	(working copy)
@@ -0,0 +1,53 @@
+// Copyright (C) 2017 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++11 } }
+// { dg-options "-O0" }
+
+#include <unordered_set>
+#include <testsuite_hooks.h>
+#include <testsuite_allocator.h>
+
+#include <ext/aligned_buffer.h>
+
+using T = int;
+
+using __gnu_test::default_init_allocator;
+
+void test01()
+{
+  typedef default_init_allocator<T> alloc_type;
+  typedef std::unordered_set<T, std::hash<T>, std::equal_to<T>,
+			     alloc_type> test_type;
+
+  __gnu_cxx::__aligned_buffer<test_type> buf;
+  __builtin_memset(buf._M_addr(), ~0, sizeof(test_type));
+
+  VERIFY( buf._M_ptr()->get_allocator().state != 0 );
+  
+  test_type *tmp = ::new(buf._M_addr()) test_type();
+
+  VERIFY( tmp->get_allocator().state == 0 );
+
+  tmp->~test_type();
+}
+
+int main()
+{
+  test01();
+  return 0;
+}
Index: testsuite/util/testsuite_allocator.h
===================================================================
--- testsuite/util/testsuite_allocator.h	(revision 248855)
+++ testsuite/util/testsuite_allocator.h	(working copy)
@@ -508,6 +508,38 @@
     bool operator!=(const SimpleAllocator<T>&, const SimpleAllocator<U>&)
     { return false; }
 
+  template<typename T>
+    struct default_init_allocator
+    {
+      using value_type = T;
+
+      default_init_allocator() = default;
+
+      template<typename U>
+        default_init_allocator(const default_init_allocator<U>& a)
+	  : state(a.state)
+        { }
+
+      T*
+      allocate(std::size_t n)
+      { return std::allocator<T>().allocate(n); }
+
+      void
+      deallocate(T* p, std::size_t n)
+      { std::allocator<T>().deallocate(p, n); }
+
+      int state;
+    };
+
+  template<typename T, typename U>
+    bool operator==(const default_init_allocator<T>& t,
+		    const default_init_allocator<U>& u)
+    { return t.state == u.state; }
+
+  template<typename T, typename U>
+    bool operator!=(const default_init_allocator<T>& t,
+		    const default_init_allocator<U>& u)
+    { return !(t == u); }
 #endif
 
   template<typename Tp>

Reply via email to