[webkit-changes] [260383] trunk/Source/WTF

Mon, 20 Apr 2020 12:11:42 -0700

Title: [260383] trunk/Source/WTF
Revision
260383
Author
[email protected]
Date
2020-04-20 12:10:00 -0700 (Mon, 20 Apr 2020)

Log Message

REGRESSION (r253987): StringImpl::adopt(Vector&&) copies only half of the characters in the Vector when copying across malloc zones
https://bugs.webkit.org/show_bug.cgi?id=210736

Reviewed by Yusuke Suzuki.

* wtf/text/StringImpl.cpp:
(WTF::StringImpl::replace): Use copyCharacters instead of memcpy.

* wtf/text/StringImpl.h: Use std::is_same_v instead of std::is_same.
(WTF::StringImpl::StringImpl): Use copyCharacters instead of memcpy.
Also drop C-style cast to cast away const.
(WTF::StringImpl::adopt): Don't even try to adopt across malloc zones.

Modified Paths

Diff

Modified: trunk/Source/WTF/ChangeLog (260382 => 260383)


--- trunk/Source/WTF/ChangeLog	2020-04-20 18:43:45 UTC (rev 260382)
+++ trunk/Source/WTF/ChangeLog	2020-04-20 19:10:00 UTC (rev 260383)
@@ -1,5 +1,20 @@
 2020-04-20  Darin Adler  <[email protected]>
 
+        REGRESSION (r253987): StringImpl::adopt(Vector&&) copies only half of the characters in the Vector when copying across malloc zones
+        https://bugs.webkit.org/show_bug.cgi?id=210736
+
+        Reviewed by Yusuke Suzuki.
+
+        * wtf/text/StringImpl.cpp:
+        (WTF::StringImpl::replace): Use copyCharacters instead of memcpy.
+
+        * wtf/text/StringImpl.h: Use std::is_same_v instead of std::is_same.
+        (WTF::StringImpl::StringImpl): Use copyCharacters instead of memcpy.
+        Also drop C-style cast to cast away const.
+        (WTF::StringImpl::adopt): Don't even try to adopt across malloc zones.
+
+2020-04-20  Darin Adler  <[email protected]>
+
         Use #import instead of #include in Objective-C and don't use #pragma once
         https://bugs.webkit.org/show_bug.cgi?id=210724

Modified: trunk/Source/WTF/wtf/text/StringImpl.cpp (260382 => 260383)


--- trunk/Source/WTF/wtf/text/StringImpl.cpp	2020-04-20 18:43:45 UTC (rev 260382)
+++ trunk/Source/WTF/wtf/text/StringImpl.cpp	2020-04-20 19:10:00 UTC (rev 260383)
@@ -1302,7 +1302,7 @@
     UChar* data;
     auto newImpl = createUninitializedInternalNonEmpty(m_length, data);
 
