[PATCH][Bug libstdc++/70303] Value-initialized debug iterators

[Jonathan Wakely]

On 01/02/21 19:30 +0100, François Dumont via Libstdc++ wrote:
>On 01/02/21 6:43 pm, Jonathan Wakely wrote:
>>On 31/01/21 16:59 +0100, François Dumont via Libstdc++ wrote:
>>>After the debug issue has been fixed in PR 98466 the problem was 
>>>not in the debug iterator implementation itself but in the deque 
>>>iterator operator- implementation.
>>>
>>>    libstdc++: Make deque iterator operator- usable with 
>>>value-init iterators
>>>
>>>    N3644 implies that operator- can be used on value-init 
>>>iterators. We now return
>>>    0 if both iterators are value initialized. If only one is 
>>>value initialized we
>>>    keep the UB by returning the result of a normal computation 
>>>which is an unexpected
>>>    value.
>>>
>>>    libstdc++/ChangeLog:
>>>
>>>            PR libstdc++/70303
>>>            * include/bits/stl_deque.h 
>>>(std::deque<>::operator-(iterator, iterator)):
>>>            Return 0 if both iterators are value-initialized.
>>>            * testsuite/23_containers/deque/70303.cc: New test.
>>>            * testsuite/23_containers/vector/70303.cc: New test.
>>>
>>>Tested under Linux x86_64, ok to commit ?
>>
>>OK.
>>
>>I don't like adding the branch there though. Even with the
>>__builtin_expect it causes worse code to be generated than the
>>original.
>>
>>This would be branchless, but a bit harder to understand:
>>
>>    return difference_type(__x._S_buffer_size())
>>      * (__x._M_node - __y._M_node - int(__x._M_node == __y._M_node))
>>      + (__x._M_cur - __x._M_first) + (__y._M_last - __y._M_cur);
>>
>>Please commit the fix and we can think about it later.
>
>I do not think this work because for value-init iterators we have 
>__x._M_node == __y._M_node == nullptr so the diff would give 
>-_S_buffer_size().
>
>But I got the idear, I'll prepare the patch.

Yeah, I just came back to the computer to say that it's wrong. But
maybe this:

    return difference_type(_S_buffer_size())
      * (__x._M_node - __y._M_node - int(__x._M_node && __y._M_node))
      + (__x._M_cur - __x._M_first) + (__y._M_last - __y._M_cur);

For value-init'd iterators we'd get _S_buffer_size() * 0 + 0 - 0

We could even just use int(__x._M_node != 0) because if one is null
and the other isn't, it's UB anyway.