Title: [250271] branches/safari-608-branch
Revision
250271
Author
alanc...@apple.com
Date
2019-09-23 17:13:27 -0700 (Mon, 23 Sep 2019)

Log Message

Cherry-pick r250116. rdar://problem/55608003

    [JSC] DFG op_call_varargs should not assume that one-previous-local of freeReg is usable
    https://bugs.webkit.org/show_bug.cgi?id=202014

    Reviewed by Saam Barati.

    JSTests:

    * stress/call-varargs-inlining-should-not-clobber-previous-to-free-register.js: Added.
    (__v0):

    Source/_javascript_Core:

    Let's look into the bytecode generated by the test.

        [   0] enter
        [   1] get_scope          loc4
        [   3] mov                loc5, loc4
        [   6] check_traps
        [   7] mov                loc6, callee
        [  10] create_direct_arguments loc7
        [  12] to_this            this
        [  15] mov                loc8, loc7
        [  18] mov                loc9, loc6
        [  21] mov                loc12, Undefined(const0)
        [  24] get_by_id          loc11, loc6, 0
        [  29] jneq_ptr           loc11, ApplyFunction, 18(->47)
        [  34] mov                loc11, loc6
        [  37] call_varargs       loc11, loc11, this, loc8, loc13, 0
        [  45] jmp                17(->62)
        [  47] mov                loc16, loc6
        [  50] mov                loc15, this
        [  53] mov                loc14, loc8
        [  56] call               loc11, loc11, 3, 22
        ...

    call_varargs uses loc13 as firstFreeReg (first usable bottom register in the current stack-frame to spread variadic arguments after this).
    This is correct. And call_varargs uses |this| as this argument for the call_varargs. This |this| argument is not in a region starting from loc13.
    And it is not in the previous place to loc13 (|this| is not loc12).

    On the other hand, DFG::ByteCodeParser's inlining path is always assuming that the previous to firstFreeReg is usable and part of arguments.
    But this is wrong. loc12 in the above bytecode is used for `[  56] call               loc11, loc11, 3, 22`'s argument later, and this call assumes
    that loc12 is not clobbered by call_varargs. But DFG and FTL clobbers it.

    The test is recursively calling the same function, and we inline the same function one-level. And stack-overflow error happens when inlined
    CallForwardVarargs (from op_call_varargs) is called. FTL recovers the frames, and at this point, outer function's loc12 is recovered to garbage since
    LoadVarargs clobbers it. And we eventually use it and crash.

        60:<!0:-> LoadVarargs(Check:Untyped:Kill:@30, MustGen, start = loc13, count = loc15, machineStart = loc7, machineCount = loc9, offset = 0, mandatoryMinimum = 0, limit = 2, R:World, W:Stack(-16),Stack(-14),Stack(-13),Heap, Exits, ClobbersExit, bc#37, ExitValid)

    This LoadVarargs clobbers loc12, loc13, and loc15 while loc12 is used.

    In all the tiers, op_call_varargs first allocates enough region to hold varargs including |this|. And we store |this| value to a correct place.
    DFG should not assume that the previous register to firstFreeReg is used for |this|.

    This patch fixes DFG::ByteCodeParser's stack region calculation for op_call_varargs inlining. And we rename maxNumArguments to maxArgumentCountIncludingThis to
    represent that `maxArgumentCountIncludingThis` includes |this| count.

    * bytecode/CallLinkInfo.cpp:
    (JSC::CallLinkInfo::setMaxArgumentCountIncludingThis):
    (JSC::CallLinkInfo::setMaxNumArguments): Deleted.
    * bytecode/CallLinkInfo.h:
    (JSC::CallLinkInfo::addressOfMaxArgumentCountIncludingThis):
    (JSC::CallLinkInfo::maxArgumentCountIncludingThis):
    (JSC::CallLinkInfo::addressOfMaxNumArguments): Deleted.
    (JSC::CallLinkInfo::maxNumArguments): Deleted.
    * bytecode/CallLinkStatus.cpp:
    (JSC::CallLinkStatus::computeFor):
    (JSC::CallLinkStatus::dump const):
    * bytecode/CallLinkStatus.h:
    (JSC::CallLinkStatus::maxArgumentCountIncludingThis const):
    (JSC::CallLinkStatus::maxNumArguments const): Deleted.
    * dfg/DFGByteCodeParser.cpp:
    (JSC::DFG::ByteCodeParser::handleVarargsInlining):
    * dfg/DFGSpeculativeJIT32_64.cpp:
    (JSC::DFG::SpeculativeJIT::emitCall):
    * dfg/DFGSpeculativeJIT64.cpp:
    (JSC::DFG::SpeculativeJIT::emitCall):
    * ftl/FTLLowerDFGToB3.cpp:
    (JSC::FTL::DFG::LowerDFGToB3::compileDirectCallOrConstruct):
    * jit/JITCall.cpp:
    (JSC::JIT::compileSetupFrame):
    * jit/JITCall32_64.cpp:
    (JSC::JIT::compileSetupFrame):
    * jit/JITOperations.cpp:

    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@250116 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Modified Paths

Added Paths

Diff

Modified: branches/safari-608-branch/JSTests/ChangeLog (250270 => 250271)


--- branches/safari-608-branch/JSTests/ChangeLog	2019-09-24 00:13:22 UTC (rev 250270)
+++ branches/safari-608-branch/JSTests/ChangeLog	2019-09-24 00:13:27 UTC (rev 250271)
@@ -1,3 +1,104 @@
+2019-09-23  Alan Coon  <alanc...@apple.com>
+
+        Cherry-pick r250116. rdar://problem/55608003
+
+    [JSC] DFG op_call_varargs should not assume that one-previous-local of freeReg is usable
+    https://bugs.webkit.org/show_bug.cgi?id=202014
+    
+    Reviewed by Saam Barati.
+    
+    JSTests:
+    
+    * stress/call-varargs-inlining-should-not-clobber-previous-to-free-register.js: Added.
+    (__v0):
+    
+    Source/_javascript_Core:
+    
+    Let's look into the bytecode generated by the test.
+    
+        [   0] enter
+        [   1] get_scope          loc4
+        [   3] mov                loc5, loc4
+        [   6] check_traps
+        [   7] mov                loc6, callee
+        [  10] create_direct_arguments loc7
+        [  12] to_this            this
+        [  15] mov                loc8, loc7
+        [  18] mov                loc9, loc6
+        [  21] mov                loc12, Undefined(const0)
+        [  24] get_by_id          loc11, loc6, 0
+        [  29] jneq_ptr           loc11, ApplyFunction, 18(->47)
+        [  34] mov                loc11, loc6
+        [  37] call_varargs       loc11, loc11, this, loc8, loc13, 0
+        [  45] jmp                17(->62)
+        [  47] mov                loc16, loc6
+        [  50] mov                loc15, this
+        [  53] mov                loc14, loc8
+        [  56] call               loc11, loc11, 3, 22
+        ...
+    
+    call_varargs uses loc13 as firstFreeReg (first usable bottom register in the current stack-frame to spread variadic arguments after this).
+    This is correct. And call_varargs uses |this| as this argument for the call_varargs. This |this| argument is not in a region starting from loc13.
+    And it is not in the previous place to loc13 (|this| is not loc12).
+    
+    On the other hand, DFG::ByteCodeParser's inlining path is always assuming that the previous to firstFreeReg is usable and part of arguments.
+    But this is wrong. loc12 in the above bytecode is used for `[  56] call               loc11, loc11, 3, 22`'s argument later, and this call assumes
+    that loc12 is not clobbered by call_varargs. But DFG and FTL clobbers it.
+    
+    The test is recursively calling the same function, and we inline the same function one-level. And stack-overflow error happens when inlined
+    CallForwardVarargs (from op_call_varargs) is called. FTL recovers the frames, and at this point, outer function's loc12 is recovered to garbage since
+    LoadVarargs clobbers it. And we eventually use it and crash.
+    
+        60:<!0:-> LoadVarargs(Check:Untyped:Kill:@30, MustGen, start = loc13, count = loc15, machineStart = loc7, machineCount = loc9, offset = 0, mandatoryMinimum = 0, limit = 2, R:World, W:Stack(-16),Stack(-14),Stack(-13),Heap, Exits, ClobbersExit, bc#37, ExitValid)
+    
+    This LoadVarargs clobbers loc12, loc13, and loc15 while loc12 is used.
+    
+    In all the tiers, op_call_varargs first allocates enough region to hold varargs including |this|. And we store |this| value to a correct place.
+    DFG should not assume that the previous register to firstFreeReg is used for |this|.
+    
+    This patch fixes DFG::ByteCodeParser's stack region calculation for op_call_varargs inlining. And we rename maxNumArguments to maxArgumentCountIncludingThis to
+    represent that `maxArgumentCountIncludingThis` includes |this| count.
+    
+    * bytecode/CallLinkInfo.cpp:
+    (JSC::CallLinkInfo::setMaxArgumentCountIncludingThis):
+    (JSC::CallLinkInfo::setMaxNumArguments): Deleted.
+    * bytecode/CallLinkInfo.h:
+    (JSC::CallLinkInfo::addressOfMaxArgumentCountIncludingThis):
+    (JSC::CallLinkInfo::maxArgumentCountIncludingThis):
+    (JSC::CallLinkInfo::addressOfMaxNumArguments): Deleted.
+    (JSC::CallLinkInfo::maxNumArguments): Deleted.
+    * bytecode/CallLinkStatus.cpp:
+    (JSC::CallLinkStatus::computeFor):
+    (JSC::CallLinkStatus::dump const):
+    * bytecode/CallLinkStatus.h:
+    (JSC::CallLinkStatus::maxArgumentCountIncludingThis const):
+    (JSC::CallLinkStatus::maxNumArguments const): Deleted.
+    * dfg/DFGByteCodeParser.cpp:
+    (JSC::DFG::ByteCodeParser::handleVarargsInlining):
+    * dfg/DFGSpeculativeJIT32_64.cpp:
+    (JSC::DFG::SpeculativeJIT::emitCall):
+    * dfg/DFGSpeculativeJIT64.cpp:
+    (JSC::DFG::SpeculativeJIT::emitCall):
+    * ftl/FTLLowerDFGToB3.cpp:
+    (JSC::FTL::DFG::LowerDFGToB3::compileDirectCallOrConstruct):
+    * jit/JITCall.cpp:
+    (JSC::JIT::compileSetupFrame):
+    * jit/JITCall32_64.cpp:
+    (JSC::JIT::compileSetupFrame):
+    * jit/JITOperations.cpp:
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@250116 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2019-09-19  Yusuke Suzuki  <ysuz...@apple.com>
+
+            [JSC] DFG op_call_varargs should not assume that one-previous-local of freeReg is usable
+            https://bugs.webkit.org/show_bug.cgi?id=202014
+
+            Reviewed by Saam Barati.
+
+            * stress/call-varargs-inlining-should-not-clobber-previous-to-free-register.js: Added.
+            (__v0):
+
 2019-09-17  Alan Coon  <alanc...@apple.com>
 
         Cherry-pick r249911. rdar://problem/55461405

Added: branches/safari-608-branch/JSTests/stress/call-varargs-inlining-should-not-clobber-previous-to-free-register.js (0 => 250271)


--- branches/safari-608-branch/JSTests/stress/call-varargs-inlining-should-not-clobber-previous-to-free-register.js	                        (rev 0)
+++ branches/safari-608-branch/JSTests/stress/call-varargs-inlining-should-not-clobber-previous-to-free-register.js	2019-09-24 00:13:27 UTC (rev 250271)
@@ -0,0 +1,9 @@
+//@ runDefault("--useConcurrentJIT=0", "--watchdog=4000", "--watchdog-exception-ok")
+(function __v0() {
+    try {
+        __v0(__v0.apply(this, arguments));
+    } catch (e) {
+        for (let i = 0; i < 10000; i++) {
+        }
+    }
+})(2);

Modified: branches/safari-608-branch/Source/_javascript_Core/ChangeLog (250270 => 250271)


--- branches/safari-608-branch/Source/_javascript_Core/ChangeLog	2019-09-24 00:13:22 UTC (rev 250270)
+++ branches/safari-608-branch/Source/_javascript_Core/ChangeLog	2019-09-24 00:13:27 UTC (rev 250271)
@@ -1,3 +1,174 @@
+2019-09-23  Alan Coon  <alanc...@apple.com>
+
+        Cherry-pick r250116. rdar://problem/55608003
+
+    [JSC] DFG op_call_varargs should not assume that one-previous-local of freeReg is usable
+    https://bugs.webkit.org/show_bug.cgi?id=202014
+    
+    Reviewed by Saam Barati.
+    
+    JSTests:
+    
+    * stress/call-varargs-inlining-should-not-clobber-previous-to-free-register.js: Added.
+    (__v0):
+    
+    Source/_javascript_Core:
+    
+    Let's look into the bytecode generated by the test.
+    
+        [   0] enter
+        [   1] get_scope          loc4
+        [   3] mov                loc5, loc4
+        [   6] check_traps
+        [   7] mov                loc6, callee
+        [  10] create_direct_arguments loc7
+        [  12] to_this            this
+        [  15] mov                loc8, loc7
+        [  18] mov                loc9, loc6
+        [  21] mov                loc12, Undefined(const0)
+        [  24] get_by_id          loc11, loc6, 0
+        [  29] jneq_ptr           loc11, ApplyFunction, 18(->47)
+        [  34] mov                loc11, loc6
+        [  37] call_varargs       loc11, loc11, this, loc8, loc13, 0
+        [  45] jmp                17(->62)
+        [  47] mov                loc16, loc6
+        [  50] mov                loc15, this
+        [  53] mov                loc14, loc8
+        [  56] call               loc11, loc11, 3, 22
+        ...
+    
+    call_varargs uses loc13 as firstFreeReg (first usable bottom register in the current stack-frame to spread variadic arguments after this).
+    This is correct. And call_varargs uses |this| as this argument for the call_varargs. This |this| argument is not in a region starting from loc13.
+    And it is not in the previous place to loc13 (|this| is not loc12).
+    
+    On the other hand, DFG::ByteCodeParser's inlining path is always assuming that the previous to firstFreeReg is usable and part of arguments.
+    But this is wrong. loc12 in the above bytecode is used for `[  56] call               loc11, loc11, 3, 22`'s argument later, and this call assumes
+    that loc12 is not clobbered by call_varargs. But DFG and FTL clobbers it.
+    
+    The test is recursively calling the same function, and we inline the same function one-level. And stack-overflow error happens when inlined
+    CallForwardVarargs (from op_call_varargs) is called. FTL recovers the frames, and at this point, outer function's loc12 is recovered to garbage since
+    LoadVarargs clobbers it. And we eventually use it and crash.
+    
+        60:<!0:-> LoadVarargs(Check:Untyped:Kill:@30, MustGen, start = loc13, count = loc15, machineStart = loc7, machineCount = loc9, offset = 0, mandatoryMinimum = 0, limit = 2, R:World, W:Stack(-16),Stack(-14),Stack(-13),Heap, Exits, ClobbersExit, bc#37, ExitValid)
+    
+    This LoadVarargs clobbers loc12, loc13, and loc15 while loc12 is used.
+    
+    In all the tiers, op_call_varargs first allocates enough region to hold varargs including |this|. And we store |this| value to a correct place.
+    DFG should not assume that the previous register to firstFreeReg is used for |this|.
+    
+    This patch fixes DFG::ByteCodeParser's stack region calculation for op_call_varargs inlining. And we rename maxNumArguments to maxArgumentCountIncludingThis to
+    represent that `maxArgumentCountIncludingThis` includes |this| count.
+    
+    * bytecode/CallLinkInfo.cpp:
+    (JSC::CallLinkInfo::setMaxArgumentCountIncludingThis):
+    (JSC::CallLinkInfo::setMaxNumArguments): Deleted.
+    * bytecode/CallLinkInfo.h:
+    (JSC::CallLinkInfo::addressOfMaxArgumentCountIncludingThis):
+    (JSC::CallLinkInfo::maxArgumentCountIncludingThis):
+    (JSC::CallLinkInfo::addressOfMaxNumArguments): Deleted.
+    (JSC::CallLinkInfo::maxNumArguments): Deleted.
+    * bytecode/CallLinkStatus.cpp:
+    (JSC::CallLinkStatus::computeFor):
+    (JSC::CallLinkStatus::dump const):
+    * bytecode/CallLinkStatus.h:
+    (JSC::CallLinkStatus::maxArgumentCountIncludingThis const):
+    (JSC::CallLinkStatus::maxNumArguments const): Deleted.
+    * dfg/DFGByteCodeParser.cpp:
+    (JSC::DFG::ByteCodeParser::handleVarargsInlining):
+    * dfg/DFGSpeculativeJIT32_64.cpp:
+    (JSC::DFG::SpeculativeJIT::emitCall):
+    * dfg/DFGSpeculativeJIT64.cpp:
+    (JSC::DFG::SpeculativeJIT::emitCall):
+    * ftl/FTLLowerDFGToB3.cpp:
+    (JSC::FTL::DFG::LowerDFGToB3::compileDirectCallOrConstruct):
+    * jit/JITCall.cpp:
+    (JSC::JIT::compileSetupFrame):
+    * jit/JITCall32_64.cpp:
+    (JSC::JIT::compileSetupFrame):
+    * jit/JITOperations.cpp:
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@250116 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2019-09-19  Yusuke Suzuki  <ysuz...@apple.com>
+
+            [JSC] DFG op_call_varargs should not assume that one-previous-local of freeReg is usable
+            https://bugs.webkit.org/show_bug.cgi?id=202014
+
+            Reviewed by Saam Barati.
+
+            Let's look into the bytecode generated by the test.
+
+                [   0] enter
+                [   1] get_scope          loc4
+                [   3] mov                loc5, loc4
+                [   6] check_traps
+                [   7] mov                loc6, callee
+                [  10] create_direct_arguments loc7
+                [  12] to_this            this
+                [  15] mov                loc8, loc7
+                [  18] mov                loc9, loc6
+                [  21] mov                loc12, Undefined(const0)
+                [  24] get_by_id          loc11, loc6, 0
+                [  29] jneq_ptr           loc11, ApplyFunction, 18(->47)
+                [  34] mov                loc11, loc6
+                [  37] call_varargs       loc11, loc11, this, loc8, loc13, 0
+                [  45] jmp                17(->62)
+                [  47] mov                loc16, loc6
+                [  50] mov                loc15, this
+                [  53] mov                loc14, loc8
+                [  56] call               loc11, loc11, 3, 22
+                ...
+
+            call_varargs uses loc13 as firstFreeReg (first usable bottom register in the current stack-frame to spread variadic arguments after this).
+            This is correct. And call_varargs uses |this| as this argument for the call_varargs. This |this| argument is not in a region starting from loc13.
+            And it is not in the previous place to loc13 (|this| is not loc12).
+
+            On the other hand, DFG::ByteCodeParser's inlining path is always assuming that the previous to firstFreeReg is usable and part of arguments.
+            But this is wrong. loc12 in the above bytecode is used for `[  56] call               loc11, loc11, 3, 22`'s argument later, and this call assumes
+            that loc12 is not clobbered by call_varargs. But DFG and FTL clobbers it.
+
+            The test is recursively calling the same function, and we inline the same function one-level. And stack-overflow error happens when inlined
+            CallForwardVarargs (from op_call_varargs) is called. FTL recovers the frames, and at this point, outer function's loc12 is recovered to garbage since
+            LoadVarargs clobbers it. And we eventually use it and crash.
+
+                60:<!0:-> LoadVarargs(Check:Untyped:Kill:@30, MustGen, start = loc13, count = loc15, machineStart = loc7, machineCount = loc9, offset = 0, mandatoryMinimum = 0, limit = 2, R:World, W:Stack(-16),Stack(-14),Stack(-13),Heap, Exits, ClobbersExit, bc#37, ExitValid)
+
+            This LoadVarargs clobbers loc12, loc13, and loc15 while loc12 is used.
+
+            In all the tiers, op_call_varargs first allocates enough region to hold varargs including |this|. And we store |this| value to a correct place.
+            DFG should not assume that the previous register to firstFreeReg is used for |this|.
+
+            This patch fixes DFG::ByteCodeParser's stack region calculation for op_call_varargs inlining. And we rename maxNumArguments to maxArgumentCountIncludingThis to
+            represent that `maxArgumentCountIncludingThis` includes |this| count.
+
+            * bytecode/CallLinkInfo.cpp:
+            (JSC::CallLinkInfo::setMaxArgumentCountIncludingThis):
+            (JSC::CallLinkInfo::setMaxNumArguments): Deleted.
+            * bytecode/CallLinkInfo.h:
+            (JSC::CallLinkInfo::addressOfMaxArgumentCountIncludingThis):
+            (JSC::CallLinkInfo::maxArgumentCountIncludingThis):
+            (JSC::CallLinkInfo::addressOfMaxNumArguments): Deleted.
+            (JSC::CallLinkInfo::maxNumArguments): Deleted.
+            * bytecode/CallLinkStatus.cpp:
+            (JSC::CallLinkStatus::computeFor):
+            (JSC::CallLinkStatus::dump const):
+            * bytecode/CallLinkStatus.h:
+            (JSC::CallLinkStatus::maxArgumentCountIncludingThis const):
+            (JSC::CallLinkStatus::maxNumArguments const): Deleted.
+            * dfg/DFGByteCodeParser.cpp:
+            (JSC::DFG::ByteCodeParser::handleVarargsInlining):
+            * dfg/DFGSpeculativeJIT32_64.cpp:
+            (JSC::DFG::SpeculativeJIT::emitCall):
+            * dfg/DFGSpeculativeJIT64.cpp:
+            (JSC::DFG::SpeculativeJIT::emitCall):
+            * ftl/FTLLowerDFGToB3.cpp:
+            (JSC::FTL::DFG::LowerDFGToB3::compileDirectCallOrConstruct):
+            * jit/JITCall.cpp:
+            (JSC::JIT::compileSetupFrame):
+            * jit/JITCall32_64.cpp:
+            (JSC::JIT::compileSetupFrame):
+            * jit/JITOperations.cpp:
+
 2019-09-17  Alan Coon  <alanc...@apple.com>
 
         Cherry-pick r249911. rdar://problem/55461405

Modified: branches/safari-608-branch/Source/_javascript_Core/bytecode/CallLinkInfo.cpp (250270 => 250271)


--- branches/safari-608-branch/Source/_javascript_Core/bytecode/CallLinkInfo.cpp	2019-09-24 00:13:22 UTC (rev 250270)
+++ branches/safari-608-branch/Source/_javascript_Core/bytecode/CallLinkInfo.cpp	2019-09-24 00:13:27 UTC (rev 250271)
@@ -190,11 +190,11 @@
     return jsCast<ExecutableBase*>(m_lastSeenCalleeOrExecutable.get());
 }
 
-void CallLinkInfo::setMaxNumArguments(unsigned value)
+void CallLinkInfo::setMaxArgumentCountIncludingThis(unsigned value)
 {
     RELEASE_ASSERT(isDirect());
     RELEASE_ASSERT(value);
-    m_maxNumArguments = value;
+    m_maxArgumentCountIncludingThis = value;
 }
 
 void CallLinkInfo::visitWeak(VM& vm)

Modified: branches/safari-608-branch/Source/_javascript_Core/bytecode/CallLinkInfo.h (250270 => 250271)


--- branches/safari-608-branch/Source/_javascript_Core/bytecode/CallLinkInfo.h	2019-09-24 00:13:22 UTC (rev 250270)
+++ branches/safari-608-branch/Source/_javascript_Core/bytecode/CallLinkInfo.h	2019-09-24 00:13:27 UTC (rev 250271)
@@ -295,17 +295,17 @@
         return static_cast<CallType>(m_callType);
     }
 
-    uint32_t* addressOfMaxNumArguments()
+    uint32_t* addressOfMaxArgumentCountIncludingThis()
     {
-        return &m_maxNumArguments;
+        return &m_maxArgumentCountIncludingThis;
     }
 
-    uint32_t maxNumArguments()
+    uint32_t maxArgumentCountIncludingThis()
     {
-        return m_maxNumArguments;
+        return m_maxArgumentCountIncludingThis;
     }
     
-    void setMaxNumArguments(unsigned);
+    void setMaxArgumentCountIncludingThis(unsigned);
 
     static ptrdiff_t offsetOfSlowPathCount()
     {
@@ -342,7 +342,7 @@
     }
 
 private:
-    uint32_t m_maxNumArguments { 0 }; // For varargs: the profiled maximum number of arguments. For direct: the number of stack slots allocated for arguments.
+    uint32_t m_maxArgumentCountIncludingThis { 0 }; // For varargs: the profiled maximum number of arguments. For direct: the number of stack slots allocated for arguments.
     CodeLocationLabel<JSInternalPtrTag> m_callReturnLocationOrPatchableJump;
     CodeLocationLabel<JSInternalPtrTag> m_hotPathBeginOrSlowPathStart;
     CodeLocationNearCall<JSInternalPtrTag> m_hotPathOther;

Modified: branches/safari-608-branch/Source/_javascript_Core/bytecode/CallLinkStatus.cpp (250270 => 250271)


--- branches/safari-608-branch/Source/_javascript_Core/bytecode/CallLinkStatus.cpp	2019-09-24 00:13:22 UTC (rev 250270)
+++ branches/safari-608-branch/Source/_javascript_Core/bytecode/CallLinkStatus.cpp	2019-09-24 00:13:27 UTC (rev 250271)
@@ -157,7 +157,7 @@
     UNUSED_PARAM(profiledBlock);
     
     CallLinkStatus result = computeFromCallLinkInfo(locker, callLinkInfo);
-    result.m_maxNumArguments = callLinkInfo.maxNumArguments();
+    result.m_maxArgumentCountIncludingThis = callLinkInfo.maxArgumentCountIncludingThis();
     return result;
 }
 
@@ -474,8 +474,8 @@
     if (!m_variants.isEmpty())
         out.print(comma, listDump(m_variants));
     
-    if (m_maxNumArguments)
-        out.print(comma, "maxNumArguments = ", m_maxNumArguments);
+    if (m_maxArgumentCountIncludingThis)
+        out.print(comma, "maxArgumentCountIncludingThis = ", m_maxArgumentCountIncludingThis);
 }
 
 } // namespace JSC

