<html>
    <head>
      <base href="https://bugs.llvm.org/">
    </head>
    <body><table border="1" cellspacing="0" cellpadding="8">
        <tr>
          <th>Bug ID</th>
          <td><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - libc++ permits conversions between std::vector<>::iterators of distinct types"
   href="https://bugs.llvm.org/show_bug.cgi?id=50714">50714</a>
          </td>
        </tr>

        <tr>
          <th>Summary</th>
          <td>libc++ permits conversions between std::vector<>::iterators of distinct types
          </td>
        </tr>

        <tr>
          <th>Product</th>
          <td>libc++
          </td>
        </tr>

        <tr>
          <th>Version</th>
          <td>12.0
          </td>
        </tr>

        <tr>
          <th>Hardware</th>
          <td>PC
          </td>
        </tr>

        <tr>
          <th>OS</th>
          <td>Linux
          </td>
        </tr>

        <tr>
          <th>Status</th>
          <td>NEW
          </td>
        </tr>

        <tr>
          <th>Severity</th>
          <td>enhancement
          </td>
        </tr>

        <tr>
          <th>Priority</th>
          <td>P
          </td>
        </tr>

        <tr>
          <th>Component</th>
          <td>All Bugs
          </td>
        </tr>

        <tr>
          <th>Assignee</th>
          <td>unassignedclangbugs@nondot.org
          </td>
        </tr>

        <tr>
          <th>Reporter</th>
          <td>mte.zych@gmail.com
          </td>
        </tr>

        <tr>
          <th>CC</th>
          <td>llvm-bugs@lists.llvm.org, mclow.lists@gmail.com
          </td>
        </tr></table>
      <p>
        <div>
        <pre>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: <a href="https://godbolt.org/z/5zzb7dTPd">https://godbolt.org/z/5zzb7dTPd</a>


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++ -> <a href="https://godbolt.org/z/bqsfW4aMb">https://godbolt.org/z/bqsfW4aMb</a>
 - [MSVC] STL      -> <a href="https://godbolt.org/z/4TnK45jcM">https://godbolt.org/z/4TnK45jcM</a>

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:
 ~ <a href="http://open-std.org/jtc1/sc22/wg21/docs/papers/2020/n4861.pdf">http://open-std.org/jtc1/sc22/wg21/docs/papers/2020/n4861.pdf</a>


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:
 ~ <a href="https://github.com/llvm/llvm-project/blob/main/libcxx/include/vector#L488">https://github.com/llvm/llvm-project/blob/main/libcxx/include/vector#L488</a>

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:
 ~ <a href="https://github.com/llvm/llvm-project/blob/main/libcxx/include/iterator#L1437">https://github.com/llvm/llvm-project/blob/main/libcxx/include/iterator#L1437</a>

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</pre>
        </div>
      </p>


      <hr>
      <span>You are receiving this mail because:</span>

      <ul>
          <li>You are on the CC list for the bug.</li>
      </ul>
    </body>
</html>