Title: [279997] trunk
Revision
279997
Author
ysuz...@apple.com
Date
2021-07-16 14:01:16 -0700 (Fri, 16 Jul 2021)

Log Message

[JSC] RegExp::dumpToStream must not ref Strings since it is called concurrently
https://bugs.webkit.org/show_bug.cgi?id=228031
rdar://80686425

Reviewed by Mark Lam.

JSTests:

* stress/regexp-dump-concurrently.js: Added.
(let.code):

Source/_javascript_Core:

RegExp::dumpToStream's escapedPattern can return m_pattern. In that case, it is refed in the concurrent thread.
This is wrong since StringImpl must not be ref-ed concurrently. This patch just revert this function to the old behavior.

* runtime/RegExp.cpp:
(JSC::RegExp::dumpToStream):

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (279996 => 279997)


--- trunk/JSTests/ChangeLog	2021-07-16 20:34:06 UTC (rev 279996)
+++ trunk/JSTests/ChangeLog	2021-07-16 21:01:16 UTC (rev 279997)
@@ -1,3 +1,14 @@
+2021-07-16  Yusuke Suzuki  <ysuz...@apple.com>
+
+        [JSC] RegExp::dumpToStream must not ref Strings since it is called concurrently
+        https://bugs.webkit.org/show_bug.cgi?id=228031
+        rdar://80686425
+
+        Reviewed by Mark Lam.
+
+        * stress/regexp-dump-concurrently.js: Added.
+        (let.code):
+
 2021-07-15  Yusuke Suzuki  <ysuz...@apple.com>
 
         [JSC] SamplingProfiler should recognize RegExp execution

Added: trunk/JSTests/stress/regexp-dump-concurrently.js (0 => 279997)


--- trunk/JSTests/stress/regexp-dump-concurrently.js	                        (rev 0)
+++ trunk/JSTests/stress/regexp-dump-concurrently.js	2021-07-16 21:01:16 UTC (rev 279997)
@@ -0,0 +1,14 @@
+//@ runDefault("--jitPolicyScale=0", "--validateAbstractInterpreterState=1")
+let code = `
+function foo() {
+  let o = /a/;
+  o.lastIndex = undefined;
+  o.toString();
+}
+for (let i = 0; i < 400; ++i) {
+  foo();
+}
+`
+for (let i=0; i<100; i++) {
+  runString(code);
+}

Modified: trunk/Source/_javascript_Core/ChangeLog (279996 => 279997)


--- trunk/Source/_javascript_Core/ChangeLog	2021-07-16 20:34:06 UTC (rev 279996)
+++ trunk/Source/_javascript_Core/ChangeLog	2021-07-16 21:01:16 UTC (rev 279997)
@@ -1,3 +1,17 @@
+2021-07-16  Yusuke Suzuki  <ysuz...@apple.com>
+
+        [JSC] RegExp::dumpToStream must not ref Strings since it is called concurrently
+        https://bugs.webkit.org/show_bug.cgi?id=228031
+        rdar://80686425
+
+        Reviewed by Mark Lam.
+
+        RegExp::dumpToStream's escapedPattern can return m_pattern. In that case, it is refed in the concurrent thread.
+        This is wrong since StringImpl must not be ref-ed concurrently. This patch just revert this function to the old behavior.
+
+        * runtime/RegExp.cpp:
+        (JSC::RegExp::dumpToStream):
+
 2021-07-15  Keith Miller  <keith_mil...@apple.com>
 
         Alias JSC graph dumping options

Modified: trunk/Source/_javascript_Core/runtime/RegExp.cpp (279996 => 279997)


--- trunk/Source/_javascript_Core/runtime/RegExp.cpp	2021-07-16 20:34:06 UTC (rev 279996)
+++ trunk/Source/_javascript_Core/runtime/RegExp.cpp	2021-07-16 21:01:16 UTC (rev 279997)
@@ -471,7 +471,9 @@
 
 void RegExp::dumpToStream(const JSCell* cell, PrintStream& out)
 {
-    out.print(jsCast<const RegExp*>(cell)->toSourceString());
+    // This function can be called concurrently. So we must not ref m_pattern.
+    auto* regExp = jsCast<const RegExp*>(cell);
+    out.print(toCString("/", regExp->pattern().impl(), "/", Yarr::flagsString(regExp->flags()).data()));
 }
 
 template <typename CharacterType>
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to