From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id E2E5638582B0 for ; Tue, 29 Nov 2022 17:14:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E2E5638582B0 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1669742090; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=839+WqtVWERgRR/DY67tNn6a3sjycnFxvco3zd0OVPw=; b=IUDr3nnDh2Vt+chAyRChFN71jECEN07n9ih50gHciRzr5+NeEyETAJWJOLptcoktuyFHRx BDZuWBL782jXXumtai3ZgioJ1Wp+rsdVh9QGfCGhwv22KkYMnhBOAY2TL+wnHg94aiowXD 8KtK6N6kBH5pg1MvoqwgbyMoKwdXGxg= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-196-xxMiRHiFM1qkt6Enu4j-Mw-1; Tue, 29 Nov 2022 12:14:47 -0500 X-MC-Unique: xxMiRHiFM1qkt6Enu4j-Mw-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 342CA3C1022D; Tue, 29 Nov 2022 17:14:47 +0000 (UTC) Received: from localhost (unknown [10.33.36.164]) by smtp.corp.redhat.com (Postfix) with ESMTP id ED3B42024CB7; Tue, 29 Nov 2022 17:14:46 +0000 (UTC) From: Jonathan Wakely To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [committed] libstdc++: Avoid bogus warning in std::vector::insert [PR107852] Date: Tue, 29 Nov 2022 17:14:46 +0000 Message-Id: <20221129171446.149751-1-jwakely@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.4 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-12.2 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Tested x86_64-linux. Pushed to trunk. -- >8 -- GCC assumes that any global variable might be modified by operator new, and so in the testcase for this PR all data members get reloaded after allocating new storage. By making local copies of the _M_start and _M_finish members we avoid that, and then the compiler has enough info to remove the dead branches that trigger bogus -Warray-bounds warnings. libstdc++-v3/ChangeLog: PR libstdc++/107852 PR libstdc++/106199 PR libstdc++/100366 * include/bits/vector.tcc (vector::_M_fill_insert): Copy _M_start and _M_finish members before allocating. (vector::_M_default_append): Likewise. (vector::_M_range_insert): Likewise. --- libstdc++-v3/include/bits/vector.tcc | 63 ++++++++++++++++------------ 1 file changed, 37 insertions(+), 26 deletions(-) diff --git a/libstdc++-v3/include/bits/vector.tcc b/libstdc++-v3/include/bits/vector.tcc index 33faabf2eae..27ef1a4ee7f 100644 --- a/libstdc++-v3/include/bits/vector.tcc +++ b/libstdc++-v3/include/bits/vector.tcc @@ -539,9 +539,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER if (__elems_after > __n) { _GLIBCXX_ASAN_ANNOTATE_GROW(__n); - std::__uninitialized_move_a(this->_M_impl._M_finish - __n, - this->_M_impl._M_finish, - this->_M_impl._M_finish, + std::__uninitialized_move_a(__old_finish - __n, + __old_finish, + __old_finish, _M_get_Tp_allocator()); this->_M_impl._M_finish += __n; _GLIBCXX_ASAN_ANNOTATE_GREW(__n); @@ -554,7 +554,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER { _GLIBCXX_ASAN_ANNOTATE_GROW(__n); this->_M_impl._M_finish = - std::__uninitialized_fill_n_a(this->_M_impl._M_finish, + std::__uninitialized_fill_n_a(__old_finish, __n - __elems_after, __x_copy, _M_get_Tp_allocator()); @@ -569,9 +569,15 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER } else { + // Make local copies of these members because the compiler thinks + // the allocator can alter them if 'this' is globally reachable. + pointer __old_start = this->_M_impl._M_start; + pointer __old_finish = this->_M_impl._M_finish; + const pointer __pos = __position.base(); + const size_type __len = _M_check_len(__n, "vector::_M_fill_insert"); - const size_type __elems_before = __position - begin(); + const size_type __elems_before = __pos - __old_start; pointer __new_start(this->_M_allocate(__len)); pointer __new_finish(__new_start); __try @@ -584,15 +590,13 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER __new_finish = std::__uninitialized_move_if_noexcept_a - (this->_M_impl._M_start, __position.base(), - __new_start, _M_get_Tp_allocator()); + (__old_start, __pos, __new_start, _M_get_Tp_allocator()); __new_finish += __n; __new_finish = std::__uninitialized_move_if_noexcept_a - (__position.base(), this->_M_impl._M_finish, - __new_finish, _M_get_Tp_allocator()); + (__pos, __old_finish, __new_finish, _M_get_Tp_allocator()); } __catch(...) { @@ -606,12 +610,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER _M_deallocate(__new_start, __len); __throw_exception_again; } - std::_Destroy(this->_M_impl._M_start, this->_M_impl._M_finish, - _M_get_Tp_allocator()); + std::_Destroy(__old_start, __old_finish, _M_get_Tp_allocator()); _GLIBCXX_ASAN_ANNOTATE_REINIT; - _M_deallocate(this->_M_impl._M_start, - this->_M_impl._M_end_of_storage - - this->_M_impl._M_start); + _M_deallocate(__old_start, + this->_M_impl._M_end_of_storage - __old_start); this->_M_impl._M_start = __new_start; this->_M_impl._M_finish = __new_finish; this->_M_impl._M_end_of_storage = __new_start + __len; @@ -645,6 +647,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER } else { + // Make local copies of these members because the compiler thinks + // the allocator can alter them if 'this' is globally reachable. + pointer __old_start = this->_M_impl._M_start; + pointer __old_finish = this->_M_impl._M_finish; + const size_type __len = _M_check_len(__n, "vector::_M_default_append"); pointer __new_start(this->_M_allocate(__len)); @@ -660,7 +667,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER _M_deallocate(__new_start, __len); __throw_exception_again; } - _S_relocate(this->_M_impl._M_start, this->_M_impl._M_finish, + _S_relocate(__old_start, __old_finish, __new_start, _M_get_Tp_allocator()); } else @@ -672,7 +679,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER __n, _M_get_Tp_allocator()); __destroy_from = __new_start + __size; std::__uninitialized_move_if_noexcept_a( - this->_M_impl._M_start, this->_M_impl._M_finish, + __old_start, __old_finish, __new_start, _M_get_Tp_allocator()); } __catch(...) @@ -683,13 +690,12 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER _M_deallocate(__new_start, __len); __throw_exception_again; } - std::_Destroy(this->_M_impl._M_start, this->_M_impl._M_finish, + std::_Destroy(__old_start, __old_finish, _M_get_Tp_allocator()); } _GLIBCXX_ASAN_ANNOTATE_REINIT; - _M_deallocate(this->_M_impl._M_start, - this->_M_impl._M_end_of_storage - - this->_M_impl._M_start); + _M_deallocate(__old_start, + this->_M_impl._M_end_of_storage - __old_start); this->_M_impl._M_start = __new_start; this->_M_impl._M_finish = __new_start + __size + __n; this->_M_impl._M_end_of_storage = __new_start + __len; @@ -782,6 +788,12 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER } else { + // Make local copies of these members because the compiler + // thinks the allocator can alter them if 'this' is globally + // reachable. + pointer __old_start = this->_M_impl._M_start; + pointer __old_finish = this->_M_impl._M_finish; + const size_type __len = _M_check_len(__n, "vector::_M_range_insert"); pointer __new_start(this->_M_allocate(__len)); @@ -790,7 +802,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER { __new_finish = std::__uninitialized_move_if_noexcept_a - (this->_M_impl._M_start, __position.base(), + (__old_start, __position.base(), __new_start, _M_get_Tp_allocator()); __new_finish = std::__uninitialized_copy_a(__first, __last, @@ -798,7 +810,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER _M_get_Tp_allocator()); __new_finish = std::__uninitialized_move_if_noexcept_a - (__position.base(), this->_M_impl._M_finish, + (__position.base(), __old_finish, __new_finish, _M_get_Tp_allocator()); } __catch(...) @@ -808,12 +820,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER _M_deallocate(__new_start, __len); __throw_exception_again; } - std::_Destroy(this->_M_impl._M_start, this->_M_impl._M_finish, + std::_Destroy(__old_start, __old_finish, _M_get_Tp_allocator()); _GLIBCXX_ASAN_ANNOTATE_REINIT; - _M_deallocate(this->_M_impl._M_start, - this->_M_impl._M_end_of_storage - - this->_M_impl._M_start); + _M_deallocate(__old_start, + this->_M_impl._M_end_of_storage - __old_start); this->_M_impl._M_start = __new_start; this->_M_impl._M_finish = __new_finish; this->_M_impl._M_end_of_storage = __new_start + __len; -- 2.38.1