- 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();