Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (227905 => 227906)
--- trunk/Source/_javascript_Core/ChangeLog 2018-01-31 18:32:05 UTC (rev 227905)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-01-31 18:57:13 UTC (rev 227906)
@@ -1,3 +1,37 @@
+2018-01-31 Keith Miller <[email protected]>
+
+ Canonicalize aquiring the JSCell lock.
+ https://bugs.webkit.org/show_bug.cgi?id=182320
+
+ Reviewed by Michael Saboff.
+
+ It's currently kinda annoying to figure out where
+ we aquire the a JSCell's lock. This patch adds a
+ helper to make it easier to grep...
+
+ * bytecode/UnlinkedCodeBlock.cpp:
+ (JSC::UnlinkedCodeBlock::visitChildren):
+ (JSC::UnlinkedCodeBlock::setInstructions):
+ (JSC::UnlinkedCodeBlock::shrinkToFit):
+ * runtime/ErrorInstance.cpp:
+ (JSC::ErrorInstance::finishCreation):
+ (JSC::ErrorInstance::materializeErrorInfoIfNeeded):
+ (JSC::ErrorInstance::visitChildren):
+ * runtime/JSArray.cpp:
+ (JSC::JSArray::shiftCountWithArrayStorage):
+ (JSC::JSArray::unshiftCountWithArrayStorage):
+ * runtime/JSCell.h:
+ (JSC::JSCell::cellLock):
+ * runtime/JSObject.cpp:
+ (JSC::JSObject::visitButterflyImpl):
+ (JSC::JSObject::convertContiguousToArrayStorage):
+ * runtime/JSPropertyNameEnumerator.cpp:
+ (JSC::JSPropertyNameEnumerator::visitChildren):
+ * runtime/SparseArrayValueMap.cpp:
+ (JSC::SparseArrayValueMap::add):
+ (JSC::SparseArrayValueMap::remove):
+ (JSC::SparseArrayValueMap::visitChildren):
+
2018-01-31 Saam Barati <[email protected]>
JSC incorrectly interpreting script, sets Global Property instead of Global Lexical variable (LiteralParser / JSONP path)
Modified: trunk/Source/_javascript_Core/bytecode/UnlinkedCodeBlock.cpp (227905 => 227906)
--- trunk/Source/_javascript_Core/bytecode/UnlinkedCodeBlock.cpp 2018-01-31 18:32:05 UTC (rev 227905)
+++ trunk/Source/_javascript_Core/bytecode/UnlinkedCodeBlock.cpp 2018-01-31 18:57:13 UTC (rev 227906)
@@ -94,7 +94,7 @@
UnlinkedCodeBlock* thisObject = jsCast<UnlinkedCodeBlock*>(cell);
ASSERT_GC_OBJECT_INHERITS(thisObject, info());
Base::visitChildren(thisObject, visitor);
- auto locker = holdLock(*thisObject);
+ auto locker = holdLock(thisObject->cellLock());
for (FunctionExpressionVector::iterator ptr = thisObject->m_functionDecls.begin(), end = thisObject->m_functionDecls.end(); ptr != end; ++ptr)
visitor.append(*ptr);
for (FunctionExpressionVector::iterator ptr = thisObject->m_functionExprs.begin(), end = thisObject->m_functionExprs.end(); ptr != end; ++ptr)
@@ -317,7 +317,7 @@
{
ASSERT(instructions);
{
- auto locker = holdLock(*this);
+ auto locker = holdLock(cellLock());
m_unlinkedInstructions = WTFMove(instructions);
}
Heap::heap(this)->reportExtraMemoryAllocated(m_unlinkedInstructions->sizeInBytes());
@@ -392,7 +392,7 @@
void UnlinkedCodeBlock::shrinkToFit()
{
- auto locker = holdLock(*this);
+ auto locker = holdLock(cellLock());
m_jumpTargets.shrinkToFit();
m_identifiers.shrinkToFit();
Modified: trunk/Source/_javascript_Core/bytecode/UnlinkedCodeBlock.h (227905 => 227906)
--- trunk/Source/_javascript_Core/bytecode/UnlinkedCodeBlock.h 2018-01-31 18:32:05 UTC (rev 227905)
+++ trunk/Source/_javascript_Core/bytecode/UnlinkedCodeBlock.h 2018-01-31 18:57:13 UTC (rev 227906)
@@ -160,7 +160,7 @@
{
createRareDataIfNecessary();
VM& vm = *this->vm();
- auto locker = lockDuringMarking(vm.heap, *this);
+ auto locker = lockDuringMarking(vm.heap, cellLock());
unsigned size = m_rareData->m_regexps.size();
m_rareData->m_regexps.append(WriteBarrier<RegExp>(vm, this, r));
return size;
@@ -191,7 +191,7 @@
void addSetConstant(IdentifierSet& set)
{
VM& vm = *this->vm();
- auto locker = lockDuringMarking(vm.heap, *this);
+ auto locker = lockDuringMarking(vm.heap, cellLock());
unsigned result = m_constantRegisters.size();
m_constantRegisters.append(WriteBarrier<Unknown>());
m_constantsSourceCodeRepresentation.append(SourceCodeRepresentation::Other);
@@ -201,7 +201,7 @@
unsigned addConstant(JSValue v, SourceCodeRepresentation sourceCodeRepresentation = SourceCodeRepresentation::Other)
{
VM& vm = *this->vm();
- auto locker = lockDuringMarking(vm.heap, *this);
+ auto locker = lockDuringMarking(vm.heap, cellLock());
unsigned result = m_constantRegisters.size();
m_constantRegisters.append(WriteBarrier<Unknown>());
m_constantRegisters.last().set(vm, this, v);
@@ -211,7 +211,7 @@
unsigned addConstant(LinkTimeConstant type)
{
VM& vm = *this->vm();
- auto locker = lockDuringMarking(vm.heap, *this);
+ auto locker = lockDuringMarking(vm.heap, cellLock());
unsigned result = m_constantRegisters.size();
ASSERT(result);
unsigned index = static_cast<unsigned>(type);
@@ -274,7 +274,7 @@
unsigned addFunctionDecl(UnlinkedFunctionExecutable* n)
{
VM& vm = *this->vm();
- auto locker = lockDuringMarking(vm.heap, *this);
+ auto locker = lockDuringMarking(vm.heap, cellLock());
unsigned size = m_functionDecls.size();
m_functionDecls.append(WriteBarrier<UnlinkedFunctionExecutable>());
m_functionDecls.last().set(vm, this, n);
@@ -285,7 +285,7 @@
unsigned addFunctionExpr(UnlinkedFunctionExecutable* n)
{
VM& vm = *this->vm();
- auto locker = lockDuringMarking(vm.heap, *this);
+ auto locker = lockDuringMarking(vm.heap, cellLock());
unsigned size = m_functionExprs.size();
m_functionExprs.append(WriteBarrier<UnlinkedFunctionExecutable>());
m_functionExprs.last().set(vm, this, n);
@@ -415,7 +415,7 @@
void createRareDataIfNecessary()
{
if (!m_rareData) {
- auto locker = lockDuringMarking(*heap(), *this);
+ auto locker = lockDuringMarking(*heap(), cellLock());
m_rareData = std::make_unique<RareData>();
}
}
Modified: trunk/Source/_javascript_Core/runtime/ErrorInstance.cpp (227905 => 227906)
--- trunk/Source/_javascript_Core/runtime/ErrorInstance.cpp 2018-01-31 18:32:05 UTC (rev 227905)
+++ trunk/Source/_javascript_Core/runtime/ErrorInstance.cpp 2018-01-31 18:57:13 UTC (rev 227906)
@@ -117,7 +117,7 @@
std::unique_ptr<Vector<StackFrame>> stackTrace = getStackTrace(exec, vm, this, useCurrentFrame);
{
- auto locker = holdLock(*this);
+ auto locker = holdLock(cellLock());
m_stackTrace = WTFMove(stackTrace);
}
vm.heap.writeBarrier(this);
@@ -209,7 +209,7 @@
addErrorInfo(vm, m_stackTrace.get(), this);
{
- auto locker = holdLock(*this);
+ auto locker = holdLock(cellLock());
m_stackTrace = nullptr;
}
@@ -234,7 +234,7 @@
Base::visitChildren(thisObject, visitor);
{
- auto locker = holdLock(*thisObject);
+ auto locker = holdLock(thisObject->cellLock());
if (thisObject->m_stackTrace) {
for (StackFrame& frame : *thisObject->m_stackTrace)
frame.visitChildren(visitor);
Modified: trunk/Source/_javascript_Core/runtime/JSArray.cpp (227905 => 227906)
--- trunk/Source/_javascript_Core/runtime/JSArray.cpp 2018-01-31 18:32:05 UTC (rev 227905)
+++ trunk/Source/_javascript_Core/runtime/JSArray.cpp 2018-01-31 18:57:13 UTC (rev 227906)
@@ -802,7 +802,7 @@
return true;
DisallowGC disallowGC;
- auto locker = holdLock(*this);
+ auto locker = holdLock(cellLock());
if (startIndex + count > vectorLength)
count = vectorLength - startIndex;
@@ -1005,7 +1005,7 @@
// Need to have GC deferred around the unshiftCountSlowCase(), since that leaves the butterfly in
// a weird state: some parts of it will be left uninitialized, which we will fill in here.
DeferGC deferGC(vm.heap);
- auto locker = holdLock(*this);
+ auto locker = holdLock(cellLock());
if (moveFront && storage->m_indexBias >= count) {
Butterfly* newButterfly = storage->butterfly()->unshift(structure(), count);
Modified: trunk/Source/_javascript_Core/runtime/JSCell.cpp (227905 => 227906)
--- trunk/Source/_javascript_Core/runtime/JSCell.cpp 2018-01-31 18:32:05 UTC (rev 227905)
+++ trunk/Source/_javascript_Core/runtime/JSCell.cpp 2018-01-31 18:57:13 UTC (rev 227906)
@@ -308,13 +308,13 @@
RELEASE_ASSERT_NOT_REACHED();
}
-void JSCell::lockSlow()
+void JSCellLock::lockSlow()
{
Atomic<IndexingType>* lock = bitwise_cast<Atomic<IndexingType>*>(&m_indexingTypeAndMisc);
IndexingTypeLockAlgorithm::lockSlow(*lock);
}
-void JSCell::unlockSlow()
+void JSCellLock::unlockSlow()
{
Atomic<IndexingType>* lock = bitwise_cast<Atomic<IndexingType>*>(&m_indexingTypeAndMisc);
IndexingTypeLockAlgorithm::unlockSlow(*lock);
Modified: trunk/Source/_javascript_Core/runtime/JSCell.h (227905 => 227906)
--- trunk/Source/_javascript_Core/runtime/JSCell.h 2018-01-31 18:32:05 UTC (rev 227905)
+++ trunk/Source/_javascript_Core/runtime/JSCell.h 2018-01-31 18:57:13 UTC (rev 227906)
@@ -50,6 +50,7 @@
class PropertyName;
class PropertyNameArray;
class Structure;
+class JSCellLock;
enum class GCDeferralContextArgPresense {
HasArg,
@@ -120,10 +121,11 @@
// Each cell has a built-in lock. Currently it's simply available for use if you need it. It's
// a full-blown WTF::Lock. Note that this lock is currently used in JSArray and that lock's
// ordering with the Structure lock is that the Structure lock must be acquired first.
- void lock();
- bool tryLock();
- void unlock();
- bool isLocked() const;
+
+ // We use this abstraction to make it easier to grep for places where we lock cells.
+ // to lock a cell you can just do:
+ // auto locker = holdLock(cell->cellLocker());
+ JSCellLock& cellLock() { return *reinterpret_cast<JSCellLock*>(this); }
JSType type() const;
IndexingType indexingTypeAndMisc() const;
@@ -272,10 +274,9 @@
private:
friend class LLIntOffsetsExtractor;
+ friend class JSCellLock;
JS_EXPORT_PRIVATE JSObject* toObjectSlow(ExecState*, JSGlobalObject*) const;
- JS_EXPORT_PRIVATE void lockSlow();
- JS_EXPORT_PRIVATE void unlockSlow();
StructureID m_structureID;
IndexingType m_indexingTypeAndMisc; // DO NOT store to this field. Always CAS.
@@ -284,6 +285,17 @@
CellState m_cellState;
};
+class JSCellLock : public JSCell {
+public:
+ void lock();
+ bool tryLock();
+ void unlock();
+ bool isLocked() const;
+private:
+ JS_EXPORT_PRIVATE void lockSlow();
+ JS_EXPORT_PRIVATE void unlockSlow();
+};
+
template<typename To, typename From>
inline To jsCast(From* from)
{
Modified: trunk/Source/_javascript_Core/runtime/JSCellInlines.h (227905 => 227906)
--- trunk/Source/_javascript_Core/runtime/JSCellInlines.h 2018-01-31 18:32:05 UTC (rev 227905)
+++ trunk/Source/_javascript_Core/runtime/JSCellInlines.h 2018-01-31 18:57:13 UTC (rev 227906)
@@ -323,7 +323,7 @@
zap();
}
-inline void JSCell::lock()
+inline void JSCellLock::lock()
{
Atomic<IndexingType>* lock = bitwise_cast<Atomic<IndexingType>*>(&m_indexingTypeAndMisc);
if (UNLIKELY(!IndexingTypeLockAlgorithm::lockFast(*lock)))
@@ -330,13 +330,13 @@
lockSlow();
}
-inline bool JSCell::tryLock()
+inline bool JSCellLock::tryLock()
{
Atomic<IndexingType>* lock = bitwise_cast<Atomic<IndexingType>*>(&m_indexingTypeAndMisc);
return IndexingTypeLockAlgorithm::tryLock(*lock);
}
-inline void JSCell::unlock()
+inline void JSCellLock::unlock()
{
Atomic<IndexingType>* lock = bitwise_cast<Atomic<IndexingType>*>(&m_indexingTypeAndMisc);
if (UNLIKELY(!IndexingTypeLockAlgorithm::unlockFast(*lock)))
@@ -343,7 +343,7 @@
unlockSlow();
}
-inline bool JSCell::isLocked() const
+inline bool JSCellLock::isLocked() const
{
Atomic<IndexingType>* lock = bitwise_cast<Atomic<IndexingType>*>(&m_indexingTypeAndMisc);
return IndexingTypeLockAlgorithm::isLocked(*lock);
Modified: trunk/Source/_javascript_Core/runtime/JSObject.cpp (227905 => 227906)
--- trunk/Source/_javascript_Core/runtime/JSObject.cpp 2018-01-31 18:32:05 UTC (rev 227905)
+++ trunk/Source/_javascript_Core/runtime/JSObject.cpp 2018-01-31 18:57:13 UTC (rev 227906)
@@ -383,7 +383,7 @@
lastOffset = structure->lastOffset();
IndexingType indexingType = structure->indexingType();
Dependency indexingTypeDependency = Dependency::fence(indexingType);
- Locker<JSCell> locker(NoLockingNecessary);
+ Locker<JSCellLock> locker(NoLockingNecessary);
switch (indexingType) {
case ALL_CONTIGUOUS_INDEXING_TYPES:
case ALL_ARRAY_STORAGE_INDEXING_TYPES:
@@ -391,7 +391,7 @@
// that can happen when the butterfly is used for array storage. We conservatively
// assume that a contiguous butterfly may transform into an array storage one, though
// this is probably more conservative than necessary.
- locker = Locker<JSCell>(*this);
+ locker = holdLock(cellLock());
break;
default:
break;
@@ -1358,7 +1358,7 @@
// Fortunately, we have the JSCell lock for this purpose!
if (vm.heap.mutatorShouldBeFenced()) {
- auto locker = holdLock(*this);
+ auto locker = holdLock(cellLock());
m_butterflyIndexingMask = newStorage->butterfly()->computeIndexingMask();
setStructureIDDirectly(nuke(structureID()));
WTF::storeStoreFence();
Modified: trunk/Source/_javascript_Core/runtime/JSPropertyNameEnumerator.cpp (227905 => 227906)
--- trunk/Source/_javascript_Core/runtime/JSPropertyNameEnumerator.cpp 2018-01-31 18:32:05 UTC (rev 227905)
+++ trunk/Source/_javascript_Core/runtime/JSPropertyNameEnumerator.cpp 2018-01-31 18:57:13 UTC (rev 227906)
@@ -71,7 +71,7 @@
m_endGenericPropertyIndex = vector.size();
{
- auto locker = lockDuringMarking(vm.heap, *this);
+ auto locker = lockDuringMarking(vm.heap, cellLock());
m_propertyNames.resizeToFit(vector.size());
}
for (unsigned i = 0; i < vector.size(); ++i) {
@@ -89,7 +89,7 @@
{
Base::visitChildren(cell, visitor);
JSPropertyNameEnumerator* thisObject = jsCast<JSPropertyNameEnumerator*>(cell);
- auto locker = holdLock(*thisObject);
+ auto locker = holdLock(thisObject->cellLock());
for (auto& propertyName : thisObject->m_propertyNames)
visitor.append(propertyName);
visitor.append(thisObject->m_prototypeChain);
Modified: trunk/Source/_javascript_Core/runtime/SparseArrayValueMap.cpp (227905 => 227906)
--- trunk/Source/_javascript_Core/runtime/SparseArrayValueMap.cpp 2018-01-31 18:32:05 UTC (rev 227905)
+++ trunk/Source/_javascript_Core/runtime/SparseArrayValueMap.cpp 2018-01-31 18:57:13 UTC (rev 227906)
@@ -77,7 +77,7 @@
AddResult result;
size_t capacity;
{
- auto locker = holdLock(*this);
+ auto locker = holdLock(cellLock());
SparseArrayEntry entry;
entry.setWithoutWriteBarrier(jsUndefined());
@@ -95,13 +95,13 @@
void SparseArrayValueMap::remove(iterator it)
{
- auto locker = holdLock(*this);
+ auto locker = holdLock(cellLock());
m_map.remove(it);
}
void SparseArrayValueMap::remove(unsigned i)
{
- auto locker = holdLock(*this);
+ auto locker = holdLock(cellLock());
m_map.remove(i);
}
@@ -197,9 +197,9 @@
void SparseArrayValueMap::visitChildren(JSCell* thisObject, SlotVisitor& visitor)
{
Base::visitChildren(thisObject, visitor);
-
+
+ auto locker = holdLock(thisObject->cellLock());
SparseArrayValueMap* thisMap = jsCast<SparseArrayValueMap*>(thisObject);
- auto locker = holdLock(*thisMap);
iterator end = thisMap->m_map.end();
for (iterator it = thisMap->m_map.begin(); it != end; ++it)
visitor.append(it->value);
