[Bug libstdc++/58415] New: __sso_string_base move constructor does not null terminate local data when moving from empty string

[Bug libstdc++/58415] New: __sso_string_base move constructor does not null terminate local data when moving from empty string

[mkirzinger at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58415

            Bug ID: 58415
           Summary: __sso_string_base move constructor does not null
                    terminate local data when moving from empty string
           Product: gcc
           Version: 4.8.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: mkirzinger at gmail dot com

When doing a move construct using a sso_string_base versa_string where it is
moving from an empty string, the contents of the string it is moving from are
not copied, leaving the local data uninitialized (and later calling c_str() on
it may produce invalid output).

Code that demonstrates one of the situations this problem can show up (compile
with -std=c++11):

------------------------------------------
#include <ext/vstring.h>
#include <cstdio>

typedef __gnu_cxx::__versa_string<char> string;

int main(int argc, char* argv[])
{
    string s1("string");
    string s2("");
    std::swap(s1, s2);
    printf("%s%s\n", s1.c_str(), s2.c_str());
}
------------------------------------------

Expected output:
string
Actual output:
stringstring

[Bug libstdc++/58415] __sso_string_base move constructor does not null terminate local data when moving from empty string

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58415

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2013-09-13
           Assignee|unassigned at gcc dot gnu.org      |paolo.carlini at oracle dot com
     Ever confirmed|0                           |1

--- Comment #1 from Paolo Carlini <paolo.carlini at oracle dot com> ---
Urgh, thanks for your report. The move constructor is wrongly setting the
length "by hand" instead of using _M_set_length, per the below. I'm going to
commit the fix momentarily to mainline and 4_8-branch. Thanks again.

Index: include/ext/sso_string_base.h
===================================================================
--- include/ext/sso_string_base.h       (revision 202561)
+++ include/ext/sso_string_base.h       (working copy)
@@ -362,8 +362,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        }

       _M_length(__rcs._M_length());
-      __rcs._M_length(0);
       __rcs._M_data(__rcs._M_local_data);
+      __rcs._M_set_length(0);
     }
 #endif

[Bug libstdc++/58415] __sso_string_base move constructor does not null terminate local data when moving from empty string

[mkirzinger at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58415

--- Comment #2 from Michael Kirzinger <mkirzinger at gmail dot com> ---
There appears to be one additional problem: if __rcs._M_is_local() is true, but
__rcs._M_length() is false, the buffer of the string being created is never
null terminated/zeroed.

Example:
----------------------------------------------------------
#include <ext/vstring.h>
#include <cstdio>
#include <cstdlib>
#include <cstring>

typedef __gnu_cxx::__versa_string<char> string;

int main()
{
    char buf[sizeof(string)+1] = "stringstringstring";

    string s1;
    string* s2 = new (buf) string(std::move(s1));

    printf("%s\n", s2->c_str());
}

[Bug libstdc++/58415] __sso_string_base move constructor does not null terminate local data when moving from empty string

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58415

--- Comment #3 from Paolo Carlini <paolo.carlini at oracle dot com> ---
Ok, thanks.

[Bug libstdc++/58415] __sso_string_base move constructor does not null terminate local data when moving from empty string

[paolo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58415

--- Comment #4 from paolo at gcc dot gnu.org <paolo at gcc dot gnu.org> ---
Author: paolo
Date: Fri Sep 13 18:32:42 2013
New Revision: 202574

URL: http://gcc.gnu.org/viewcvs?rev=202574&root=gcc&view=rev
Log:
2013-09-13  Paolo Carlini  <paolo.carlini@oracle.com>

    PR libstdc++/58415
    * include/ext/sso_string_base.h (__sso_string_base<>::
    __sso_string_base(__sso_string_base&&)): Fix thinkos about
    _M_length vs _M_set_length.
    * testsuite/ext/vstring/cons/58415-1.cc: New.
    * testsuite/ext/vstring/cons/58415-2.cc: Likewise.

Added:
    trunk/libstdc++-v3/testsuite/ext/vstring/cons/58415-1.cc
    trunk/libstdc++-v3/testsuite/ext/vstring/cons/58415-2.cc
Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/include/ext/sso_string_base.h

[Bug libstdc++/58415] __sso_string_base move constructor does not null terminate local data when moving from empty string

[paolo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58415

--- Comment #5 from paolo at gcc dot gnu.org <paolo at gcc dot gnu.org> ---
Author: paolo
Date: Fri Sep 13 18:33:17 2013
New Revision: 202575

URL: http://gcc.gnu.org/viewcvs?rev=202575&root=gcc&view=rev
Log:
2013-09-13  Paolo Carlini  <paolo.carlini@oracle.com>

    PR libstdc++/58415
    * include/ext/sso_string_base.h (__sso_string_base<>::
    __sso_string_base(__sso_string_base&&)): Fix thinkos about
    _M_length vs _M_set_length.
    * testsuite/ext/vstring/cons/58415-1.cc: New.
    * testsuite/ext/vstring/cons/58415-2.cc: Likewise.

Added:
    branches/gcc-4_8-branch/libstdc++-v3/testsuite/ext/vstring/cons/58415-1.cc
    branches/gcc-4_8-branch/libstdc++-v3/testsuite/ext/vstring/cons/58415-2.cc
Modified:
    branches/gcc-4_8-branch/libstdc++-v3/ChangeLog
    branches/gcc-4_8-branch/libstdc++-v3/include/ext/sso_string_base.h

[Bug libstdc++/58415] __sso_string_base move constructor does not null terminate local data when moving from empty string

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58415

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED
           Assignee|paolo.carlini at oracle dot com    |unassigned at gcc dot gnu.org
   Target Milestone|---                         |4.8.2

--- Comment #6 from Paolo Carlini <paolo.carlini at oracle dot com> ---
Fixed mainline and 4.8.2.