From patchwork Wed May 7 12:07:44 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Wakely X-Patchwork-Id: 346572 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.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 1CA171401EB for ; Wed, 7 May 2014 22:08:07 +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:date :from:to:subject:message-id:mime-version:content-type; q=dns; s= default; b=NrZKlH/Fg4UVkNwO2wlR4Ezp0cTCXwap2BHU+8AMPorzRLeRr1REh 3+yoWOEAUZ0XoHfh4sW8xblI3fVW2htCactXXzvgMYuHKSWt8j/sU3K/t8SimGa7 Spv49KUpEQeal8pL6xmk3aIKqbPTNNsaa/IZsgf3NUrxGiuFhqvxe8= 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:date :from:to:subject:message-id:mime-version:content-type; s= default; bh=mEFoZvj3s8v7t+iU1Jyv8O91YIY=; b=Lo6B++EOzw5H51FuyueP lHVHH5g8SD1TbI4VDhCLZkpiAWosrLQmXJ+dRQKjjsUWTjw66MZpFuyIOD0/wjar aynnqvYz7eDEhxM+coreR2MzblGpGEZELzJkEuv6OaXJRvUdPDFlCrESwOrGt4JV /kBjLKudbr9pj2/Ft5E3jhU= Received: (qmail 28066 invoked by alias); 7 May 2014 12:07:49 -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 28048 invoked by uid 89); 7 May 2014 12:07:48 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 07 May 2014 12:07:46 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s47C7jGT026777 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 7 May 2014 08:07:45 -0400 Received: from localhost (vpn1-5-30.ams2.redhat.com [10.36.5.30]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id s47C7i7d026502; Wed, 7 May 2014 08:07:45 -0400 Date: Wed, 7 May 2014 13:07:44 +0100 From: Jonathan Wakely To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [patch] libstdc++/61086 - fix ubsan errors in std::vector Message-ID: <20140507120743.GB18109@redhat.com> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) The testcase in the PR calls __position._M_const_cast() to get a mutable iterator and that dereferences the pointer as suggested in http://gcc.gnu.org/ml/libstdc++/2013-05/msg00031.html That's invalid because the pointer is not dereferenceable (in this case it's null but is past-the-end at all times). I played around with changing the __normal_iterator so we would do __postition._M_const_cast(begin()) then decided we don't need it at all and can just as easily obtain a mutable iterator using: auto __pos = begin() + (__position - cbegin()); I plan to commit the attached patch to trunk and 4.9 soon. I've tested it on x86_64-linux but not added a testcase because we don't test with -fsanitize (though we should do) and it only shows up with Clang anyway. commit 566623def309c70387e41da2346ff89aa7619b13 Author: Jonathan Wakely Date: Wed May 7 12:17:41 2014 +0100 PR libstdc++/61086 * include/bits/stl_iterator.h (__normal_iterator::_M_const_cast): Remove. * include/bits/stl_vector.h (vector::insert, vector::erase): Use arithmetic to obtain a mutable iterator from const_iterator. * include/bits/vector.tcc (vector::insert): Likewise. * include/debug/vector (vector::erase): Likewise. * testsuite/23_containers/vector/requirements/dr438/assign_neg.cc: Adjust dg-error line number. * testsuite/23_containers/vector/requirements/dr438/ constructor_1_neg.cc: Likewise. * testsuite/23_containers/vector/requirements/dr438/ constructor_2_neg.cc: Likewise. * testsuite/23_containers/vector/requirements/dr438/insert_neg.cc: Likewise. diff --git a/libstdc++-v3/include/bits/stl_iterator.h b/libstdc++-v3/include/bits/stl_iterator.h index 16f992c..f4522a4 100644 --- a/libstdc++-v3/include/bits/stl_iterator.h +++ b/libstdc++-v3/include/bits/stl_iterator.h @@ -736,21 +736,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _Container>::__type>& __i) _GLIBCXX_NOEXCEPT : _M_current(__i.base()) { } -#if __cplusplus >= 201103L - __normal_iterator - _M_const_cast() const noexcept - { - using _PTraits = std::pointer_traits; - return __normal_iterator - (_PTraits::pointer_to(const_cast - (*_M_current))); - } -#else - __normal_iterator - _M_const_cast() const - { return *this; } -#endif - // Forward iterator requirements reference operator*() const _GLIBCXX_NOEXCEPT diff --git a/libstdc++-v3/include/bits/stl_vector.h b/libstdc++-v3/include/bits/stl_vector.h index 3d3a2cf..0a56c65 100644 --- a/libstdc++-v3/include/bits/stl_vector.h +++ b/libstdc++-v3/include/bits/stl_vector.h @@ -1051,7 +1051,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER insert(const_iterator __position, size_type __n, const value_type& __x) { difference_type __offset = __position - cbegin(); - _M_fill_insert(__position._M_const_cast(), __n, __x); + _M_fill_insert(begin() + __offset, __n, __x); return begin() + __offset; } #else @@ -1096,7 +1096,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER _InputIterator __last) { difference_type __offset = __position - cbegin(); - _M_insert_dispatch(__position._M_const_cast(), + _M_insert_dispatch(begin() + __offset, __first, __last, __false_type()); return begin() + __offset; } @@ -1144,10 +1144,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER iterator #if __cplusplus >= 201103L erase(const_iterator __position) + { return _M_erase(begin() + (__position - cbegin())); } #else erase(iterator __position) + { return _M_erase(__position); } #endif - { return _M_erase(__position._M_const_cast()); } /** * @brief Remove a range of elements. @@ -1170,10 +1171,15 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER iterator #if __cplusplus >= 201103L erase(const_iterator __first, const_iterator __last) + { + const auto __beg = begin(); + const auto __cbeg = cbegin(); + return _M_erase(__beg + (__first - __cbeg), __beg + (__last - __cbeg)); + } #else erase(iterator __first, iterator __last) + { return _M_erase(__first, __last); } #endif - { return _M_erase(__first._M_const_cast(), __last._M_const_cast()); } /** * @brief Swaps data with another %vector. diff --git a/libstdc++-v3/include/bits/vector.tcc b/libstdc++-v3/include/bits/vector.tcc index 299e614..5c3dfae 100644 --- a/libstdc++-v3/include/bits/vector.tcc +++ b/libstdc++-v3/include/bits/vector.tcc @@ -121,14 +121,17 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER else { #if __cplusplus >= 201103L + const auto __pos = begin() + (__position - cbegin()); if (this->_M_impl._M_finish != this->_M_impl._M_end_of_storage) { _Tp __x_copy = __x; - _M_insert_aux(__position._M_const_cast(), std::move(__x_copy)); + _M_insert_aux(__pos, std::move(__x_copy)); } else + _M_insert_aux(__pos, __x); +#else + _M_insert_aux(__position, __x); #endif - _M_insert_aux(__position._M_const_cast(), __x); } return iterator(this->_M_impl._M_start + __n); } @@ -307,7 +310,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER ++this->_M_impl._M_finish; } else - _M_insert_aux(__position._M_const_cast(), + _M_insert_aux(begin() + (__position - cbegin()), std::forward<_Args>(__args)...); return iterator(this->_M_impl._M_start + __n); } diff --git a/libstdc++-v3/include/debug/vector b/libstdc++-v3/include/debug/vector index 427d19f..f92a77f 100644 --- a/libstdc++-v3/include/debug/vector +++ b/libstdc++-v3/include/debug/vector @@ -647,7 +647,7 @@ namespace __debug } else #if __cplusplus >= 201103L - return iterator(__first.base()._M_const_cast(), this); + return begin() + (__first.base() - cbegin().base()); #else return __first; #endif diff --git a/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/assign_neg.cc b/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/assign_neg.cc index 191fbc7..655c7e8 100644 --- a/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/assign_neg.cc +++ b/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/assign_neg.cc @@ -18,7 +18,7 @@ // . // { dg-do compile } -// { dg-error "no matching" "" { target *-*-* } 1320 } +// { dg-error "no matching" "" { target *-*-* } 1326 } #include diff --git a/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_1_neg.cc b/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_1_neg.cc index 8818a88..d938aa2 100644 --- a/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_1_neg.cc +++ b/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_1_neg.cc @@ -18,7 +18,7 @@ // . // { dg-do compile } -// { dg-error "no matching" "" { target *-*-* } 1246 } +// { dg-error "no matching" "" { target *-*-* } 1252 } #include diff --git a/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_2_neg.cc b/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_2_neg.cc index 09499bc..f888285 100644 --- a/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_2_neg.cc +++ b/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_2_neg.cc @@ -18,7 +18,7 @@ // . // { dg-do compile } -// { dg-error "no matching" "" { target *-*-* } 1246 } +// { dg-error "no matching" "" { target *-*-* } 1252 } #include #include diff --git a/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/insert_neg.cc b/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/insert_neg.cc index 674e3b5..316249b 100644 --- a/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/insert_neg.cc +++ b/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/insert_neg.cc @@ -18,7 +18,7 @@ // . // { dg-do compile } -// { dg-error "no matching" "" { target *-*-* } 1361 } +// { dg-error "no matching" "" { target *-*-* } 1367 } #include