Changeset 246379 in webkit

Timestamp:

Jun 12, 2019, 3:20:54 PM (7 years ago)

Author:

Kocsen Chung

Message:

Cherry-pick r246071. rdar://problem/51656838

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

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

Location:

branches/safari-607-branch

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)

branches/safari-607-branch/JSTests/ChangeLog

@@ +1 @@
+2019-06-12  Null  <null@apple.com>
+
+        Cherry-pick r246071. rdar://problem/51656838
+
+    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):
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@246071 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    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-05-30  Kocsen Chung  <kocsen_chung@apple.com>
 

branches/safari-607-branch/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2019-06-12  Null  <null@apple.com>
+
+        Cherry-pick r246071. rdar://problem/51656838
+
+    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):
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@246071 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    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-12  Null  <null@apple.com>
 

branches/safari-607-branch/Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp

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

branches/safari-607-branch/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

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