From b88852059823163b0fa0f624f5ceb49931c3cc68 Mon Sep 17 00:00:00 2001
From: Marco Bubke <marco.bubke@qt.io>
Date: Wed, 30 Nov 2016 16:21:25 +0100
Subject: [PATCH] Utils: Reserve in smallstring was flaky

We have to copy the content of SmallString before we instantiate a new
constructor in the same memory. So the content to which data() is pointing
can be already invalid.

Change-Id: I3a0ab4f9ac0c1219c2bd75fc4412eaf56209ca64
Reviewed-by: Tobias Hunger <tobias.hunger@qt.io>
---
 src/libs/utils/smallstring.h             | 23 ++++++++++++++------
 tests/unit/unittest/smallstring-test.cpp | 27 ++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/src/libs/utils/smallstring.h b/src/libs/utils/smallstring.h
index b0af23aac9..eae9c29dc4 100644
--- a/src/libs/utils/smallstring.h
+++ b/src/libs/utils/smallstring.h
@@ -102,12 +102,7 @@ public:
         } else {
             m_data.allocated.data.pointer = Memory::allocate(capacity + 1);
             std::memcpy(m_data.allocated.data.pointer, string, size);
-            m_data.allocated.data.pointer[size] = 0;
-            m_data.allocated.data.size = size;
-            m_data.allocated.data.capacity = capacity;
-            m_data.allocated.shortStringSize = 0;
-            m_data.allocated.isReference = true;
-            m_data.allocated.isReadOnlyReference = false;
+            initializeLongString(size, capacity);
         }
     }
 
@@ -245,8 +240,12 @@ public:
                 m_data.allocated.data.capacity = newCapacity;
             } else {
                 const size_type oldSize = size();
+                const char *oldData = data();
 
-                new (this) BasicSmallString(data(), oldSize, newCapacity);
+                char *newData = Memory::allocate(newCapacity + 1);
+                std::memcpy(newData, oldData, oldSize);
+                m_data.allocated.data.pointer = newData;
+                initializeLongString(oldSize, newCapacity);
             }
         }
     }
@@ -660,6 +659,16 @@ private:
     {
     }
 
+    void initializeLongString(size_type size, size_type capacity)
+    {
+        m_data.allocated.data.pointer[size] = 0;
+        m_data.allocated.data.size = size;
+        m_data.allocated.data.capacity = capacity;
+        m_data.allocated.shortStringSize = 0;
+        m_data.allocated.isReference = true;
+        m_data.allocated.isReadOnlyReference = false;
+    }
+
     char &at(size_type index)
     {
         return *(data() + index);
diff --git a/tests/unit/unittest/smallstring-test.cpp b/tests/unit/unittest/smallstring-test.cpp
index 70e944ffa4..05925d72b6 100644
--- a/tests/unit/unittest/smallstring-test.cpp
+++ b/tests/unit/unittest/smallstring-test.cpp
@@ -546,6 +546,15 @@ TEST(SmallString, AppendShortSmallString)
     ASSERT_THAT(text, SmallString("some text"));
 }
 
+TEST(SmallString, AppendLongSmallStringToShortSmallString)
+{
+    SmallString text("some ");
+
+    text.append(SmallString("very very very very very long string"));
+
+    ASSERT_THAT(text, SmallString("some very very very very very long string"));
+}
+
 TEST(SmallString, AppendLongSmallString)
 {
     SmallString longText("some very very very very very very very very very very very long string");
@@ -833,6 +842,24 @@ TEST(SmallString, ReserveMuchBiggerThanReference)
     ASSERT_THAT(text.capacity(), 100);
 }
 
+TEST(SmallString, TextIsCopiedAfterReserveFromShortToLongString)
+{
+    SmallString text("text");
+
+    text.reserve(100);
+
+    ASSERT_THAT(text, "text");
+}
+
+TEST(SmallString, TextIsCopiedAfterReserveReferenceToLongString)
+{
+    SmallString text("some very very very very very very very very very very very long string");
+
+    text.reserve(100);
+
+    ASSERT_THAT(text, "some very very very very very very very very very very very long string");
+}
+
 TEST(SmallString, ReserveSmallerThanShortSmallString)
 {
     SmallString text = SmallString::fromUtf8("text");
-- 
GitLab