Merge r246071 - Argument elimination should check for negative indice…

…s in GetByVal

https://bugs.webkit.org/show_bug.cgi?id=198302
<rdar://problem/51188095>

Reviewed by Filip Pizlo.

JSTests:

* stress/eliminate-arguments-negative-rest-access.js: Added.
(inlinee):
(opt):

Source/JavaScriptCore:

In DFG::ArgumentEliminationPhase, the index is treated as unsigned, but there's no check
for overflow in the addition. In compileGetMyArgumentByVal, there's a check for overflow,
but the index is treated as signed, resulting in an index lower than numberOfArgumentsToSkip.

* dfg/DFGArgumentsEliminationPhase.cpp:
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileGetMyArgumentByVal):

JSTests/ChangeLog

@@ -1,3 +1,15 @@
+2019-06-04  Tadeu Zagallo  <tzagallo@apple.com>
+
+        Argument elimination should check for negative indices in GetByVal
+        https://bugs.webkit.org/show_bug.cgi?id=198302
+        <rdar://problem/51188095>
+
+        Reviewed by Filip Pizlo.
+
+        * stress/eliminate-arguments-negative-rest-access.js: Added.
+        (inlinee):
+        (opt):
+
 2019-06-10  Tadeu Zagallo  <tzagallo@apple.com>
 
         AI BitURShift's result should not be unsigned

JSTests/stress/eliminate-arguments-negative-rest-access.js

@@ -0,0 +1,16 @@
+//@ requireOptions("--forceEagerCompilation=1")
+
+function inlinee(index, value, ...rest) {
+    return rest[index | 0];
+}
+
+function opt() {
+    return inlinee(-1, 0x1234);
+}
+noInline(opt);
+
+for (let i = 0; i < 1e6; i++) {
+    const value = opt();
+    if (value !== undefined)
+        throw new Error(`${i}: ${value}`);
+}

Source/JavaScriptCore/ChangeLog

@@ -1,3 +1,19 @@
+2019-06-04  Tadeu Zagallo  <tzagallo@apple.com>
+
+        Argument elimination should check for negative indices in GetByVal
+        https://bugs.webkit.org/show_bug.cgi?id=198302
+        <rdar://problem/51188095>
+
+        Reviewed by Filip Pizlo.
+
+        In DFG::ArgumentEliminationPhase, the index is treated as unsigned, but there's no check
+        for overflow in the addition. In compileGetMyArgumentByVal, there's a check for overflow,
+        but the index is treated as signed, resulting in an index lower than numberOfArgumentsToSkip.
+
+        * dfg/DFGArgumentsEliminationPhase.cpp:
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileGetMyArgumentByVal):
+
 2019-06-10  Tadeu Zagallo  <tzagallo@apple.com>
 
         AI BitURShift's result should not be unsigned

Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp

@@ -756,11 +756,11 @@ class ArgumentsEliminationPhase : public Phase {
                         InlineCallFrame* inlineCallFrame = candidate->origin.semantic.inlineCallFrame;
                         index += numberOfArgumentsToSkip;
 
-                        bool safeToGetStack;
+                        bool safeToGetStack = index >= numberOfArgumentsToSkip;
                         if (inlineCallFrame)
-                            safeToGetStack = index < inlineCallFrame->argumentCountIncludingThis - 1;
+                            safeToGetStack &= index < inlineCallFrame->argumentCountIncludingThis - 1;
                         else {
-                            safeToGetStack =
+                            safeToGetStack &=
                                 index < static_cast<unsigned>(codeBlock()->numParameters()) - 1;
                         }
                         if (safeToGetStack) {

Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

@@ -4254,13 +4254,15 @@ class LowerDFGToB3 {
 
         LValue numberOfArgs = m_out.sub(numberOfArgsIncludingThis, m_out.int32One);
         LValue indexToCheck = originalIndex;
+        LValue numberOfArgumentsToSkip = m_out.int32Zero;
         if (m_node->numberOfArgumentsToSkip()) {
-            CheckValue* check = m_out.speculateAdd(indexToCheck, m_out.constInt32(m_node->numberOfArgumentsToSkip()));
+            numberOfArgumentsToSkip = m_out.constInt32(m_node->numberOfArgumentsToSkip());
+            CheckValue* check = m_out.speculateAdd(indexToCheck, numberOfArgumentsToSkip);
             blessSpeculation(check, Overflow, noValue(), nullptr, m_origin);
             indexToCheck = check;
         }
 
-        LValue isOutOfBounds = m_out.aboveOrEqual(indexToCheck, numberOfArgs);
+        LValue isOutOfBounds = m_out.bitOr(m_out.aboveOrEqual(indexToCheck, numberOfArgs), m_out.below(indexToCheck, numberOfArgumentsToSkip));
         LBasicBlock continuation = nullptr;
         LBasicBlock lastNext = nullptr;
         ValueFromBlock slowResult;