<vector>, <string>: Avoid dereferencing null (fancy) pointers when calling _Seek_to/_Unwrapped on or comparing vector and basic_string's iterators
#4275
Conversation
frederick-vs-ja
changed the title
<vector>: Avoid dereferencing null pointers when _Seek_to-ing vector's iterators<vector>, <string>: Avoid dereferencing null pointers when _Seek_to-ing vector and basic_string's iterators
|
The test failure is EDG complaining about comparing null pointers with I've approved the change in principle, but we'll need to avoid the test failure and file a new EDG bug before merging. EDIT: template <class I>
constexpr void meow(I first, const I last) {
_STD _Adl_verify_range(first, last);
first._Seek_to(first._Unwrapped());
}
constexpr bool test_seeking_vector_iterators() {
using VIt = vector<int, min_allocator<int>>::iterator;
meow(VIt{}, VIt{});
return true;
}
static_assert(test_seeking_vector_iterators());The |
Wow... it seems that the real error comes from I think we should use Lines 2032 to 2034 in b84bf09 Lines 2156 to 2158 in b84bf09 Lines 216 to 218 in b84bf09 Lines 344 to 346 in b84bf09
Slightly reduced example constexpr bool test_seeking() {
using VIt = std::vector<int, min_allocator<int>>::iterator;
auto first = VIt{};
_Verify_range(first, first);
(void) first._Unwrapped();
return true;
}
static_assert(test_seeking());IIUC, in constant evaluation:
|
frederick-vs-ja
changed the title
<vector>, <string>: Avoid dereferencing null pointers when _Seek_to-ing vector and basic_string's iterators<vector>, <string>: Avoid dereferencing null (fancy) pointers when _Seek_to/_Unwrapped-ing vector and basic_string's iterators
frederick-vs-ja
changed the title
<vector>, <string>: Avoid dereferencing null (fancy) pointers when _Seek_to/_Unwrapped-ing vector and basic_string's iterators<vector>, <string>: Avoid dereferencing null (fancy) pointers when calling _Seek_to/_Unwrapped on or comparing vector and basic_string's iterators
Good catch! |
| // some portions of this file are derived from libc++'s test files: | ||
| // * support/min_allocator.h |
There was a problem hiding this comment.
No change requested: Note that tests/std can include headers from libcxx's support directory. (We've arranged for this to work in both the GitHub and MSVC-internal test harnesses.) It looks like <min_allocator.h> defines a bunch of other stuff that we don't need here, so distilling it down to what we need is reasonable, I just wanted to mention this if it's ever useful in the future.
| template <class ID> | ||
| class min_pointer<const void, ID> { | ||
| const void* ptr_; | ||
|
|
||
| public: | ||
| min_pointer() = default; | ||
| min_pointer(nullptr_t) noexcept : ptr_(nullptr) {} | ||
| template <class T> | ||
| min_pointer(min_pointer<T, ID> p) noexcept : ptr_(p.ptr_) {} | ||
|
|
||
| explicit operator bool() const { | ||
| return ptr_ != nullptr; | ||
| } | ||
|
|
||
| friend bool operator==(min_pointer x, min_pointer y) { | ||
| return x.ptr_ == y.ptr_; | ||
| } | ||
| friend bool operator!=(min_pointer x, min_pointer y) { | ||
| return !(x == y); | ||
| } | ||
| template <class U, class XID> | ||
| friend class min_pointer; | ||
| }; |
There was a problem hiding this comment.
No change requested: I observe that min_pointer<const void, ID> lacks constexpr when the other specializations have it. This appears to be the case in libc++'s original code, so maybe they were trying to do something special there.
|
Thanks! I pushed a conflict-free merge with |
|
I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed. |
|
Thanks for investigating and fixing these libcxx failures - I'm so happy to see this! 🎩 😻 🎉 |
Per CWG-2823 (recently voted into the Standard), dereferencing a null pointer is UB in the involved case. Currently, MSVC correctly diagnoses this while Clang 17 doesn't.
Perhaps other iterator's
_Seek_tofunctions (edit: other thanbasic_string's) are fine, sincebasic_string's iterators obtained from containers are always non-null, and_Seek_tofunctions don't seem to dereference pointers themselves.Edit:
basic_string's iterator may also fail instd::find(I{}, I{}, T{}) == I{}when the iterator contains a null fancy pointer.Unblocks libcxx tests:
std/containers/sequences/vector/vector.erasure/erase.pass.cppstd/containers/sequences/vector/vector.erasure/erase_if.pass.cpp