Title: [219182] trunk
- Revision
- 219182
- Author
- [email protected]
- Date
- 2017-07-05 19:31:35 -0700 (Wed, 05 Jul 2017)
Log Message
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):
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (219181 => 219182)
--- trunk/JSTests/ChangeLog 2017-07-06 02:30:40 UTC (rev 219181)
+++ trunk/JSTests/ChangeLog 2017-07-06 02:31:35 UTC (rev 219182)
@@ -1,3 +1,13 @@
+2017-07-05 Yusuke Suzuki <[email protected]>
+
+ WTF::StringImpl::copyChars segfaults when built with GCC 7
+ https://bugs.webkit.org/show_bug.cgi?id=173407
+
+ Reviewed by Andreas Kling.
+
+ * stress/string-repeat-copy-chars-crash.js: Added.
+ (shouldBe):
+
2017-07-03 Saam Barati <[email protected]>
Skip unshiftCountSlowCase-correct-postCapacity.js on debug builds since it takes a long time to run.
Added: trunk/JSTests/stress/string-repeat-copy-chars-crash.js (0 => 219182)
--- trunk/JSTests/stress/string-repeat-copy-chars-crash.js (rev 0)
+++ trunk/JSTests/stress/string-repeat-copy-chars-crash.js 2017-07-06 02:31:35 UTC (rev 219182)
@@ -0,0 +1,8 @@
+function shouldBe(actual, expected) {
+ if (actual !== expected)
+ throw new Error('bad value: ' + actual);
+}
+
+var s = 'xxxxxxxxxxxxxxAxxxxxxxxxxxxxxxxxxxxA–';
+var result = s.replace(/A/g, 'b');
+shouldBe(result, 'xxxxxxxxxxxxxxbxxxxxxxxxxxxxxxxxxxxb–');
Modified: trunk/Source/WTF/ChangeLog (219181 => 219182)
--- trunk/Source/WTF/ChangeLog 2017-07-06 02:30:40 UTC (rev 219181)
+++ trunk/Source/WTF/ChangeLog 2017-07-06 02:31:35 UTC (rev 219182)
@@ -1,5 +1,31 @@
2017-07-05 Yusuke Suzuki <[email protected]>
+ WTF::StringImpl::copyChars segfaults when built with GCC 7
+ https://bugs.webkit.org/show_bug.cgi?id=173407
+
+ Reviewed by Andreas Kling.
+
+ 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):
+
+2017-07-05 Yusuke Suzuki <[email protected]>
+
Use std::lock_guard instead of std::unique_lock if move semantics and try_lock is not necessary
https://bugs.webkit.org/show_bug.cgi?id=174148
Modified: trunk/Source/WTF/wtf/text/StringImpl.h (219181 => 219182)
--- trunk/Source/WTF/wtf/text/StringImpl.h 2017-07-06 02:30:40 UTC (rev 219181)
+++ trunk/Source/WTF/wtf/text/StringImpl.h 2017-07-06 02:31:35 UTC (rev 219182)
@@ -627,25 +627,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<const uint32_t*>(source);
- uint32_t* destCharacters = reinterpret_cast<uint32_t*>(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)
@@ -859,9 +841,6 @@
return *tailPointer<StringImpl*>();
}
- // 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<CaseConvertType type, typename CharacterType> static Ref<StringImpl> convertASCIICase(StringImpl&, const CharacterType*, unsigned);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
