Skip to content

Commit

Permalink
[JSC] Handler IC Getter / Setter / Proxy accessors should not use m_g…
Browse files Browse the repository at this point in the history 
…lobalObject

https://bugs.webkit.org/show_bug.cgi?id=274485
rdar://128494111

Reviewed by Mark Lam.

Since Handler IC can be shared across JSGlobalObjects, the generated code should not rely on that.
This patch fixes so that all access are done through JSGlobalObject offered at runtime.
We made all necessary fields of JSGlobalObject from JIT.

* Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp:
(JSC::InlineCacheCompiler::generateWithGuard):
(JSC::InlineCacheCompiler::generateImpl):
(JSC::InlineCacheCompiler::emitProxyObjectAccess):
* Source/JavaScriptCore/runtime/JSGlobalObject.cpp:
(JSC::JSGlobalObject::init):
(JSC::JSGlobalObject::visitChildrenImpl):
* Source/JavaScriptCore/runtime/JSGlobalObject.h:
(JSC::JSGlobalObject::symbolPrototype const):
(JSC::JSGlobalObject::dateStructure const):
(JSC::JSGlobalObject::symbolObjectStructure const):
(JSC::JSGlobalObject::offsetOfPerformProxyObjectHasFunction):
(JSC::JSGlobalObject::offsetOfPerformProxyObjectGetFunction):
(JSC::JSGlobalObject::offsetOfPerformProxyObjectGetByValFunction):
(JSC::JSGlobalObject::offsetOfPerformProxyObjectSetStrictFunction):
(JSC::JSGlobalObject::offsetOfPerformProxyObjectSetSloppyFunction):
(JSC::JSGlobalObject::offsetOfNullSetterStrictFunction):
(JSC::JSGlobalObject::offsetOfStringPrototype):
(JSC::JSGlobalObject::offsetOfBigIntPrototype):
(JSC::JSGlobalObject::offsetOfSymbolPrototype):
* Source/JavaScriptCore/runtime/JSGlobalObjectInlines.h:
(JSC::JSGlobalObject::performProxyObjectHasFunction const):
(JSC::JSGlobalObject::performProxyObjectGetFunction const):
(JSC::JSGlobalObject::performProxyObjectGetFunctionConcurrently const):
(JSC::JSGlobalObject::performProxyObjectGetByValFunction const):
(JSC::JSGlobalObject::performProxyObjectGetByValFunctionConcurrently const):
(JSC::JSGlobalObject::performProxyObjectSetSloppyFunction const):
(JSC::JSGlobalObject::performProxyObjectSetSloppyFunctionConcurrently const):
(JSC::JSGlobalObject::performProxyObjectSetStrictFunction const):
(JSC::JSGlobalObject::performProxyObjectSetStrictFunctionConcurrently const):
* Source/JavaScriptCore/runtime/StructureInlines.h:

Canonical link: https://commits.webkit.org/279136@main
  • Loading branch information
@Constellation
Constellation committed May 22, 2024
1 parent 19697c5 commit 57affb6
Show file tree
Hide file tree
Showing 8 changed files with 112 additions and 35 deletions.
2 changes: 0 additions & 2 deletions LayoutTests/platform/mac/TestExpectations
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2540,5 +2540,3 @@ imported/w3c/web-platform-tests/navigation-api/navigation-history-entry/current-
imported/w3c/web-platform-tests/navigation-api/navigation-history-entry/entries-after-navigations-in-multiple-windows.html [ Failure ]
imported/w3c/web-platform-tests/navigation-api/navigation-history-entry/key-id-location-replace-cross-origin.html [ Failure ]
imported/w3c/web-platform-tests/navigation-api/navigation-methods/sandboxing-navigate-parent.html [ Failure ]

