Changeset 228638 in webkit


Ignore:
Timestamp:
Feb 19, 2018 2:47:07 AM (6 years ago)
Author:
Carlos Garcia Campos
Message:

Merge r228193 - put_to_scope/get_from_scope should not cache lexical scopes when expecting a global object
https://bugs.webkit.org/show_bug.cgi?id=182549
<rdar://problem/36189995>

Reviewed by Saam Barati.

JSTests:

  • stress/var-injection-cache-invalidation.js: Added.

(allocateLotsOfThings):
(test):

Source/JavaScriptCore:

Previously, the llint/baseline caching for put_to_scope and
get_from_scope would cache lexical environments when the
varInjectionWatchpoint had been fired for global properties. Code
in the DFG does not follow this same assumption so we could
potentially return the wrong result. Additionally, the baseline
would write barrier the global object rather than the lexical
enviroment object. This patch makes it so that we do not cache
anything other than the global object for when the resolve type is
GlobalPropertyWithVarInjectionChecks or GlobalProperty.

  • assembler/MacroAssembler.cpp:

(JSC::MacroAssembler::jitAssert):

  • assembler/MacroAssembler.h:
  • jit/JITPropertyAccess.cpp:

(JSC::JIT::emit_op_get_from_scope):
(JSC::JIT::emit_op_put_to_scope):

  • runtime/CommonSlowPaths.h:

(JSC::CommonSlowPaths::tryCachePutToScopeGlobal):
(JSC::CommonSlowPaths::tryCacheGetFromScopeGlobal):

  • runtime/Options.h:
Location:
releases/WebKitGTK/webkit-2.20
Files:
1 added
7 edited

Legend:

