Changeset 246071 in webkit

Timestamp:

Jun 4, 2019, 10:56:59 AM (7 years ago)

Author:

Tadeu Zagallo

Message:

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.

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):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/eliminate-arguments-negative-rest-access.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp (modified) (1 diff)
• Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp (modified) (1 diff)

trunk/JSTests/ChangeLog

@@ +1 @@
+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-03  Caio Lima  <ticaiolima@gmail.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+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-04  Tadeu Zagallo  <tzagallo@apple.com>
 

trunk/Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp

@@ -763 +763 @@
                         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;
                         }

trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

@@ -4462 +4462 @@
         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;