[Stlport-devel] rope bug fixes I would like to get accepted

[Peter H. <pe...@sy...>]

Hi all,

I have here two patches for rope implementation in STLport 5.0.2.
This was detected on Windows with msvc 7.1 compiler and the default
stl_user_config but I believe it is platform/config independent.

I'm not really sure how to get them to the official trunk in the
next point release, so I though this is a good place to start.

I'm attaching the patch which fixes these two bugs:

Bug 1)
- fixed by second hunk of _rope.h file
- when a rope is constructed from a single character then the
  character buffer inside rope leaf is not terminated by zero;
  this leads to incorrect results returned from c_str() method

Bug 2)
- fixed by the rest for the patch
- concatenation of a string to a rope could lead to a write after
  the space allocated to the rope; this was corrupting data of
  other/unrelated objects possibly leading to crashes
- the problem is that the check whether there is enough space
  preallocated in a rope leaf did not take the terminating zero
  correctly into account
- an easy way how to simulate this bug is to concatenate one
  character to a rope created from a string like this "1234567"
  (notice that the string is just one character shorter than
  the allocation granularity); then the character being added
  was overwriting the old termination zero and the new termination
  zero was written past the preallocated buffer

Kind regards,
   Peter Hercek.


The patch is below:

Index: stlport/stl/_rope.c
===================================================================
--- stlport/stl/_rope.c (revision 32821)
+++ stlport/stl/_rope.c (working copy)
@@ -409,7 +409,8 @@
   }
   __r->_M_decr(); // - ptr, __r->_M_ref_count == 1 or 0
   size_t __old_len = __r->_M_size._M_data;
-  if (_S_allocated_capacity(__old_len) >= __old_len + __len) {
+  // Chek whether we can still fit to the old buffer
+  if ( _S_rounded_up_size(__old_len) == 
_S_rounded_up_size(__old_len+__len) ) {
     // The space has been partially initialized for the standard
     // character types.  But that doesn't matter for those types.
     uninitialized_copy_n(__iter, __len, __r->_M_data + __old_len);
Index: stlport/stl/_rope.h
===================================================================
--- stlport/stl/_rope.h (revision 32821)
+++ stlport/stl/_rope.h (working copy)
@@ -1153,10 +1153,6 @@
     return _RopeRep::_S_rounded_up_size(__n);
   }

-  static size_t _S_allocated_capacity(size_t __n) {
-    return _RopeRep::_S_rounded_up_size_aux(__n, __false_type());
-  }
-
   // Allocate and construct a RopeLeaf using the supplied allocator
   // Takes ownership of s instead of copying.
   static _RopeLeaf* _S_new_RopeLeaf(_CharT *__s,
@@ -1348,9 +1344,11 @@

   rope(_CharT __c, const allocator_type& __a = allocator_type())
     : _M_tree_ptr(__a, (_RopeRep*)0) {
-    _CharT* __buf = _M_tree_ptr.allocate(_S_rounded_up_size(1));
+    _CharT* __buf = _M_tree_ptr.allocate(_S_rounded_up_size(2));

     _Copy_Construct(__buf, __c);
+    _S_construct_null( __buf + sizeof(_CharT) );
+
     _STLP_TRY {
       _M_tree_ptr._M_data = _S_new_RopeLeaf(__buf, 1, __a);
     }