libstdc++/4354: string::assign behaving unexpectedly when destination string == source string

[pcarlini@unitus.it]

>Number:         4354
>Category:       libstdc++
>Synopsis:       string::assign behaving unexpectedly when destination string == source string
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    unassigned
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Sep 18 16:46:00 PDT 2001
>Closed-Date:
>Last-Modified:
>Originator:     Paolo Carlini / Roberto Bagnara
>Release:        gcc-3.0.1 (current 3.1 and 3.0.2 snapshots too)
>Organization:
>Environment:
x86, glibc-2.2.4, linux-2.4.8, binutils-2.11.2
>Description:
The following simple program prints "merge" instead of
"mergesort_ap_variant.pl", as it (probably) should:

#include <iostream>
#include <string>

using namespace std;

int main() {
  string aux = "../BenchCtiTr/apt/mergesort_ap_variant.pl";
  string::size_type i = aux.rfind("/");
  if (i != string::npos)
    aux.assign(aux, i+1, string::npos);
  cout << aux.c_str() << endl;
}

>How-To-Repeat:
The member function string::assign behaves unexpectedly when
dealing with the same string as source and destination.


 
>Fix:
I traced back the problem to the following function in
basic_string.tcc (lines 442-462):

--------------------------------------

  template<typename _CharT, typename _Traits, typename _Alloc>
    template<typename _ForwardIter>
      basic_string<_CharT, _Traits, _Alloc>&
      basic_string<_CharT, _Traits, _Alloc>::
      _M_replace(iterator __i1, iterator __i2, _ForwardIter __k1,
   _ForwardIter __k2, forward_iterator_tag)
      {
 size_type __dold = __i2 - __i1;
 size_type __dmax = this->max_size();
 size_type __dnew = static_cast<size_type>(distance(__k1, __k2));

 if (__dmax <= __dnew)
   __throw_length_error("basic_string::_M_replace");
 size_type __off = __i1 - _M_ibegin();
 _M_mutate(__off, __dold, __dnew);
 // Invalidated __i1, __i2
 if (__dnew)
   _S_copy_chars(_M_data() + __off, __k1, __k2);

 return *this;
      }

---------------------------------------

Irrespective of the fact that we are dealing with the
very same string as source and destination, _M_mutate is
called (with __dold == 41 and __dnew == 23) **before** the
actual copy operation (_S_copy_chars) is carried out.
Therefore, the aux string, as source of the assign, is
clobbered, i.e. is truncated at the 23rd character
("../BenchCtiTr/apt/merge"), and the eventual copy
operation (_S_copy_chars) produces aux == "merge".
Indeed, tentatively exchanging the _M_mutate and
_S_copy_chars calls leads to the expected behaviour when
source string == destination string.

Therefore, it seems likely that this special case should be
detected and dealt with in that way (i.e., swapping the
aforementioned calls) modulo perhaps reference counting
issues which I do not fully understand at present :(

>Release-Note:
>Audit-Trail:
>Unformatted: