Skip to content

Commit

Permalink
[JSC] SymbolTableRareData should be set after store-store-fence
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=248114
rdar://102536737

Reviewed by Mark Lam.

This patch inserts storeStoreFence when setting SymbolTableRareData since
this data structure is read by concurrent GC thread.

* Source/JavaScriptCore/runtime/SymbolTable.cpp:
(JSC::SymbolTable::visitChildrenImpl):
(JSC::SymbolTable::cloneScopePart):
(JSC::SymbolTable::prepareForTypeProfiling):
(JSC::SymbolTable::setRareDataCodeBlock):
(JSC::SymbolTable::ensureRareDataSlow):
* Source/JavaScriptCore/runtime/SymbolTable.h:

Canonical link: https://commits.webkit.org/256868@main
  • Loading branch information
Constellation committed Nov 19, 2022
1 parent 33e16a6 commit 1d5e1b4
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 27 deletions.
28 changes: 17 additions & 11 deletions Source/JavaScriptCore/runtime/SymbolTable.cpp
Expand Up @@ -103,8 +103,8 @@ void SymbolTable::visitChildrenImpl(JSCell* thisCell, Visitor& visitor)

visitor.append(thisSymbolTable->m_arguments);

if (thisSymbolTable->m_rareData)
visitor.append(thisSymbolTable->m_rareData->m_codeBlock);
if (auto* rareData = thisSymbolTable->m_rareData.get())
visitor.append(rareData->m_codeBlock);

// Save some memory. This is O(n) to rebuild and we do so on the fly.
ConcurrentJSLocker locker(thisSymbolTable->m_lock);
Expand Down Expand Up @@ -164,7 +164,7 @@ SymbolTable* SymbolTable::cloneScopePart(VM& vm)
result->m_arguments.set(vm, result, arguments);

if (m_rareData) {
result->m_rareData = makeUnique<SymbolTableRareData>();
result->ensureRareData();

{
auto iter = m_rareData->m_uniqueIDMap.begin();
Expand Down Expand Up @@ -201,11 +201,11 @@ void SymbolTable::prepareForTypeProfiling(const ConcurrentJSLocker&)
if (m_rareData)
return;

m_rareData = makeUnique<SymbolTableRareData>();
auto& rareData = ensureRareData();

for (auto iter = m_map.begin(), end = m_map.end(); iter != end; ++iter) {
m_rareData->m_uniqueIDMap.set(iter->key, TypeProfilerNeedsUniqueIDGeneration);
m_rareData->m_offsetToVariableMap.set(iter->value.varOffset(), iter->key);
rareData.m_uniqueIDMap.set(iter->key, TypeProfilerNeedsUniqueIDGeneration);
rareData.m_offsetToVariableMap.set(iter->value.varOffset(), iter->key);
}
}

Expand All @@ -219,11 +219,9 @@ CodeBlock* SymbolTable::rareDataCodeBlock()

void SymbolTable::setRareDataCodeBlock(CodeBlock* codeBlock)
{
if (!m_rareData)
m_rareData = makeUnique<SymbolTableRareData>();

ASSERT(!m_rareData->m_codeBlock);
m_rareData->m_codeBlock.set(codeBlock->vm(), this, codeBlock);
auto& rareData = ensureRareData();
ASSERT(!rareData.m_codeBlock);
rareData.m_codeBlock.set(codeBlock->vm(), this, codeBlock);
}

GlobalVariableID SymbolTable::uniqueIDForVariable(const ConcurrentJSLocker&, UniquedStringImpl* key, VM& vm)
Expand Down Expand Up @@ -285,6 +283,14 @@ RefPtr<TypeSet> SymbolTable::globalTypeSetForVariable(const ConcurrentJSLocker&
return iter->value;
}

SymbolTable::SymbolTableRareData& SymbolTable::ensureRareDataSlow()
{
auto rareData = makeUnique<SymbolTableRareData>();
WTF::storeStoreFence();
m_rareData = WTFMove(rareData);
return *m_rareData;
}

void SymbolTable::dump(PrintStream& out) const
{
ConcurrentJSLocker locker(m_lock);
Expand Down
44 changes: 28 additions & 16 deletions Source/JavaScriptCore/runtime/SymbolTable.h
Expand Up @@ -597,22 +597,27 @@ class SymbolTable final : public JSCell {
add(locker, key, std::forward<Entry>(entry));
}

bool hasPrivateNames() const { return m_rareData && m_rareData->m_privateNames.size(); }
bool hasPrivateNames() const
{
if (auto* rareData = m_rareData.get())
return rareData->m_privateNames.size();
return false;
}

ALWAYS_INLINE PrivateNameIteratorRange privateNames()
{
// Use of the IteratorRange must be guarded to prevent ASSERT failures in checkValidity().
ASSERT(hasPrivateNames());
return makeIteratorRange(m_rareData->m_privateNames.begin(), m_rareData->m_privateNames.end());
auto& rareData = ensureRareData();
return makeIteratorRange(rareData.m_privateNames.begin(), rareData.m_privateNames.end());
}

void addPrivateName(const RefPtr<UniquedStringImpl>& key, PrivateNameEntry value)
{
ASSERT(key && !key->isSymbol());
if (!m_rareData)
m_rareData = WTF::makeUnique<SymbolTableRareData>();

ASSERT(m_rareData->m_privateNames.find(key) == m_rareData->m_privateNames.end());
m_rareData->m_privateNames.add(key, value);
auto& rareData = ensureRareData();
ASSERT(rareData.m_privateNames.find(key) == rareData.m_privateNames.end());
rareData.m_privateNames.add(key, value);
}

template<typename Entry>
Expand Down Expand Up @@ -740,26 +745,33 @@ class SymbolTable final : public JSCell {
void finalizeUnconditionally(VM&);
void dump(PrintStream&) const;

struct SymbolTableRareData {
WTF_MAKE_STRUCT_FAST_ALLOCATED;
UniqueIDMap m_uniqueIDMap;
OffsetToVariableMap m_offsetToVariableMap;
UniqueTypeSetMap m_uniqueTypeSetMap;
WriteBarrier<CodeBlock> m_codeBlock;
PrivateNameEnvironment m_privateNames;
};

private:
JS_EXPORT_PRIVATE SymbolTable(VM&);
~SymbolTable();
SymbolTableRareData& ensureRareData()
{
if (LIKELY(m_rareData))
return *m_rareData;
return ensureRareDataSlow();
}

JS_EXPORT_PRIVATE void finishCreation(VM&);
JS_EXPORT_PRIVATE SymbolTableRareData& ensureRareDataSlow();

Map m_map;
ScopeOffset m_maxScopeOffset;
public:
mutable ConcurrentJSLock m_lock;

struct SymbolTableRareData {
WTF_MAKE_STRUCT_FAST_ALLOCATED;
UniqueIDMap m_uniqueIDMap;
OffsetToVariableMap m_offsetToVariableMap;
UniqueTypeSetMap m_uniqueTypeSetMap;
WriteBarrier<CodeBlock> m_codeBlock;
PrivateNameEnvironment m_privateNames;
};

private:
unsigned m_usesNonStrictEval : 1;
unsigned m_nestedLexicalScope : 1; // Non-function LexicalScope.
Expand Down

0 comments on commit 1d5e1b4

Please sign in to comment.