Unmodified
Added
Removed
  • releases/WebKitGTK/webkit-2.20/JSTests/ChangeLog

    r228047 r228638  
     12018-02-06  Keith Miller  <keith_miller@apple.com>
     2
     3        put_to_scope/get_from_scope should not cache lexical scopes when expecting a global object
     4        https://bugs.webkit.org/show_bug.cgi?id=182549
     5        <rdar://problem/36189995>
     6
     7        Reviewed by Saam Barati.
     8
     9        * stress/var-injection-cache-invalidation.js: Added.
     10        (allocateLotsOfThings):
     11        (test):
     12
    1132018-02-03  Yusuke Suzuki  <utatane.tea@gmail.com>
    214
  • releases/WebKitGTK/webkit-2.20/Source/JavaScriptCore/ChangeLog

    r228633 r228638  
     12018-02-06  Keith Miller  <keith_miller@apple.com>
     2
     3        put_to_scope/get_from_scope should not cache lexical scopes when expecting a global object
     4        https://bugs.webkit.org/show_bug.cgi?id=182549
     5        <rdar://problem/36189995>
     6
     7        Reviewed by Saam Barati.
     8
     9        Previously, the llint/baseline caching for put_to_scope and
     10        get_from_scope would cache lexical environments when the
     11        varInjectionWatchpoint had been fired for global properties. Code
     12        in the DFG does not follow this same assumption so we could
     13        potentially return the wrong result. Additionally, the baseline
     14        would write barrier the global object rather than the lexical
     15        enviroment object. This patch makes it so that we do not cache
     16        anything other than the global object for when the resolve type is
     17        GlobalPropertyWithVarInjectionChecks or GlobalProperty.
     18
     19        * assembler/MacroAssembler.cpp:
     20        (JSC::MacroAssembler::jitAssert):
     21        * assembler/MacroAssembler.h:
     22        * jit/JITPropertyAccess.cpp:
     23        (JSC::JIT::emit_op_get_from_scope):
     24        (JSC::JIT::emit_op_put_to_scope):
     25        * runtime/CommonSlowPaths.h:
     26        (JSC::CommonSlowPaths::tryCachePutToScopeGlobal):
     27        (JSC::CommonSlowPaths::tryCacheGetFromScopeGlobal):
     28        * runtime/Options.h:
     29
    1302018-02-11  Guillaume Emont  <guijemont@igalia.com>
    231
  • releases/WebKitGTK/webkit-2.20/Source/JavaScriptCore/assembler/MacroAssembler.cpp

    r222871 r228638  
    2929#if ENABLE(ASSEMBLER)
    3030
     31#include "Options.h"
    3132#include "ProbeContext.h"
    3233#include <wtf/PrintStream.h>
     34#include <wtf/ScopedLambda.h>
    3335
    3436namespace JSC {
    3537
    3638const double MacroAssembler::twoToThe32 = (double)0x100000000ull;
     39
     40void MacroAssembler::jitAssert(const ScopedLambda<Jump(void)>& functor)
     41{
     42    if (Options::enableJITDebugAssetions()) {
     43        Jump passed = functor();
     44        breakpoint();
     45        passed.link(this);
     46    }
     47}
    3748
    3849#if ENABLE(MASM_PROBE)
  • releases/WebKitGTK/webkit-2.20/Source/JavaScriptCore/assembler/MacroAssembler.h

    r227643 r228638  
    6161
    6262#include "MacroAssemblerHelpers.h"
     63
     64namespace WTF {
     65
     66template<typename FunctionType>
     67class ScopedLambda;
     68
     69} // namespace WTF
    6370
    6471namespace JSC {
     
    18841891        urshift32(src, trustedImm32ForShift(amount), dest);
    18851892    }
     1893
     1894    // If the result jump is taken that means the assert passed.
     1895    void jitAssert(const WTF::ScopedLambda<Jump(void)>&);
    18861896
    18871897#if ENABLE(MASM_PROBE)
  • releases/WebKitGTK/webkit-2.20/Source/JavaScriptCore/jit/JITPropertyAccess.cpp

    r227874 r228638  
    4444#include "SlowPathCall.h"
    4545#include "StructureStubInfo.h"
     46#include <wtf/ScopedLambda.h>
    4647#include <wtf/StringPrintStream.h>
    4748
     
    858859        case GlobalProperty:
    859860        case GlobalPropertyWithVarInjectionChecks: {
    860             emitLoadWithStructureCheck(scope, structureSlot); // Structure check covers var injection.
     861            emitLoadWithStructureCheck(scope, structureSlot); // Structure check covers var injection since we don't cache structures for anything but the GlobalObject. Additionally, resolve_scope handles checking for the var injection.
    861862            GPRReg base = regT0;
    862863            GPRReg result = regT0;
    863864            GPRReg offset = regT1;
    864865            GPRReg scratch = regT2;
    865            
     866
     867            jitAssert(scopedLambda<Jump(void)>([&] () -> Jump {
     868                return branchPtr(Equal, base, TrustedImmPtr(m_codeBlock->globalObject()));
     869            }));
     870
    866871            load32(operandSlot, offset);
    867872            if (!ASSERT_DISABLED) {
     
    986991        case GlobalProperty:
    987992        case GlobalPropertyWithVarInjectionChecks: {
    988             emitLoadWithStructureCheck(scope, structureSlot); // Structure check covers var injection.
     993            emitLoadWithStructureCheck(scope, structureSlot); // Structure check covers var injection since we don't cache structures for anything but the GlobalObject. Additionally, resolve_scope handles checking for the var injection.
    989994            emitGetVirtualRegister(value, regT2);
    990            
     995
     996            jitAssert(scopedLambda<Jump(void)>([&] () -> Jump {
     997                return branchPtr(Equal, regT0, TrustedImmPtr(m_codeBlock->globalObject()));
     998            }));
     999
    9911000            loadPtr(Address(regT0, JSObject::butterflyOffset()), regT0);
    9921001            loadPtr(operandSlot, regT1);
  • releases/WebKitGTK/webkit-2.20/Source/JavaScriptCore/runtime/CommonSlowPaths.h

    r226310 r228638  
    139139   
    140140    if (resolveType == GlobalProperty || resolveType == GlobalPropertyWithVarInjectionChecks) {
     141        JSGlobalObject* globalObject = codeBlock->globalObject();
     142        ASSERT(globalObject == scope || globalObject->varInjectionWatchpoint()->hasBeenInvalidated());
    141143        if (!slot.isCacheablePut()
    142144            || slot.base() != scope
     145            || scope != globalObject
    143146            || !scope->structure()->propertyAccessesAreCacheable())
    144147            return;
     
    184187
    185188    // Covers implicit globals. Since they don't exist until they first execute, we didn't know how to cache them at compile time.
    186     if (slot.isCacheableValue() && slot.slotBase() == scope && scope->structure()->propertyAccessesAreCacheable()) {
    187         if (resolveType == GlobalProperty || resolveType == GlobalPropertyWithVarInjectionChecks) {
    188             CodeBlock* codeBlock = exec->codeBlock();
     189    if (resolveType == GlobalProperty || resolveType == GlobalPropertyWithVarInjectionChecks) {
     190        CodeBlock* codeBlock = exec->codeBlock();
     191        JSGlobalObject* globalObject = codeBlock->globalObject();
     192        ASSERT(scope == globalObject || globalObject->varInjectionWatchpoint()->hasBeenInvalidated());
     193        if (slot.isCacheableValue() && slot.slotBase() == scope && scope == globalObject && scope->structure()->propertyAccessesAreCacheable()) {
    189194            Structure* structure = scope->structure(vm);
    190195            {
  • releases/WebKitGTK/webkit-2.20/Source/JavaScriptCore/runtime/Options.h

    r227617 r228638  
    251251    v(bool, ftlCrashes, false, Normal, nullptr) /* fool-proof way of checking that you ended up in the FTL. ;-) */\
    252252    v(bool, clobberAllRegsInFTLICSlowPath, !ASSERT_DISABLED, Normal, nullptr) \
     253    v(bool, enableJITDebugAssetions, !ASSERT_DISABLED, Normal, nullptr) \
    253254    v(bool, useAccessInlining, true, Normal, nullptr) \
    254255    v(unsigned, maxAccessVariantListSize, 8, Normal, nullptr) \
Note: See TracChangeset for help on using the changeset viewer.