public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] PR libstdc++/80553 don't allow destroying non-destructible types
@ 2017-04-28 13:33 Jonathan Wakely
  2017-05-02  9:17 ` Jonathan Wakely
  0 siblings, 1 reply; 5+ messages in thread
From: Jonathan Wakely @ 2017-04-28 13:33 UTC (permalink / raw)
  To: libstdc++, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1340 bytes --]

We optimize _Destroy and _Destroy_n to do nothing when the type has a
trivial destructor, which means we do nothing (instead of giving an
error) when trying to destroy types with deleted destructors.

Because deleted destructors are trivial. Because C++.

This adds static assertions to reject attempts to destroy
indestructible objects. I put them in the top-level functions, not the
specialization for trivial destructors, which means we also get the
static assertion for types with a private destructor. It's debatable
whether that's better: you get a failed "type is destructible"
assertion instead of an error saying the destructor is private, which
is a bit more precise. If people prefer the compiler error about a
private destructor we could move the assertions into the
specializations for types with trivial destructors.

	PR libstdc++/80553
	* include/bits/stl_construct.h (_Destroy, _Destroy_n): Add static
	assertions to ensure type is destructible.
	(destroy_at, destroy, destroy_n): Move from stl_uninitialized.h.
	* include/bits/stl_uninitialized.h (destroy_at, destroy, destroy_n):
	Move to stl_construct.h.
	* testsuite/20_util/specialized_algorithms/memory_management_tools/
	destroy_neg.cc: New test.
	* testsuite/23_containers/vector/cons/destructible_neg.cc: New test.

Tested powerpc64le-linux, committed to trunk.



[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 9499 bytes --]

commit 2b0c41d7cc1182382582842ccc70f6effc997d6f
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Apr 28 12:34:27 2017 +0100

    PR libstdc++/80553 don't allow destroying non-destructible types
    
    	PR libstdc++/80553
    	* include/bits/stl_construct.h (_Destroy, _Destroy_n): Add static
    	assertions to ensure type is destructible.
    	(destroy_at, destroy, destroy_n): Move from stl_uninitialized.h.
    	* include/bits/stl_uninitialized.h (destroy_at, destroy, destroy_n):
    	Move to stl_construct.h.
    	* testsuite/20_util/specialized_algorithms/memory_management_tools/
    	destroy_neg.cc: New test.
    	* testsuite/23_containers/vector/cons/destructible_neg.cc: New test.

diff --git a/libstdc++-v3/include/bits/stl_construct.h b/libstdc++-v3/include/bits/stl_construct.h
index c1504e9..71483b4 100644
--- a/libstdc++-v3/include/bits/stl_construct.h
+++ b/libstdc++-v3/include/bits/stl_construct.h
@@ -128,6 +128,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     {
       typedef typename iterator_traits<_ForwardIterator>::value_type
                        _Value_type;
+#if __cplusplus >= 201103L
+      // A deleted destructor is trivial, this ensures we reject such types:
+      static_assert(is_destructible<_Value_type>::value,
+		    "value type is destructible");
+#endif
       std::_Destroy_aux<__has_trivial_destructor(_Value_type)>::
 	__destroy(__first, __last);
     }
@@ -151,10 +156,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       template<typename _ForwardIterator, typename _Size>
         static _ForwardIterator
         __destroy_n(_ForwardIterator __first, _Size __count)
-      {
-	 std::advance(__first, __count);
-	 return __first;
-      }
+	{
+	  std::advance(__first, __count);
+	  return __first;
+	}
     };
 
   /**
@@ -168,6 +173,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     {
       typedef typename iterator_traits<_ForwardIterator>::value_type
                        _Value_type;
+#if __cplusplus >= 201103L
+      // A deleted destructor is trivial, this ensures we reject such types:
+      static_assert(is_destructible<_Value_type>::value,
+		    "value type is destructible");
+#endif
       return std::_Destroy_n_aux<__has_trivial_destructor(_Value_type)>::
 	__destroy_n(__first, __count);
     }
@@ -196,6 +206,29 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _Destroy(__first, __last);
     }
 
+#if __cplusplus > 201402L
+  template <typename _Tp>
+    inline void
+    destroy_at(_Tp* __location)
+    {
+      std::_Destroy(__location);
+    }
+
+  template <typename _ForwardIterator>
+    inline void
+    destroy(_ForwardIterator __first, _ForwardIterator __last)
+    {
+      std::_Destroy(__first, __last);
+    }
+
+  template <typename _ForwardIterator, typename _Size>
+    inline _ForwardIterator
+    destroy_n(_ForwardIterator __first, _Size __count)
+    {
+      return std::_Destroy_n(__first, __count);
+    }
+#endif
+
 _GLIBCXX_END_NAMESPACE_VERSION
 } // namespace std
 
diff --git a/libstdc++-v3/include/bits/stl_uninitialized.h b/libstdc++-v3/include/bits/stl_uninitialized.h
index d0e2b2d..c2ba863 100644
--- a/libstdc++-v3/include/bits/stl_uninitialized.h
+++ b/libstdc++-v3/include/bits/stl_uninitialized.h
@@ -831,77 +831,54 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     inline void
     uninitialized_default_construct(_ForwardIterator __first,
 				    _ForwardIterator __last)
-  {
-    __uninitialized_default_novalue(__first, __last);
-  }
+    {
+      __uninitialized_default_novalue(__first, __last);
+    }
 
   template <typename _ForwardIterator, typename _Size>
     inline _ForwardIterator
     uninitialized_default_construct_n(_ForwardIterator __first, _Size __count)
-  {
-    return __uninitialized_default_novalue_n(__first, __count);
-  }
+    {
+      return __uninitialized_default_novalue_n(__first, __count);
+    }
 
   template <typename _ForwardIterator>
     inline void
     uninitialized_value_construct(_ForwardIterator __first,
 				  _ForwardIterator __last)
-  {
-    return __uninitialized_default(__first, __last);
-  }
+    {
+      return __uninitialized_default(__first, __last);
+    }
 
   template <typename _ForwardIterator, typename _Size>
     inline _ForwardIterator
     uninitialized_value_construct_n(_ForwardIterator __first, _Size __count)
-  {
-    return __uninitialized_default_n(__first, __count);
-  }
+    {
+      return __uninitialized_default_n(__first, __count);
+    }
 
   template <typename _InputIterator, typename _ForwardIterator>
     inline _ForwardIterator
     uninitialized_move(_InputIterator __first, _InputIterator __last,
 		       _ForwardIterator __result)
-  {
-    return std::uninitialized_copy
-      (_GLIBCXX_MAKE_MOVE_ITERATOR(__first),
-       _GLIBCXX_MAKE_MOVE_ITERATOR(__last), __result);
-  }
+    {
+      return std::uninitialized_copy
+	(_GLIBCXX_MAKE_MOVE_ITERATOR(__first),
+	 _GLIBCXX_MAKE_MOVE_ITERATOR(__last), __result);
+    }
 
   template <typename _InputIterator, typename _Size, typename _ForwardIterator>
     inline pair<_InputIterator, _ForwardIterator>
     uninitialized_move_n(_InputIterator __first, _Size __count,
 			 _ForwardIterator __result)
-  {
-    auto __res = std::__uninitialized_copy_n_pair
-      (_GLIBCXX_MAKE_MOVE_ITERATOR(__first),
-       __count, __result);
-    return {__res.first.base(), __res.second};
-  }
-
-  template <typename _Tp>
-    inline void
-    destroy_at(_Tp* __location)
-  {
-    std::_Destroy(__location);
-  }
-
-  template <typename _ForwardIterator>
-    inline void
-    destroy(_ForwardIterator __first, _ForwardIterator __last)
-  {
-    std::_Destroy(__first, __last);
-  }
-
-  template <typename _ForwardIterator, typename _Size>
-    inline _ForwardIterator
-    destroy_n(_ForwardIterator __first, _Size __count)
-  {
-    return std::_Destroy_n(__first, __count);
-  }
-
+    {
+      auto __res = std::__uninitialized_copy_n_pair
+	(_GLIBCXX_MAKE_MOVE_ITERATOR(__first),
+	 __count, __result);
+      return {__res.first.base(), __res.second};
+    }
 #endif
 
-
 _GLIBCXX_END_NAMESPACE_VERSION
 } // namespace
 
diff --git a/libstdc++-v3/testsuite/20_util/specialized_algorithms/memory_management_tools/destroy_neg.cc b/libstdc++-v3/testsuite/20_util/specialized_algorithms/memory_management_tools/destroy_neg.cc
new file mode 100644
index 0000000..663b2c0
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/specialized_algorithms/memory_management_tools/destroy_neg.cc
@@ -0,0 +1,50 @@
+// 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-options "-std=gnu++17" }
+// { dg-do compile { target c++1z } }
+
+#include <memory>
+
+// This has a trivial destructor, but should not be destructible!
+struct DeletedDtor {
+  ~DeletedDtor() = delete;
+};
+
+void
+test01()
+{
+  alignas(DeletedDtor) unsigned char buf[sizeof(DeletedDtor)];
+  auto p = ::new (buf) DeletedDtor();
+  std::destroy(p, p + 1);	// { dg-error "here" }
+  std::destroy_n(p, 1);		// { dg-error "here" }
+}
+
+class PrivateDtor {
+  ~PrivateDtor() { }
+};
+
+void
+test02()
+{
+  alignas(PrivateDtor) unsigned char buf[sizeof(PrivateDtor)];
+  auto p = ::new (buf) PrivateDtor();
+  std::destroy(p, p + 1);	// { dg-error "here" }
+  std::destroy_n(p, 1);		// { dg-error "here" }
+}
+
+// { dg-error "value type is destructible" "" { target *-*-* } 0 }
diff --git a/libstdc++-v3/testsuite/23_containers/vector/cons/destructible_neg.cc b/libstdc++-v3/testsuite/23_containers/vector/cons/destructible_neg.cc
new file mode 100644
index 0000000..4898595
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/vector/cons/destructible_neg.cc
@@ -0,0 +1,44 @@
+// 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 compile { target c++11 } }
+
+#include <vector>
+
+// PR libstdc++/80553
+
+struct DeletedDtor {
+  ~DeletedDtor() = delete;
+};
+
+class PrivateDtor {
+  ~PrivateDtor() { }
+};
+
+void
+test01()
+{
+  std::vector<DeletedDtor> v; // { dg-error "here" }
+}
+
+void
+test02()
+{
+  std::vector<PrivateDtor> v; // { dg-error "here" }
+}
+
+// { dg-error "value type is destructible" "" { target *-*-* } 0 }

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-05-02 15:58 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-28 13:33 [PATCH] PR libstdc++/80553 don't allow destroying non-destructible types Jonathan Wakely
2017-05-02  9:17 ` Jonathan Wakely
2017-05-02  9:52   ` Jonathan Wakely
2017-05-02 15:31     ` Marc Glisse
2017-05-02 16:19       ` Jonathan Wakely

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).