[libc++] Fix constructing bitset from non-null-terminated arrays

[frederick-vs-ja]

Unconditional evaluation of char_traits<_CharT>::length(__str) is problematic, because it causes UB when __str points to a non-null-terminated array. We should only call length (currently, in basic_string_view's constructor) when __n == npos per [bitset.cons]/8.

Drive-by change: Reduction of conditional compilation, given that

• both basic_string_view<_CharT>::size_type and basic_string<_CharT>::size_type must be size_t, and thus
• both basic_string_view<_CharT>::npos and basic_string<_CharT>::npos must be size_t(-1).

For the type sameness in the standard wording, see:

• [string.view.template.general]
• [basic.string.general]
• [allocator.traits.types]/6
• [default.allocator.general]/1

Fixes #143684.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: A. Jiang (frederick-vs-ja)

Changes

Unconditional evaluation of char_traits<_CharT>::length(__str) is problematic, because it causes UB when __str points to a non-null-terminated array. We should only call length (currently, in basic_string_view's constructor) when __n == npos per [bitset.cons]/8.

Drive-by change: Reduction of conditional compilation, given that

• both basic_string_view<_CharT>::size_type and basic_string<_CharT>::size_type must be size_t, and thus
• both basic_string_view<_CharT>::npos and basic_string<_CharT>::npos must be size_t(-1).

For the type sameness in the standard wording, see:

• [string.view.template.general]
• [basic.string.general]
• [allocator.traits.types]/6
• [default.allocator.general]/1

Fixes #143684.

---

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

2 Files Affected:

• (modified) libcxx/include/bitset (+5-8)
• (modified) libcxx/test/std/utilities/template.bitset/bitset.cons/char_ptr_ctor.pass.cpp (+28)

diff --git a/libcxx/include/bitset b/libcxx/include/bitset
index 88dc0e08c995d..8f2c0611b6436 100644
--- a/libcxx/include/bitset
+++ b/libcxx/include/bitset
@@ -645,16 +645,13 @@ public:
   template <class _CharT, __enable_if_t<_IsCharLikeType<_CharT>::value, int> = 0>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 explicit bitset(
       const _CharT* __str,
-#  if _LIBCPP_STD_VER >= 26
-      typename basic_string_view<_CharT>::size_type __n = basic_string_view<_CharT>::npos,
-#  else
-      typename basic_string<_CharT>::size_type __n = basic_string<_CharT>::npos,
-#  endif
+      size_t __n    = static_cast<size_t>(-1),
       _CharT __zero = _CharT('0'),
       _CharT __one  = _CharT('1')) {
-
-    size_t __rlen = std::min(__n, char_traits<_CharT>::length(__str));
-    __init_from_string_view(basic_string_view<_CharT>(__str, __rlen), __zero, __one);
+    if (__n == static_cast<size_t>(-1))
+      __init_from_string_view(basic_string_view<_CharT>(__str), __zero, __one);
+    else
+      __init_from_string_view(basic_string_view<_CharT>(__str, __n), __zero, __one);
   }
 #  if _LIBCPP_STD_VER >= 26
   template <class _CharT, class _Traits>
diff --git a/libcxx/test/std/utilities/template.bitset/bitset.cons/char_ptr_ctor.pass.cpp b/libcxx/test/std/utilities/template.bitset/bitset.cons/char_ptr_ctor.pass.cpp
index 86b144ed87b70..8ad7d513ea7c5 100644
--- a/libcxx/test/std/utilities/template.bitset/bitset.cons/char_ptr_ctor.pass.cpp
+++ b/libcxx/test/std/utilities/template.bitset/bitset.cons/char_ptr_ctor.pass.cpp
@@ -72,6 +72,34 @@ TEST_CONSTEXPR_CXX23 void test_char_pointer_ctor()
     for (std::size_t i = 10; i < v.size(); ++i)
         assert(v[i] == false);
   }
+  // See https://github.com/llvm/llvm-project/issues/143684
+  {
+    const char not_null_terminated[] = {'1', '0', '1', '0', '1', '0', '1', '0', '1', '0'};
+    std::bitset<N> v(not_null_terminated, 10);
+    std::size_t M = std::min<std::size_t>(v.size(), 10);
+    for (std::size_t i = 0; i < M; ++i)
+      assert(v[i] == (not_null_terminated[M - 1 - i] == '1'));
+    for (std::size_t i = 10; i < v.size(); ++i)
+      assert(v[i] == false);
+  }
+  {
+    const char not_null_terminated[] = {'1', 'a', '1', 'a', '1', 'a', '1', 'a', '1', 'a'};
+    std::bitset<N> v(not_null_terminated, 10, 'a');
+    std::size_t M = std::min<std::size_t>(v.size(), 10);
+    for (std::size_t i = 0; i < M; ++i)
+      assert(v[i] == (not_null_terminated[M - 1 - i] == '1'));
+    for (std::size_t i = 10; i < v.size(); ++i)
+      assert(v[i] == false);
+  }
+  {
+    const char not_null_terminated[] = {'b', 'a', 'b', 'a', 'b', 'a', 'b', 'a', 'b', 'a'};
+    std::bitset<N> v(not_null_terminated, 10, 'a', 'b');
+    std::size_t M = std::min<std::size_t>(v.size(), 10);
+    for (std::size_t i = 0; i < M; ++i)
+      assert(v[i] == (not_null_terminated[M - 1 - i] == 'b'));
+    for (std::size_t i = 10; i < v.size(); ++i)
+      assert(v[i] == false);
+  }
 }
 
 TEST_CONSTEXPR_CXX23 bool test() {

[philnik777]

LGTM % nit assuming the CI is green.

[ldionne]

CI seems to be as green as it'll ever be in the current conditions. Merging.