⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 259638 in webkit


Ignore:
Timestamp:
Apr 7, 2020, 9:04:57 AM (6 years ago)
Author:
ysuzuki@apple.com
Message:

[JSC] Inlined IC should get right JSGlobalObject
​https://bugs.webkit.org/show_bug.cgi?id=210092

Reviewed by Tadeu Zagallo.

JSTests:

  • stress/getter-setter-globalobject-in-ic.js: Added.

(shouldBe):
(valueFunc):
(accessorFunc):
(valueTest):
(accessorTest):

Source/JavaScriptCore:

In DFG / FTL, CodeBlock in AccessCase is the DFG / FTL CodeBlock which includes all the inlined CodeBlocks.
If inlining happens with CodeBlock which has different JSGlobalObject, CodeBlock->globalObject() is different
to the actual lexical JSGlobalObject of the IC. So basically, we should not rely on codeBlock->globalObject() in IC.

This patch passes the correct lexical JSGlobalObject to IC to use. We do not retain this JSGlobalObject.
Since this is lexical JSGlobalObject of that IC, the owner CodeBlock of this IC should already retain it (even if this
JSGlobalObject is one of inlined CodeBlock since the owner CodeBlock retains inlined lower-tier CodeBlocks).

  • bytecode/AccessCase.cpp:

(JSC::AccessCase::generateImpl):

  • bytecode/PolymorphicAccess.cpp:

(JSC::PolymorphicAccess::regenerate):

  • bytecode/PolymorphicAccess.h:
  • bytecode/StructureStubInfo.cpp:

(JSC::StructureStubInfo::addAccessCase):

  • bytecode/StructureStubInfo.h:
  • jit/Repatch.cpp:

(JSC::tryCacheGetBy):
(JSC::tryCacheArrayGetByVal):
(JSC::tryCachePutByID):
(JSC::tryCacheDeleteBy):
(JSC::tryCacheInByID):
(JSC::tryCacheInstanceOf):

  • tools/JSDollarVM.cpp:
Location:
trunk
Files:
1 added
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/JSTests/ChangeLog

    r259636 r259638  
     12020-04-07  Yusuke Suzuki  <ysuzuki@apple.com>
     2
     3        [JSC] Inlined IC should get right JSGlobalObject
     4        https://bugs.webkit.org/show_bug.cgi?id=210092
     5
     6        Reviewed by Tadeu Zagallo.
     7
     8        * stress/getter-setter-globalobject-in-ic.js: Added.
     9        (shouldBe):
     10        (valueFunc):
     11        (accessorFunc):
     12        (valueTest):
     13        (accessorTest):
     14
    1152020-04-07  Yusuke Suzuki  <ysuzuki@apple.com>
    216

  • trunk/Source/JavaScriptCore/ChangeLog

    r259636 r259638  
     12020-04-07  Yusuke Suzuki  <ysuzuki@apple.com>
     2
     3        [JSC] Inlined IC should get right JSGlobalObject
     4        https://bugs.webkit.org/show_bug.cgi?id=210092
     5
     6        Reviewed by Tadeu Zagallo.
     7
     8        In DFG / FTL, CodeBlock in AccessCase is the DFG / FTL CodeBlock which includes all the inlined CodeBlocks.
     9        If inlining happens with CodeBlock which has different JSGlobalObject, CodeBlock->globalObject() is different
     10        to the actual lexical JSGlobalObject of the IC. So basically, we should not rely on codeBlock->globalObject() in IC.
     11
     12        This patch passes the correct lexical JSGlobalObject to IC to use. We do not retain this JSGlobalObject.
     13        Since this is lexical JSGlobalObject of that IC, the owner CodeBlock of this IC should already retain it (even if this
     14        JSGlobalObject is one of inlined CodeBlock since the owner CodeBlock retains inlined lower-tier CodeBlocks).
     15
     16        * bytecode/AccessCase.cpp:
     17        (JSC::AccessCase::generateImpl):
     18        * bytecode/PolymorphicAccess.cpp:
     19        (JSC::PolymorphicAccess::regenerate):
     20        * bytecode/PolymorphicAccess.h:
     21        * bytecode/StructureStubInfo.cpp:
     22        (JSC::StructureStubInfo::addAccessCase):
     23        * bytecode/StructureStubInfo.h:
     24        * jit/Repatch.cpp:
     25        (JSC::tryCacheGetBy):
     26        (JSC::tryCacheArrayGetByVal):
     27        (JSC::tryCachePutByID):
     28        (JSC::tryCacheDeleteBy):
     29        (JSC::tryCacheInByID):
     30        (JSC::tryCacheInstanceOf):
     31        * tools/JSDollarVM.cpp:
     32
    1332020-04-07  Yusuke Suzuki  <ysuzuki@apple.com>
    234

  • trunk/Source/JavaScriptCore/bytecode/AccessCase.cpp

    r259583 r259638  
    13681368    VM& vm = state.m_vm;
    13691369    CodeBlock* codeBlock = jit.codeBlock();
     1370    JSGlobalObject* globalObject = state.m_globalObject;
    13701371    StructureStubInfo& stubInfo = *state.stubInfo;
    13711372    JSValueRegs valueRegs = state.valueRegs;
    … …  
    16581659            done.append(jit.jump());
    16591660
    1660             // FIXME: Revisit JSGlobalObject.
    1661             // https://bugs.webkit.org/show_bug.cgi?id=203204
    16621661            slowCase.link(&jit);
    16631662            jit.move(loadedValueGPR, GPRInfo::regT0);
    … …  
    16671666#endif
    16681667            jit.move(CCallHelpers::TrustedImmPtr(access.callLinkInfo()), GPRInfo::regT2);
    1669             jit.move(CCallHelpers::TrustedImmPtr(state.m_globalObject), GPRInfo::regT3);
     1668            jit.move(CCallHelpers::TrustedImmPtr(globalObject), GPRInfo::regT3);
    16701669            slowPathCall = jit.nearCall();
    16711670            if (m_type == Getter)
    … …  
    17111710            // https://bugs.webkit.org/show_bug.cgi?id=158014
    17121711            GPRReg baseForCustom = m_type == CustomValueGetter || m_type == CustomValueSetter ? baseForAccessGPR : baseForCustomGetGPR;
    1713             // FIXME: Revisit JSGlobalObject.
    1714             // https://bugs.webkit.org/show_bug.cgi?id=203204
     1712            // We do not need to keep globalObject alive since the owner CodeBlock (even if JSGlobalObject* is one of CodeBlock that is inlined and held by DFG CodeBlock)
     1713            // must keep it alive.
    17151714            if (m_type == CustomValueGetter || m_type == CustomAccessorGetter) {
    17161715                RELEASE_ASSERT(m_identifier);
    17171716                jit.setupArguments<PropertySlot::GetValueFunc>(
    1718                     CCallHelpers::TrustedImmPtr(codeBlock->globalObject()),
     1717                    CCallHelpers::TrustedImmPtr(globalObject),
    17191718                    CCallHelpers::CellValue(baseForCustom),
    17201719                    CCallHelpers::TrustedImmPtr(uid()));
    17211720            } else {
    17221721                jit.setupArguments<PutPropertySlot::PutValueFunc>(
    1723                     CCallHelpers::TrustedImmPtr(codeBlock->globalObject()),
     1722                    CCallHelpers::TrustedImmPtr(globalObject),
    17241723                    CCallHelpers::CellValue(baseForCustom),
    17251724                    valueRegs);

  • trunk/Source/JavaScriptCore/bytecode/PolymorphicAccess.cpp

    r257399 r259638  
    389389
    390390AccessGenerationResult PolymorphicAccess::regenerate(
    391     const GCSafeConcurrentJSLocker& locker, VM& vm, CodeBlock* codeBlock, StructureStubInfo& stubInfo)
     391    const GCSafeConcurrentJSLocker& locker, VM& vm, JSGlobalObject* globalObject, CodeBlock* codeBlock, StructureStubInfo& stubInfo)
    392392{
    393393    SuperSamplerScope superSamplerScope(false);
    … …  
    396396        dataLog("Regenerate with m_list: ", listDump(m_list), "\n");
    397397
    398     AccessGenerationState state(vm, codeBlock->globalObject());
     398    AccessGenerationState state(vm, globalObject);
    399399
    400400    state.access = this;

  • trunk/Source/JavaScriptCore/bytecode/PolymorphicAccess.h

    r254464 r259638  
    145145        const GCSafeConcurrentJSLocker&, VM&, CodeBlock*, StructureStubInfo&, std::unique_ptr<AccessCase>);
    146146   
    147     AccessGenerationResult regenerate(const GCSafeConcurrentJSLocker&, VM&, CodeBlock*, StructureStubInfo&);
     147    AccessGenerationResult regenerate(const GCSafeConcurrentJSLocker&, VM&, JSGlobalObject*, CodeBlock*, StructureStubInfo&);
    148148   
    149149    bool isEmpty() const { return m_list.isEmpty(); }

  • trunk/Source/JavaScriptCore/bytecode/StructureStubInfo.cpp

    r259175 r259638  
    141141
    142142AccessGenerationResult StructureStubInfo::addAccessCase(
    143     const GCSafeConcurrentJSLocker& locker, CodeBlock* codeBlock, CacheableIdentifier ident, std::unique_ptr<AccessCase> accessCase)
     143    const GCSafeConcurrentJSLocker& locker, JSGlobalObject* globalObject, CodeBlock* codeBlock, CacheableIdentifier ident, std::unique_ptr<AccessCase> accessCase)
    144144{
    145145    checkConsistency();
    … …  
    220220        clearBufferedStructures();
    221221       
    222         result = u.stub->regenerate(locker, vm, codeBlock, *this);
     222        result = u.stub->regenerate(locker, vm, globalObject, codeBlock, *this);
    223223       
    224224        if (StructureStubInfoInternal::verbose)

  • trunk/Source/JavaScriptCore/bytecode/StructureStubInfo.h

    r259175 r259638  
    8484    void initInByIdSelf(CodeBlock*, Structure* baseObjectStructure, PropertyOffset, CacheableIdentifier);
    8585
    86     AccessGenerationResult addAccessCase(const GCSafeConcurrentJSLocker&, CodeBlock*, CacheableIdentifier, std::unique_ptr<AccessCase>);
     86    AccessGenerationResult addAccessCase(const GCSafeConcurrentJSLocker&, JSGlobalObject*, CodeBlock*, CacheableIdentifier, std::unique_ptr<AccessCase>);
    8787
    8888    void reset(CodeBlock*);

  • trunk/Source/JavaScriptCore/jit/Repatch.cpp

    r259463 r259638  
    397397        LOG_IC((ICEvent::GetByAddAccessCase, baseValue.classInfoOrNull(vm), Identifier::fromUid(vm, propertyName.uid()), slot.slotBase() == baseValue));
    398398
    399         result = stubInfo.addAccessCase(locker, codeBlock, propertyName, WTFMove(newCase));
     399        result = stubInfo.addAccessCase(locker, globalObject, codeBlock, propertyName, WTFMove(newCase));
    400400
    401401        if (result.generatedSomeCode()) {
    … …  
    496496        }
    497497
    498         result = stubInfo.addAccessCase(locker, codeBlock, nullptr, AccessCase::create(vm, codeBlock, accessType, nullptr));
     498        result = stubInfo.addAccessCase(locker, globalObject, codeBlock, nullptr, AccessCase::create(vm, codeBlock, accessType, nullptr));
    499499
    500500        if (result.generatedSomeCode()) {
    … …  
    715715        LOG_IC((ICEvent::PutByIdAddAccessCase, oldStructure->classInfo(), ident, slot.base() == baseValue));
    716716       
    717         result = stubInfo.addAccessCase(locker, codeBlock, propertyName, WTFMove(newCase));
     717        result = stubInfo.addAccessCase(locker, globalObject, codeBlock, propertyName, WTFMove(newCase));
    718718
    719719        if (result.generatedSomeCode()) {
    … …  
    779779        }
    780780
    781         result = stubInfo.addAccessCase(locker, codeBlock, propertyName, WTFMove(newCase));
     781        result = stubInfo.addAccessCase(locker, globalObject, codeBlock, propertyName, WTFMove(newCase));
    782782
    783783        if (result.generatedSomeCode()) {
    … …  
    897897            vm, codeBlock, wasFound ? AccessCase::InHit : AccessCase::InMiss, propertyName, wasFound ? slot.cachedOffset() : invalidOffset, structure, conditionSet, WTFMove(prototypeAccessChain));
    898898
    899         result = stubInfo.addAccessCase(locker, codeBlock, propertyName, WTFMove(newCase));
     899        result = stubInfo.addAccessCase(locker, globalObject, codeBlock, propertyName, WTFMove(newCase));
    900900
    901901        if (result.generatedSomeCode()) {
    … …  
    965965        LOG_IC((ICEvent::InstanceOfAddAccessCase, structure->classInfo(), Identifier()));
    966966       
    967         result = stubInfo.addAccessCase(locker, codeBlock, nullptr, WTFMove(newCase));
     967        result = stubInfo.addAccessCase(locker, globalObject, codeBlock, nullptr, WTFMove(newCase));
    968968       
    969969        if (result.generatedSomeCode()) {

  • trunk/Source/JavaScriptCore/tools/JSDollarVM.cpp

    r259480 r259638  
    13451345}
    13461346
     1347static EncodedJSValue customGetAccessorGlobalObject(JSGlobalObject* globalObject, EncodedJSValue, PropertyName)
     1348{
     1349    return JSValue::encode(globalObject);
     1350}
     1351
     1352static EncodedJSValue customGetValueGlobalObject(JSGlobalObject* globalObject, EncodedJSValue, PropertyName)
     1353{
     1354    return JSValue::encode(globalObject);
     1355}
     1356
    13471357static bool customSetAccessor(JSGlobalObject* globalObject, EncodedJSValue thisObject, EncodedJSValue encodedValue)
    13481358{
    … …  
    13841394    putDirectCustomAccessor(vm, Identifier::fromString(vm, "customAccessor"),
    13851395        CustomGetterSetter::create(vm, customGetAccessor, customSetAccessor), static_cast<unsigned>(PropertyAttribute::CustomAccessor));
     1396    putDirectCustomAccessor(vm, Identifier::fromString(vm, "customValueGlobalObject"),
     1397        CustomGetterSetter::create(vm, customGetValueGlobalObject, nullptr), 0);
     1398    putDirectCustomAccessor(vm, Identifier::fromString(vm, "customAccessorGlobalObject"),
     1399        CustomGetterSetter::create(vm, customGetAccessorGlobalObject, nullptr), static_cast<unsigned>(PropertyAttribute::CustomAccessor));
     1400
    13861401}
    13871402
Note: See TracChangeset for help on using the changeset viewer.