-    memcpy(data, m_data16, i * sizeof(UChar));
+    copyCharacters(data, m_data16, i);
     for (unsigned j = i; j != m_length; ++j) {
         UChar character = m_data16[j];
         if (character == target)

Modified: trunk/Source/WTF/wtf/text/StringImpl.h (260382 => 260383)


--- trunk/Source/WTF/wtf/text/StringImpl.h	2020-04-20 18:43:45 UTC (rev 260382)
+++ trunk/Source/WTF/wtf/text/StringImpl.h	2020-04-20 19:10:00 UTC (rev 260383)
@@ -892,11 +892,12 @@
 inline StringImpl::StringImpl(MallocPtr<LChar, Malloc> characters, unsigned length)
     : StringImplShape(s_refCountIncrement, length, static_cast<const LChar*>(nullptr), s_hashFlag8BitBuffer | StringNormal | BufferOwned)
 {
-    if constexpr (std::is_same<Malloc, StringImplMalloc>::value)
+    if constexpr (std::is_same_v<Malloc, StringImplMalloc>)
         m_data8 = characters.leakPtr();
     else {
-        m_data8 = static_cast<const LChar*>(StringImplMalloc::malloc(length));
-        memcpy((void*)m_data8, characters.get(), length);
+        auto data8 = static_cast<LChar*>(StringImplMalloc::malloc(length * sizeof(LChar)));
+        copyCharacters(data8, characters.get(), length);
+        m_data8 = data8;
     }
 
     ASSERT(m_data8);
@@ -927,11 +928,12 @@
 inline StringImpl::StringImpl(MallocPtr<UChar, Malloc> characters, unsigned length)
     : StringImplShape(s_refCountIncrement, length, static_cast<const UChar*>(nullptr), StringNormal | BufferOwned)
 {
-    if constexpr (std::is_same<Malloc, StringImplMalloc>::value)
+    if constexpr (std::is_same_v<Malloc, StringImplMalloc>)
         m_data16 = characters.leakPtr();
     else {
-        m_data16 = static_cast<const UChar*>(StringImplMalloc::malloc(length * sizeof(UChar)));
-        memcpy((void*)m_data16, characters.get(), length * sizeof(UChar));
+        auto data16 = static_cast<UChar*>(StringImplMalloc::malloc(length * sizeof(UChar)));
+        copyCharacters(data16, characters.get(), length);
+        m_data16 = data16;
     }
 
     ASSERT(m_data16);
@@ -1031,22 +1033,15 @@
 template<typename CharacterType, size_t inlineCapacity, typename OverflowHandler, size_t minCapacity, typename Malloc>
 inline Ref<StringImpl> StringImpl::adopt(Vector<CharacterType, inlineCapacity, OverflowHandler, minCapacity, Malloc>&& vector)
 {
-    if (size_t size = vector.size()) {
-        ASSERT(vector.data());
-        if (size > MaxLength)
+    if constexpr (std::is_same_v<Malloc, StringImplMalloc>) {
+        auto length = vector.size();
+        if (!length)
+            return *empty();
+        if (length > MaxLength)
             CRASH();
-
-        if constexpr (std::is_same<Malloc, StringImplMalloc>::value)
-            return adoptRef(*new StringImpl(vector.releaseBuffer(), size));
-        else {
-            // We have to copy between malloc zones.
-            auto vectorBuffer = vector.releaseBuffer();
-            auto stringImplBuffer = MallocPtr<CharacterType, StringImplMalloc>::malloc(size);
-            memcpy(stringImplBuffer.get(), vectorBuffer.get(), size);
-            return adoptRef(*new StringImpl(WTFMove(stringImplBuffer), size));
-        }
-    }
-    return *empty();
+        return adoptRef(*new StringImpl(vector.releaseBuffer(), length));
+    } else
+        return create(vector.data(), vector.size());
 }
 
 inline size_t StringImpl::cost() const
@@ -1134,9 +1129,9 @@
 template<typename SourceCharacterType, typename DestinationCharacterType>
 inline void StringImpl::copyCharacters(DestinationCharacterType* destination, const SourceCharacterType* source, unsigned numCharacters)
 {
-    static_assert(std::is_same<SourceCharacterType, LChar>::value || std::is_same<SourceCharacterType, UChar>::value);
-    static_assert(std::is_same<DestinationCharacterType, LChar>::value || std::is_same<DestinationCharacterType, UChar>::value);
-    if constexpr (std::is_same<SourceCharacterType, DestinationCharacterType>::value) {
+    static_assert(std::is_same_v<SourceCharacterType, LChar> || std::is_same_v<SourceCharacterType, UChar>);
+    static_assert(std::is_same_v<DestinationCharacterType, LChar> || std::is_same_v<DestinationCharacterType, UChar>);
+    if constexpr (std::is_same_v<SourceCharacterType, DestinationCharacterType>) {
         if (numCharacters == 1) {
             *destination = *source;
             return;

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to