[libcxx-commits] [libcxx] [libc++] Add exception guard for vector<bool>::__init_with_sentinel (PR #115491)

Peng Liu via libcxx-commits libcxx-commits at lists.llvm.org
Mon Nov 18 07:33:50 PST 2024


https://github.com/winner245 updated https://github.com/llvm/llvm-project/pull/115491

>From f6c209f95d65e0a8edc34601abea0e8e8ad8aa01 Mon Sep 17 00:00:00 2001
From: Peng Liu <winner245 at hotmail.com>
Date: Wed, 13 Nov 2024 09:45:01 -0500
Subject: [PATCH 1/5] Add exception tests for vector<bool>

---
 .../sequences/vector.bool/exceptions.pass.cpp | 124 ++++++++++++++++++
 libcxx/test/support/exception_test_helpers.h  |  80 +++++++++++
 2 files changed, 204 insertions(+)
 create mode 100644 libcxx/test/std/containers/sequences/vector.bool/exceptions.pass.cpp
 create mode 100644 libcxx/test/support/exception_test_helpers.h

diff --git a/libcxx/test/std/containers/sequences/vector.bool/exceptions.pass.cpp b/libcxx/test/std/containers/sequences/vector.bool/exceptions.pass.cpp
new file mode 100644
index 00000000000000..c6c27dfec59ea3
--- /dev/null
+++ b/libcxx/test/std/containers/sequences/vector.bool/exceptions.pass.cpp
@@ -0,0 +1,124 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: no-exceptions
+
+// Check that vector<bool> constructors don't leak memory when an operation inside the constructor throws an exception
+
+#include <cstddef>
+#include <memory>
+#include <type_traits>
+#include <vector>
+
+#include "exception_test_helpers.h"
+#include "test_iterators.h"
+
+int main(int, char**) {
+  using AllocVec = std::vector<bool, throwing_allocator<bool> >;
+  try { // Throw in vector() from allocator
+    AllocVec vec; // Throw on default construction
+  } catch (int) {
+  }
+  check_new_delete_called();
+
+#if TEST_STD_VER >= 14 
+  try { // Throw in vector(size_type, const allocator_type&) from allocator
+    throwing_allocator<bool> alloc(false, true); // Throw on copy only
+    AllocVec get_alloc(0, alloc);
+  } catch (int) {
+  }
+  check_new_delete_called();
+#endif  // TEST_STD_VER >= 14
+
+  try { // Throw in vector(size_type, const value_type&, const allocator_type&) from allocator
+    throwing_allocator<bool> alloc(false, true); // Throw on copy only
+    AllocVec get_alloc(0, true, alloc);
+  } catch (int) {
+  }
+  check_new_delete_called();
+
+  try { // Throw in vector(InputIterator, InputIterator) from input iterator
+    std::vector<bool> vec((throwing_iterator<bool, std::input_iterator_tag>()), throwing_iterator<bool, std::input_iterator_tag>(2));
+  } catch (int) {
+  }
+  check_new_delete_called();
+
+  try { // Throw in vector(InputIterator, InputIterator) from forward iterator
+    std::vector<bool> vec((throwing_iterator<bool, std::forward_iterator_tag>()), throwing_iterator<bool, std::forward_iterator_tag>(2));
+  } catch (int) {
+  }
+  check_new_delete_called();
+
+  try { // Throw in vector(InputIterator, InputIterator) from allocator
+    bool a[] = {true, true};
+    AllocVec vec(cpp17_input_iterator<bool*>(a), cpp17_input_iterator<bool*>(a + 2));  // throwing_allocator throws on default construction
+  } catch (int) {
+  }
+  check_new_delete_called();
+
+  try { // Throw in vector(InputIterator, InputIterator, const allocator_type&) from input iterator
+    std::allocator<bool> alloc;
+    std::vector<bool> vec(throwing_iterator<bool, std::input_iterator_tag>(), throwing_iterator<bool, std::input_iterator_tag>(2), alloc);
+  } catch (int) {
+  }
+  check_new_delete_called();
+
+  try { // Throw in vector(InputIterator, InputIterator, const allocator_type&) from forward iterator
+    std::allocator<bool> alloc;
+    std::vector<bool> vec(throwing_iterator<bool, std::forward_iterator_tag>(), throwing_iterator<bool, std::forward_iterator_tag>(2), alloc);
+  } catch (int) {
+  }
+  check_new_delete_called();
+
+  try { // Throw in vector(InputIterator, InputIterator, const allocator_type&) from allocator
+    bool a[] = {true, true};
+    throwing_allocator<bool> alloc(false, true); // Throw on copy only
+    AllocVec vec(cpp17_input_iterator<bool*>(a), cpp17_input_iterator<bool*>(a + 2), alloc);
+  } catch (int) { 
+  }
+  check_new_delete_called();
+
+  try { // Throw in vector(InputIterator, InputIterator, const allocator_type&) from allocator
+    bool a[] = {true, true};
+    throwing_allocator<bool> alloc(false, true); // Throw on copy only
+    AllocVec vec(forward_iterator<bool*>(a), forward_iterator<bool*>(a + 2), alloc);
+  } catch (int) {
+  }
+  check_new_delete_called();
+
+#if TEST_STD_VER >= 11
+  try { // Throw in vector(const vector&, const allocator_type&) from allocator
+    throwing_allocator<bool> alloc(false, false);
+    AllocVec vec(alloc);
+    vec.emplace_back(true);
+    alloc.throw_on_copy_ = true;
+    AllocVec vec2(vec, alloc);
+  } catch (int) {
+  }
+  check_new_delete_called();
+
+  try { // Throw in vector(vector&&, const allocator_type&) from allocator
+    throwing_allocator<bool> alloc(false, false);
+    AllocVec vec(alloc);
+    vec.emplace_back(true);
+    alloc.throw_on_copy_ = true;
+    AllocVec vec2(std::move(vec), alloc);
+  } catch (int) {
+  }
+  check_new_delete_called();
+
+  try { // Throw in vector(initializer_list<value_type>, const allocator_type&) constructor from allocator
+    throwing_allocator<bool> alloc(false, true); // Throw on copy only
+    AllocVec vec({true, true}, alloc);
+  } catch (int) {
+  }
+  check_new_delete_called();
+#endif // TEST_STD_VER >= 11
+
+  return 0;
+}
diff --git a/libcxx/test/support/exception_test_helpers.h b/libcxx/test/support/exception_test_helpers.h
new file mode 100644
index 00000000000000..4f680e9486e0a5
--- /dev/null
+++ b/libcxx/test/support/exception_test_helpers.h
@@ -0,0 +1,80 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef VECTOR_EXCEPTION_TEST_HELPER_H
+#define VECTOR_EXCEPTION_TEST_HELPER_H
+
+#include "count_new.h"
+
+template <class T>
+struct throwing_allocator {
+  using value_type      = T;
+  using is_always_equal = std::false_type;
+
+  bool throw_on_copy_   = false; 
+
+  throwing_allocator(bool throw_on_ctor = true, bool throw_on_copy = false) : throw_on_copy_(throw_on_copy) {
+    if (throw_on_ctor)
+      throw 0;
+  }
+
+  template <class U>
+  throwing_allocator(const throwing_allocator<U>& rhs) : throw_on_copy_(rhs.throw_on_copy_) {
+    if (throw_on_copy_)
+      throw 0;
+  }
+
+  T* allocate(std::size_t n) { return std::allocator<T>().allocate(n); }
+  void deallocate(T* ptr, std::size_t n) { std::allocator<T>().deallocate(ptr, n); }
+
+  template <class U>
+  friend bool operator==(const throwing_allocator&, const throwing_allocator<U>&) { return true; }
+};
+
+template <class T, class IterCat>
+struct throwing_iterator {
+  using iterator_category = IterCat;
+  using difference_type   = std::ptrdiff_t;
+  using value_type        = T;
+  using reference         = T&;
+  using pointer           = T*;
+
+  int i_;
+  T v_;
+
+  throwing_iterator(int i = 0, const T& v = T()) : i_(i), v_(v) {}
+
+  reference operator*() {
+    if (i_ == 1)
+      throw 1;
+    return v_;
+  }
+
+  friend bool operator==(const throwing_iterator& lhs, const throwing_iterator& rhs) { return lhs.i_ == rhs.i_; }
+  friend bool operator!=(const throwing_iterator& lhs, const throwing_iterator& rhs) { return lhs.i_ != rhs.i_; }
+
+  throwing_iterator& operator++() {
+    ++i_;
+    return *this;
+  }
+
+  throwing_iterator operator++(int) {
+    auto tmp = *this;
+    ++i_;
+    return tmp;
+  }
+};
+
+inline void check_new_delete_called() {
+  assert(globalMemCounter.new_called == globalMemCounter.delete_called);
+  assert(globalMemCounter.new_array_called == globalMemCounter.delete_array_called);
+  assert(globalMemCounter.aligned_new_called == globalMemCounter.aligned_delete_called);
+  assert(globalMemCounter.aligned_new_array_called == globalMemCounter.aligned_delete_array_called);
+}
+
+#endif // VECTOR_EXCEPTION_TEST_HELPER_H
\ No newline at end of file

>From 9b694dde02acf00f7589d1e0715ba50ee83c295f Mon Sep 17 00:00:00 2001
From: Peng Liu <winner245 at hotmail.com>
Date: Fri, 8 Nov 2024 00:23:42 -0500
Subject: [PATCH 2/5] Add exception guard for
 vector<bool>::__init_with_sentinel

---
 libcxx/include/__vector/vector_bool.h | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/libcxx/include/__vector/vector_bool.h b/libcxx/include/__vector/vector_bool.h
index 3b03f65b7ee549..36eb7f350ac406 100644
--- a/libcxx/include/__vector/vector_bool.h
+++ b/libcxx/include/__vector/vector_bool.h
@@ -390,18 +390,12 @@ class _LIBCPP_TEMPLATE_VIS vector<bool, _Allocator> {
   template <class _InputIterator, class _Sentinel>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void
   __init_with_sentinel(_InputIterator __first, _Sentinel __last) {
-#if _LIBCPP_HAS_EXCEPTIONS
-    try {
-#endif // _LIBCPP_HAS_EXCEPTIONS
-      for (; __first != __last; ++__first)
-        push_back(*__first);
-#if _LIBCPP_HAS_EXCEPTIONS
-    } catch (...) {
-      if (__begin_ != nullptr)
-        __storage_traits::deallocate(__alloc_, __begin_, __cap_);
-      throw;
-    }
-#endif // _LIBCPP_HAS_EXCEPTIONS
+    auto __guard = std::__make_exception_guard(__destroy_vector(*this));
+
+    for (; __first != __last; ++__first)
+      push_back(*__first);
+
+    __guard.__complete();
   }
 
   template <class _Iterator, class _Sentinel>

>From 31b58234674282afaa86bc3555de650feef83bfa Mon Sep 17 00:00:00 2001
From: Peng Liu <winner245 at hotmail.com>
Date: Wed, 13 Nov 2024 10:01:30 -0500
Subject: [PATCH 3/5] Run clang-format

---
 .../sequences/vector.bool/exceptions.pass.cpp | 26 ++++++++++++-------
 libcxx/test/support/exception_test_helpers.h  |  6 +++--
 2 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/libcxx/test/std/containers/sequences/vector.bool/exceptions.pass.cpp b/libcxx/test/std/containers/sequences/vector.bool/exceptions.pass.cpp
index c6c27dfec59ea3..a6f9488347a5c0 100644
--- a/libcxx/test/std/containers/sequences/vector.bool/exceptions.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector.bool/exceptions.pass.cpp
@@ -20,20 +20,20 @@
 
 int main(int, char**) {
   using AllocVec = std::vector<bool, throwing_allocator<bool> >;
-  try { // Throw in vector() from allocator
+  try {           // Throw in vector() from allocator
     AllocVec vec; // Throw on default construction
   } catch (int) {
   }
   check_new_delete_called();
 
-#if TEST_STD_VER >= 14 
-  try { // Throw in vector(size_type, const allocator_type&) from allocator
+#if TEST_STD_VER >= 14
+  try {                                          // Throw in vector(size_type, const allocator_type&) from allocator
     throwing_allocator<bool> alloc(false, true); // Throw on copy only
     AllocVec get_alloc(0, alloc);
   } catch (int) {
   }
   check_new_delete_called();
-#endif  // TEST_STD_VER >= 14
+#endif // TEST_STD_VER >= 14
 
   try { // Throw in vector(size_type, const value_type&, const allocator_type&) from allocator
     throwing_allocator<bool> alloc(false, true); // Throw on copy only
@@ -43,34 +43,40 @@ int main(int, char**) {
   check_new_delete_called();
 
   try { // Throw in vector(InputIterator, InputIterator) from input iterator
-    std::vector<bool> vec((throwing_iterator<bool, std::input_iterator_tag>()), throwing_iterator<bool, std::input_iterator_tag>(2));
+    std::vector<bool> vec(
+        (throwing_iterator<bool, std::input_iterator_tag>()), throwing_iterator<bool, std::input_iterator_tag>(2));
   } catch (int) {
   }
   check_new_delete_called();
 
   try { // Throw in vector(InputIterator, InputIterator) from forward iterator
-    std::vector<bool> vec((throwing_iterator<bool, std::forward_iterator_tag>()), throwing_iterator<bool, std::forward_iterator_tag>(2));
+    std::vector<bool> vec(
+        (throwing_iterator<bool, std::forward_iterator_tag>()), throwing_iterator<bool, std::forward_iterator_tag>(2));
   } catch (int) {
   }
   check_new_delete_called();
 
   try { // Throw in vector(InputIterator, InputIterator) from allocator
     bool a[] = {true, true};
-    AllocVec vec(cpp17_input_iterator<bool*>(a), cpp17_input_iterator<bool*>(a + 2));  // throwing_allocator throws on default construction
+    AllocVec vec(cpp17_input_iterator<bool*>(a),
+                 cpp17_input_iterator<bool*>(a + 2)); // throwing_allocator throws on default construction
   } catch (int) {
   }
   check_new_delete_called();
 
   try { // Throw in vector(InputIterator, InputIterator, const allocator_type&) from input iterator
     std::allocator<bool> alloc;
-    std::vector<bool> vec(throwing_iterator<bool, std::input_iterator_tag>(), throwing_iterator<bool, std::input_iterator_tag>(2), alloc);
+    std::vector<bool> vec(
+        throwing_iterator<bool, std::input_iterator_tag>(), throwing_iterator<bool, std::input_iterator_tag>(2), alloc);
   } catch (int) {
   }
   check_new_delete_called();
 
   try { // Throw in vector(InputIterator, InputIterator, const allocator_type&) from forward iterator
     std::allocator<bool> alloc;
-    std::vector<bool> vec(throwing_iterator<bool, std::forward_iterator_tag>(), throwing_iterator<bool, std::forward_iterator_tag>(2), alloc);
+    std::vector<bool> vec(throwing_iterator<bool, std::forward_iterator_tag>(),
+                          throwing_iterator<bool, std::forward_iterator_tag>(2),
+                          alloc);
   } catch (int) {
   }
   check_new_delete_called();
@@ -79,7 +85,7 @@ int main(int, char**) {
     bool a[] = {true, true};
     throwing_allocator<bool> alloc(false, true); // Throw on copy only
     AllocVec vec(cpp17_input_iterator<bool*>(a), cpp17_input_iterator<bool*>(a + 2), alloc);
-  } catch (int) { 
+  } catch (int) {
   }
   check_new_delete_called();
 
diff --git a/libcxx/test/support/exception_test_helpers.h b/libcxx/test/support/exception_test_helpers.h
index 4f680e9486e0a5..44be6916cb0226 100644
--- a/libcxx/test/support/exception_test_helpers.h
+++ b/libcxx/test/support/exception_test_helpers.h
@@ -16,7 +16,7 @@ struct throwing_allocator {
   using value_type      = T;
   using is_always_equal = std::false_type;
 
-  bool throw_on_copy_   = false; 
+  bool throw_on_copy_ = false;
 
   throwing_allocator(bool throw_on_ctor = true, bool throw_on_copy = false) : throw_on_copy_(throw_on_copy) {
     if (throw_on_ctor)
@@ -33,7 +33,9 @@ struct throwing_allocator {
   void deallocate(T* ptr, std::size_t n) { std::allocator<T>().deallocate(ptr, n); }
 
   template <class U>
-  friend bool operator==(const throwing_allocator&, const throwing_allocator<U>&) { return true; }
+  friend bool operator==(const throwing_allocator&, const throwing_allocator<U>&) {
+    return true;
+  }
 };
 
 template <class T, class IterCat>

>From faf92d865465026d074d4db761b039ea9b07b14c Mon Sep 17 00:00:00 2001
From: Peng Liu <winner245 at hotmail.com>
Date: Wed, 13 Nov 2024 10:20:06 -0500
Subject: [PATCH 4/5] Update header guard to match the header name

---
 libcxx/test/support/exception_test_helpers.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libcxx/test/support/exception_test_helpers.h b/libcxx/test/support/exception_test_helpers.h
index 44be6916cb0226..96cf407a94ae4e 100644
--- a/libcxx/test/support/exception_test_helpers.h
+++ b/libcxx/test/support/exception_test_helpers.h
@@ -6,8 +6,8 @@
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef VECTOR_EXCEPTION_TEST_HELPER_H
-#define VECTOR_EXCEPTION_TEST_HELPER_H
+#ifndef EXCEPTION_TEST_HELPER_H
+#define EXCEPTION_TEST_HELPER_H
 
 #include "count_new.h"
 
@@ -79,4 +79,4 @@ inline void check_new_delete_called() {
   assert(globalMemCounter.aligned_new_array_called == globalMemCounter.aligned_delete_array_called);
 }
 
-#endif // VECTOR_EXCEPTION_TEST_HELPER_H
\ No newline at end of file
+#endif // EXCEPTION_TEST_HELPER_H
\ No newline at end of file

>From ca65517a30f8a0248c9644a9daba80ba847844ad Mon Sep 17 00:00:00 2001
From: Peng Liu <winner245 at hotmail.com>
Date: Wed, 13 Nov 2024 11:33:08 -0500
Subject: [PATCH 5/5] Fix compatibility

---
 .../std/containers/sequences/vector.bool/exceptions.pass.cpp  | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libcxx/test/std/containers/sequences/vector.bool/exceptions.pass.cpp b/libcxx/test/std/containers/sequences/vector.bool/exceptions.pass.cpp
index a6f9488347a5c0..49a20b8b7f8961 100644
--- a/libcxx/test/std/containers/sequences/vector.bool/exceptions.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector.bool/exceptions.pass.cpp
@@ -101,7 +101,7 @@ int main(int, char**) {
   try { // Throw in vector(const vector&, const allocator_type&) from allocator
     throwing_allocator<bool> alloc(false, false);
     AllocVec vec(alloc);
-    vec.emplace_back(true);
+    vec.push_back(true);
     alloc.throw_on_copy_ = true;
     AllocVec vec2(vec, alloc);
   } catch (int) {
@@ -111,7 +111,7 @@ int main(int, char**) {
   try { // Throw in vector(vector&&, const allocator_type&) from allocator
     throwing_allocator<bool> alloc(false, false);
     AllocVec vec(alloc);
-    vec.emplace_back(true);
+    vec.push_back(true);
     alloc.throw_on_copy_ = true;
     AllocVec vec2(std::move(vec), alloc);
   } catch (int) {



More information about the libcxx-commits mailing list