From patchwork Mon Feb 1 18:43:09 2021 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: 1434275 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=kqqx0dFo; dkim-atps=neutral Received: from sourceware.org (unknown [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4DTxfP44SNz9t2G for ; Tue, 2 Feb 2021 05:43:22 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 2210C3947C0F; Mon, 1 Feb 2021 18:43:17 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 2210C3947C0F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1612204997; bh=uPjQ2kto+ztJagQVEler3GIPnVbpRMlyUM4AwaUEgeM=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=kqqx0dFo96+ie5nWOgudJ9pnAgkApgXR2+zh8Z+MxAwrN4VV665OWHZ/XlUseY5K0 K+Q1td0K3fvd/YVgTQfS8k7UdicD34OvV4GmbuUNAy8LuHx1NCyVdSW7/VE54ObVN1 yJt3Mth0o257j86i+7sVgK1GgY79yVi6uZzVkf1w= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-wm1-x330.google.com (mail-wm1-x330.google.com [IPv6:2a00:1450:4864:20::330]) by sourceware.org (Postfix) with ESMTPS id 604F939450F1; Mon, 1 Feb 2021 18:43:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 604F939450F1 Received: by mail-wm1-x330.google.com with SMTP id 190so223873wmz.0; Mon, 01 Feb 2021 10:43:13 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:to:from:subject:message-id:date:user-agent :mime-version:content-language; bh=uPjQ2kto+ztJagQVEler3GIPnVbpRMlyUM4AwaUEgeM=; b=uSuAqWbVY3uKeVMepkeBiSK8yWgMzBhjYa0Im6gRFnpmqmFRQ9ftYO4oNzuZ9qNtph vRvwcXQ+2Pw90dCjMl5aLzs+LsHlXOTrx41CQg4KiYTsEZVyXDV//+UXJF1lDSLGIwMJ Yy/227HF9q7eIgI1PWWbOpWBKfeYD/NbQh8uf7qT78Szq3SwMsRyNETAdQyuP/WDnUUA PWZxQdABa/axOkuMBmH7emCalfdk0u0WfOm89oY9aJrKTMlw7mOcOwaAZkv5YVUokATL r2qETHUUPGbeA0idDG77bCiVdVAWTV605ywrcwpPd8ccTjjjiqFR233bzBtXoPq7Izqs McqA== X-Gm-Message-State: AOAM531QEApO2li1I4Y9FPKBdsAhvhz/bwezEDLYYwWqQ+IQZ/hSQ9x7 XpwXApLfb5HRLI9iQHwQHCdEQ2bShmA= X-Google-Smtp-Source: ABdhPJyVdyCcWXg12Wm1mSRqq4AdDq0dF5v6J4b8LW28vBVkr8Hj8dDqpwfq0bFwdTdSjem6DE9ApA== X-Received: by 2002:a1c:4c07:: with SMTP id z7mr193893wmf.129.1612204991946; Mon, 01 Feb 2021 10:43:11 -0800 (PST) Received: from [10.56.5.117] ([109.190.253.15]) by smtp.googlemail.com with ESMTPSA id d2sm32006898wre.39.2021.02.01.10.43.10 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 01 Feb 2021 10:43:11 -0800 (PST) To: "libstdc++@gcc.gnu.org" , gcc-patches Subject: [PATCH][_GLIBCXX_DEBUG] Enhance detection of invalid iterators usage Message-ID: Date: Mon, 1 Feb 2021 19:43:09 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 Content-Language: fr X-Spam-Status: No, score=-6.7 required=5.0 tests=BAYES_00, BODY_8BITS, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_SHORT, RCVD_IN_ABUSEAT, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: =?utf-8?q?Fran=C3=A7ois_Dumont_via_Gcc-patches?= From: =?utf-8?q?Fran=C3=A7ois_Dumont?= Reply-To: =?utf-8?q?Fran=C3=A7ois_Dumont?= Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" At the moment some iterators like std::list<>::end() looks like value-init iterators once detached. I think using an iterator in such a state is wrong so here is a patch to detect this. This patch also add a new iterator state: singular (value-initialized) Example of the output of the forward_list/iterator2_neg.cc test: In function:     bool __gnu_debug::operator!=(const _Self&, const _Self&) Error: attempt to compare a singular iterator to a singular (value-initialized) iterator. Objects involved in the operation:     iterator "__lhs" @ 0x0x7fffb777d4b0 {       type = std::__cxx1998::_Fwd_list_iterator (mutable iterator);       state = singular;     }     iterator "__rhs" @ 0x0x7fffb777d4e0 {       type = std::__cxx1998::_Fwd_list_iterator (mutable iterator);       state = singular (value-initialized);     }     libstdc++: [_GLIBCXX_DEBUG] Do not consider detach iterators as value-initialized     An attach iterator has its _M_version set to something != 0. This value shall be preserved     when detaching it so that the iterator does not look like a value initialized one.     libstdc++-v3/ChangeLog:             * include/debug/formatter.h (__singular_value_init): New _Iterator_state enum entry.             (_Parameter<>(const _Safe_iterator<>&, const char*, _Is_iterator)): Check if iterator             parameter is value-initialized.             (_Parameter<>(const _Safe_local_iterator<>&, const char*, _Is_iterator)): Likewise.             * include/debug/safe_iterator.h (_Safe_iterator<>::_M_value_initialized()): New. Adapt             checks.             * include/debug/safe_local_iterator.h (_Safe_local_iterator<>::_M_value_initialized()): New.             Adapt checks.             * src/c++11/debug.cc (_Safe_iterator_base::_M_reset): Do not reset _M_version.             (print_field(PrintContext&, const _Parameter&, const char*)): Adapt state_names.             * testsuite/23_containers/deque/debug/iterator1_neg.cc: New test.             * testsuite/23_containers/deque/debug/iterator2_neg.cc: New test.             * testsuite/23_containers/forward_list/debug/iterator1_neg.cc: New test.             * testsuite/23_containers/forward_list/debug/iterator2_neg.cc: New test. Tested under Linux x86_64. Ok to commit ? François diff --git a/libstdc++-v3/include/debug/formatter.h b/libstdc++-v3/include/debug/formatter.h index 7140fed5e83..1b96de07261 100644 --- a/libstdc++-v3/include/debug/formatter.h +++ b/libstdc++-v3/include/debug/formatter.h @@ -185,6 +185,7 @@ namespace __gnu_debug __rbegin, // dereferenceable, and at the reverse-beginning __rmiddle, // reverse-dereferenceable, not at the reverse-beginning __rend, // reverse-past-the-end + __singular_value_init, // singular, value initialized __last_state }; @@ -278,7 +279,12 @@ namespace __gnu_debug _M_variant._M_iterator._M_seq_type = _GLIBCXX_TYPEID(_Sequence); if (__it._M_singular()) - _M_variant._M_iterator._M_state = __singular; + { + if (__it._M_value_initialized()) + _M_variant._M_iterator._M_state = __singular_value_init; + else + _M_variant._M_iterator._M_state = __singular; + } else { if (__it._M_is_before_begin()) @@ -306,7 +312,12 @@ namespace __gnu_debug _M_variant._M_iterator._M_seq_type = _GLIBCXX_TYPEID(_Sequence); if (__it._M_singular()) - _M_variant._M_iterator._M_state = __singular; + { + if (__it._M_value_initialized()) + _M_variant._M_iterator._M_state = __singular_value_init; + else + _M_variant._M_iterator._M_state = __singular; + } else { if (__it._M_is_end()) diff --git a/libstdc++-v3/include/debug/safe_iterator.h b/libstdc++-v3/include/debug/safe_iterator.h index a10df190969..99d8830e45e 100644 --- a/libstdc++-v3/include/debug/safe_iterator.h +++ b/libstdc++-v3/include/debug/safe_iterator.h @@ -41,8 +41,8 @@ #define _GLIBCXX_DEBUG_VERIFY_OPERANDS(_Lhs, _Rhs, _BadMsgId, _DiffMsgId) \ _GLIBCXX_DEBUG_VERIFY(!_Lhs._M_singular() && !_Rhs._M_singular() \ - || (_Lhs.base() == _Iterator() \ - && _Rhs.base() == _Iterator()), \ + || (_Lhs._M_value_initialized() \ + && _Rhs._M_value_initialized()), \ _M_message(_BadMsgId) \ ._M_iterator(_Lhs, #_Lhs) \ ._M_iterator(_Rhs, #_Rhs)); \ @@ -177,7 +177,7 @@ namespace __gnu_debug // _GLIBCXX_RESOLVE_LIB_DEFECTS // DR 408. Is vector > forbidden? _GLIBCXX_DEBUG_VERIFY(!__x._M_singular() - || __x.base() == _Iterator(), + || __x._M_value_initialized(), _M_message(__msg_init_copy_singular) ._M_iterator(*this, "this") ._M_iterator(__x, "other")); @@ -193,7 +193,7 @@ namespace __gnu_debug : _Iter_base() { _GLIBCXX_DEBUG_VERIFY(!__x._M_singular() - || __x.base() == _Iterator(), + || __x._M_value_initialized(), _M_message(__msg_init_copy_singular) ._M_iterator(*this, "this") ._M_iterator(__x, "other")); @@ -220,7 +220,7 @@ namespace __gnu_debug // _GLIBCXX_RESOLVE_LIB_DEFECTS // DR 408. Is vector > forbidden? _GLIBCXX_DEBUG_VERIFY(!__x._M_singular() - || __x.base() == _MutableIterator(), + || __x._M_value_initialized(), _M_message(__msg_init_const_singular) ._M_iterator(*this, "this") ._M_iterator(__x, "other")); @@ -236,7 +236,7 @@ namespace __gnu_debug // _GLIBCXX_RESOLVE_LIB_DEFECTS // DR 408. Is vector > forbidden? _GLIBCXX_DEBUG_VERIFY(!__x._M_singular() - || __x.base() == _Iterator(), + || __x._M_value_initialized(), _M_message(__msg_copy_singular) ._M_iterator(*this, "this") ._M_iterator(__x, "other")); @@ -266,7 +266,7 @@ namespace __gnu_debug operator=(_Safe_iterator&& __x) noexcept { _GLIBCXX_DEBUG_VERIFY(!__x._M_singular() - || __x.base() == _Iterator(), + || __x._M_value_initialized(), _M_message(__msg_copy_singular) ._M_iterator(*this, "this") ._M_iterator(__x, "other")); @@ -403,6 +403,11 @@ namespace __gnu_debug _M_incrementable() const { return !this->_M_singular() && !_M_is_end(); } + /// Is the iterator value-initialized? + bool + _M_value_initialized() const + { return _M_version == 0 && base() == _Iter_base(); } + // Can we advance the iterator @p __n steps (@p __n may be negative) bool _M_can_advance(difference_type __n, bool __strict = false) const; diff --git a/libstdc++-v3/include/debug/safe_local_iterator.h b/libstdc++-v3/include/debug/safe_local_iterator.h index 31c48e65a24..97ae56c5a7c 100644 --- a/libstdc++-v3/include/debug/safe_local_iterator.h +++ b/libstdc++-v3/include/debug/safe_local_iterator.h @@ -33,8 +33,8 @@ #define _GLIBCXX_DEBUG_VERIFY_OPERANDS(_Lhs, _Rhs) \ _GLIBCXX_DEBUG_VERIFY(!_Lhs._M_singular() && !_Rhs._M_singular() \ - || (_Lhs.base() == _Iterator{} \ - && _Rhs.base() == _Iterator{}), \ + || (_Lhs._M_value_initialized() \ + && _Rhs._M_value_initialized()), \ _M_message(__msg_iter_compare_bad) \ ._M_iterator(_Lhs, "lhs") \ ._M_iterator(_Rhs, "rhs")); \ @@ -127,7 +127,7 @@ namespace __gnu_debug // _GLIBCXX_RESOLVE_LIB_DEFECTS // DR 408. Is vector > forbidden? _GLIBCXX_DEBUG_VERIFY(!__x._M_singular() - || __x.base() == _Iterator(), + || __x._M_value_initialized(), _M_message(__msg_init_copy_singular) ._M_iterator(*this, "this") ._M_iterator(__x, "other")); @@ -142,7 +142,7 @@ namespace __gnu_debug : _Iter_base() { _GLIBCXX_DEBUG_VERIFY(!__x._M_singular() - || __x.base() == _Iterator(), + || __x._M_value_initialized(), _M_message(__msg_init_copy_singular) ._M_iterator(*this, "this") ._M_iterator(__x, "other")); @@ -167,7 +167,7 @@ namespace __gnu_debug // _GLIBCXX_RESOLVE_LIB_DEFECTS // DR 408. Is vector > forbidden? _GLIBCXX_DEBUG_VERIFY(!__x._M_singular() - || __x.base() == _MutableIterator(), + || __x._M_value_initialized(), _M_message(__msg_init_const_singular) ._M_iterator(*this, "this") ._M_iterator(__x, "other")); @@ -183,7 +183,7 @@ namespace __gnu_debug // _GLIBCXX_RESOLVE_LIB_DEFECTS // DR 408. Is vector > forbidden? _GLIBCXX_DEBUG_VERIFY(!__x._M_singular() - || __x.base() == _Iterator(), + || __x._M_value_initialized(), _M_message(__msg_copy_singular) ._M_iterator(*this, "this") ._M_iterator(__x, "other")); @@ -212,7 +212,7 @@ namespace __gnu_debug operator=(_Safe_local_iterator&& __x) noexcept { _GLIBCXX_DEBUG_VERIFY(!__x._M_singular() - || __x.base() == _Iterator(), + || __x._M_value_initialized(), _M_message(__msg_copy_singular) ._M_iterator(*this, "this") ._M_iterator(__x, "other")); @@ -343,6 +343,11 @@ namespace __gnu_debug _M_incrementable() const { return !this->_M_singular() && !_M_is_end(); } + /// Is the iterator value-initialized? + bool + _M_value_initialized() const + { return _M_version == 0 && base() == _Iter_base{}; } + // Is the iterator range [*this, __rhs) valid? bool _M_valid_range(const _Safe_local_iterator& __rhs, diff --git a/libstdc++-v3/src/c++11/debug.cc b/libstdc++-v3/src/c++11/debug.cc index 5a642097d17..5e446b96df8 100644 --- a/libstdc++-v3/src/c++11/debug.cc +++ b/libstdc++-v3/src/c++11/debug.cc @@ -411,7 +411,8 @@ namespace __gnu_debug _M_reset() throw () { __atomic_store_n(&_M_sequence, (_Safe_sequence_base*)0, __ATOMIC_RELEASE); - _M_version = 0; + // Detach iterator shall not look like a value-initialized one. + // _M_version = 0; _M_prior = 0; _M_next = 0; } @@ -737,7 +738,8 @@ namespace "before-begin", "dereferenceable (start-of-reverse-sequence)", "dereferenceable (reverse)", - "past-the-reverse-end" + "past-the-reverse-end", + "singular (value-initialized)" }; print_word(ctx, state_names[iterator._M_state]); } diff --git a/libstdc++-v3/testsuite/23_containers/deque/debug/iterator1_neg.cc b/libstdc++-v3/testsuite/23_containers/deque/debug/iterator1_neg.cc new file mode 100644 index 00000000000..faf68dfce3b --- /dev/null +++ b/libstdc++-v3/testsuite/23_containers/deque/debug/iterator1_neg.cc @@ -0,0 +1,39 @@ +// Copyright (C) 2021 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. +// +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// . +// +// { dg-do run { xfail *-*-* } } +// { dg-require-debug-mode "" } + +#include + +#include + +void test01() +{ + typedef typename std::deque::iterator It; + std::deque dq; + dq.push_back(1); + + It it = It(); + VERIFY( dq.begin() != it ); +} + +int main() +{ + test01(); + return 0; +} diff --git a/libstdc++-v3/testsuite/23_containers/deque/debug/iterator2_neg.cc b/libstdc++-v3/testsuite/23_containers/deque/debug/iterator2_neg.cc new file mode 100644 index 00000000000..ae2247312dd --- /dev/null +++ b/libstdc++-v3/testsuite/23_containers/deque/debug/iterator2_neg.cc @@ -0,0 +1,42 @@ +// Copyright (C) 2021 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. +// +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// . +// +// { dg-do run { xfail *-*-* } } +// { dg-require-debug-mode "" } + +#include + +#include + +void test01() +{ + typedef typename std::deque::iterator It; + It it; + { + std::deque dq; + it = dq.begin(); + } + + It value_init_it = It(); + VERIFY( it != value_init_it ); +} + +int main() +{ + test01(); + return 0; +} diff --git a/libstdc++-v3/testsuite/23_containers/forward_list/debug/iterator1_neg.cc b/libstdc++-v3/testsuite/23_containers/forward_list/debug/iterator1_neg.cc new file mode 100644 index 00000000000..fcb6f9e23a6 --- /dev/null +++ b/libstdc++-v3/testsuite/23_containers/forward_list/debug/iterator1_neg.cc @@ -0,0 +1,39 @@ +// Copyright (C) 2021 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. +// +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// . +// +// { dg-do run { xfail *-*-* } } +// { dg-require-debug-mode "" } + +#include + +#include + +void test01() +{ + typedef typename std::forward_list::iterator It; + std::forward_list fl; + fl.push_front(1); + + It it = It(); + VERIFY( fl.begin() != it ); +} + +int main() +{ + test01(); + return 0; +} diff --git a/libstdc++-v3/testsuite/23_containers/forward_list/debug/iterator2_neg.cc b/libstdc++-v3/testsuite/23_containers/forward_list/debug/iterator2_neg.cc new file mode 100644 index 00000000000..c350327ed37 --- /dev/null +++ b/libstdc++-v3/testsuite/23_containers/forward_list/debug/iterator2_neg.cc @@ -0,0 +1,42 @@ +// Copyright (C) 2021 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. +// +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// . +// +// { dg-do run { target c++11 xfail *-*-* } } +// { dg-require-debug-mode "" } + +#include + +#include + +void test01() +{ + typedef typename std::forward_list::iterator It; + It it; + { + std::forward_list fl; + it = fl.begin(); + } + + It value_init_it{}; + VERIFY( it != value_init_it ); +} + +int main() +{ + test01(); + return 0; +}