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

Changeset 244067 in webkit


Ignore:
Timestamp:
Apr 8, 2019, 8:23:15 PM (7 years ago)
Author:
ysuzuki@apple.com
Message:

[JSC] DFG should respect node's strict flag
​https://bugs.webkit.org/show_bug.cgi?id=196617

Reviewed by Saam Barati.

JSTests:

  • stress/put-by-val-direct-should-respect-strict-mode-of-inlining-codeblock.js: Added.

(shouldEqual):
(makeUnwriteableUnconfigurableObject):
(runTest):

  • stress/put-dynamic-var-strict-and-sloppy.js: Added.

(shouldBe):
(shouldThrow):
(with.result):
(with.putValueStrict):
(with.putValueSloppy):

Source/JavaScriptCore:

We accidentally use codeBlock->isStrictMode() directly in DFG and FTL. But this is wrong since this CodeBlock is the top level DFG/FTL CodeBlock,
and this code does not respect the isStrictMode flag for the inlined CodeBlocks. In this patch, we start using isStrictModeFor(CodeOrigin) consistently
in DFG and FTL to get the right isStrictMode flag for the DFG node.
And we also split compilePutDynamicVar into compilePutDynamicVarStrict and compilePutDynamicVarNonStrict since (1) it is cleaner than accessing inlined
callframe in the operation function, and (2) it is aligned to the other functions like operationPutByValDirectNonStrict etc.
This bug is discovered by RandomizingFuzzerAgent by expanding the DFG coverage.

  • dfg/DFGAbstractInterpreterInlines.h:

(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):

  • dfg/DFGConstantFoldingPhase.cpp:

(JSC::DFG::ConstantFoldingPhase::foldConstants):

  • dfg/DFGFixupPhase.cpp:

(JSC::DFG::FixupPhase::fixupToThis):

  • dfg/DFGOperations.cpp:
  • dfg/DFGOperations.h:
  • dfg/DFGPredictionPropagationPhase.cpp:
  • dfg/DFGSpeculativeJIT.cpp:

(JSC::DFG::SpeculativeJIT::compileDoublePutByVal):
(JSC::DFG::SpeculativeJIT::compilePutByValForIntTypedArray):
(JSC::DFG::SpeculativeJIT::compilePutDynamicVar):
(JSC::DFG::SpeculativeJIT::compileToThis):

  • dfg/DFGSpeculativeJIT32_64.cpp:

(JSC::DFG::SpeculativeJIT::compileContiguousPutByVal):
(JSC::DFG::SpeculativeJIT::compile):

  • dfg/DFGSpeculativeJIT64.cpp:

(JSC::DFG::SpeculativeJIT::compile):

  • ftl/FTLLowerDFGToB3.cpp:

(JSC::FTL::DFG::LowerDFGToB3::compilePutByVal):
(JSC::FTL::DFG::LowerDFGToB3::compilePutDynamicVar):

Location:
trunk
Files:
2 added
12 edited

Legend:

