From patchwork Mon May 21 04:28:02 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeremy Sawicki X-Patchwork-Id: 917331 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-478034-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=sawicki.us Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="TWbXV2vE"; dkim-atps=neutral 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 40q5PK6YCLz9s37 for ; Mon, 21 May 2018 14:28:19 +1000 (AEST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to :message-id:date:mime-version:content-type:subject:from; q=dns; s=default; b=gwVyQ/9xWWyT9U7Sn7yVH7iM48/k9Zl+RHsUbr//ROe4pWwsfg zt9r2FmWoxj7pvgtwa7Yy4DBhuoGcBkhEBsm1QWccvfm5hlqzHh3iLf9AGkO436O FeWCSOuh941qZaj8DnVz5MHiw0MuZCtbHfy4GuyHajo8Q8gSn6ym1rvCw= 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:to :message-id:date:mime-version:content-type:subject:from; s= default; bh=xBjg5d4dR+7htm/Y/NeRiR9dD2s=; b=TWbXV2vEj5cbC1QGsXEV s2AiZJxbPd/OlYEMrDZmjqsOC38mQZURMFxUpRbovJhJvcihiCgpyPF3taDP5pIk ZdYckeHX8nS7uV8anM9pf5AVZSJNbgwopWbJj6rMWeZDizNfuHvLjbNGzOD1/lwO ggkx+ai+tZTFchhsgtWLgTs= Received: (qmail 109504 invoked by alias); 21 May 2018 04:28:11 -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 109471 invoked by uid 89); 21 May 2018 04:28:09 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-9.1 required=5.0 tests=BAYES_00, GIT_PATCH_2, GIT_PATCH_3, KAM_ASCII_DIVIDERS, KAM_INFOUSMEBIZ, KAM_LAZY_DOMAIN_SECURITY, KAM_LOTSOFHASH, KAM_SHORT, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 spammy=rope, VERIFY, H*F:D*us, __gnu_cxx X-HELO: smtp1.phpwebhosting.com Received: from smtp1.phpwebhosting.com (HELO smtp1.phpwebhosting.com) (173.236.14.186) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with SMTP; Mon, 21 May 2018 04:28:07 +0000 Received: (qmail 14041 invoked from network); 21 May 2018 04:35:44 -0000 Received: from unknown (HELO mech.phpwebhosting.com) (tmda@sawicki.us@96.127.160.86) by smtp1.phpwebhosting.com with SMTP; Mon, 21 May 2018 00:35:44 -0400 Received: from [IPv6:::1] (localhost.localdomain [127.0.0.1]) by mech.phpwebhosting.com (tmda-ofmipd) with ESMTP; Mon, 21 May 2018 00:20:14 -0400 To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Message-ID: Date: Sun, 20 May 2018 21:28:02 -0700 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 Subject: [PATCH] Rope iterators: don't retain pointers when copied X-Delivery-Agent: TMDA/1.1.12 (Macallan) From: Jeremy Sawicki Rope iterators sometimes contain pointers to an internal buffer inside the iterator itself. When such an iterator is copied, the copy incorrectly retains pointers to the original. This patch takes the simple approach of not copying the cached information when the internal buffer is being used, instead requiring it to be recomputed when the copied iterator is dereferenced. An alternative would be to adjust the pointers so they refer to the buffer in the copy. I tested on Linux x64 with "/configure", "make bootstrap", and "make -k check", three times: (1) with no changes (as a baseline), (2) with only the new test (to make sure it fails), and (3) with the new test and bug fix (to make sure the test passes and nothing else changes). * include/ext/rope (_Rope_iterator_base(const _Rope_iterator_base&)) (_Rope_const_iterator::operator=(const _Rope_const_iterator&)) (_Rope_iterator::operator=(const _Rope_iterator&)): Copied/assigned rope iterators don't retain pointers to the iterator they were copied/assigned from. * testsuite/ext/rope/7.cc: New. Index: libstdc++-v3/include/ext/rope =================================================================== --- libstdc++-v3/include/ext/rope (revision 260403) +++ libstdc++-v3/include/ext/rope (working copy) @@ -1119,7 +1119,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _Rope_iterator_base(const _Rope_iterator_base& __x) { - if (0 != __x._M_buf_ptr) + if (0 != __x._M_buf_ptr && __x._M_buf_start != __x._M_tmp_buf) *this = __x; else { @@ -1166,7 +1166,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _Rope_const_iterator& operator=(const _Rope_const_iterator& __x) { - if (0 != __x._M_buf_ptr) + if (0 != __x._M_buf_ptr && __x._M_buf_start != __x._M_tmp_buf) *(static_cast<_Rope_iterator_base<_CharT, _Alloc>*>(this)) = __x; else { @@ -1345,7 +1345,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _RopeRep* __old = this->_M_root; _RopeRep::_S_ref(__x._M_root); - if (0 != __x._M_buf_ptr) + if (0 != __x._M_buf_ptr && __x._M_buf_start != __x._M_tmp_buf) { _M_root_rope = __x._M_root_rope; *(static_cast<_Rope_iterator_base<_CharT, _Alloc>*>(this)) = __x; Index: libstdc++-v3/testsuite/ext/rope/7.cc =================================================================== --- libstdc++-v3/testsuite/ext/rope/7.cc (nonexistent) +++ libstdc++-v3/testsuite/ext/rope/7.cc (working copy) @@ -0,0 +1,95 @@ +// Copyright (C) 2018 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 +// . + +// rope (SGI extension) + +// { dg-do run } + +#include +#include + +void +test01() +{ + using __gnu_cxx::crope; + + char str_a[] = + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + char str_b[] = + "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"; + + // Create ropes with leaf nodes longer than __lazy_threshold = 128 + // so substring nodes will be created by the next step + crope leaf_rope_a(str_a); + crope leaf_rope_b(str_b); + + // Create ropes with substring nodes referencing the leaf nodes + // of the prior ropes + crope substring_rope_a(leaf_rope_a.begin() + 1, + leaf_rope_a.begin() + 199); + crope substring_rope_b(leaf_rope_b.begin() + 1, + leaf_rope_b.begin() + 199); + + // Create iterators to substring_rope_a + crope::const_iterator cit_orig = substring_rope_a.begin(); + crope::iterator mit_orig = substring_rope_a.mutable_begin(); + + // Dereference the iterators so they cache a portion of the substring + // node in their internal buffers + *cit_orig; + *mit_orig; + + // Copy the original iterators, via both copy constructors and + // assignment operators. Prior to the bug fix, these iterators + // retained pointers to the internal buffers of the original + // iterators. + crope::const_iterator cit_copy(cit_orig); + crope::iterator mit_copy(mit_orig); + crope::const_iterator cit_assign; cit_assign = cit_orig; + crope::iterator mit_assign; mit_assign = mit_orig; + + // Modify the original iterators to refer to substring_rope_b + cit_orig = substring_rope_b.begin(); + mit_orig = substring_rope_b.mutable_begin(); + + // Dereference the original iterators so they fill their internal + // buffers with part of substring_rope_b + *cit_orig; + *mit_orig; + + // Verify that the copied iterators return data from + // substring_rope_a, not substring_rope_b + VERIFY(*cit_copy == 'a'); + VERIFY(*mit_copy == 'a'); + VERIFY(*cit_assign == 'a'); + VERIFY(*mit_assign == 'a'); +} + +int +main() +{ + test01(); + return 0; +}