[libcxx-commits] [libcxx] [libc++] Add a utility to check whether a range is valid (PR #87665)

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Fri Apr 12 10:59:01 PDT 2024


https://github.com/ldionne updated https://github.com/llvm/llvm-project/pull/87665

>From edcf19d4480372074cd1b31c148ff6ca6541b129 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Thu, 4 Apr 2024 13:17:08 -0400
Subject: [PATCH 1/4] [libc++] Add a utility to check whether a range is valid

In the future, this utility could be made to also work with iterators,
including bounded iterators. We could also query the ASAN runtime for
this information when it's around.
---
 libcxx/include/CMakeLists.txt                 |  1 +
 .../include/__utility/is_pointer_in_range.h   |  5 +-
 libcxx/include/__utility/is_valid_range.h     | 38 +++++++++++
 .../libcxx/utilities/is_valid_range.pass.cpp  | 64 +++++++++++++++++++
 4 files changed, 106 insertions(+), 2 deletions(-)
 create mode 100644 libcxx/include/__utility/is_valid_range.h
 create mode 100644 libcxx/test/libcxx/utilities/is_valid_range.pass.cpp

diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt
index db3980342f50bf..a3dce266ea912d 100644
--- a/libcxx/include/CMakeLists.txt
+++ b/libcxx/include/CMakeLists.txt
@@ -856,6 +856,7 @@ set(files
   __utility/in_place.h
   __utility/integer_sequence.h
   __utility/is_pointer_in_range.h
+  __utility/is_valid_range.h
   __utility/move.h
   __utility/no_destroy.h
   __utility/pair.h
diff --git a/libcxx/include/__utility/is_pointer_in_range.h b/libcxx/include/__utility/is_pointer_in_range.h
index 68cdfea6f94529..fa032d80cb2630 100644
--- a/libcxx/include/__utility/is_pointer_in_range.h
+++ b/libcxx/include/__utility/is_pointer_in_range.h
@@ -17,6 +17,7 @@
 #include <__type_traits/is_constant_evaluated.h>
 #include <__type_traits/void_t.h>
 #include <__utility/declval.h>
+#include <__utility/is_valid_range.h>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
@@ -34,9 +35,9 @@ struct __is_less_than_comparable<_Tp, _Up, __void_t<decltype(std::declval<_Tp>()
 template <class _Tp, class _Up, __enable_if_t<__is_less_than_comparable<const _Tp*, const _Up*>::value, int> = 0>
 _LIBCPP_CONSTEXPR_SINCE_CXX14 _LIBCPP_HIDE_FROM_ABI _LIBCPP_NO_SANITIZE("address") bool __is_pointer_in_range(
     const _Tp* __begin, const _Tp* __end, const _Up* __ptr) {
-  if (__libcpp_is_constant_evaluated()) {
-    _LIBCPP_ASSERT_VALID_INPUT_RANGE(__builtin_constant_p(__begin <= __end), "__begin and __end do not form a range");
+  _LIBCPP_ASSERT_VALID_INPUT_RANGE(std::__is_valid_range(__begin, __end), "[__begin, __end) is not a valid range");
 
+  if (__libcpp_is_constant_evaluated()) {
     // If this is not a constant during constant evaluation we know that __ptr is not part of the allocation where
     // [__begin, __end) is.
     if (!__builtin_constant_p(__begin <= __ptr && __ptr < __end))
diff --git a/libcxx/include/__utility/is_valid_range.h b/libcxx/include/__utility/is_valid_range.h
new file mode 100644
index 00000000000000..0cf1ab5420a36b
--- /dev/null
+++ b/libcxx/include/__utility/is_valid_range.h
@@ -0,0 +1,38 @@
+//===----------------------------------------------------------------------===//
+//
+// 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 _LIBCPP___UTILITY_IS_VALID_RANGE_H
+#define _LIBCPP___UTILITY_IS_VALID_RANGE_H
+
+#include <__algorithm/comp.h>
+#include <__config>
+#include <__type_traits/is_constant_evaluated.h>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#  pragma GCC system_header
+#endif
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+template <class _Tp>
+_LIBCPP_CONSTEXPR_SINCE_CXX14 _LIBCPP_HIDE_FROM_ABI _LIBCPP_NO_SANITIZE("address") bool __is_valid_range(
+    const _Tp* __first, const _Tp* __last) {
+  if (__libcpp_is_constant_evaluated()) {
+    // If this is not a constant during constant evaluation, that is because __first and __last are not
+    // part of the same allocation. If they are part of the same allocation, we must still make sure they
+    // are ordered properly.
+    return __builtin_constant_p(__first <= __last) && __first <= __last;
+  }
+
+  // Checking this for unrelated pointers is technically UB, but no compiler optimizes based on it (currently).
+  return !__less<>()(__last, __first);
+}
+
+_LIBCPP_END_NAMESPACE_STD
+
+#endif // _LIBCPP___UTILITY_IS_VALID_RANGE_H
diff --git a/libcxx/test/libcxx/utilities/is_valid_range.pass.cpp b/libcxx/test/libcxx/utilities/is_valid_range.pass.cpp
new file mode 100644
index 00000000000000..e5ce71c2bdc48a
--- /dev/null
+++ b/libcxx/test/libcxx/utilities/is_valid_range.pass.cpp
@@ -0,0 +1,64 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include <__utility/is_valid_range.h>
+#include <cassert>
+
+#include "test_macros.h"
+
+template <class T>
+TEST_CONSTEXPR_CXX14 void check_type() {
+  {
+    T i = 0;
+    T j = 0;
+    assert(std::__is_valid_range(&i, &i));
+
+    assert(!std::__is_valid_range(&i, &j));
+    assert(!std::__is_valid_range(&i + 1, &i));
+
+    // We detect this one as being a valid range.
+    // Ideally we would detect it as an invalid range, but this may not be implementable.
+    assert(std::__is_valid_range(&i, &i + 1));
+  }
+
+  {
+    T arr[3] = {1, 2, 3};
+    assert(std::__is_valid_range(&arr[0], &arr[0]));
+    assert(std::__is_valid_range(&arr[0], &arr[1]));
+    assert(std::__is_valid_range(&arr[0], &arr[2]));
+
+    assert(!std::__is_valid_range(&arr[1], &arr[0]));
+    assert(!std::__is_valid_range(&arr[2], &arr[0]));
+  }
+
+#if TEST_STD_VER >= 20
+  {
+    T* arr = new int[4]{1, 2, 3, 4};
+    assert(std::__is_valid_range(arr, arr + 4));
+    delete[] arr;
+  }
+#endif
+}
+
+TEST_CONSTEXPR_CXX14 bool test() {
+  check_type<int>();
+  check_type<const int>();
+  check_type<volatile int>();
+  check_type<const volatile int>();
+
+  return true;
+}
+
+int main(int, char**) {
+  test();
+#if TEST_STD_VER >= 14
+  static_assert(test(), "");
+#endif
+
+  return 0;
+}

>From 469834e3cc92328020b54046f111ada07317e69b Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Thu, 4 Apr 2024 13:51:30 -0400
Subject: [PATCH 2/4] clang-format

---
 libcxx/include/__utility/is_valid_range.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libcxx/include/__utility/is_valid_range.h b/libcxx/include/__utility/is_valid_range.h
index 0cf1ab5420a36b..2c6b6627fe4287 100644
--- a/libcxx/include/__utility/is_valid_range.h
+++ b/libcxx/include/__utility/is_valid_range.h
@@ -20,8 +20,8 @@
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 template <class _Tp>
-_LIBCPP_CONSTEXPR_SINCE_CXX14 _LIBCPP_HIDE_FROM_ABI _LIBCPP_NO_SANITIZE("address") bool __is_valid_range(
-    const _Tp* __first, const _Tp* __last) {
+_LIBCPP_CONSTEXPR_SINCE_CXX14 _LIBCPP_HIDE_FROM_ABI _LIBCPP_NO_SANITIZE("address") bool
+__is_valid_range(const _Tp* __first, const _Tp* __last) {
   if (__libcpp_is_constant_evaluated()) {
     // If this is not a constant during constant evaluation, that is because __first and __last are not
     // part of the same allocation. If they are part of the same allocation, we must still make sure they

>From 5e03959bc5fef365c08f6db63d54c9ab3c936b93 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Fri, 12 Apr 2024 09:21:33 -0400
Subject: [PATCH 3/4] Review comments and CI fixes

---
 libcxx/include/__utility/is_pointer_in_range.h       | 1 -
 libcxx/include/__utility/is_valid_range.h            | 1 -
 libcxx/include/libcxx.imp                            | 1 +
 libcxx/include/module.modulemap                      | 1 +
 libcxx/test/libcxx/utilities/is_valid_range.pass.cpp | 5 +----
 5 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/libcxx/include/__utility/is_pointer_in_range.h b/libcxx/include/__utility/is_pointer_in_range.h
index fa032d80cb2630..9eee8bf811c603 100644
--- a/libcxx/include/__utility/is_pointer_in_range.h
+++ b/libcxx/include/__utility/is_pointer_in_range.h
@@ -44,7 +44,6 @@ _LIBCPP_CONSTEXPR_SINCE_CXX14 _LIBCPP_HIDE_FROM_ABI _LIBCPP_NO_SANITIZE("address
       return false;
   }
 
-  // Checking this for unrelated pointers is technically UB, but no compiler optimizes based on it (currently).
   return !__less<>()(__ptr, __begin) && __less<>()(__ptr, __end);
 }
 
diff --git a/libcxx/include/__utility/is_valid_range.h b/libcxx/include/__utility/is_valid_range.h
index 2c6b6627fe4287..7286662dbf3092 100644
--- a/libcxx/include/__utility/is_valid_range.h
+++ b/libcxx/include/__utility/is_valid_range.h
@@ -29,7 +29,6 @@ __is_valid_range(const _Tp* __first, const _Tp* __last) {
     return __builtin_constant_p(__first <= __last) && __first <= __last;
   }
 
-  // Checking this for unrelated pointers is technically UB, but no compiler optimizes based on it (currently).
   return !__less<>()(__last, __first);
 }
 
diff --git a/libcxx/include/libcxx.imp b/libcxx/include/libcxx.imp
index 2cb1fa5e1e2aa0..2bf41e89954834 100644
--- a/libcxx/include/libcxx.imp
+++ b/libcxx/include/libcxx.imp
@@ -851,6 +851,7 @@
   { include: [ "<__utility/in_place.h>", "private", "<utility>", "public" ] },
   { include: [ "<__utility/integer_sequence.h>", "private", "<utility>", "public" ] },
   { include: [ "<__utility/is_pointer_in_range.h>", "private", "<utility>", "public" ] },
+  { include: [ "<__utility/is_valid_range.h>", "private", "<utility>", "public" ] },
   { include: [ "<__utility/move.h>", "private", "<utility>", "public" ] },
   { include: [ "<__utility/no_destroy.h>", "private", "<utility>", "public" ] },
   { include: [ "<__utility/pair.h>", "private", "<utility>", "public" ] },
diff --git a/libcxx/include/module.modulemap b/libcxx/include/module.modulemap
index 6d4dcc2511f3ec..03922ff41365f9 100644
--- a/libcxx/include/module.modulemap
+++ b/libcxx/include/module.modulemap
@@ -2064,6 +2064,7 @@ module std_private_utility_forward_like           [system] { header "__utility/f
 module std_private_utility_in_place               [system] { header "__utility/in_place.h" }
 module std_private_utility_integer_sequence       [system] { header "__utility/integer_sequence.h" }
 module std_private_utility_is_pointer_in_range    [system] { header "__utility/is_pointer_in_range.h" }
+module std_private_utility_is_valid_range         [system] { header "__utility/is_valid_range.h" }
 module std_private_utility_move                   [system] {
   header "__utility/move.h"
   export std_private_type_traits_is_copy_constructible
diff --git a/libcxx/test/libcxx/utilities/is_valid_range.pass.cpp b/libcxx/test/libcxx/utilities/is_valid_range.pass.cpp
index e5ce71c2bdc48a..1cbc560e5e1824 100644
--- a/libcxx/test/libcxx/utilities/is_valid_range.pass.cpp
+++ b/libcxx/test/libcxx/utilities/is_valid_range.pass.cpp
@@ -17,13 +17,10 @@ TEST_CONSTEXPR_CXX14 void check_type() {
     T i = 0;
     T j = 0;
     assert(std::__is_valid_range(&i, &i));
+    assert(std::__is_valid_range(&i, &i + 1));
 
     assert(!std::__is_valid_range(&i, &j));
     assert(!std::__is_valid_range(&i + 1, &i));
-
-    // We detect this one as being a valid range.
-    // Ideally we would detect it as an invalid range, but this may not be implementable.
-    assert(std::__is_valid_range(&i, &i + 1));
   }
 
   {

>From 8fd00e2a3224c04028791ba14b99199a4a93be93 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Fri, 12 Apr 2024 13:58:47 -0400
Subject: [PATCH 4/4] Make tests portable

---
 .../libcxx/utilities/is_valid_range.pass.cpp  | 41 +++++++++++--------
 1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/libcxx/test/libcxx/utilities/is_valid_range.pass.cpp b/libcxx/test/libcxx/utilities/is_valid_range.pass.cpp
index 1cbc560e5e1824..345e2feeda81c7 100644
--- a/libcxx/test/libcxx/utilities/is_valid_range.pass.cpp
+++ b/libcxx/test/libcxx/utilities/is_valid_range.pass.cpp
@@ -11,42 +11,49 @@
 
 #include "test_macros.h"
 
-template <class T>
+template <class T, class TQualified>
 TEST_CONSTEXPR_CXX14 void check_type() {
   {
-    T i = 0;
-    T j = 0;
-    assert(std::__is_valid_range(&i, &i));
-    assert(std::__is_valid_range(&i, &i + 1));
+    // We need to ensure that the addresses of i and j are ordered as &i < &j for
+    // the test below to work portably, so we define them in a struct.
+    struct {
+      T i = 0;
+      T j = 0;
+    } storage;
+    assert(std::__is_valid_range(static_cast<TQualified*>(&storage.i), static_cast<TQualified*>(&storage.i)));
+    assert(std::__is_valid_range(static_cast<TQualified*>(&storage.i), static_cast<TQualified*>(&storage.i + 1)));
 
-    assert(!std::__is_valid_range(&i, &j));
-    assert(!std::__is_valid_range(&i + 1, &i));
+    assert(!std::__is_valid_range(static_cast<TQualified*>(&storage.j), static_cast<TQualified*>(&storage.i)));
+    assert(!std::__is_valid_range(static_cast<TQualified*>(&storage.i + 1), static_cast<TQualified*>(&storage.i)));
+
+    // We detect this as being a valid range even though it is not really valid.
+    assert(std::__is_valid_range(static_cast<TQualified*>(&storage.i), static_cast<TQualified*>(&storage.j)));
   }
 
   {
     T arr[3] = {1, 2, 3};
-    assert(std::__is_valid_range(&arr[0], &arr[0]));
-    assert(std::__is_valid_range(&arr[0], &arr[1]));
-    assert(std::__is_valid_range(&arr[0], &arr[2]));
+    assert(std::__is_valid_range(static_cast<TQualified*>(&arr[0]), static_cast<TQualified*>(&arr[0])));
+    assert(std::__is_valid_range(static_cast<TQualified*>(&arr[0]), static_cast<TQualified*>(&arr[1])));
+    assert(std::__is_valid_range(static_cast<TQualified*>(&arr[0]), static_cast<TQualified*>(&arr[2])));
 
-    assert(!std::__is_valid_range(&arr[1], &arr[0]));
-    assert(!std::__is_valid_range(&arr[2], &arr[0]));
+    assert(!std::__is_valid_range(static_cast<TQualified*>(&arr[1]), static_cast<TQualified*>(&arr[0])));
+    assert(!std::__is_valid_range(static_cast<TQualified*>(&arr[2]), static_cast<TQualified*>(&arr[0])));
   }
 
 #if TEST_STD_VER >= 20
   {
     T* arr = new int[4]{1, 2, 3, 4};
-    assert(std::__is_valid_range(arr, arr + 4));
+    assert(std::__is_valid_range(static_cast<TQualified*>(arr), static_cast<TQualified*>(arr + 4)));
     delete[] arr;
   }
 #endif
 }
 
 TEST_CONSTEXPR_CXX14 bool test() {
-  check_type<int>();
-  check_type<const int>();
-  check_type<volatile int>();
-  check_type<const volatile int>();
+  check_type<int, int>();
+  check_type<int, int const>();
+  check_type<int, int volatile>();
+  check_type<int, int const volatile>();
 
   return true;
 }



More information about the libcxx-commits mailing list