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 CAC7E3858023 for ; Mon, 8 Aug 2022 13:19:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org CAC7E3858023 Received: from mail-qk1-f200.google.com (mail-qk1-f200.google.com [209.85.222.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-312-Cejxb62fMwaxbEHmt8yDmg-1; Mon, 08 Aug 2022 09:19:40 -0400 X-MC-Unique: Cejxb62fMwaxbEHmt8yDmg-1 Received: by mail-qk1-f200.google.com with SMTP id bi22-20020a05620a319600b006b92f4b2ebbso4879593qkb.22 for ; Mon, 08 Aug 2022 06:19:40 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc; bh=H08rnclY2IXNTpgWzqYSIV7SCtkAY+Wxq+JJPZZfYdQ=; b=uQleU1WnCNAoDqAO8Xy4e+iYlsqaIwtGhQKF5BopwgxS7Ry8K4VD+GoR8Jrj4DkAvu 13EA68aYS7i2zFF5EzxbPspnZ1l933Ux2gtTEnImUIiW/9TwwasFrRPZORBxJYagPpxF +ZwxsSnf6WLBgCCV9gN2xgQw0XsNvKigavXaT8yq29w733+KavgzGRfuL6HZ0MAN6kY2 6ox3/fDKk4FEdaAQiTaHcX5/dO+M5ckvCpNQw+pQAImRqkVP4NcpBVd+5LTk69c9vGqH tsHvoBLTRj1hoMA3hg72XRv6RYHdVfrpvdo9g64qo1mh2gkb1C3onQVJbT/Sajs6FqS7 aY3g== X-Gm-Message-State: ACgBeo3jKPz68EladHNoZYe4UAHc+P+tQbCCIefd1IPQBxwQhzK40ysp NAnmdi0aqlRayK0zA+31E8JjoAl8dzuqVy850GwTu1iThIdQyVIrK6N2SIhFsWKlWX6L7ovHCbW UBVIKxIY5D0d5spbThBc5FeU4zgTDasA= X-Received: by 2002:ad4:5be2:0:b0:476:7e0d:815f with SMTP id k2-20020ad45be2000000b004767e0d815fmr15375653qvc.57.1659964779980; Mon, 08 Aug 2022 06:19:39 -0700 (PDT) X-Google-Smtp-Source: AA6agR5uxuMlDAysLmZwfi/WFcWpZqJi6h1weRtn3XSshYlMLK6KMPwMPJQtKYSr3vKXLaW8X6r+LG5ZIXgqn/Hd8Mo= X-Received: by 2002:ad4:5be2:0:b0:476:7e0d:815f with SMTP id k2-20020ad45be2000000b004767e0d815fmr15375640qvc.57.1659964779759; Mon, 08 Aug 2022 06:19:39 -0700 (PDT) MIME-Version: 1.0 References: <3b6d1c3d-8bc7-e5e5-0a76-58d960f86e3f@gmail.com> In-Reply-To: From: Jonathan Wakely Date: Mon, 8 Aug 2022 14:19:29 +0100 Message-ID: Subject: Re: [PATCH][_GLIBCXX_DEBUG] Refine singular iterator state To: =?UTF-8?Q?Fran=C3=A7ois_Dumont?= Cc: "libstdc++@gcc.gnu.org" , gcc-patches X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-14.1 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, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libstdc++@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libstdc++ mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 08 Aug 2022 13:19:43 -0000 On Mon, 8 Aug 2022 at 06:07, Fran=C3=A7ois Dumont via Libstdc++ wrote: > > Another version of this patch with just a new test case showing what > wrong code was unnoticed previously by the _GLIBCXX_DEBUG mode. > > On 04/08/22 22:56, Fran=C3=A7ois Dumont wrote: > > This an old patch I had prepared a long time ago, I don't think I ever > > submitted it. > > > > libstdc++: [_GLIBCXX_DEBUG] Do not consider detached iterators as > > value-initialized > > > > An attach iterator has its _M_version set to something !=3D 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 _GLIBCXX_DEBUG mode. > > > > Ok to commit ? > > > > Fran=C3=A7ois >diff --git a/libstdc++-v3/src/c++11/debug.cc b/libstdc++-v3/src/c++11/debu= g.cc >index 4706defedf1..cf8e6f48081 100644 >--- a/libstdc++-v3/src/c++11/debug.cc >+++ b/libstdc++-v3/src/c++11/debug.cc >@@ -426,7 +426,8 @@ namespace __gnu_debug > _M_reset() throw () > { > __atomic_store_n(&_M_sequence, (_Safe_sequence_base*)0, __ATOMIC_RELE= ASE); >- _M_version =3D 0; >+ // Detach iterator shall not look like a value-initialized one. >+ // _M_version =3D 0; > _M_prior =3D 0; > _M_next =3D 0; > } I think this would be clearer as "Do not reset version, so that a detached iterator does not look like a value-initialized one." >+// { 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 =3D It(); >+ VERIFY( dq.begin() !=3D it ); Is there any reason to use VERIFY here? We're expecting the comparison to abort in the debug mode checks, right? Which would happen if we just do: (void) dq.begin() =3D=3D it; Using VERIFY just makes it look like we're expecting the test to be XFAIL because the assertion will fail, but that's not what is being tested. OK for trunk with those changes, thanks.