Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (218640 => 218641)
--- trunk/Source/_javascript_Core/ChangeLog 2017-06-21 18:37:02 UTC (rev 218640)
+++ trunk/Source/_javascript_Core/ChangeLog 2017-06-21 18:42:44 UTC (rev 218641)
@@ -1,3 +1,34 @@
+2017-06-21 Saam Barati <[email protected]>
+
+ Make it clear that regenerating ICs are holding the CodeBlock's lock by passing the locker as a parameter
+ https://bugs.webkit.org/show_bug.cgi?id=173609
+
+ Reviewed by Keith Miller.
+
+ This patch makes many of the IC generating functions require a locker as
+ a parameter. We do this in other places in JSC to indicate that
+ a particular API is only valid while a particular lock is held.
+ This is the case when generating ICs. This patch just makes it
+ explicit in the IC generating interface.
+
+ * bytecode/PolymorphicAccess.cpp:
+ (JSC::PolymorphicAccess::addCases):
+ (JSC::PolymorphicAccess::addCase):
+ (JSC::PolymorphicAccess::commit):
+ (JSC::PolymorphicAccess::regenerate):
+ * bytecode/PolymorphicAccess.h:
+ * bytecode/StructureStubInfo.cpp:
+ (JSC::StructureStubInfo::addAccessCase):
+ (JSC::StructureStubInfo::initStub): Deleted.
+ * bytecode/StructureStubInfo.h:
+ * jit/Repatch.cpp:
+ (JSC::tryCacheGetByID):
+ (JSC::repatchGetByID):
+ (JSC::tryCachePutByID):
+ (JSC::repatchPutByID):
+ (JSC::tryRepatchIn):
+ (JSC::repatchIn):
+
2017-06-20 Myles C. Maxfield <[email protected]>
Disable font variations on macOS Sierra and iOS 10
Modified: trunk/Source/_javascript_Core/bytecode/PolymorphicAccess.cpp (218640 => 218641)
--- trunk/Source/_javascript_Core/bytecode/PolymorphicAccess.cpp 2017-06-21 18:37:02 UTC (rev 218640)
+++ trunk/Source/_javascript_Core/bytecode/PolymorphicAccess.cpp 2017-06-21 18:42:44 UTC (rev 218641)
@@ -210,8 +210,8 @@
PolymorphicAccess::~PolymorphicAccess() { }
AccessGenerationResult PolymorphicAccess::addCases(
- VM& vm, CodeBlock* codeBlock, StructureStubInfo& stubInfo, const Identifier& ident,
- Vector<std::unique_ptr<AccessCase>, 2> originalCasesToAdd)
+ const GCSafeConcurrentJSLocker& locker, VM& vm, CodeBlock* codeBlock, StructureStubInfo& stubInfo,
+ const Identifier& ident, Vector<std::unique_ptr<AccessCase>, 2> originalCasesToAdd)
{
SuperSamplerScope superSamplerScope(false);
@@ -258,7 +258,7 @@
// Now add things to the new list. Note that at this point, we will still have old cases that
// may be replaced by the new ones. That's fine. We will sort that out when we regenerate.
for (auto& caseToAdd : casesToAdd) {
- commit(vm, m_watchpoints, codeBlock, stubInfo, ident, *caseToAdd);
+ commit(locker, vm, m_watchpoints, codeBlock, stubInfo, ident, *caseToAdd);
m_list.append(WTFMove(caseToAdd));
}
@@ -269,12 +269,12 @@
}
AccessGenerationResult PolymorphicAccess::addCase(
- VM& vm, CodeBlock* codeBlock, StructureStubInfo& stubInfo, const Identifier& ident,
- std::unique_ptr<AccessCase> newAccess)
+ const GCSafeConcurrentJSLocker& locker, VM& vm, CodeBlock* codeBlock, StructureStubInfo& stubInfo,
+ const Identifier& ident, std::unique_ptr<AccessCase> newAccess)
{
Vector<std::unique_ptr<AccessCase>, 2> newAccesses;
newAccesses.append(WTFMove(newAccess));
- return addCases(vm, codeBlock, stubInfo, ident, WTFMove(newAccesses));
+ return addCases(locker, vm, codeBlock, stubInfo, ident, WTFMove(newAccesses));
}
bool PolymorphicAccess::visitWeak(VM& vm) const
@@ -310,7 +310,7 @@
}
void PolymorphicAccess::commit(
- VM& vm, std::unique_ptr<WatchpointsOnStructureStubInfo>& watchpoints, CodeBlock* codeBlock,
+ const GCSafeConcurrentJSLocker&, VM& vm, std::unique_ptr<WatchpointsOnStructureStubInfo>& watchpoints, CodeBlock* codeBlock,
StructureStubInfo& stubInfo, const Identifier& ident, AccessCase& accessCase)
{
// NOTE: We currently assume that this is relatively rare. It mainly arises for accesses to
@@ -329,7 +329,7 @@
}
AccessGenerationResult PolymorphicAccess::regenerate(
- VM& vm, CodeBlock* codeBlock, StructureStubInfo& stubInfo, const Identifier& ident)
+ const GCSafeConcurrentJSLocker& locker, VM& vm, CodeBlock* codeBlock, StructureStubInfo& stubInfo, const Identifier& ident)
{
SuperSamplerScope superSamplerScope(false);
@@ -410,7 +410,7 @@
bool allGuardedByStructureCheck = true;
bool hasJSGetterSetterCall = false;
for (auto& newCase : cases) {
- commit(vm, state.watchpoints, codeBlock, stubInfo, ident, *newCase);
+ commit(locker, vm, state.watchpoints, codeBlock, stubInfo, ident, *newCase);
allGuardedByStructureCheck &= newCase->guardedByStructureCheck();
if (newCase->type() == AccessCase::Getter || newCase->type() == AccessCase::Setter)
hasJSGetterSetterCall = true;
Modified: trunk/Source/_javascript_Core/bytecode/PolymorphicAccess.h (218640 => 218641)
--- trunk/Source/_javascript_Core/bytecode/PolymorphicAccess.h 2017-06-21 18:37:02 UTC (rev 218640)
+++ trunk/Source/_javascript_Core/bytecode/PolymorphicAccess.h 2017-06-21 18:42:44 UTC (rev 218641)
@@ -125,12 +125,12 @@
// When this fails (returns GaveUp), this will leave the old stub intact but you should not try
// to call this method again for that PolymorphicAccess instance.
AccessGenerationResult addCases(
- VM&, CodeBlock*, StructureStubInfo&, const Identifier&, Vector<std::unique_ptr<AccessCase>, 2>);
+ const GCSafeConcurrentJSLocker&, VM&, CodeBlock*, StructureStubInfo&, const Identifier&, Vector<std::unique_ptr<AccessCase>, 2>);
AccessGenerationResult addCase(
- VM&, CodeBlock*, StructureStubInfo&, const Identifier&, std::unique_ptr<AccessCase>);
+ const GCSafeConcurrentJSLocker&, VM&, CodeBlock*, StructureStubInfo&, const Identifier&, std::unique_ptr<AccessCase>);
- AccessGenerationResult regenerate(VM&, CodeBlock*, StructureStubInfo&, const Identifier&);
+ AccessGenerationResult regenerate(const GCSafeConcurrentJSLocker&, VM&, CodeBlock*, StructureStubInfo&, const Identifier&);
bool isEmpty() const { return m_list.isEmpty(); }
unsigned size() const { return m_list.size(); }
@@ -164,12 +164,9 @@
typedef Vector<std::unique_ptr<AccessCase>, 2> ListType;
void commit(
- VM&, std::unique_ptr<WatchpointsOnStructureStubInfo>&, CodeBlock*, StructureStubInfo&,
+ const GCSafeConcurrentJSLocker&, VM&, std::unique_ptr<WatchpointsOnStructureStubInfo>&, CodeBlock*, StructureStubInfo&,
const Identifier&, AccessCase&);
- MacroAssemblerCodePtr regenerate(
- VM&, CodeBlock*, StructureStubInfo&, const Identifier&, ListType& cases);
-
ListType m_list;
RefPtr<JITStubRoutine> m_stubRoutine;
std::unique_ptr<WatchpointsOnStructureStubInfo> m_watchpoints;
Modified: trunk/Source/_javascript_Core/bytecode/StructureStubInfo.cpp (218640 => 218641)
--- trunk/Source/_javascript_Core/bytecode/StructureStubInfo.cpp 2017-06-21 18:37:02 UTC (rev 218640)
+++ trunk/Source/_javascript_Core/bytecode/StructureStubInfo.cpp 2017-06-21 18:42:44 UTC (rev 218641)
@@ -78,12 +78,6 @@
u.byIdSelf.offset = offset;
}
-void StructureStubInfo::initStub(CodeBlock*, std::unique_ptr<PolymorphicAccess> stub)
-{
- cacheType = CacheType::Stub;
- u.stub = stub.release();
-}
-
void StructureStubInfo::deref()
{
switch (cacheType) {
@@ -117,7 +111,7 @@
}
AccessGenerationResult StructureStubInfo::addAccessCase(
- CodeBlock* codeBlock, const Identifier& ident, std::unique_ptr<AccessCase> accessCase)
+ const GCSafeConcurrentJSLocker& locker, CodeBlock* codeBlock, const Identifier& ident, std::unique_ptr<AccessCase> accessCase)
{
VM& vm = *codeBlock->vm();
@@ -130,7 +124,7 @@
AccessGenerationResult result;
if (cacheType == CacheType::Stub) {
- result = u.stub->addCase(vm, codeBlock, *this, ident, WTFMove(accessCase));
+ result = u.stub->addCase(locker, vm, codeBlock, *this, ident, WTFMove(accessCase));
if (verbose)
dataLog("Had stub, result: ", result, "\n");
@@ -151,7 +145,7 @@
accessCases.append(WTFMove(accessCase));
- result = access->addCases(vm, codeBlock, *this, ident, WTFMove(accessCases));
+ result = access->addCases(locker, vm, codeBlock, *this, ident, WTFMove(accessCases));
if (verbose)
dataLog("Created stub, result: ", result, "\n");
@@ -161,7 +155,8 @@
return result;
}
- initStub(codeBlock, WTFMove(access));
+ cacheType = CacheType::Stub;
+ u.stub = access.release();
}
RELEASE_ASSERT(!result.generatedSomeCode());
@@ -186,7 +181,7 @@
// PolymorphicAccess.
bufferedStructures.clear();
- result = u.stub->regenerate(vm, codeBlock, *this, ident);
+ result = u.stub->regenerate(locker, vm, codeBlock, *this, ident);
if (verbose)
dataLog("Regeneration result: ", result, "\n");
Modified: trunk/Source/_javascript_Core/bytecode/StructureStubInfo.h (218640 => 218641)
--- trunk/Source/_javascript_Core/bytecode/StructureStubInfo.h 2017-06-21 18:37:02 UTC (rev 218640)
+++ trunk/Source/_javascript_Core/bytecode/StructureStubInfo.h 2017-06-21 18:42:44 UTC (rev 218641)
@@ -71,9 +71,8 @@
void initGetByIdSelf(CodeBlock*, Structure* baseObjectStructure, PropertyOffset);
void initArrayLength();
void initPutByIdReplace(CodeBlock*, Structure* baseObjectStructure, PropertyOffset);
- void initStub(CodeBlock*, std::unique_ptr<PolymorphicAccess>);
- AccessGenerationResult addAccessCase(CodeBlock*, const Identifier&, std::unique_ptr<AccessCase>);
+ AccessGenerationResult addAccessCase(const GCSafeConcurrentJSLocker&, CodeBlock*, const Identifier&, std::unique_ptr<AccessCase>);
void reset(CodeBlock*);
Modified: trunk/Source/_javascript_Core/jit/Repatch.cpp (218640 => 218641)
--- trunk/Source/_javascript_Core/jit/Repatch.cpp 2017-06-21 18:37:02 UTC (rev 218640)
+++ trunk/Source/_javascript_Core/jit/Repatch.cpp 2017-06-21 18:42:44 UTC (rev 218641)
@@ -157,7 +157,7 @@
return operationTryGetById;
}
-static InlineCacheAction tryCacheGetByID(ExecState* exec, JSValue baseValue, const Identifier& propertyName, const PropertySlot& slot, StructureStubInfo& stubInfo, GetByIDKind kind)
+static InlineCacheAction tryCacheGetByID(const GCSafeConcurrentJSLocker& locker, ExecState* exec, JSValue baseValue, const Identifier& propertyName, const PropertySlot& slot, StructureStubInfo& stubInfo, GetByIDKind kind)
{
if (forceICFailure(exec))
return GiveUpOnCache;
@@ -323,7 +323,7 @@
LOG_IC((ICEvent::GetByIdAddAccessCase, baseValue.classInfoOrNull(vm), propertyName));
- AccessGenerationResult result = stubInfo.addAccessCase(codeBlock, propertyName, WTFMove(newCase));
+ AccessGenerationResult result = stubInfo.addAccessCase(locker, codeBlock, propertyName, WTFMove(newCase));
if (result.generatedSomeCode()) {
LOG_IC((ICEvent::GetByIdReplaceWithJump, baseValue.classInfoOrNull(vm), propertyName));
@@ -340,7 +340,7 @@
SuperSamplerScope superSamplerScope(false);
GCSafeConcurrentJSLocker locker(exec->codeBlock()->m_lock, exec->vm().heap);
- if (tryCacheGetByID(exec, baseValue, propertyName, slot, stubInfo, kind) == GiveUpOnCache)
+ if (tryCacheGetByID(locker, exec, baseValue, propertyName, slot, stubInfo, kind) == GiveUpOnCache)
ftlThunkAwareRepatchCall(exec->codeBlock(), stubInfo.slowPathCallLocation(), appropriateGenericGetByIdFunction(kind));
}
@@ -368,7 +368,7 @@
return operationPutByIdNonStrictOptimize;
}
-static InlineCacheAction tryCachePutByID(ExecState* exec, JSValue baseValue, Structure* structure, const Identifier& ident, const PutPropertySlot& slot, StructureStubInfo& stubInfo, PutKind putKind)
+static InlineCacheAction tryCachePutByID(const GCSafeConcurrentJSLocker& locker, ExecState* exec, JSValue baseValue, Structure* structure, const Identifier& ident, const PutPropertySlot& slot, StructureStubInfo& stubInfo, PutKind putKind)
{
if (forceICFailure(exec))
return GiveUpOnCache;
@@ -476,7 +476,7 @@
LOG_IC((ICEvent::PutByIdAddAccessCase, structure->classInfo(), ident));
- AccessGenerationResult result = stubInfo.addAccessCase(codeBlock, ident, WTFMove(newCase));
+ AccessGenerationResult result = stubInfo.addAccessCase(locker, codeBlock, ident, WTFMove(newCase));
if (result.generatedSomeCode()) {
LOG_IC((ICEvent::PutByIdReplaceWithJump, structure->classInfo(), ident));
@@ -494,13 +494,13 @@
SuperSamplerScope superSamplerScope(false);
GCSafeConcurrentJSLocker locker(exec->codeBlock()->m_lock, exec->vm().heap);
- if (tryCachePutByID(exec, baseValue, structure, propertyName, slot, stubInfo, putKind) == GiveUpOnCache)
+ if (tryCachePutByID(locker, exec, baseValue, structure, propertyName, slot, stubInfo, putKind) == GiveUpOnCache)
ftlThunkAwareRepatchCall(exec->codeBlock(), stubInfo.slowPathCallLocation(), appropriateGenericPutByIdFunction(slot, putKind));
}
static InlineCacheAction tryRepatchIn(
- ExecState* exec, JSCell* base, const Identifier& ident, bool wasFound,
- const PropertySlot& slot, StructureStubInfo& stubInfo)
+ const GCSafeConcurrentJSLocker& locker, ExecState* exec, JSCell* base, const Identifier& ident,
+ bool wasFound, const PropertySlot& slot, StructureStubInfo& stubInfo)
{
if (forceICFailure(exec))
return GiveUpOnCache;
@@ -535,7 +535,7 @@
std::unique_ptr<AccessCase> newCase = AccessCase::create(
vm, codeBlock, wasFound ? AccessCase::InHit : AccessCase::InMiss, invalidOffset, structure, conditionSet);
- AccessGenerationResult result = stubInfo.addAccessCase(codeBlock, ident, WTFMove(newCase));
+ AccessGenerationResult result = stubInfo.addAccessCase(locker, codeBlock, ident, WTFMove(newCase));
if (result.generatedSomeCode()) {
LOG_IC((ICEvent::InReplaceWithJump, structure->classInfo(), ident));
@@ -556,7 +556,7 @@
{
SuperSamplerScope superSamplerScope(false);
GCSafeConcurrentJSLocker locker(exec->codeBlock()->m_lock, exec->vm().heap);
- if (tryRepatchIn(exec, base, ident, wasFound, slot, stubInfo) == GiveUpOnCache)
+ if (tryRepatchIn(locker, exec, base, ident, wasFound, slot, stubInfo) == GiveUpOnCache)
ftlThunkAwareRepatchCall(exec->codeBlock(), stubInfo.slowPathCallLocation(), operationIn);
}
