Changeset 259602 in webkit

Timestamp:

Apr 6, 2020, 4:05:18 PM (6 years ago)

Author:

Alan Coon

Message:

Cherry-pick r259424. rdar://problem/61352472

[JSC] RecordedStatuses's assignment should be guarded by CodeBlock's lock
​https://bugs.webkit.org/show_bug.cgi?id=209935
<rdar://problem/59443383>

Reviewed by Mark Lam.

Previously RecordedStatuses are not touched by GC. But now, GC visits RecordedStatuses.
This means that modifying RecordedStatuses should be guarded by CodeBlock's lock if
it is reachable from CodeBlock.
In DFG::Plan::reallyAdd, we already installed DFG::JITCode into the CodeBlock so that
RecordedStatuses is reachable from CodeBlock. We should lock CodeBlock's lock while
performing WTFMove(RecordedStatuses).

We do not need to emit write-barrier here because (1) DFG::Plan::reallyAdd is executed
while GC is deferred and (2) we emit write-barrier to CodeBlock before deferred GC is executed.

• dfg/DFGPlan.cpp: (JSC::DFG::Plan::reallyAdd):

git-svn-id: ​https://svn.webkit.org/repository/webkit/trunk@259424 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Location:

branches/safari-609-branch/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• dfg/DFGPlan.cpp (modified) (2 diffs)

branches/safari-609-branch/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2020-04-06  Alan Coon  <alancoon@apple.com>
+
+        Cherry-pick r259424. rdar://problem/61352472
+
+    [JSC] RecordedStatuses's assignment should be guarded by CodeBlock's lock
+    https://bugs.webkit.org/show_bug.cgi?id=209935
+    <rdar://problem/59443383>
+    
+    Reviewed by Mark Lam.
+    
+    Previously RecordedStatuses are not touched by GC. But now, GC visits RecordedStatuses.
+    This means that modifying RecordedStatuses should be guarded by CodeBlock's lock if
+    it is reachable from CodeBlock.
+    In DFG::Plan::reallyAdd, we already installed DFG::JITCode into the CodeBlock so that
+    RecordedStatuses is reachable from CodeBlock. We should lock CodeBlock's lock while
+    performing `WTFMove(RecordedStatuses)`.
+    
+    We do not need to emit write-barrier here because (1) DFG::Plan::reallyAdd is executed
+    while GC is deferred and (2) we emit write-barrier to CodeBlock before deferred GC is executed.
+    
+    * dfg/DFGPlan.cpp:
+    (JSC::DFG::Plan::reallyAdd):
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@259424 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2020-04-02  Yusuke Suzuki  <ysuzuki@apple.com>
+
+            [JSC] RecordedStatuses's assignment should be guarded by CodeBlock's lock
+            https://bugs.webkit.org/show_bug.cgi?id=209935
+            <rdar://problem/59443383>
+
+            Reviewed by Mark Lam.
+
+            Previously RecordedStatuses are not touched by GC. But now, GC visits RecordedStatuses.
+            This means that modifying RecordedStatuses should be guarded by CodeBlock's lock if
+            it is reachable from CodeBlock.
+            In DFG::Plan::reallyAdd, we already installed DFG::JITCode into the CodeBlock so that
+            RecordedStatuses is reachable from CodeBlock. We should lock CodeBlock's lock while
+            performing `WTFMove(RecordedStatuses)`.
+
+            We do not need to emit write-barrier here because (1) DFG::Plan::reallyAdd is executed
+            while GC is deferred and (2) we emit write-barrier to CodeBlock before deferred GC is executed.
+
+            * dfg/DFGPlan.cpp:
+            (JSC::DFG::Plan::reallyAdd):
+
 2020-04-06  Alan Coon  <alancoon@apple.com>
 

branches/safari-609-branch/Source/JavaScriptCore/dfg/DFGPlan.cpp

@@ -559 +559 @@
 void Plan::reallyAdd(CommonData* commonData)
 {
+    ASSERT(m_vm->heap.isDeferred());
     m_watchpoints.reallyAdd(m_codeBlock, *commonData);
     m_identifiers.reallyAdd(*m_vm, commonData);
@@ -564 +565 @@
     m_transitions.reallyAdd(*m_vm, commonData);
     m_globalProperties.reallyAdd(m_codeBlock, m_identifiers, *commonData);
-    commonData->recordedStatuses = WTFMove(m_recordedStatuses);
+    {
+        ConcurrentJSLocker locker(m_codeBlock->m_lock);
+        commonData->recordedStatuses = WTFMove(m_recordedStatuses);
+    }
 }