- Revision
- 286312
- Author
- sbar...@apple.com
- Date
- 2021-11-30 12:00:44 -0800 (Tue, 30 Nov 2021)
Log Message
GetMyArgumentByValOutOfBounds needs to check for negative indices
https://bugs.webkit.org/show_bug.cgi?id=232966
<rdar://problem/85519898>
Reviewed by Yusuke Suzuki.
JSTests:
* get-my-argument-by-val-negative-1.js: Added.
* get-my-argument-by-val-negative-2.js: Added.
* get-my-argument-by-val-negative-3.js: Added.
Source/_javascript_Core:
Negative indices inside of GetMyArgumentByValOutOfBounds would cause
us to have the resulting value be undefined, instead of a full blown
lookup that properly consults the prototype chain and such. The reason for
this is negative indices would show up as "out of bounds", which would
lead this node to result in undefined. But negative indices really should
be treated as string property names, and can't be treated like normal out
of bounds positive integers.
This patch makes it so we speculate that we don't see negative indices. If
we do see negative indices, we stop performing the transformation inside
of arguments elimination so we don't end up in an OSR exit loop.
* dfg/DFGArgumentsEliminationPhase.cpp:
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileGetMyArgumentByVal):
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (286311 => 286312)
--- trunk/JSTests/ChangeLog 2021-11-30 19:52:28 UTC (rev 286311)
+++ trunk/JSTests/ChangeLog 2021-11-30 20:00:44 UTC (rev 286312)
@@ -1,3 +1,15 @@
+2021-11-30 Saam Barati <sbar...@apple.com>
+
+ GetMyArgumentByValOutOfBounds needs to check for negative indices
+ https://bugs.webkit.org/show_bug.cgi?id=232966
+ <rdar://problem/85519898>
+
+ Reviewed by Yusuke Suzuki.
+
+ * get-my-argument-by-val-negative-1.js: Added.
+ * get-my-argument-by-val-negative-2.js: Added.
+ * get-my-argument-by-val-negative-3.js: Added.
+
2021-11-29 Yusuke Suzuki <ysuz...@apple.com>
[JSC] jumpForTypedArrayOutOfBounds should use asAnyInt since it uses isAnyInt
Added: trunk/JSTests/get-my-argument-by-val-negative-1.js (0 => 286312)
--- trunk/JSTests/get-my-argument-by-val-negative-1.js (rev 0)
+++ trunk/JSTests/get-my-argument-by-val-negative-1.js 2021-11-30 20:00:44 UTC (rev 286312)
@@ -0,0 +1,25 @@
+function assert(b) {
+ if (!b)
+ throw new Error;
+}
+
+function main() {
+ let result;
+
+ const v13 = [0, 0];
+ Array.prototype[-80887344] = v13;
+
+ const func = (i, ...rest) => {
+ result = rest[i];
+ };
+ noInline(func);
+
+ for (let v30 = 0; v30 < 10000; v30++) {
+ func(0);
+ }
+
+ func(-80887344);
+ assert(result === v13);
+}
+noDFG(main);
+main();
Added: trunk/JSTests/get-my-argument-by-val-negative-2.js (0 => 286312)
--- trunk/JSTests/get-my-argument-by-val-negative-2.js (rev 0)
+++ trunk/JSTests/get-my-argument-by-val-negative-2.js 2021-11-30 20:00:44 UTC (rev 286312)
@@ -0,0 +1,25 @@
+function assert(b) {
+ if (!b)
+ throw new Error;
+}
+
+function main() {
+ let result;
+
+ const v13 = [0, 0];
+ Object.prototype[-80887344] = v13;
+
+ const func = function func(i) {
+ result = arguments[i];
+ };
+ noInline(func);
+
+ for (let v30 = 0; v30 < 10000; v30++) {
+ func(3);
+ }
+
+ func(-80887344);
+ assert(result === v13);
+}
+noDFG(main);
+main();
Added: trunk/JSTests/get-my-argument-by-val-negative-3.js (0 => 286312)
--- trunk/JSTests/get-my-argument-by-val-negative-3.js (rev 0)
+++ trunk/JSTests/get-my-argument-by-val-negative-3.js 2021-11-30 20:00:44 UTC (rev 286312)
@@ -0,0 +1,25 @@
+function assert(b) {
+ if (!b)
+ throw new Error;
+}
+
+function main() {
+ let result;
+
+ const v13 = [0, 0];
+ Array.prototype[-1] = v13;
+
+ const func = function func(i, ...args) {
+ result = args[i];
+ };
+ noInline(func);
+
+ for (let v30 = 0; v30 < 10000; v30++) {
+ func(1000, 10);
+ }
+
+ func(-1, 10);
+ assert(result === v13);
+}
+noDFG(main);
+main();
Modified: trunk/Source/_javascript_Core/ChangeLog (286311 => 286312)
--- trunk/Source/_javascript_Core/ChangeLog 2021-11-30 19:52:28 UTC (rev 286311)
+++ trunk/Source/_javascript_Core/ChangeLog 2021-11-30 20:00:44 UTC (rev 286312)
@@ -1,3 +1,27 @@
+2021-11-30 Saam Barati <sbar...@apple.com>
+
+ GetMyArgumentByValOutOfBounds needs to check for negative indices
+ https://bugs.webkit.org/show_bug.cgi?id=232966
+ <rdar://problem/85519898>
+
+ Reviewed by Yusuke Suzuki.
+
+ Negative indices inside of GetMyArgumentByValOutOfBounds would cause
+ us to have the resulting value be undefined, instead of a full blown
+ lookup that properly consults the prototype chain and such. The reason for
+ this is negative indices would show up as "out of bounds", which would
+ lead this node to result in undefined. But negative indices really should
+ be treated as string property names, and can't be treated like normal out
+ of bounds positive integers.
+
+ This patch makes it so we speculate that we don't see negative indices. If
+ we do see negative indices, we stop performing the transformation inside
+ of arguments elimination so we don't end up in an OSR exit loop.
+
+ * dfg/DFGArgumentsEliminationPhase.cpp:
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::compileGetMyArgumentByVal):
+
2021-11-30 Geza Lore <gl...@igalia.com>
[JSC] Unify most Baseline ops between JSVALUE64 and JSVALUE32_64
Modified: trunk/Source/_javascript_Core/dfg/DFGArgumentsEliminationPhase.cpp (286311 => 286312)
--- trunk/Source/_javascript_Core/dfg/DFGArgumentsEliminationPhase.cpp 2021-11-30 19:52:28 UTC (rev 286311)
+++ trunk/Source/_javascript_Core/dfg/DFGArgumentsEliminationPhase.cpp 2021-11-30 20:00:44 UTC (rev 286312)
@@ -312,7 +312,10 @@
break;
case GetByVal:
- escapeBasedOnArrayMode(node->arrayMode(), m_graph.varArgChild(node, 0), node);
+ if (m_graph.hasExitSite(node, NegativeIndex))
+ escape(m_graph.varArgChild(node, 0), node);
+ else
+ escapeBasedOnArrayMode(node->arrayMode(), m_graph.varArgChild(node, 0), node);
escape(m_graph.varArgChild(node, 1), node);
escape(m_graph.varArgChild(node, 2), node);
break;
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (286311 => 286312)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2021-11-30 19:52:28 UTC (rev 286311)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2021-11-30 20:00:44 UTC (rev 286312)
@@ -5652,18 +5652,18 @@
VirtualRegister argumentCountRegister = AssemblyHelpers::argumentCount(inlineCallFrame);
numberOfArgsIncludingThis = m_out.load32(payloadFor(argumentCountRegister));
}
+
+ speculate(NegativeIndex, noValue(), nullptr, m_out.lessThan(originalIndex, m_out.int32Zero));
LValue numberOfArgs = m_out.sub(numberOfArgsIncludingThis, m_out.int32One);
LValue indexToCheck = originalIndex;
- LValue numberOfArgumentsToSkip = m_out.int32Zero;
if (m_node->numberOfArgumentsToSkip()) {
- numberOfArgumentsToSkip = m_out.constInt32(m_node->numberOfArgumentsToSkip());
- CheckValue* check = m_out.speculateAdd(indexToCheck, numberOfArgumentsToSkip);
+ CheckValue* check = m_out.speculateAdd(indexToCheck, m_out.constInt32(m_node->numberOfArgumentsToSkip()));
blessSpeculation(check, Overflow, noValue(), nullptr, m_origin);
indexToCheck = check;
}
- LValue isOutOfBounds = m_out.bitOr(m_out.aboveOrEqual(indexToCheck, numberOfArgs), m_out.below(indexToCheck, numberOfArgumentsToSkip));
+ LValue isOutOfBounds = m_out.aboveOrEqual(indexToCheck, numberOfArgs);
LBasicBlock continuation = nullptr;
LBasicBlock lastNext = nullptr;
ValueFromBlock slowResult;
