[PATCH][_GLIBCXX_DEBUG] Improve valid_range check

[François Dumont]

On 11/26/19 1:21 PM, Jonathan Wakely wrote:
> On 26/11/19 12:33 +0100, Stephan Bergmann wrote:
>> On 22/11/2019 18:59, Jonathan Wakely wrote:
>>> On 22/11/19 18:38 +0100, François Dumont wrote:
>>>> Â Â Â  I noticed that we are not checking that iterators are not 
>>>> singular in valid_range. Moreover __check_singular signature for 
>>>> pointers is not intercepting all kind of pointers in terms of 
>>>> qualification.
>>>>
>>>> Â Â Â  I'd like to commit it next week but considering we are in stage 
>>>> 3 I need proper acceptance.
>>>>
>>>> Â Â Â  * include/debug/functions.h: Remove <bits/move.h> include.
>>>> Â Â Â  (__check_singular_aux, __check_singular): Move...
>>>> Â Â Â  * include/debug/helper_functions.h:
>>>> Â Â Â  (__check_singular_aux, __check_singular): ...here.
>>>> Â Â Â  (__valid_range_aux): Adapt to use latter.
>>>> Â Â Â  * testsuite/25_algorithms/copy/debug/2_neg.cc: New.
>>>>
>>>> Tested under Linux x86_64 normal and debug modes.
>>>
>>> OK for trunk, thanks.
>>
>> The curly braces...
>>
>>> diff --git a/libstdc++-v3/include/debug/helper_functions.h 
>>> b/libstdc++-v3/include/debug/helper_functions.h
>>> index c3e7478f649..5a858754875 100644
>>> --- a/libstdc++-v3/include/debug/helper_functions.h
>>> +++ b/libstdc++-v3/include/debug/helper_functions.h
>> [...]
>>> @@ -138,14 +156,23 @@ namespace __gnu_debug
>>> Â Â Â  inline bool
>>> Â Â Â  __valid_range_aux(_InputIterator __first, _InputIterator __last,
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â  std::input_iterator_tag)
>>> -Â Â Â  { return true; }
>>> +Â Â Â  {
>>> +Â Â Â Â Â  if (__first != __last)
>>> +Â Â Â  return !__check_singular(__first) && !__check_singular(__last);
>>> +
>>> +Â Â Â Â Â  return true;
>>> +Â Â Â  }
>>> Â  template<typename _InputIterator>
>>> Â Â Â  _GLIBCXX_CONSTEXPR
>>> Â Â Â  inline bool
>>> Â Â Â  __valid_range_aux(_InputIterator __first, _InputIterator __last,
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â  std::random_access_iterator_tag)
>>> -Â Â Â  { return __first <= __last; }
>>> +Â Â Â  {
>>> +Â Â Â Â Â  return
>>> +Â Â Â  __valid_range_aux(__first, __last, std::input_iterator_tag{})
>>
>> ...^^^ here...
>>
>>> +Â Â Â  && __first <= __last;
>>> +Â Â Â  }
>>> Â  /** We have iterators, so figure out what kind of iterators they are
>>> Â Â  *Â  to see if we can check the range ahead of time.
>>> @@ -167,6 +194,9 @@ namespace __gnu_debug
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â  typename _Distance_traits<_InputIterator>::__type& 
>>> __dist,
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â  std::__false_type)
>>> Â Â Â  {
>>> +Â Â Â Â Â  if (!__valid_range_aux(__first, __last, 
>>> std::input_iterator_tag{}))
>>
>> ...and ^^^ here are not allowed pre C++11.  Replacing those with
>>
>> Â std::input_iterator_tag()
>>
>> should fix it.
>
> Indeed. We should also have tests that use "-std=gnu++98
> -D_GLIBCXX_DEBUG" so they'd catch this.
>
> François, can you take care of the fix please?
>
>
>
Sure, I am about to do so.

However I wasn't sure about this syntax before the commit so I had run 
the new 2_neg.cc with:

make CXXFLAGS=-std=c++98 check-debug

and it worked fine and still is !

I also try -std=gnu++98 and made sure that pch had been updated by 
re-building libstdc++ first. I just can't get the expected compilation 
error.

Maybe I need to rebuild the whole stuff to get an error...

Sorry