[webkit-changes] [249706] trunk/Source/JavaScriptCore

Mon, 09 Sep 2019 22:04:08 -0700

Title: [249706] trunk/Source/_javascript_Core
Revision
249706
Author
ysuz...@apple.com
Date
2019-09-09 22:03:13 -0700 (Mon, 09 Sep 2019)

Log Message

[JSC] CodeBlock::m_constantRegisters should be guarded by ConcurrentJSLock when Vector reallocate memory
https://bugs.webkit.org/show_bug.cgi?id=201622

Reviewed by Mark Lam.

CodeBlock::visitChildren takes ConcurrentJSLock while iterating m_constantRegisters, some of the places reallocate
this Vector without taking a lock. If a Vector memory is reallocated while iterating it in concurrent collector,
the concurrent collector can see a garbage. This patch guards m_constantRegisters reallocation with ConcurrentJSLock.

* bytecode/CodeBlock.cpp:
(JSC::CodeBlock::finishCreation):
(JSC::CodeBlock::setConstantRegisters):
* bytecode/CodeBlock.h:
(JSC::CodeBlock::addConstant):
(JSC::CodeBlock::addConstantLazily):
* dfg/DFGDesiredWatchpoints.cpp:
(JSC::DFG::ArrayBufferViewWatchpointAdaptor::add):
(JSC::DFG::SymbolTableAdaptor::add):
(JSC::DFG::FunctionExecutableAdaptor::add):
* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::registerFrozenValues):
* dfg/DFGJITFinalizer.cpp:
(JSC::DFG::JITFinalizer::finalizeCommon):
* dfg/DFGLazyJSValue.cpp:
(JSC::DFG::LazyJSValue::emit const):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (249705 => 249706)


--- trunk/Source/_javascript_Core/ChangeLog	2019-09-10 04:51:02 UTC (rev 249705)
+++ trunk/Source/_javascript_Core/ChangeLog	2019-09-10 05:03:13 UTC (rev 249706)
@@ -1,3 +1,31 @@
+2019-09-09  Yusuke Suzuki  <ysuz...@apple.com>
+
+        [JSC] CodeBlock::m_constantRegisters should be guarded by ConcurrentJSLock when Vector reallocate memory
+        https://bugs.webkit.org/show_bug.cgi?id=201622
+
+        Reviewed by Mark Lam.
+
+        CodeBlock::visitChildren takes ConcurrentJSLock while iterating m_constantRegisters, some of the places reallocate
+        this Vector without taking a lock. If a Vector memory is reallocated while iterating it in concurrent collector,
+        the concurrent collector can see a garbage. This patch guards m_constantRegisters reallocation with ConcurrentJSLock.
+
+        * bytecode/CodeBlock.cpp:
+        (JSC::CodeBlock::finishCreation):
+        (JSC::CodeBlock::setConstantRegisters):
+        * bytecode/CodeBlock.h:
+        (JSC::CodeBlock::addConstant):
+        (JSC::CodeBlock::addConstantLazily):
+        * dfg/DFGDesiredWatchpoints.cpp:
+        (JSC::DFG::ArrayBufferViewWatchpointAdaptor::add):
+        (JSC::DFG::SymbolTableAdaptor::add):
+        (JSC::DFG::FunctionExecutableAdaptor::add):
+        * dfg/DFGGraph.cpp:
+        (JSC::DFG::Graph::registerFrozenValues):
+        * dfg/DFGJITFinalizer.cpp:
+        (JSC::DFG::JITFinalizer::finalizeCommon):
+        * dfg/DFGLazyJSValue.cpp:
+        (JSC::DFG::LazyJSValue::emit const):
+
 2019-09-09  Robin Morisset  <rmoris...@apple.com>
 
         [Air] highOrderAdjacents in AbstractColoringAllocator::conservativeHeuristic should be some kind of array

Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp (249705 => 249706)


--- trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp	2019-09-10 04:51:02 UTC (rev 249705)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp	2019-09-10 05:03:13 UTC (rev 249706)
@@ -596,7 +596,7 @@
                 if (op.type == ModuleVar) {
                     // Keep the linked module environment strongly referenced.
                     if (stronglyReferencedModuleEnvironments.add(jsCast<JSModuleEnvironment*>(op.lexicalEnvironment)).isNewEntry)
-                        addConstant(op.lexicalEnvironment);
+                        addConstant(ConcurrentJSLocker(m_lock), op.lexicalEnvironment);
                     metadata.m_lexicalEnvironment.set(vm, this, op.lexicalEnvironment);
                 } else
                     metadata.m_symbolTable.set(vm, this, op.lexicalEnvironment->symbolTable());
@@ -899,7 +899,10 @@
 
     ASSERT(constants.size() == constantsSourceCodeRepresentation.size());
     size_t count = constants.size();