Unmodified
Added
Removed

  • trunk/JSTests/ChangeLog

    r244058 r244067  
     12019-04-08  Yusuke Suzuki  <ysuzuki@apple.com>
     2
     3        [JSC] DFG should respect node's strict flag
     4        https://bugs.webkit.org/show_bug.cgi?id=196617
     5
     6        Reviewed by Saam Barati.
     7
     8        * stress/put-by-val-direct-should-respect-strict-mode-of-inlining-codeblock.js: Added.
     9        (shouldEqual):
     10        (makeUnwriteableUnconfigurableObject):
     11        (runTest):
     12        * stress/put-dynamic-var-strict-and-sloppy.js: Added.
     13        (shouldBe):
     14        (shouldThrow):
     15        (with.result):
     16        (with.putValueStrict):
     17        (with.putValueSloppy):
     18
    1192019-04-08  Yusuke Suzuki  <ysuzuki@apple.com>
    220

  • trunk/Source/JavaScriptCore/ChangeLog

    r244065 r244067  
     12019-04-08  Yusuke Suzuki  <ysuzuki@apple.com>
     2
     3        [JSC] DFG should respect node's strict flag
     4        https://bugs.webkit.org/show_bug.cgi?id=196617
     5
     6        Reviewed by Saam Barati.
     7
     8        We accidentally use codeBlock->isStrictMode() directly in DFG and FTL. But this is wrong since this CodeBlock is the top level DFG/FTL CodeBlock,
     9        and this code does not respect the isStrictMode flag for the inlined CodeBlocks. In this patch, we start using isStrictModeFor(CodeOrigin) consistently
     10        in DFG and FTL to get the right isStrictMode flag for the DFG node.
     11        And we also split compilePutDynamicVar into compilePutDynamicVarStrict and compilePutDynamicVarNonStrict since (1) it is cleaner than accessing inlined
     12        callframe in the operation function, and (2) it is aligned to the other functions like operationPutByValDirectNonStrict etc.
     13        This bug is discovered by RandomizingFuzzerAgent by expanding the DFG coverage.
     14
     15        * dfg/DFGAbstractInterpreterInlines.h:
     16        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
     17        * dfg/DFGConstantFoldingPhase.cpp:
     18        (JSC::DFG::ConstantFoldingPhase::foldConstants):
     19        * dfg/DFGFixupPhase.cpp:
     20        (JSC::DFG::FixupPhase::fixupToThis):
     21        * dfg/DFGOperations.cpp:
     22        * dfg/DFGOperations.h:
     23        * dfg/DFGPredictionPropagationPhase.cpp:
     24        * dfg/DFGSpeculativeJIT.cpp:
     25        (JSC::DFG::SpeculativeJIT::compileDoublePutByVal):
     26        (JSC::DFG::SpeculativeJIT::compilePutByValForIntTypedArray):
     27        (JSC::DFG::SpeculativeJIT::compilePutDynamicVar):
     28        (JSC::DFG::SpeculativeJIT::compileToThis):
     29        * dfg/DFGSpeculativeJIT32_64.cpp:
     30        (JSC::DFG::SpeculativeJIT::compileContiguousPutByVal):
     31        (JSC::DFG::SpeculativeJIT::compile):
     32        * dfg/DFGSpeculativeJIT64.cpp:
     33        (JSC::DFG::SpeculativeJIT::compile):
     34        * ftl/FTLLowerDFGToB3.cpp:
     35        (JSC::FTL::DFG::LowerDFGToB3::compilePutByVal):
     36        (JSC::FTL::DFG::LowerDFGToB3::compilePutDynamicVar):
     37
    1382019-04-08  Don Olmstead  <don.olmstead@sony.com>
    239

  • trunk/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h

    r243418 r244067  
    25322532        AbstractValue& source = forNode(node->child1());
    25332533        AbstractValue& destination = forNode(node);
    2534         bool strictMode = m_graph.executableFor(node->origin.semantic)->isStrictMode();
     2534        bool strictMode = m_graph.isStrictModeFor(node->origin.semantic);
    25352535
    25362536        ToThisResult result = isToThisAnIdentity(m_vm, strictMode, source);

  • trunk/Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp

    r243232 r244067  
    696696
    697697            case ToThis: {
    698                 ToThisResult result = isToThisAnIdentity(m_graph.m_vm, m_graph.executableFor(node->origin.semantic)->isStrictMode(), m_state.forNode(node->child1()));
     698                ToThisResult result = isToThisAnIdentity(m_graph.m_vm, m_graph.isStrictModeFor(node->origin.semantic), m_state.forNode(node->child1()));
    699699                if (result == ToThisResult::Identity) {
    700700                    node->convertToIdentity();

  • trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp

    r243232 r244067  
    26562656    void fixupToThis(Node* node)
    26572657    {
    2658         ECMAMode ecmaMode = m_graph.executableFor(node->origin.semantic)->isStrictMode() ? StrictMode : NotStrictMode;
    2659 
    2660         if (ecmaMode == StrictMode) {
     2658        bool isStrictMode = m_graph.isStrictModeFor(node->origin.semantic);
     2659
     2660        if (isStrictMode) {
    26612661            if (node->child1()->shouldSpeculateBoolean()) {
    26622662                fixEdge<BooleanUse>(node->child1());
    … …  
    27112711
    27122712        if (node->child1()->shouldSpeculateOther()) {
    2713             if (ecmaMode == StrictMode) {
     2713            if (isStrictMode) {
    27142714                fixEdge<OtherUse>(node->child1());
    27152715                node->convertToIdentity();

  • trunk/Source/JavaScriptCore/dfg/DFGOperations.cpp

    r243835 r244067  
    28762876}
    28772877
    2878 void JIT_OPERATION operationPutDynamicVar(ExecState* exec, JSObject* scope, EncodedJSValue value, UniquedStringImpl* impl, unsigned getPutInfoBits)
    2879 {
    2880     VM& vm = exec->vm();
    2881     NativeCallFrameTracer tracer(&vm, exec);
     2878ALWAYS_INLINE static void putDynamicVar(ExecState* exec, VM& vm, JSObject* scope, EncodedJSValue value, UniquedStringImpl* impl, unsigned getPutInfoBits, bool isStrictMode)
     2879{
    28822880    auto throwScope = DECLARE_THROW_SCOPE(vm);
    28832881
    … …  
    29032901    }
    29042902
    2905     CodeOrigin origin = exec->codeOrigin();
    2906     auto* inlineCallFrame = origin.inlineCallFrame();
    2907     bool strictMode;
    2908     if (inlineCallFrame)
    2909         strictMode = inlineCallFrame->baselineCodeBlock->isStrictMode();
    2910     else
    2911         strictMode = exec->codeBlock()->isStrictMode();
    2912     PutPropertySlot slot(scope, strictMode, PutPropertySlot::UnknownContext, isInitialization(getPutInfo.initializationMode()));
     2903    PutPropertySlot slot(scope, isStrictMode, PutPropertySlot::UnknownContext, isInitialization(getPutInfo.initializationMode()));
    29132904    throwScope.release();
    29142905    scope->methodTable(vm)->put(scope, exec, ident, JSValue::decode(value), slot);
     2906}
     2907
     2908void JIT_OPERATION operationPutDynamicVarStrict(ExecState* exec, JSObject* scope, EncodedJSValue value, UniquedStringImpl* impl, unsigned getPutInfoBits)
     2909{
     2910    VM& vm = exec->vm();
     2911    NativeCallFrameTracer tracer(&vm, exec);
     2912    constexpr bool isStrictMode = true;
     2913    return putDynamicVar(exec, vm, scope, value, impl, getPutInfoBits, isStrictMode);
     2914}
     2915
     2916void JIT_OPERATION operationPutDynamicVarNonStrict(ExecState* exec, JSObject* scope, EncodedJSValue value, UniquedStringImpl* impl, unsigned getPutInfoBits)
     2917{
     2918    VM& vm = exec->vm();
     2919    NativeCallFrameTracer tracer(&vm, exec);
     2920    constexpr bool isStrictMode = false;
     2921    return putDynamicVar(exec, vm, scope, value, impl, getPutInfoBits, isStrictMode);
    29152922}
    29162923

  • trunk/Source/JavaScriptCore/dfg/DFGOperations.h

    r242715 r244067  
    269269EncodedJSValue JIT_OPERATION operationResolveScopeForHoistingFuncDeclInEval(ExecState*, JSScope*, UniquedStringImpl*);
    270270EncodedJSValue JIT_OPERATION operationGetDynamicVar(ExecState*, JSObject* scope, UniquedStringImpl*, unsigned);
    271 void JIT_OPERATION operationPutDynamicVar(ExecState*, JSObject* scope, EncodedJSValue, UniquedStringImpl*, unsigned);
     271void JIT_OPERATION operationPutDynamicVarStrict(ExecState*, JSObject* scope, EncodedJSValue, UniquedStringImpl*, unsigned);
     272void JIT_OPERATION operationPutDynamicVarNonStrict(ExecState*, JSObject* scope, EncodedJSValue, UniquedStringImpl*, unsigned);
    272273
    273274int64_t JIT_OPERATION operationConvertBoxedDoubleToInt52(EncodedJSValue);

  • trunk/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp

    r242715 r244067  
    448448        case ToThis: {
    449449            // ToThis in methods for primitive types should speculate primitive types in strict mode.
    450             ECMAMode ecmaMode = m_graph.executableFor(node->origin.semantic)->isStrictMode() ? StrictMode : NotStrictMode;
    451             if (ecmaMode == StrictMode) {
     450            bool isStrictMode = m_graph.isStrictModeFor(node->origin.semantic);
     451            if (isStrictMode) {
    452452                if (node->child1()->shouldSpeculateBoolean()) {
    453453                    changed |= mergePrediction(SpecBoolean);
    … …  
    497497
    498498            SpeculatedType prediction = node->child1()->prediction();
    499             if (ecmaMode == StrictMode)
     499            if (isStrictMode)
    500500                changed |= mergePrediction(node->getHeapPrediction());
    501501            else if (prediction) {

  • trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

    r244058 r244067  
    21232123            slowPathCall(
    21242124                slowCase, this,
    2125                 m_jit.codeBlock()->isStrictMode()
     2125                m_jit.isStrictModeFor(node->origin.semantic)
    21262126                    ? (node->op() == PutByValDirect ? operationPutDoubleByValDirectBeyondArrayBoundsStrict : operationPutDoubleByValBeyondArrayBoundsStrict)
    21272127                    : (node->op() == PutByValDirect ? operationPutDoubleByValDirectBeyondArrayBoundsNonStrict : operationPutDoubleByValBeyondArrayBoundsNonStrict),
    … …  
    31543154            addSlowPathGenerator(slowPathCall(
    31553155                slowPathCases, this,
    3156                 m_jit.codeBlock()->isStrictMode() ? operationPutByValDirectCellStrict : operationPutByValDirectCellNonStrict,
     3156                m_jit.isStrictModeFor(node->origin.semantic) ? operationPutByValDirectCellStrict : operationPutByValDirectCellNonStrict,
    31573157                NoResult, base, JSValueRegs(propertyTagGPR, property), JSValueRegs(valueTagGPR, valueGPR)));
    31583158        } else {
    31593159            addSlowPathGenerator(slowPathCall(
    31603160                slowPathCases, this,
    3161                 m_jit.codeBlock()->isStrictMode() ? operationPutByValCellStrict : operationPutByValCellNonStrict,
     3161                m_jit.isStrictModeFor(node->origin.semantic) ? operationPutByValCellStrict : operationPutByValCellNonStrict,
    31623162                NoResult, base, JSValueRegs(propertyTagGPR, property), JSValueRegs(valueTagGPR, valueGPR)));
    31633163        }
    … …  
    1106111061
    1106211062    flushRegisters();
    11063     callOperation(operationPutDynamicVar, NoResult, scopeGPR, valueRegs, identifierUID(node->identifierNumber()), node->getPutInfo());
     11063    callOperation(m_jit.isStrictModeFor(node->origin.semantic) ? operationPutDynamicVarStrict : operationPutDynamicVarNonStrict, NoResult, scopeGPR, valueRegs, identifierUID(node->identifierNumber()), node->getPutInfo());
    1106411064    m_jit.exceptionCheck();
    1106511065    noResult(node);
    … …  
    1233012330
    1233112331    J_JITOperation_EJ function;
    12332     if (m_jit.graph().executableFor(node->origin.semantic)->isStrictMode())
     12332    if (m_jit.isStrictModeFor(node->origin.semantic))
    1233312333        function = operationToThisStrict;
    1233412334    else

  • trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp

    r243232 r244067  
    17861786            addSlowPathGenerator(slowPathCall(
    17871787                slowCase, this,
    1788                 m_jit.codeBlock()->isStrictMode() ? operationPutByValDirectBeyondArrayBoundsStrict : operationPutByValDirectBeyondArrayBoundsNonStrict,
     1788                m_jit.isStrictModeFor(node->origin.semantic) ? operationPutByValDirectBeyondArrayBoundsStrict : operationPutByValDirectBeyondArrayBoundsNonStrict,
    17891789                NoResult, baseReg, propertyReg, JSValueRegs(valueTag, valuePayloadReg)));
    17901790        } else {
    17911791            addSlowPathGenerator(slowPathCall(
    17921792                slowCase, this,
    1793                 m_jit.codeBlock()->isStrictMode() ? operationPutByValBeyondArrayBoundsStrict : operationPutByValBeyondArrayBoundsNonStrict,
     1793                m_jit.isStrictModeFor(node->origin.semantic) ? operationPutByValBeyondArrayBoundsStrict : operationPutByValBeyondArrayBoundsNonStrict,
    17941794                NoResult, baseReg, propertyReg, JSValueRegs(valueTag, valuePayloadReg)));
    17951795        }
    … …  
    25552555            flushRegisters();
    25562556            if (node->op() == PutByValDirect)
    2557                 callOperation(m_jit.codeBlock()->isStrictMode() ? operationPutByValDirectCellStrict : operationPutByValDirectCellNonStrict, baseGPR, propertyRegs, valueRegs);
     2557                callOperation(m_jit.isStrictModeFor(node->origin.semantic) ? operationPutByValDirectCellStrict : operationPutByValDirectCellNonStrict, baseGPR, propertyRegs, valueRegs);
    25582558            else
    2559                 callOperation(m_jit.codeBlock()->isStrictMode() ? operationPutByValCellStrict : operationPutByValCellNonStrict, baseGPR, propertyRegs, valueRegs);
     2559                callOperation(m_jit.isStrictModeFor(node->origin.semantic) ? operationPutByValCellStrict : operationPutByValCellNonStrict, baseGPR, propertyRegs, valueRegs);
    25602560            m_jit.exceptionCheck();
    25612561           
    … …  
    26682668                    addSlowPathGenerator(slowPathCall(
    26692669                        slowCases, this,
    2670                         m_jit.codeBlock()->isStrictMode() ? operationPutByValDirectBeyondArrayBoundsStrict : operationPutByValDirectBeyondArrayBoundsNonStrict,
     2670                        m_jit.isStrictModeFor(node->origin.semantic) ? operationPutByValDirectBeyondArrayBoundsStrict : operationPutByValDirectBeyondArrayBoundsNonStrict,
    26712671                        NoResult, baseReg, propertyReg, JSValueRegs(valueTagReg, valuePayloadReg)));
    26722672                } else {
    26732673                    addSlowPathGenerator(slowPathCall(
    26742674                        slowCases, this,
    2675                         m_jit.codeBlock()->isStrictMode() ? operationPutByValBeyondArrayBoundsStrict : operationPutByValBeyondArrayBoundsNonStrict,
     2675                        m_jit.isStrictModeFor(node->origin.semantic) ? operationPutByValBeyondArrayBoundsStrict : operationPutByValBeyondArrayBoundsNonStrict,
    26762676                        NoResult, baseReg, propertyReg, JSValueRegs(valueTagReg, valuePayloadReg)));
    26772677                }

  • trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp

    r243344 r244067  
    27272727                addSlowPathGenerator(slowPathCall(
    27282728                    slowCase, this,
    2729                     m_jit.codeBlock()->isStrictMode()
     2729                    m_jit.isStrictModeFor(node->origin.semantic)
    27302730                        ? (node->op() == PutByValDirect ? operationPutByValDirectBeyondArrayBoundsStrict : operationPutByValBeyondArrayBoundsStrict)
    27312731                        : (node->op() == PutByValDirect ? operationPutByValDirectBeyondArrayBoundsNonStrict : operationPutByValBeyondArrayBoundsNonStrict),
    … …  
    28112811                addSlowPathGenerator(slowPathCall(
    28122812                    slowCases, this,
    2813                     m_jit.codeBlock()->isStrictMode()
     2813                    m_jit.isStrictModeFor(node->origin.semantic)
    28142814                        ? (node->op() == PutByValDirect ? operationPutByValDirectBeyondArrayBoundsStrict : operationPutByValBeyondArrayBoundsStrict)
    28152815                        : (node->op() == PutByValDirect ? operationPutByValDirectBeyondArrayBoundsNonStrict : operationPutByValBeyondArrayBoundsNonStrict),

  • trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

    r243959 r244067  
    45664566               
    45674567                contiguousPutByValOutOfBounds(
    4568                     codeBlock()->isStrictMode()
     4568                    m_graph.isStrictModeFor(m_node->origin.semantic)
    45694569                        ? (m_node->op() == PutByValDirect ? operationPutByValDirectBeyondArrayBoundsStrict : operationPutByValBeyondArrayBoundsStrict)
    45704570                        : (m_node->op() == PutByValDirect ? operationPutByValDirectBeyondArrayBoundsNonStrict : operationPutByValBeyondArrayBoundsNonStrict),
    … …  
    45924592               
    45934593                contiguousPutByValOutOfBounds(
    4594                     codeBlock()->isStrictMode()
     4594                    m_graph.isStrictModeFor(m_node->origin.semantic)
    45954595                        ? (m_node->op() == PutByValDirect ? operationPutDoubleByValDirectBeyondArrayBoundsStrict : operationPutDoubleByValBeyondArrayBoundsStrict)
    45964596                        : (m_node->op() == PutByValDirect ? operationPutDoubleByValDirectBeyondArrayBoundsNonStrict : operationPutDoubleByValBeyondArrayBoundsNonStrict),
    … …  
    46324632                index, m_out.load32NonNegative(storage, m_heaps.ArrayStorage_vectorLength));
    46334633
    4634             auto slowPathFunction = codeBlock()->isStrictMode()
     4634            auto slowPathFunction = m_graph.isStrictModeFor(m_node->origin.semantic)
    46354635                ? (m_node->op() == PutByValDirect ? operationPutByValDirectBeyondArrayBoundsStrict : operationPutByValBeyondArrayBoundsStrict)
    46364636                : (m_node->op() == PutByValDirect ? operationPutByValDirectBeyondArrayBoundsNonStrict : operationPutByValBeyondArrayBoundsNonStrict);
    … …  
    1235612356    {
    1235712357        UniquedStringImpl* uid = m_graph.identifiers()[m_node->identifierNumber()];
    12358         setJSValue(vmCall(Void, m_out.operation(operationPutDynamicVar),
     12358        setJSValue(vmCall(Void, m_out.operation(m_graph.isStrictModeFor(m_node->origin.semantic) ? operationPutDynamicVarStrict : operationPutDynamicVarNonStrict),
    1235912359            m_callFrame, lowCell(m_node->child1()), lowJSValue(m_node->child2()), m_out.constIntPtr(uid), m_out.constInt32(m_node->getPutInfo())));
    1236012360    }
Note: See TracChangeset for help on using the changeset viewer.