From patchwork Fri Aug 30 20:24:55 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Fran=C3=A7ois_Dumont?= X-Patchwork-Id: 271432 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "www.sourceware.org", Issuer "StartCom Class 1 Primary Intermediate Server CA" (not verified)) by ozlabs.org (Postfix) with ESMTPS id AB0782C00CF for ; Sat, 31 Aug 2013 06:25:11 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; q=dns; s=default; b=aP4piHUXb35f3EvsK e78i48qxoVX/jX4pT/MzfCmFgOFJissR6TllR1sNHpTQtdOHAcd+EKj64YQCAf5d WYmUIb916zSVC2mjtARRLlUWrr2EwKWp7x1P5eoqSpjBpDDBa0alhw3EJcH8C2ns NCqMvq55fxmJAWZAZxsG8rdB7k= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; s=default; bh=yfevWpV08Kyht7u0xrtkXsK QjxY=; b=nLYjPW6e6plILglKRH34qQgxXvc2FHIdl83BLLFaFFa0PQ/cGub1i79 Km9beAD5gKH6/gX1QnW8qSeDOnkyNOh09otuch6kozO7SORFM4YoI/by6x4LBYX6 ea77PQSnWLYO93C0I26aFqxRqIFAZMCzA0KlxMKY+5/BMd8YLKfg= Received: (qmail 1759 invoked by alias); 30 Aug 2013 20:25:04 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 1735 invoked by uid 89); 30 Aug 2013 20:25:03 -0000 Received: from mail-wi0-f170.google.com (HELO mail-wi0-f170.google.com) (209.85.212.170) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Fri, 30 Aug 2013 20:25:03 +0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.7 required=5.0 tests=ALL_TRUSTED, AWL, BAYES_00, FREEMAIL_FROM, KHOP_THREADED autolearn=ham version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mail-wi0-f170.google.com Received: by mail-wi0-f170.google.com with SMTP id hi5so2706wib.5 for ; Fri, 30 Aug 2013 13:24:58 -0700 (PDT) X-Received: by 10.194.89.38 with SMTP id bl6mr3197819wjb.50.1377894298916; Fri, 30 Aug 2013 13:24:58 -0700 (PDT) Received: from localhost.localdomain (arf62-1-82-237-250-248.fbx.proxad.net. [82.237.250.248]) by mx.google.com with ESMTPSA id iz19sm50001wic.9.1969.12.31.16.00.00 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 30 Aug 2013 13:24:57 -0700 (PDT) Message-ID: <5220FF97.9060309@gmail.com> Date: Fri, 30 Aug 2013 22:24:55 +0200 From: =?ISO-8859-1?Q?Fran=E7ois_Dumont?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120829 Thunderbird/15.0 MIME-Version: 1.0 To: Paolo Carlini CC: libstdc++@gcc.gnu.org, gcc-patches Subject: Re: PR 58191 patch References: <521D12CF.8020902@gmail.com> <521D1560.4080206@oracle.com> In-Reply-To: <521D1560.4080206@oracle.com> Hi I finally generalized this method to other debug functions, it is more consistent and clean the implementation of the debug checks. For 4.8 branch I will limit it to just what need to be really fixed. 2013-08-30 François Dumont PR libstdc++/58191 * include/debug/macros.h (__glibcxx_check_partitioned_lower): Add __gnu_debug::__base calls on iterators passed to internal debug check. (__glibcxx_check_partitioned_lower_pred): Likewise. (__glibcxx_check_partitioned_upper): Likewise. (__glibcxx_check_partitioned_upper_pred): Likewise. (__glibcxx_check_sorted): Likewise. (__glibcxx_check_sorted_pred): Likewise. (__glibcxx_check_sorted_set): Likewise. (__glibcxx_check_sorted_set_pred): Likewise. * include/debug/functions.h (__check_partitioned_lower): Remove code to detect safe iterators. (__check_partitioned_upper): Likewise. (__check_sorted): Likewise. François On 08/27/2013 11:08 PM, Paolo Carlini wrote: > On 08/27/2013 10:57 PM, François Dumont wrote: >> Hi >> >> Here is a patch to fix the small PR 58191 regression. I don't >> remember why I hadn't used the __gnu_debug::__base from the start >> rather than trying to reproduce its behavior within the >> __check_partitioned* methods. This way we only detect random access >> safe iterator to enhance performance but do not check the iterator >> category otherwise, concept checks are there for that reason. >> >> The patch is generated from the 4.8 branch. I still need to reg >> test it but if it succeeded is it ok to commit ? Just in trunk or >> also in 4.8 branch ? > Thanks. Let's play safe, let's apply it to mainline and let's ask > people on the audit trail of the bug too to test it. If everything > goes well let's backport it to the branch in a few weeks. > > Thanks again, > Paolo. > Index: include/debug/functions.h =================================================================== --- include/debug/functions.h (revision 201966) +++ include/debug/functions.h (working copy) @@ -336,15 +336,6 @@ return true; } - // For performance reason, as the iterator range has been validated, check on - // random access safe iterators is done using the base iterator. - template - inline bool - __check_sorted_aux(const _Safe_iterator<_Iterator, _Sequence>& __first, - const _Safe_iterator<_Iterator, _Sequence>& __last, - std::random_access_iterator_tag __tag) - { return __check_sorted_aux(__first.base(), __last.base(), __tag); } - // Can't check if an input iterator sequence is sorted, because we can't step // through the sequence. template @@ -371,17 +362,6 @@ return true; } - // For performance reason, as the iterator range has been validated, check on - // random access safe iterators is done using the base iterator. - template - inline bool - __check_sorted_aux(const _Safe_iterator<_Iterator, _Sequence>& __first, - const _Safe_iterator<_Iterator, _Sequence>& __last, - _Predicate __pred, - std::random_access_iterator_tag __tag) - { return __check_sorted_aux(__first.base(), __last.base(), __pred, __tag); } - // Determine if a sequence is sorted. template inline bool @@ -470,11 +450,13 @@ return __check_sorted_set_aux(__first, __last, __pred, _SameType()); } + // _GLIBCXX_RESOLVE_LIB_DEFECTS + // 270. Binary search requirements overly strict + // Determine if a sequence is partitioned w.r.t. this element. template inline bool - __check_partitioned_lower_aux(_ForwardIterator __first, - _ForwardIterator __last, const _Tp& __value, - std::forward_iterator_tag) + __check_partitioned_lower(_ForwardIterator __first, + _ForwardIterator __last, const _Tp& __value) { while (__first != __last && *__first < __value) ++__first; @@ -487,38 +469,11 @@ return __first == __last; } - // For performance reason, as the iterator range has been validated, check on - // random access safe iterators is done using the base iterator. - template - inline bool - __check_partitioned_lower_aux( - const _Safe_iterator<_Iterator, _Sequence>& __first, - const _Safe_iterator<_Iterator, _Sequence>& __last, - const _Tp& __value, - std::random_access_iterator_tag __tag) - { - return __check_partitioned_lower_aux(__first.base(), __last.base(), - __value, __tag); - } - - // _GLIBCXX_RESOLVE_LIB_DEFECTS - // 270. Binary search requirements overly strict - // Determine if a sequence is partitioned w.r.t. this element. template inline bool - __check_partitioned_lower(_ForwardIterator __first, + __check_partitioned_upper(_ForwardIterator __first, _ForwardIterator __last, const _Tp& __value) { - return __check_partitioned_lower_aux(__first, __last, __value, - std::__iterator_category(__first)); - } - - template - inline bool - __check_partitioned_upper_aux(_ForwardIterator __first, - _ForwardIterator __last, const _Tp& __value, - std::forward_iterator_tag) - { while (__first != __last && !(__value < *__first)) ++__first; if (__first != __last) @@ -530,35 +485,12 @@ return __first == __last; } - // For performance reason, as the iterator range has been validated, check on - // random access safe iterators is done using the base iterator. - template - inline bool - __check_partitioned_upper_aux( - const _Safe_iterator<_Iterator, _Sequence>& __first, - const _Safe_iterator<_Iterator, _Sequence>& __last, - const _Tp& __value, - std::random_access_iterator_tag __tag) - { - return __check_partitioned_upper_aux(__first.base(), __last.base(), - __value, __tag); - } - - template - inline bool - __check_partitioned_upper(_ForwardIterator __first, - _ForwardIterator __last, const _Tp& __value) - { - return __check_partitioned_upper_aux(__first, __last, __value, - std::__iterator_category(__first)); - } - + // Determine if a sequence is partitioned w.r.t. this element. template inline bool - __check_partitioned_lower_aux(_ForwardIterator __first, - _ForwardIterator __last, const _Tp& __value, - _Pred __pred, - std::forward_iterator_tag) + __check_partitioned_lower(_ForwardIterator __first, + _ForwardIterator __last, const _Tp& __value, + _Pred __pred) { while (__first != __last && bool(__pred(*__first, __value))) ++__first; @@ -571,39 +503,12 @@ return __first == __last; } - // For performance reason, as the iterator range has been validated, check on - // random access safe iterators is done using the base iterator. - template - inline bool - __check_partitioned_lower_aux( - const _Safe_iterator<_Iterator, _Sequence>& __first, - const _Safe_iterator<_Iterator, _Sequence>& __last, - const _Tp& __value, _Pred __pred, - std::random_access_iterator_tag __tag) - { - return __check_partitioned_lower_aux(__first.base(), __last.base(), - __value, __pred, __tag); - } - - // Determine if a sequence is partitioned w.r.t. this element. template inline bool - __check_partitioned_lower(_ForwardIterator __first, + __check_partitioned_upper(_ForwardIterator __first, _ForwardIterator __last, const _Tp& __value, _Pred __pred) { - return __check_partitioned_lower_aux(__first, __last, __value, __pred, - std::__iterator_category(__first)); - } - - template - inline bool - __check_partitioned_upper_aux(_ForwardIterator __first, - _ForwardIterator __last, const _Tp& __value, - _Pred __pred, - std::forward_iterator_tag) - { while (__first != __last && !bool(__pred(__value, *__first))) ++__first; if (__first != __last) @@ -615,31 +520,6 @@ return __first == __last; } - // For performance reason, as the iterator range has been validated, check on - // random access safe iterators is done using the base iterator. - template - inline bool - __check_partitioned_upper_aux( - const _Safe_iterator<_Iterator, _Sequence>& __first, - const _Safe_iterator<_Iterator, _Sequence>& __last, - const _Tp& __value, _Pred __pred, - std::random_access_iterator_tag __tag) - { - return __check_partitioned_upper_aux(__first.base(), __last.base(), - __value, __pred, __tag); - } - - template - inline bool - __check_partitioned_upper(_ForwardIterator __first, - _ForwardIterator __last, const _Tp& __value, - _Pred __pred) - { - return __check_partitioned_upper_aux(__first, __last, __value, __pred, - std::__iterator_category(__first)); - } - // Helper struct to detect random access safe iterators. template struct __is_safe_random_iterator Index: include/debug/macros.h =================================================================== --- include/debug/macros.h (revision 201966) +++ include/debug/macros.h (working copy) @@ -229,7 +229,9 @@ // Verify that the iterator range [_First, _Last) is sorted #define __glibcxx_check_sorted(_First,_Last) \ __glibcxx_check_valid_range(_First,_Last); \ -_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_sorted(_First, _Last), \ + _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_sorted( \ + __gnu_debug::__base(_First), \ + __gnu_debug::__base(_Last)), \ _M_message(__gnu_debug::__msg_unsorted) \ ._M_iterator(_First, #_First) \ ._M_iterator(_Last, #_Last)) @@ -238,7 +240,9 @@ predicate _Pred. */ #define __glibcxx_check_sorted_pred(_First,_Last,_Pred) \ __glibcxx_check_valid_range(_First,_Last); \ -_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_sorted(_First, _Last, _Pred), \ +_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_sorted( \ + __gnu_debug::__base(_First), \ + __gnu_debug::__base(_Last), _Pred), \ _M_message(__gnu_debug::__msg_unsorted_pred) \ ._M_iterator(_First, #_First) \ ._M_iterator(_Last, #_Last) \ @@ -248,7 +252,8 @@ #define __glibcxx_check_sorted_set(_First1,_Last1,_First2) \ __glibcxx_check_valid_range(_First1,_Last1); \ _GLIBCXX_DEBUG_VERIFY( \ - __gnu_debug::__check_sorted_set(_First1, _Last1, _First2), \ + __gnu_debug::__check_sorted_set(__gnu_debug::__base(_First1), \ + __gnu_debug::__base(_Last1), _First2),\ _M_message(__gnu_debug::__msg_unsorted) \ ._M_iterator(_First1, #_First1) \ ._M_iterator(_Last1, #_Last1)) @@ -257,7 +262,9 @@ #define __glibcxx_check_sorted_set_pred(_First1,_Last1,_First2,_Pred) \ __glibcxx_check_valid_range(_First1,_Last1); \ _GLIBCXX_DEBUG_VERIFY( \ - __gnu_debug::__check_sorted_set(_First1, _Last1, _First2, _Pred), \ + __gnu_debug::__check_sorted_set(__gnu_debug::__base(_First1), \ + __gnu_debug::__base(_Last1), \ + _First2, _Pred), \ _M_message(__gnu_debug::__msg_unsorted_pred) \ ._M_iterator(_First1, #_First1) \ ._M_iterator(_Last1, #_Last1) \ @@ -267,8 +274,9 @@ w.r.t. the value _Value. */ #define __glibcxx_check_partitioned_lower(_First,_Last,_Value) \ __glibcxx_check_valid_range(_First,_Last); \ -_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_partitioned_lower(_First, _Last, \ - _Value), \ +_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_partitioned_lower( \ + __gnu_debug::__base(_First), \ + __gnu_debug::__base(_Last), _Value), \ _M_message(__gnu_debug::__msg_unpartitioned) \ ._M_iterator(_First, #_First) \ ._M_iterator(_Last, #_Last) \ @@ -276,8 +284,9 @@ #define __glibcxx_check_partitioned_upper(_First,_Last,_Value) \ __glibcxx_check_valid_range(_First,_Last); \ -_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_partitioned_upper(_First, _Last, \ - _Value), \ +_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_partitioned_upper( \ + __gnu_debug::__base(_First), \ + __gnu_debug::__base(_Last), _Value), \ _M_message(__gnu_debug::__msg_unpartitioned) \ ._M_iterator(_First, #_First) \ ._M_iterator(_Last, #_Last) \ @@ -287,8 +296,9 @@ w.r.t. the value _Value and predicate _Pred. */ #define __glibcxx_check_partitioned_lower_pred(_First,_Last,_Value,_Pred) \ __glibcxx_check_valid_range(_First,_Last); \ -_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_partitioned_lower(_First, _Last, \ - _Value, _Pred), \ +_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_partitioned_lower( \ + __gnu_debug::__base(_First), \ + __gnu_debug::__base(_Last), _Value, _Pred), \ _M_message(__gnu_debug::__msg_unpartitioned_pred) \ ._M_iterator(_First, #_First) \ ._M_iterator(_Last, #_Last) \ @@ -299,8 +309,9 @@ w.r.t. the value _Value and predicate _Pred. */ #define __glibcxx_check_partitioned_upper_pred(_First,_Last,_Value,_Pred) \ __glibcxx_check_valid_range(_First,_Last); \ -_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_partitioned_upper(_First, _Last, \ - _Value, _Pred), \ +_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_partitioned_upper( \ + __gnu_debug::__base(_First), \ + __gnu_debug::__base(_Last), _Value, _Pred), \ _M_message(__gnu_debug::__msg_unpartitioned_pred) \ ._M_iterator(_First, #_First) \ ._M_iterator(_Last, #_Last) \