-    m_constantRegisters.resizeToFit(count);
+    {
+        ConcurrentJSLocker locker(m_lock);
+        m_constantRegisters.resizeToFit(count);
+    }
     for (size_t i = 0; i < count; i++) {
         JSValue constant = constants[i].get();

Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.h (249705 => 249706)


--- trunk/Source/_javascript_Core/bytecode/CodeBlock.h	2019-09-10 04:51:02 UTC (rev 249705)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.h	2019-09-10 05:03:13 UTC (rev 249706)
@@ -546,7 +546,7 @@
 
     Vector<WriteBarrier<Unknown>>& constants() { return m_constantRegisters; }
     Vector<SourceCodeRepresentation>& constantsSourceCodeRepresentation() { return m_constantsSourceCodeRepresentation; }
-    unsigned addConstant(JSValue v)
+    unsigned addConstant(const ConcurrentJSLocker&, JSValue v)
     {
         unsigned result = m_constantRegisters.size();
         m_constantRegisters.append(WriteBarrier<Unknown>());
@@ -555,7 +555,7 @@
         return result;
     }
 
-    unsigned addConstantLazily()
+    unsigned addConstantLazily(const ConcurrentJSLocker&)
     {
         unsigned result = m_constantRegisters.size();
         m_constantRegisters.append(WriteBarrier<Unknown>());

Modified: trunk/Source/_javascript_Core/dfg/DFGDesiredWatchpoints.cpp (249705 => 249706)


--- trunk/Source/_javascript_Core/dfg/DFGDesiredWatchpoints.cpp	2019-09-10 04:51:02 UTC (rev 249705)
+++ trunk/Source/_javascript_Core/dfg/DFGDesiredWatchpoints.cpp	2019-09-10 05:03:13 UTC (rev 249706)
@@ -43,7 +43,7 @@
     ArrayBufferNeuteringWatchpointSet* neuteringWatchpoint =
         ArrayBufferNeuteringWatchpointSet::create(vm);
     neuteringWatchpoint->set().add(watchpoint);
-    codeBlock->addConstant(neuteringWatchpoint);
+    codeBlock->addConstant(ConcurrentJSLocker(codeBlock->m_lock), neuteringWatchpoint);
     // FIXME: We don't need to set this watchpoint at all for shared buffers.
     // https://bugs.webkit.org/show_bug.cgi?id=164108
     vm.heap.addReference(neuteringWatchpoint, view->possiblySharedBuffer());
@@ -52,7 +52,7 @@
 void SymbolTableAdaptor::add(
     CodeBlock* codeBlock, SymbolTable* symbolTable, CommonData& common)
 {
-    codeBlock->addConstant(symbolTable); // For common users, it doesn't really matter if it's weak or not. If references to it go away, we go away, too.
+    codeBlock->addConstant(ConcurrentJSLocker(codeBlock->m_lock), symbolTable); // For common users, it doesn't really matter if it's weak or not. If references to it go away, we go away, too.
     symbolTable->singleton().add(common.watchpoints.add(codeBlock));
 }
 
@@ -59,7 +59,7 @@
 void FunctionExecutableAdaptor::add(
     CodeBlock* codeBlock, FunctionExecutable* executable, CommonData& common)
 {
-    codeBlock->addConstant(executable); // For common users, it doesn't really matter if it's weak or not. If references to it go away, we go away, too.
+    codeBlock->addConstant(ConcurrentJSLocker(codeBlock->m_lock), executable); // For common users, it doesn't really matter if it's weak or not. If references to it go away, we go away, too.
     executable->singleton().add(common.watchpoints.add(codeBlock));
 }

Modified: trunk/Source/_javascript_Core/dfg/DFGGraph.cpp (249705 => 249706)


--- trunk/Source/_javascript_Core/dfg/DFGGraph.cpp	2019-09-10 04:51:02 UTC (rev 249705)
+++ trunk/Source/_javascript_Core/dfg/DFGGraph.cpp	2019-09-10 05:03:13 UTC (rev 249706)
@@ -1405,6 +1405,7 @@
 
 void Graph::registerFrozenValues()
 {
+    ConcurrentJSLocker locker(m_codeBlock->m_lock);
     m_codeBlock->constants().shrink(0);
     m_codeBlock->constantsSourceCodeRepresentation().resize(0);
     for (FrozenValue* value : m_frozenValues) {
@@ -1420,7 +1421,7 @@
             break;
         }
         case StrongValue: {
-            unsigned constantIndex = m_codeBlock->addConstantLazily();
+            unsigned constantIndex = m_codeBlock->addConstantLazily(locker);
             // We already have a barrier on the code block.
             m_codeBlock->constants()[constantIndex].setWithoutWriteBarrier(value->value());
             break;

Modified: trunk/Source/_javascript_Core/dfg/DFGJITFinalizer.cpp (249705 => 249706)


--- trunk/Source/_javascript_Core/dfg/DFGJITFinalizer.cpp	2019-09-10 04:51:02 UTC (rev 249705)
+++ trunk/Source/_javascript_Core/dfg/DFGJITFinalizer.cpp	2019-09-10 05:03:13 UTC (rev 249706)
@@ -82,8 +82,11 @@
 void JITFinalizer::finalizeCommon()
 {
     // Some JIT finalizers may have added more constants. Shrink-to-fit those things now.
-    m_plan.codeBlock()->constants().shrinkToFit();
-    m_plan.codeBlock()->constantsSourceCodeRepresentation().shrinkToFit();
+    {
+        ConcurrentJSLocker locker(m_plan.codeBlock()->m_lock);
+        m_plan.codeBlock()->constants().shrinkToFit();
+        m_plan.codeBlock()->constantsSourceCodeRepresentation().shrinkToFit();
+    }
 
 #if ENABLE(FTL_JIT)
     m_jitCode->optimizeAfterWarmUp(m_plan.codeBlock());

Modified: trunk/Source/_javascript_Core/dfg/DFGLazyJSValue.cpp (249705 => 249706)


--- trunk/Source/_javascript_Core/dfg/DFGLazyJSValue.cpp	2019-09-10 04:51:02 UTC (rev 249705)
+++ trunk/Source/_javascript_Core/dfg/DFGLazyJSValue.cpp	2019-09-10 05:03:13 UTC (rev 249706)
@@ -254,7 +254,7 @@
             JSValue realValue = thisValue.getValue(codeBlock->vm());
             RELEASE_ASSERT(realValue.isCell());
 
-            codeBlock->addConstant(realValue);
+            codeBlock->addConstant(ConcurrentJSLocker(codeBlock->m_lock), realValue);
             
             if (thisValue.m_kind == NewStringImpl)
                 thisValue.u.stringImpl->deref();

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to