<vector>, <string>: Avoid dereferencing null (fancy) pointers... (#…

…4275)

... when calling `_Seek_to`/`_Unwrapped` on or comparing `vector` and `basic_string`'s iterators

Co-authored-by: Stephan T. Lavavej <stl@nuwen.net>

stl/inc/vector

@@ -169,7 +169,7 @@ public:
 #if _HAS_CXX20
     _NODISCARD constexpr strong_ordering operator<=>(const _Vector_const_iterator& _Right) const noexcept {
         _Compat(_Right);
-        return _Unfancy(_Ptr) <=> _Unfancy(_Right._Ptr);
+        return _STD _Unfancy_maybe_null(_Ptr) <=> _STD _Unfancy_maybe_null(_Right._Ptr);
     }
 #else // ^^^ _HAS_CXX20 / !_HAS_CXX20 vvv
     _NODISCARD bool operator!=(const _Vector_const_iterator& _Right) const noexcept {
@@ -214,11 +214,11 @@ public:
     using _Prevent_inheriting_unwrap = _Vector_const_iterator;
 
     _NODISCARD _CONSTEXPR20 const value_type* _Unwrapped() const noexcept {
-        return _Unfancy(_Ptr);
+        return _STD _Unfancy_maybe_null(_Ptr);
     }
 
     _CONSTEXPR20 void _Seek_to(const value_type* _It) noexcept {
-        _Ptr = _Refancy<_Tptr>(const_cast<value_type*>(_It));
+        _Ptr = _STD _Refancy_maybe_null<_Tptr>(const_cast<value_type*>(_It));
     }
 
     _Tptr _Ptr; // pointer to element in vector
@@ -342,7 +342,7 @@ public:
     using _Prevent_inheriting_unwrap = _Vector_iterator;
 
     _NODISCARD _CONSTEXPR20 value_type* _Unwrapped() const noexcept {
-        return _Unfancy(this->_Ptr);
+        return _STD _Unfancy_maybe_null(this->_Ptr);
     }
 };
 
@@ -425,17 +425,6 @@ public:
     pointer _Myend; // pointer to end of array
 };
 
