[libcxx-commits] [libcxx] [libc++][hardening] Add hardening assertions to std::deque (PR #79397)

[via libcxx-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libcxx

Author: David Benjamin (davidben)

<details>
<summary>Changes</summary>

This aligns std::deque with std::vector w.r.t. hardening checks. There's probably more that can be done with iterators, but start with this.

This caught a bug with one of libc++'s tests. One of the erase calls in asan_caterpillar.pass.cpp was a no-op because the iterators were in the other order. (deque::erase happened to cleanly do nothing when the distance is negative.)

Fixes #<!-- -->63809

---
Full diff: https://github.com/llvm/llvm-project/pull/79397.diff


3 Files Affected:

- (modified) libcxx/include/deque (+10) 
- (modified) libcxx/test/libcxx/containers/sequences/deque/asan_caterpillar.pass.cpp (+1-1) 
- (added) libcxx/test/libcxx/containers/sequences/deque/assert.pass.cpp (+54) 


``````````diff
diff --git a/libcxx/include/deque b/libcxx/include/deque
index fca8b3d6e2c737..68a8dd51cc8925 100644
--- a/libcxx/include/deque
+++ b/libcxx/include/deque
@@ -1487,6 +1487,7 @@ void deque<_Tp, _Allocator>::shrink_to_fit() _NOEXCEPT {
 
 template <class _Tp, class _Allocator>
 inline typename deque<_Tp, _Allocator>::reference deque<_Tp, _Allocator>::operator[](size_type __i) _NOEXCEPT {
+  _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(__i < size(), "deque::operator[] index out of bounds");
   size_type __p = __start_ + __i;
   return *(*(__map_.begin() + __p / __block_size) + __p % __block_size);
 }
@@ -1494,6 +1495,7 @@ inline typename deque<_Tp, _Allocator>::reference deque<_Tp, _Allocator>::operat
 template <class _Tp, class _Allocator>
 inline typename deque<_Tp, _Allocator>::const_reference
 deque<_Tp, _Allocator>::operator[](size_type __i) const _NOEXCEPT {
+  _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(__i < size(), "deque::operator[] index out of bounds");
   size_type __p = __start_ + __i;
   return *(*(__map_.begin() + __p / __block_size) + __p % __block_size);
 }
@@ -1516,22 +1518,26 @@ inline typename deque<_Tp, _Allocator>::const_reference deque<_Tp, _Allocator>::
 
 template <class _Tp, class _Allocator>
 inline typename deque<_Tp, _Allocator>::reference deque<_Tp, _Allocator>::front() _NOEXCEPT {
+  _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(!empty(), "deque::front called on an empty deque");
   return *(*(__map_.begin() + __start_ / __block_size) + __start_ % __block_size);
 }
 
 template <class _Tp, class _Allocator>
 inline typename deque<_Tp, _Allocator>::const_reference deque<_Tp, _Allocator>::front() const _NOEXCEPT {
+  _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(!empty(), "deque::front called on an empty deque");
   return *(*(__map_.begin() + __start_ / __block_size) + __start_ % __block_size);
 }
 
 template <class _Tp, class _Allocator>
 inline typename deque<_Tp, _Allocator>::reference deque<_Tp, _Allocator>::back() _NOEXCEPT {
+  _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(!empty(), "deque::back called on an empty deque");
   size_type __p = size() + __start_ - 1;
   return *(*(__map_.begin() + __p / __block_size) + __p % __block_size);
 }
 
 template <class _Tp, class _Allocator>
 inline typename deque<_Tp, _Allocator>::const_reference deque<_Tp, _Allocator>::back() const _NOEXCEPT {
+  _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(!empty(), "deque::back called on an empty deque");
   size_type __p = size() + __start_ - 1;
   return *(*(__map_.begin() + __p / __block_size) + __p % __block_size);
 }
@@ -2255,6 +2261,7 @@ void deque<_Tp, _Allocator>::__add_back_capacity(size_type __n) {
 
 template <class _Tp, class _Allocator>
 void deque<_Tp, _Allocator>::pop_front() {
+  _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(!empty(), "deque::pop_front called on an empty deque");
   size_type __old_sz    = size();
   size_type __old_start = __start_;
   allocator_type& __a   = __alloc();
@@ -2395,6 +2402,8 @@ void deque<_Tp, _Allocator>::__move_construct_backward_and_check(
 
 template <class _Tp, class _Allocator>
 typename deque<_Tp, _Allocator>::iterator deque<_Tp, _Allocator>::erase(const_iterator __f) {
+  _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(
+      __f != end(), "deque::erase(iterator) called with a non-dereferenceable iterator");
   size_type __old_sz    = size();
   size_type __old_start = __start_;
   iterator __b          = begin();
@@ -2420,6 +2429,7 @@ typename deque<_Tp, _Allocator>::iterator deque<_Tp, _Allocator>::erase(const_it
 
 template <class _Tp, class _Allocator>
 typename deque<_Tp, _Allocator>::iterator deque<_Tp, _Allocator>::erase(const_iterator __f, const_iterator __l) {
+  _LIBCPP_ASSERT_VALID_INPUT_RANGE(__f <= __l, "deque::erase(first, last) called with an invalid range");
   size_type __old_sz    = size();
   size_type __old_start = __start_;
   difference_type __n   = __l - __f;
diff --git a/libcxx/test/libcxx/containers/sequences/deque/asan_caterpillar.pass.cpp b/libcxx/test/libcxx/containers/sequences/deque/asan_caterpillar.pass.cpp
index d3c6e0493ee28f..4c7b2317d04d3a 100644
--- a/libcxx/test/libcxx/containers/sequences/deque/asan_caterpillar.pass.cpp
+++ b/libcxx/test/libcxx/containers/sequences/deque/asan_caterpillar.pass.cpp
@@ -25,7 +25,7 @@ void test1() {
 
   for (int i = 0; i < 1100; i += 1) {
     test.insert(test.begin(), buff, buff + 320);
-    test.erase(test.end(), test.end() - 320);
+    test.erase(test.end() - 320, test.end());
   }
 
   test.insert(test.begin(), buff, buff + 32000);
diff --git a/libcxx/test/libcxx/containers/sequences/deque/assert.pass.cpp b/libcxx/test/libcxx/containers/sequences/deque/assert.pass.cpp
new file mode 100644
index 00000000000000..a0c80ce30ec78d
--- /dev/null
+++ b/libcxx/test/libcxx/containers/sequences/deque/assert.pass.cpp
@@ -0,0 +1,54 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// <deque>
+
+// Test hardening assertions for std::deque.
+
+// REQUIRES: has-unix-headers
+// UNSUPPORTED: libcpp-hardening-mode=none
+
+#include <deque>
+
+#include "check_assertion.h"
+
+int main(int, char**) {
+  std::deque<int> c;
+  TEST_LIBCPP_ASSERT_FAILURE(c.front(), "deque::front called on an empty deque");
+  TEST_LIBCPP_ASSERT_FAILURE(c.back(), "deque::back called on an empty deque");
+  TEST_LIBCPP_ASSERT_FAILURE(c[0], "deque::operator[] index out of bounds");
+  TEST_LIBCPP_ASSERT_FAILURE(c.pop_front(), "deque::pop_front called on an empty deque");
+  TEST_LIBCPP_ASSERT_FAILURE(c.pop_back(), "deque::pop_back called on an empty deque");
+
+  // Repeat the test with a const reference to test the const overloads.
+  {
+    const std::deque<int>& cc = c;
+    TEST_LIBCPP_ASSERT_FAILURE(cc.front(), "deque::front called on an empty deque");
+    TEST_LIBCPP_ASSERT_FAILURE(cc.back(), "deque::back called on an empty deque");
+    TEST_LIBCPP_ASSERT_FAILURE(cc[0], "deque::operator[] index out of bounds");
+  }
+
+  c.push_back(1);
+  c.push_back(2);
+  c.push_back(3);
+  TEST_LIBCPP_ASSERT_FAILURE(c[3], "deque::operator[] index out of bounds");
+  TEST_LIBCPP_ASSERT_FAILURE(c[100], "deque::operator[] index out of bounds");
+
+  // Repeat the test with a const reference to test the const overloads.
+  {
+    const std::deque<int>& cc = c;
+    TEST_LIBCPP_ASSERT_FAILURE(cc[3], "deque::operator[] index out of bounds");
+    TEST_LIBCPP_ASSERT_FAILURE(cc[100], "deque::operator[] index out of bounds");
+  }
+
+  TEST_LIBCPP_ASSERT_FAILURE(c.erase(c.end()), "deque::erase(iterator) called with a non-dereferenceable iterator");
+  TEST_LIBCPP_ASSERT_FAILURE(
+      c.erase(c.begin() + 1, c.begin()), "deque::erase(first, last) called with an invalid range");
+
+  return 0;
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/79397