From 7175db5f8c38b3f3f3146a085719285ea440e459 Mon Sep 17 00:00:00 2001 From: "utatane.tea@gmail.com" Date: Thu, 6 Jul 2017 02:31:35 +0000 Subject: [PATCH] WTF::StringImpl::copyChars segfaults when built with GCC 7 https://bugs.webkit.org/show_bug.cgi?id=173407 Reviewed by Andreas Kling. JSTests: * stress/string-repeat-copy-chars-crash.js: Added. (shouldBe): Source/WTF: With GCC 7, StringImpl::copyChars() behaves as unexpected. This function violates strict aliasing rule. This optimization is originally introduced to improve performance in SunSpider's string tests in 2008. When running it in my Linux box, it no longer causes any observable difference. So, we just remove this optimization. baseline patched string-base64 7.7544+-0.1761 7.6138+-0.2071 might be 1.0185x faster string-fasta 10.5429+-0.2746 ? 10.7500+-0.2669 ? might be 1.0196x slower string-tagcloud 14.8588+-0.2828 14.8039+-0.3039 string-unpack-code 36.1769+-0.4251 35.3397+-0.5398 might be 1.0237x faster string-validate-input 8.5182+-0.2206 8.3514+-0.2179 might be 1.0200x faster * wtf/text/StringImpl.h: (WTF::StringImpl::copyChars): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@219182 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- JSTests/ChangeLog | 10 +++++++++ JSTests/stress/string-repeat-copy-chars-crash.js | 8 ++++++++ Source/WTF/ChangeLog | 26 ++++++++++++++++++++++++ Source/WTF/wtf/text/StringImpl.h | 23 +-------------------- 4 files changed, 45 insertions(+), 22 deletions(-) create mode 100644 JSTests/stress/string-repeat-copy-chars-crash.js diff -urN qtwebkit.orig/Source/WTF/wtf/text/StringImpl.h qtwebkit/Source/WTF/wtf/text/StringImpl.h --- qtwebkit.orig/Source/WTF/wtf/text/StringImpl.h 2018-02-12 12:14:02.778420779 +0000 +++ qtwebkit/Source/WTF/wtf/text/StringImpl.h 2018-02-12 12:18:30.385666017 +0000 @@ -629,25 +629,7 @@ *destination = *source; return; } - - if (numCharacters <= s_copyCharsInlineCutOff) { - unsigned i = 0; -#if (CPU(X86) || CPU(X86_64)) - const unsigned charsPerInt = sizeof(uint32_t) / sizeof(T); - - if (numCharacters > charsPerInt) { - unsigned stopCount = numCharacters & ~(charsPerInt - 1); - - const uint32_t* srcCharacters = reinterpret_cast(source); - uint32_t* destCharacters = reinterpret_cast(destination); - for (unsigned j = 0; i < stopCount; i += charsPerInt, ++j) - destCharacters[j] = srcCharacters[j]; - } -#endif - for (; i < numCharacters; ++i) - destination[i] = source[i]; - } else - memcpy(destination, source, numCharacters * sizeof(T)); + memcpy(destination, source, numCharacters * sizeof(T)); } ALWAYS_INLINE static void copyChars(UChar* destination, const LChar* source, unsigned numCharacters) @@ -771,9 +753,6 @@ return reinterpret_cast(m_data16) == reinterpret_cast(this + 1); } - // This number must be at least 2 to avoid sharing empty, null as well as 1 character strings from SmallStrings. - static const unsigned s_copyCharsInlineCutOff = 20; - BufferOwnership bufferOwnership() const { return static_cast(m_hashAndFlags & s_hashMaskBufferOwnership); } bool isStatic() const { return m_refCount & s_refCountFlagIsStaticString; } template PassRefPtr stripMatchedCharacters(UCharPredicate); diff -urN qtwebkit.orig/Source/WTF/wtf/text/StringImpl.h.rej qtwebkit/Source/WTF/wtf/text/StringImpl.h.rej --- qtwebkit.orig/Source/WTF/wtf/text/StringImpl.h.rej 1970-01-01 00:00:00.000000000 +0000 +++ qtwebkit/Source/WTF/wtf/text/StringImpl.h.rej 2018-02-12 12:17:26.235366283 +0000 @@ -0,0 +1,12 @@ +--- Source/WTF/wtf/text/StringImpl.h ++++ Source/WTF/wtf/text/StringImpl.h +@@ -841,9 +823,6 @@ + return *tailPointer(); + } + +- // This number must be at least 2 to avoid sharing empty, null as well as 1 character strings from SmallStrings. +- static const unsigned s_copyCharsInlineCutOff = 20; +- + enum class CaseConvertType { Upper, Lower }; + template static Ref convertASCIICase(StringImpl&, const CharacterType*, unsigned); +