Modified: branches/safari-608-branch/Source/_javascript_Core/bytecode/CallLinkStatus.h (250270 => 250271)


--- branches/safari-608-branch/Source/_javascript_Core/bytecode/CallLinkStatus.h	2019-09-24 00:13:22 UTC (rev 250270)
+++ branches/safari-608-branch/Source/_javascript_Core/bytecode/CallLinkStatus.h	2019-09-24 00:13:27 UTC (rev 250271)
@@ -104,7 +104,7 @@
 
     bool isClosureCall() const; // Returns true if any callee is a closure call.
     
-    unsigned maxNumArguments() const { return m_maxNumArguments; }
+    unsigned maxArgumentCountIncludingThis() const { return m_maxArgumentCountIncludingThis; }
     
     bool finalize(VM&);
     
@@ -129,7 +129,7 @@
     bool m_couldTakeSlowPath { false };
     bool m_isProved { false };
     bool m_isBasedOnStub { false };
-    unsigned m_maxNumArguments { 0 };
+    unsigned m_maxArgumentCountIncludingThis { 0 };
 };
 
 } // namespace JSC

Modified: branches/safari-608-branch/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (250270 => 250271)


--- branches/safari-608-branch/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp	2019-09-24 00:13:22 UTC (rev 250270)
+++ branches/safari-608-branch/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp	2019-09-24 00:13:27 UTC (rev 250271)
@@ -1831,7 +1831,7 @@
     NodeType callOp, InlineCallFrame::Kind kind)
 {
     VERBOSE_LOG("Handling inlining (Varargs)...\nStack: ", currentCodeOrigin(), "\n");
-    if (callLinkStatus.maxNumArguments() > Options::maximumVarargsForInlining()) {
+    if (callLinkStatus.maxArgumentCountIncludingThis() > Options::maximumVarargsForInlining()) {
         VERBOSE_LOG("Bailing inlining: too many arguments for varargs inlining.\n");
         return false;
     }
@@ -1849,16 +1849,16 @@
         mandatoryMinimum = 0;
     
     // includes "this"
-    unsigned maxNumArguments = std::max(callLinkStatus.maxNumArguments(), mandatoryMinimum + 1);
+    unsigned maxArgumentCountIncludingThis = std::max(callLinkStatus.maxArgumentCountIncludingThis(), mandatoryMinimum + 1);
 
     CodeSpecializationKind specializationKind = InlineCallFrame::specializationKindFor(kind);
-    if (inliningCost(callVariant, maxNumArguments, kind) > getInliningBalance(callLinkStatus, specializationKind)) {
+    if (inliningCost(callVariant, maxArgumentCountIncludingThis, kind) > getInliningBalance(callLinkStatus, specializationKind)) {
         VERBOSE_LOG("Bailing inlining: inlining cost too high.\n");
         return false;
     }
     
-    int registerOffset = firstFreeReg + 1;
-    registerOffset -= maxNumArguments; // includes "this"
+    int registerOffset = firstFreeReg;
+    registerOffset -= maxArgumentCountIncludingThis;
     registerOffset -= CallFrame::headerSizeInRegisters;
     registerOffset = -WTF::roundUpToMultipleOf(stackAlignmentRegisters(), -registerOffset);
 
@@ -1879,7 +1879,7 @@
         data->start = VirtualRegister(remappedArgumentStart + 1);
         data->count = VirtualRegister(remappedRegisterOffset + CallFrameSlot::argumentCount);
         data->offset = argumentsOffset;
-        data->limit = maxNumArguments;
+        data->limit = maxArgumentCountIncludingThis;
         data->mandatoryMinimum = mandatoryMinimum;
         
         if (callOp == TailCallForwardVarargs)
@@ -1908,7 +1908,7 @@
         
         set(VirtualRegister(argumentStart), get(thisArgument), ImmediateNakedSet);
         unsigned numSetArguments = 0;
-        for (unsigned argument = 1; argument < maxNumArguments; ++argument) {
+        for (unsigned argument = 1; argument < maxArgumentCountIncludingThis; ++argument) {
             VariableAccessData* variable = newVariableAccessData(VirtualRegister(remappedArgumentStart + argument));
             variable->mergeShouldNeverUnbox(true); // We currently have nowhere to put the type check on the LoadVarargs. LoadVarargs is effectful, so after it finishes, we cannot exit.
             
@@ -1942,7 +1942,7 @@
     // those arguments. Even worse, if the intrinsic decides to exit, it won't really have anywhere to
     // exit to: LoadVarargs is effectful and it's part of the op_call_varargs, so we can't exit without
     // calling LoadVarargs twice.
-    inlineCall(callTargetNode, result, callVariant, registerOffset, maxNumArguments, kind, nullptr, insertChecks);
+    inlineCall(callTargetNode, result, callVariant, registerOffset, maxArgumentCountIncludingThis, kind, nullptr, insertChecks);
 
     for (VirtualRegister reg : setArgumentMaybes)
         setDirect(reg, jsConstant(jsUndefined()), ImmediateNakedSet);

Modified: branches/safari-608-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp (250270 => 250271)


--- branches/safari-608-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp	2019-09-24 00:13:22 UTC (rev 250270)
+++ branches/safari-608-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp	2019-09-24 00:13:27 UTC (rev 250271)
@@ -826,7 +826,7 @@
 
     if (isDirect) {
         info->setExecutableDuringCompilation(executable);
-        info->setMaxNumArguments(numAllocatedArgs);
+        info->setMaxArgumentCountIncludingThis(numAllocatedArgs);
 
         if (isTail) {
             RELEASE_ASSERT(node->op() == DirectTailCall);

Modified: branches/safari-608-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (250270 => 250271)


--- branches/safari-608-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp	2019-09-24 00:13:22 UTC (rev 250270)
+++ branches/safari-608-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp	2019-09-24 00:13:27 UTC (rev 250271)
@@ -776,7 +776,7 @@
     
     if (isDirect) {
         callLinkInfo->setExecutableDuringCompilation(executable);
-        callLinkInfo->setMaxNumArguments(numAllocatedArgs);
+        callLinkInfo->setMaxArgumentCountIncludingThis(numAllocatedArgs);
 
         if (isTail) {
             RELEASE_ASSERT(node->op() == DirectTailCall);

Modified: branches/safari-608-branch/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (250270 => 250271)


--- branches/safari-608-branch/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2019-09-24 00:13:22 UTC (rev 250270)
+++ branches/safari-608-branch/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2019-09-24 00:13:27 UTC (rev 250271)
@@ -8074,7 +8074,7 @@
                         CallLinkInfo::DirectTailCall, node->origin.semantic, InvalidGPRReg);
                     callLinkInfo->setExecutableDuringCompilation(executable);
                     if (numAllocatedArgs > numPassedArgs)
-                        callLinkInfo->setMaxNumArguments(numAllocatedArgs);
+                        callLinkInfo->setMaxArgumentCountIncludingThis(numAllocatedArgs);
                     
                     jit.addLinkTask(
                         [=] (LinkBuffer& linkBuffer) {
@@ -8108,7 +8108,7 @@
                     node->origin.semantic, InvalidGPRReg);
                 callLinkInfo->setExecutableDuringCompilation(executable);
                 if (numAllocatedArgs > numPassedArgs)
-                    callLinkInfo->setMaxNumArguments(numAllocatedArgs);
+                    callLinkInfo->setMaxArgumentCountIncludingThis(numAllocatedArgs);
                 
                 params.addLatePath(
                     [=] (CCallHelpers& jit) {

Modified: branches/safari-608-branch/Source/_javascript_Core/jit/JITCall.cpp (250270 => 250271)


--- branches/safari-608-branch/Source/_javascript_Core/jit/JITCall.cpp	2019-09-24 00:13:22 UTC (rev 250270)
+++ branches/safari-608-branch/Source/_javascript_Core/jit/JITCall.cpp	2019-09-24 00:13:27 UTC (rev 250271)
@@ -111,9 +111,9 @@
 
     // Profile the argument count.
     load32(Address(regT1, CallFrameSlot::argumentCount * static_cast<int>(sizeof(Register)) + PayloadOffset), regT2);
-    load32(info->addressOfMaxNumArguments(), regT0);
+    load32(info->addressOfMaxArgumentCountIncludingThis(), regT0);
     Jump notBiggest = branch32(Above, regT0, regT2);
-    store32(regT2, info->addressOfMaxNumArguments());
+    store32(regT2, info->addressOfMaxArgumentCountIncludingThis());
     notBiggest.link(this);
     
     // Initialize 'this'.

Modified: branches/safari-608-branch/Source/_javascript_Core/jit/JITCall32_64.cpp (250270 => 250271)


--- branches/safari-608-branch/Source/_javascript_Core/jit/JITCall32_64.cpp	2019-09-24 00:13:22 UTC (rev 250270)
+++ branches/safari-608-branch/Source/_javascript_Core/jit/JITCall32_64.cpp	2019-09-24 00:13:27 UTC (rev 250271)
@@ -202,9 +202,9 @@
 
     // Profile the argument count.
     load32(Address(regT1, CallFrameSlot::argumentCount * static_cast<int>(sizeof(Register)) + PayloadOffset), regT2);
-    load32(info->addressOfMaxNumArguments(), regT0);
+    load32(info->addressOfMaxArgumentCountIncludingThis(), regT0);
     Jump notBiggest = branch32(Above, regT0, regT2);
-    store32(regT2, info->addressOfMaxNumArguments());
+    store32(regT2, info->addressOfMaxArgumentCountIncludingThis());
     notBiggest.link(this);
     
     // Initialize 'this'.

Modified: branches/safari-608-branch/Source/_javascript_Core/jit/JITOperations.cpp (250270 => 250271)


--- branches/safari-608-branch/Source/_javascript_Core/jit/JITOperations.cpp	2019-09-24 00:13:22 UTC (rev 250270)
+++ branches/safari-608-branch/Source/_javascript_Core/jit/JITOperations.cpp	2019-09-24 00:13:27 UTC (rev 250271)
@@ -1139,7 +1139,7 @@
         EXCEPTION_ASSERT_UNUSED(throwScope, throwScope.exception() == error);
         if (UNLIKELY(error))
             return;
-        unsigned argumentStackSlots = callLinkInfo->maxNumArguments();
+        unsigned argumentStackSlots = callLinkInfo->maxArgumentCountIncludingThis();
         if (argumentStackSlots < static_cast<size_t>(codeBlock->numParameters()))
             codePtr = functionExecutable->entrypointFor(kind, MustCheckArity);
         else
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to