webkit.org/b/274485 imported/w3c/web-platform-tests/html/dom/idlharness.https.html [ Skip ]
83 changes: 67 additions & 16 deletions Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1418,7 +1418,6 @@ void InlineCacheCompiler::generateWithGuard(unsigned index, AccessCase& accessCa

accessCase.checkConsistency(*m_stubInfo);

JSGlobalObject* globalObject = m_globalObject;
CCallHelpers& jit = *m_jit;
JIT_COMMENT(jit, "Begin generateWithGuard");
VM& vm = m_vm;
Expand Down Expand Up @@ -1465,9 +1464,28 @@ void InlineCacheCompiler::generateWithGuard(unsigned index, AccessCase& accessCa
}
} else {
if (structure->hasMonoProto()) {
JSValue prototype = structure->prototypeForLookup(globalObject);
RELEASE_ASSERT(prototype.isObject());
jit.move(CCallHelpers::TrustedImmPtr(asObject(prototype)), baseForAccessGPR);
if (!useHandlerIC() || structure->isObject()) {
JSValue prototype = structure->prototypeForLookup(m_globalObject);
RELEASE_ASSERT(prototype.isObject());
jit.move(CCallHelpers::TrustedImmPtr(asObject(prototype)), baseForAccessGPR);
} else {
ASSERT(useHandlerIC());
jit.loadPtr(CCallHelpers::Address(GPRInfo::jitDataRegister, BaselineJITData::offsetOfGlobalObject()), baseForAccessGPR);
switch (structure->typeInfo().type()) {
case StringType:
jit.loadPtr(CCallHelpers::Address(baseForAccessGPR, JSGlobalObject::offsetOfStringPrototype()), baseForAccessGPR);
break;
case HeapBigIntType:
jit.loadPtr(CCallHelpers::Address(baseForAccessGPR, JSGlobalObject::offsetOfBigIntPrototype()), baseForAccessGPR);
break;
case SymbolType:
jit.loadPtr(CCallHelpers::Address(baseForAccessGPR, JSGlobalObject::offsetOfSymbolPrototype()), baseForAccessGPR);
break;
default:
RELEASE_ASSERT_NOT_REACHED();
break;
}
}
} else {
RELEASE_ASSERT(structure->isObject()); // Primitives must have a stored prototype. We use prototypeForLookup for them.
#if USE(JSVALUE64)
Expand Down Expand Up @@ -2933,8 +2951,6 @@ void InlineCacheCompiler::generateImpl(unsigned index, AccessCase& accessCase)

ASSERT(baseGPR != scratchGPR);

JSGlobalObject* globalObject = m_globalObject;

// Create a JS call using a JS call inline cache. Assume that:
//
// - SP is aligned and represents the extent of the calling compiler's stack usage.
Expand Down Expand Up @@ -2968,7 +2984,11 @@ void InlineCacheCompiler::generateImpl(unsigned index, AccessCase& accessCase)
if (ecmaMode.isStrict()) {
CCallHelpers::Jump shouldNotThrowError = jit.branchIfNotType(scratchGPR, NullSetterFunctionType);
// We replace setter with this AccessCase's JSGlobalObject::nullSetterStrictFunction, which will throw an error with the right JSGlobalObject.
jit.move(CCallHelpers::TrustedImmPtr(globalObject->nullSetterStrictFunction()), scratchGPR);
if (useHandlerIC()) {
jit.loadPtr(CCallHelpers::Address(GPRInfo::jitDataRegister, BaselineJITData::offsetOfGlobalObject()), scratchGPR);
jit.loadPtr(CCallHelpers::Address(scratchGPR, JSGlobalObject::offsetOfNullSetterStrictFunction()), scratchGPR);
} else
jit.move(CCallHelpers::TrustedImmPtr(m_globalObject->nullSetterStrictFunction()), scratchGPR);
shouldNotThrowError.link(&jit);
}
}
Expand Down Expand Up @@ -3602,8 +3622,6 @@ void InlineCacheCompiler::emitProxyObjectAccess(unsigned index, ProxyObjectAcces
GPRReg scratchGPR = m_scratchGPR;
GPRReg thisGPR = m_stubInfo->thisValueIsInExtraGPR() ? m_stubInfo->thisGPR() : baseGPR;

JSGlobalObject* globalObject = m_globalObject;

jit.load8(CCallHelpers::Address(baseGPR, JSCell::typeInfoTypeOffset()), scratchGPR);
fallThrough.append(jit.branch32(CCallHelpers::NotEqual, scratchGPR, CCallHelpers::TrustedImm32(ProxyObjectType)));

Expand All @@ -3618,24 +3636,19 @@ void InlineCacheCompiler::emitProxyObjectAccess(unsigned index, ProxyObjectAcces
setSpillStateForJSCall(spillState);

unsigned numberOfParameters;
JSFunction* proxyInternalMethod = nullptr;

switch (accessCase.m_type) {
case AccessCase::ProxyObjectHas:
numberOfParameters = 2;
proxyInternalMethod = globalObject->performProxyObjectHasFunction();
break;
case AccessCase::ProxyObjectLoad:
numberOfParameters = 3;
proxyInternalMethod = globalObject->performProxyObjectGetFunction();
break;
case AccessCase::IndexedProxyObjectLoad:
numberOfParameters = 3;
proxyInternalMethod = globalObject->performProxyObjectGetByValFunction();
break;
case AccessCase::ProxyObjectStore:
numberOfParameters = 4;
proxyInternalMethod = ecmaMode.isStrict() ? globalObject->performProxyObjectSetStrictFunction() : globalObject->performProxyObjectSetSloppyFunction();
break;
default:
RELEASE_ASSERT_NOT_REACHED();
Expand Down Expand Up @@ -3677,8 +3690,46 @@ void InlineCacheCompiler::emitProxyObjectAccess(unsigned index, ProxyObjectAcces
break;
}

ASSERT(proxyInternalMethod);
jit.move(CCallHelpers::TrustedImmPtr(proxyInternalMethod), scratchGPR);
switch (accessCase.m_type) {
case AccessCase::ProxyObjectHas: {
if (useHandlerIC()) {
jit.loadPtr(CCallHelpers::Address(GPRInfo::jitDataRegister, BaselineJITData::offsetOfGlobalObject()), scratchGPR);
jit.loadPtr(CCallHelpers::Address(scratchGPR, JSGlobalObject::offsetOfPerformProxyObjectHasFunction()), scratchGPR);
} else
jit.move(CCallHelpers::TrustedImmPtr(m_globalObject->performProxyObjectHasFunction()), scratchGPR);
break;
}
case AccessCase::ProxyObjectLoad: {
if (useHandlerIC()) {
jit.loadPtr(CCallHelpers::Address(GPRInfo::jitDataRegister, BaselineJITData::offsetOfGlobalObject()), scratchGPR);
jit.loadPtr(CCallHelpers::Address(scratchGPR, JSGlobalObject::offsetOfPerformProxyObjectGetFunction()), scratchGPR);
} else
jit.move(CCallHelpers::TrustedImmPtr(m_globalObject->performProxyObjectGetFunction()), scratchGPR);
break;
}
case AccessCase::IndexedProxyObjectLoad: {
if (useHandlerIC()) {
jit.loadPtr(CCallHelpers::Address(GPRInfo::jitDataRegister, BaselineJITData::offsetOfGlobalObject()), scratchGPR);
jit.loadPtr(CCallHelpers::Address(scratchGPR, JSGlobalObject::offsetOfPerformProxyObjectGetByValFunction()), scratchGPR);
} else
jit.move(CCallHelpers::TrustedImmPtr(m_globalObject->performProxyObjectGetByValFunction()), scratchGPR);
break;
}
case AccessCase::ProxyObjectStore: {
if (useHandlerIC()) {
jit.loadPtr(CCallHelpers::Address(GPRInfo::jitDataRegister, BaselineJITData::offsetOfGlobalObject()), scratchGPR);
if (ecmaMode.isStrict())
jit.loadPtr(CCallHelpers::Address(scratchGPR, JSGlobalObject::offsetOfPerformProxyObjectSetStrictFunction()), scratchGPR);
else
jit.loadPtr(CCallHelpers::Address(scratchGPR, JSGlobalObject::offsetOfPerformProxyObjectSetSloppyFunction()), scratchGPR);
} else
jit.move(CCallHelpers::TrustedImmPtr(ecmaMode.isStrict() ? m_globalObject->performProxyObjectSetStrictFunction() : m_globalObject->performProxyObjectSetSloppyFunction()), scratchGPR);
break;
}
default:
RELEASE_ASSERT_NOT_REACHED();
}

jit.storeCell(scratchGPR, calleeFrame.withOffset(CallFrameSlot::callee * sizeof(Register)));

if (useHandlerIC()) {
Expand Down
4 changes: 0 additions & 4 deletions Source/JavaScriptCore/dfg/DFGJITCode.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,6 @@ static bool attemptToWatch(CodeBlock* codeBlock, WatchpointSet& set, CodeBlockJe

bool JITData::tryInitialize(VM& vm, CodeBlock* codeBlock, const JITCode& jitCode)
{
// We share the same layout for particular fields in all JITData to make our data IC assume this.
ASSERT(BaselineJITData::offsetOfGlobalObject() == JITData::offsetOfGlobalObject());
ASSERT(BaselineJITData::offsetOfStackOffset() == JITData::offsetOfStackOffset());

m_globalObject = codeBlock->globalObject();
m_stackOffset = codeBlock->stackPointerOffset() * sizeof(Register);

Expand Down
8 changes: 8 additions & 0 deletions Source/JavaScriptCore/llint/LLIntData.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@
#include "LLIntData.h"

#include "ArithProfile.h"
#include "BaselineJITCode.h"
#include "CodeBlock.h"
#include "DFGJITCode.h"
#include "JSCConfig.h"
#include "LLIntCLoop.h"
#include "LLIntEntrypoint.h"
Expand Down Expand Up @@ -367,6 +369,12 @@ void Data::performAssertions(VM& vm)
ASSERT(arithProfile.lhsObservedType().isOnlyInt32());
ASSERT(arithProfile.rhsObservedType().isOnlyNumber());
}

#if ENABLE(DFG_JIT)
// We share the same layout for particular fields in all JITData to make our data IC assume this.
RELEASE_ASSERT(BaselineJITData::offsetOfGlobalObject() == DFG::JITData::offsetOfGlobalObject());
RELEASE_ASSERT(BaselineJITData::offsetOfStackOffset() == DFG::JITData::offsetOfStackOffset());
#endif
}
IGNORE_WARNINGS_END

Expand Down
11 changes: 10 additions & 1 deletion Source/JavaScriptCore/runtime/JSGlobalObject.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,6 @@ const GlobalObjectMethodTable* JSGlobalObject::baseGlobalObjectMethodTable()
Map JSGlobalObject::m_mapStructure DontEnum|ClassStructure
Number JSGlobalObject::m_numberObjectStructure DontEnum|ClassStructure
Set JSGlobalObject::m_setStructure DontEnum|ClassStructure
Symbol JSGlobalObject::m_symbolObjectStructure DontEnum|ClassStructure
WeakMap JSGlobalObject::m_weakMapStructure DontEnum|ClassStructure
WeakSet JSGlobalObject::m_weakSetStructure DontEnum|ClassStructure
@end
Expand Down Expand Up @@ -891,6 +890,11 @@ void JSGlobalObject::init(VM& vm)
});

m_functionProtoHasInstanceSymbolFunction.set(vm, this, hasInstanceSymbolFunction);
m_performProxyObjectHasFunction.set(vm, this, jsCast<JSFunction*>(linkTimeConstant(LinkTimeConstant::performProxyObjectHas)));
m_performProxyObjectGetFunction.set(vm, this, jsCast<JSFunction*>(linkTimeConstant(LinkTimeConstant::performProxyObjectGet)));
m_performProxyObjectGetByValFunction.set(vm, this, jsCast<JSFunction*>(linkTimeConstant(LinkTimeConstant::performProxyObjectGetByVal)));
m_performProxyObjectSetStrictFunction.set(vm, this, jsCast<JSFunction*>(linkTimeConstant(LinkTimeConstant::performProxyObjectSetStrict)));
m_performProxyObjectSetSloppyFunction.set(vm, this, jsCast<JSFunction*>(linkTimeConstant(LinkTimeConstant::performProxyObjectSetSloppy)));

m_nullGetterFunction.set(vm, this, NullGetterFunction::create(vm, NullGetterFunction::createStructure(vm, this, m_functionPrototype.get())));
Structure* nullSetterFunctionStructure = NullSetterFunction::createStructure(vm, this, m_functionPrototype.get());
Expand Down Expand Up @@ -2496,6 +2500,11 @@ void JSGlobalObject::visitChildrenImpl(JSCell* cell, Visitor& visitor)
visitor.append(thisObject->m_objectProtoValueOfFunction);
thisObject->m_numberProtoToStringFunction.visit(visitor);
visitor.append(thisObject->m_functionProtoHasInstanceSymbolFunction);
visitor.append(thisObject->m_performProxyObjectHasFunction);
visitor.append(thisObject->m_performProxyObjectGetFunction);
visitor.append(thisObject->m_performProxyObjectGetByValFunction);
visitor.append(thisObject->m_performProxyObjectSetStrictFunction);
visitor.append(thisObject->m_performProxyObjectSetSloppyFunction);
visitor.append(thisObject->m_regExpProtoSymbolReplace);
thisObject->m_throwTypeErrorArgumentsCalleeGetterSetter.visit(visitor);
thisObject->m_moduleLoader.visit(visitor);
Expand Down
20 changes: 17 additions & 3 deletions Source/JavaScriptCore/runtime/JSGlobalObject.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ constexpr bool typeExposedByDefault = true;
macro(String, string, stringObject, StringObject, String, object, typeExposedByDefault) \
macro(JSPromise, promise, promise, JSPromise, Promise, object, typeExposedByDefault) \
macro(BigInt, bigInt, bigIntObject, BigIntObject, BigInt, object, typeExposedByDefault) \
macro(Symbol, symbol, symbolObject, SymbolObject, Symbol, object, typeExposedByDefault) \
macro(WeakObjectRef, weakObjectRef, weakObjectRef, JSWeakObjectRef, WeakRef, object, typeExposedByDefault) \
macro(FinalizationRegistry, finalizationRegistry, finalizationRegistry, JSFinalizationRegistry, FinalizationRegistry, object, typeExposedByDefault) \

Expand All @@ -147,7 +148,6 @@ constexpr bool typeExposedByDefault = true;
macro(Map, map, map, JSMap, Map, object, typeExposedByDefault) \
macro(Number, number, numberObject, NumberObject, Number, object, typeExposedByDefault) \
macro(Set, set, set, JSSet, Set, object, typeExposedByDefault) \
macro(Symbol, symbol, symbolObject, SymbolObject, Symbol, object, typeExposedByDefault) \
DEFINE_STANDARD_BUILTIN(macro, WeakMap, weakMap) \
DEFINE_STANDARD_BUILTIN(macro, WeakSet, weakSet) \

Expand Down Expand Up @@ -259,6 +259,11 @@ class JSGlobalObject : public JSSegmentedVariableObject {
LazyProperty<JSGlobalObject, JSFunction> m_numberProtoToStringFunction;
WriteBarrier<JSFunction> m_objectProtoValueOfFunction;
WriteBarrier<JSFunction> m_functionProtoHasInstanceSymbolFunction;
WriteBarrier<JSFunction> m_performProxyObjectHasFunction;
WriteBarrier<JSFunction> m_performProxyObjectGetFunction;
WriteBarrier<JSFunction> m_performProxyObjectGetByValFunction;
WriteBarrier<JSFunction> m_performProxyObjectSetStrictFunction;
WriteBarrier<JSFunction> m_performProxyObjectSetSloppyFunction;
WriteBarrier<JSObject> m_regExpProtoSymbolReplace;
LazyProperty<JSGlobalObject, GetterSetter> m_throwTypeErrorArgumentsCalleeGetterSetter;

Expand Down Expand Up @@ -716,7 +721,7 @@ class JSGlobalObject : public JSSegmentedVariableObject {
JSObject* numberPrototype() const { return m_numberObjectStructure.prototypeInitializedOnMainThread(this); }
BigIntPrototype* bigIntPrototype() const { return m_bigIntPrototype.get(); }
JSObject* datePrototype() const { return m_dateStructure.prototype(this); }
JSObject* symbolPrototype() const { return m_symbolObjectStructure.prototypeInitializedOnMainThread(this); }
SymbolPrototype* symbolPrototype() const { return m_symbolPrototype.get(); }
ShadowRealmPrototype* shadowRealmPrototype() const { return m_shadowRealmPrototype.get(); }
RegExpPrototype* regExpPrototype() const { return m_regExpPrototype.get(); }
JSObject* errorPrototype() const { return m_errorStructure.prototype(this); }
Expand Down Expand Up @@ -776,7 +781,6 @@ class JSGlobalObject : public JSSegmentedVariableObject {
Structure* glibWrapperObjectStructure() const { return m_glibWrapperObjectStructure.get(this); }
#endif
Structure* dateStructure() const { return m_dateStructure.get(this); }
Structure* symbolObjectStructure() const { return m_symbolObjectStructure.get(this); }
Structure* nullPrototypeObjectStructure() const { return m_nullPrototypeObjectStructure.get(); }
Structure* errorStructure() const { return m_errorStructure.get(this); }
inline Structure* errorStructure(ErrorType) const;
Expand Down Expand Up @@ -820,6 +824,7 @@ class JSGlobalObject : public JSSegmentedVariableObject {
Structure* mapIteratorStructure() const { return m_mapIteratorStructure.get(); }
Structure* setIteratorStructure() const { return m_setIteratorStructure.get(); }
Structure* stringObjectStructure() const { return m_stringObjectStructure.get(); }
Structure* symbolObjectStructure() const { return m_symbolObjectStructure.get(); }
Structure* iteratorResultObjectStructure() const { return m_iteratorResultObjectStructure.get(this); }
Structure* dataPropertyDescriptorObjectStructure() const { return m_dataPropertyDescriptorObjectStructure.get(this); }
Structure* accessorPropertyDescriptorObjectStructure() const { return m_accessorPropertyDescriptorObjectStructure.get(this); }
Expand Down Expand Up @@ -881,6 +886,15 @@ class JSGlobalObject : public JSSegmentedVariableObject {
static ptrdiff_t offsetOfVarInjectionWatchpoint() { return OBJECT_OFFSETOF(JSGlobalObject, m_varInjectionWatchpointSet); }
static ptrdiff_t offsetOfVarReadOnlyWatchpoint() { return OBJECT_OFFSETOF(JSGlobalObject, m_varReadOnlyWatchpointSet); }
static ptrdiff_t offsetOfFunctionProtoHasInstanceSymbolFunction() { return OBJECT_OFFSETOF(JSGlobalObject, m_functionProtoHasInstanceSymbolFunction); }
static ptrdiff_t offsetOfPerformProxyObjectHasFunction() { return OBJECT_OFFSETOF(JSGlobalObject, m_performProxyObjectHasFunction); }
static ptrdiff_t offsetOfPerformProxyObjectGetFunction() { return OBJECT_OFFSETOF(JSGlobalObject, m_performProxyObjectGetFunction); }
static ptrdiff_t offsetOfPerformProxyObjectGetByValFunction() { return OBJECT_OFFSETOF(JSGlobalObject, m_performProxyObjectGetByValFunction); }
static ptrdiff_t offsetOfPerformProxyObjectSetStrictFunction() { return OBJECT_OFFSETOF(JSGlobalObject, m_performProxyObjectSetStrictFunction); }
static ptrdiff_t offsetOfPerformProxyObjectSetSloppyFunction() { return OBJECT_OFFSETOF(JSGlobalObject, m_performProxyObjectSetSloppyFunction); }
static ptrdiff_t offsetOfNullSetterStrictFunction() { return OBJECT_OFFSETOF(JSGlobalObject, m_nullSetterStrictFunction); }
static ptrdiff_t offsetOfStringPrototype() { return OBJECT_OFFSETOF(JSGlobalObject, m_stringPrototype); }
static ptrdiff_t offsetOfBigIntPrototype() { return OBJECT_OFFSETOF(JSGlobalObject, m_bigIntPrototype); }
static ptrdiff_t offsetOfSymbolPrototype() { return OBJECT_OFFSETOF(JSGlobalObject, m_symbolPrototype); }

#if ENABLE(REMOTE_INSPECTOR)
// FIXME: <http://webkit.org/b/246237> Local inspection should be controlled by `inspectable` API.
Expand Down
Loading

0 comments on commit 57affb6

Please sign in to comment.