-template <class _Ptrty>
-constexpr auto _Unfancy_maybe_null(_Ptrty _Ptr) noexcept {
-    // converts from a (potentially null) fancy pointer to a plain pointer
-    return _Ptr ? _STD addressof(*_Ptr) : nullptr;
-}
-
-template <class _Ty>
-constexpr _Ty* _Unfancy_maybe_null(_Ty* _Ptr) noexcept { // do nothing for plain pointers
-    return _Ptr;
-}
-
 _EXPORT_STD template <class _Ty, class _Alloc = allocator<_Ty>>
 class vector { // varying size array of values
 private:

stl/inc/xmemory

@@ -36,6 +36,17 @@ _NODISCARD constexpr _Ty* _Unfancy(_Ty* _Ptr) noexcept { // do nothing for plain
     return _Ptr;
 }
 
+template <class _Ptrty>
+constexpr auto _Unfancy_maybe_null(_Ptrty _Ptr) noexcept {
+    // converts from a (potentially null) fancy pointer to a plain pointer
+    return _Ptr ? _STD addressof(*_Ptr) : nullptr;
+}
+
+template <class _Ty>
+constexpr _Ty* _Unfancy_maybe_null(_Ty* _Ptr) noexcept { // do nothing for plain pointers
+    return _Ptr;
+}
+
 template <class _Ty>
 struct _NODISCARD _Tidy_guard { // class with destructor that calls _Tidy
     _Ty* _Target;
@@ -300,6 +311,16 @@ _CONSTEXPR20 _Pointer _Refancy(_Pointer _Ptr) noexcept {
     return _Ptr;
 }
 
+template <class _Pointer, enable_if_t<!is_pointer_v<_Pointer>, int> = 0>
+_CONSTEXPR20 _Pointer _Refancy_maybe_null(typename pointer_traits<_Pointer>::element_type* _Ptr) noexcept {
+    return _Ptr == nullptr ? _Pointer() : pointer_traits<_Pointer>::pointer_to(*_Ptr);
+}
+
+template <class _Pointer, enable_if_t<is_pointer_v<_Pointer>, int> = 0>
+_CONSTEXPR20 _Pointer _Refancy_maybe_null(_Pointer _Ptr) noexcept {
+    return _Ptr;
+}
+
 template <class _NoThrowFwdIt, class _NoThrowSentinel>
 _CONSTEXPR20 void _Destroy_range(_NoThrowFwdIt _First, _NoThrowSentinel _Last) noexcept;
 

stl/inc/xstring

@@ -1984,7 +1984,7 @@ public:
 #if _HAS_CXX20
     _NODISCARD constexpr strong_ordering operator<=>(const _String_const_iterator& _Right) const noexcept {
         _Compat(_Right);
-        return _Unfancy(_Ptr) <=> _Unfancy(_Right._Ptr);
+        return _STD _Unfancy_maybe_null(_Ptr) <=> _STD _Unfancy_maybe_null(_Right._Ptr);
     }
 #else // ^^^ _HAS_CXX20 / !_HAS_CXX20 vvv
     _NODISCARD bool operator!=(const _String_const_iterator& _Right) const noexcept {
@@ -2030,11 +2030,11 @@ public:
     using _Prevent_inheriting_unwrap = _String_const_iterator;
 
     _NODISCARD _CONSTEXPR20 const value_type* _Unwrapped() const noexcept {
-        return _Unfancy(_Ptr);
+        return _STD _Unfancy_maybe_null(_Ptr);
     }
 
     _CONSTEXPR20 void _Seek_to(const value_type* _It) noexcept {
-        _Ptr = _Refancy<pointer>(const_cast<value_type*>(_It));
+        _Ptr = _STD _Refancy_maybe_null<pointer>(const_cast<value_type*>(_It));
     }
 
     pointer _Ptr; // pointer to element in string
@@ -2154,7 +2154,7 @@ public:
     using _Prevent_inheriting_unwrap = _String_iterator;
 
     _NODISCARD _CONSTEXPR20 value_type* _Unwrapped() const noexcept {
-        return const_cast<value_type*>(_Unfancy(this->_Ptr));
+        return const_cast<value_type*>(_STD _Unfancy_maybe_null(this->_Ptr));
     }
 };
 

tests/libcxx/expected_results.txt

@@ -1005,14 +1005,6 @@ std/containers/sequences/vector.bool/vector_bool.pass.cpp FAIL
 std/containers/sequences/vector/iterators.pass.cpp:0 FAIL
 std/containers/sequences/vector/iterators.pass.cpp:1 FAIL
 
-# Not analyzed.
-# error C4854: binding dereferenced null pointer to reference has undefined behavior
-# note: while evaluating constexpr function 'std::_Refancy'
-std/containers/sequences/vector/vector.erasure/erase.pass.cpp:0 FAIL
-std/containers/sequences/vector/vector.erasure/erase.pass.cpp:1 FAIL
-std/containers/sequences/vector/vector.erasure/erase_if.pass.cpp:0 FAIL
-std/containers/sequences/vector/vector.erasure/erase_if.pass.cpp:1 FAIL
-
 # Not analyzed. Inspecting shift operators for quoted().
 std/input.output/iostream.format/quoted.manip/quoted_traits.compile.pass.cpp FAIL
 

tests/std/test.lst

@@ -234,6 +234,7 @@ tests\GH_003840_tellg_when_reading_lf_file_in_text_mode
 tests\GH_003867_output_nan
 tests\GH_004023_mdspan_fwd_prod_overflow
 tests\GH_004040_container_nonmember_functions
+tests\GH_004275_seeking_fancy_iterators
 tests\LWG2381_num_get_floating_point
 tests\LWG2597_complex_branch_cut
 tests\LWG3018_shared_ptr_function

tests/std/tests/GH_004275_seeking_fancy_iterators/env.lst

@@ -0,0 +1,4 @@
+# Copyright (c) Microsoft Corporation.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+RUNALL_INCLUDE ..\usual_matrix.lst