This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH][_GLIBCXX_DEBUG] Improve valid_range check


On 26/11/19 20:07 +0100, François Dumont wrote:
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 !

The testsuite doesn't use CXXFLAGS.


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...

No, you need to pass the right flags so they are used by the
testsuite. This will do it:

make -C testsuite/  check-debug debug_flags=unix/-D_GLIBCXX_DEBUG/-std=gnu++98/-Wsystem-headers

But since it only shows up with -Wsystem-headers, there's no point
trying to test for it.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]