libc++ permits conversions between std::vector<>::iterators of distinct types #50058
Description
| Bugzilla Link | 50714 |
| Version | 12.0 |
| OS | Linux |
| Reporter | LLVM Bugzilla Contributor |
| CC | @dwblaikie,@mclow |
Extended Description
Hello!
Recently I've noticed that, libc++ permits conversions
between std::vector<>::iterators of two different element_types,
as long as pointers to these element_types are convertible:
struct foo { float f; };
struct bar : foo { int i; };
auto vec = std::vector
{
bar { foo { 1.1f }, 7 },
bar { foo { 0.8f }, 5 },
};
typename std::vector<foo>::iterator it = vec.begin();
// ^
// note: The value_type of the iterator 'it' is foo, not bar!
++it;
auto fp_value = it->f; // bug: iterator 'it' is not valid!
This is very dangerous, because the iterator 'it' expects
to iterate over a continuous block of memory, containing only foo objects:
+-----+-----+-----
| foo | foo | foo ...
+-----+-----+-----
^ ^ ^
it it+1 it+2 ...
However, this is not what is actually happening in memory:
+-----------+-----------+-------
| bar | bar | bar ...
+-----------+-----------+-------
| foo | | foo | | foo | ...
+-----------+-----------+-------
^ ^ ^
it it+1 it+2 ...
This contradiction makes std::next(it) is not dereferenceable
and will result in undefined behavior (UB),
when an object will be accessed though the iterator 'it'.
Compiler Explorer link: https://godbolt.org/z/5zzb7dTPd
Correct me if I'm wrong,
but ideally this bug should be reported at compile-time by a C++ compiler.
BTW, this is what other C++ standard library implementations are already doing:
- [GCC] libstdc++ -> https://godbolt.org/z/bqsfW4aMb
- [MSVC] STL -> https://godbolt.org/z/4TnK45jcM
Also, the ISO C++ standard only requires that std::vector<>::iterator
must be convertible to std::vector<>::const_iterator,
so other conversions could be disabled, right?
Table 73: Container requirements [tab:container.req]
+-------------+---------------+------------------------------------+
| Expression | Return type | Note |
+-------------+---------------+------------------------------------+
| | iterator type | any iterator category that meets |
| X::iterator | whose value | the forward iterator requirements. |
| | type is T | convertible to X::const_iterator |
+-------------+---------------+------------------------------------+
ISO C++20 Working Draft:
~ http://open-std.org/jtc1/sc22/wg21/docs/papers/2020/n4861.pdf
Why this conversion is even allowed by libc++?
From what I've investigated, in the libc++ implementation,
the std::vector<>::iterator is an alias to the __wrap_iter<> class template:
template <class _Tp, class _Allocator /* = allocator<_Tp> */>
class _LIBCPP_TEMPLATE_VIS vector
{
...
typedef __wrap_iter<pointer> iterator;
typedef __wrap_iter<const_pointer> const_iterator;
...
};
[GitHub] llvm-project/libcxx/include/vector:
~ https://github.com/llvm/llvm-project/blob/main/libcxx/include/vector#L488
Looking at the __wrap_iter<> class template revels the reason why
this dangerous conversion was even allowed by the Clang compiler:
template <class _Iter>
class __wrap_iter
{
...
template <class _Up> ...
__wrap_iter(const __wrap_iter<_Up>& __u,
std::enable_if_t<
std::is_convertible_v<_Up, iterator_type>>* = nullptr)
... { ... }
...
};
[GitHub] llvm-project/libcxx/include/iterator:
~ https://github.com/llvm/llvm-project/blob/main/libcxx/include/iterator#L1437
Essentially, as long as type_x* is convertible to type_y*,
std::vector<type_x>::iterator is convertible to std::vector<type_y>::iterator.
Every time I see such a weakly-constrained, non-explicit conversion constructor,
red flags are immediately being raised in my brain, since
this could lead to quite surprising results and can potentially cause issues ...
Do you think it would be beneficial to further constrain
conversions of std::vector<>::iterator?
Would it make sense to allow only conversion
from std::vector<>::iterator to std::vector<>::const_iterator?
Thank you, Mateusz Zych