Rope iterators: don't retain pointers when copied
authorJeremy Sawicki <jeremy-gcc@sawicki.us>
Tue, 14 Aug 2018 11:23:50 +0000 (11:23 +0000)
committerJonathan Wakely <redi@gcc.gnu.org>
Tue, 14 Aug 2018 11:23:50 +0000 (12:23 +0100)
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.

2018-08-14  Jeremy Sawicki  <jeremy-gcc@sawicki.us>

* include/ext/rope (_Rope_iterator_base(const _Rope_iterator_base&))
(_Rope_const_iterator::operator=(const _Rope_const_iterator&))
(_Rope_iterator::operator=(const _Rope_iterator&)): Ensure
copied/assigned rope iterators don't retain pointers to the iterator
they were copied/assigned from.
* testsuite/ext/rope/7.cc: New.

From-SVN: r263534

libstdc++-v3/ChangeLog
libstdc++-v3/include/ext/rope
libstdc++-v3/testsuite/ext/rope/7.cc [new file with mode: 0644]

index 312f39c41ba0093c2d3f2e85ff5d8edd42a97eca..65890ea4b6b4e8f3811b0ea4086ce7d8a245e251 100644 (file)
@@ -1,3 +1,12 @@
+2018-08-14  Jeremy Sawicki  <jeremy-gcc@sawicki.us>
+
+       * include/ext/rope (_Rope_iterator_base(const _Rope_iterator_base&))
+       (_Rope_const_iterator::operator=(const _Rope_const_iterator&))
+       (_Rope_iterator::operator=(const _Rope_iterator&)): Ensure
+       copied/assigned rope iterators don't retain pointers to the iterator
+       they were copied/assigned from.
+       * testsuite/ext/rope/7.cc: New.
+
 2018-08-13  Jonathan Wakely  <jwakely@redhat.com>
 
        PR libstdc++/45093
index a53d2e07a32462a69d60f49720ce2783b007e221..a172989a2f9f8bcfe08f7ac1f6662a8cc013dbcc 100644 (file)
@@ -1119,7 +1119,7 @@ protected:
     
       _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 @@ protected:
       _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 @@ protected:
         _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;
diff --git a/libstdc++-v3/testsuite/ext/rope/7.cc b/libstdc++-v3/testsuite/ext/rope/7.cc
new file mode 100644 (file)
index 0000000..a927494
--- /dev/null
@@ -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
+// <http://www.gnu.org/licenses/>.
+
+// rope (SGI extension)
+
+// { dg-do run }
+
+#include <ext/rope>
+#include <testsuite_hooks.h>
+
+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;
+}