- Revision
- 227998
- Author
- mark....@apple.com
- Date
- 2018-02-01 23:30:30 -0800 (Thu, 01 Feb 2018)
Log Message
Fix broken bounds check in FTL's compileGetMyArgumentByVal().
https://bugs.webkit.org/show_bug.cgi?id=182419
<rdar://problem/37044945>
Reviewed by Saam Barati.
JSTests:
* stress/regress-182419.js: Added.
Source/_javascript_Core:
In compileGetMyArgumentByVal(), it computes:
limit = m_out.sub(limit, m_out.constInt32(m_node->numberOfArgumentsToSkip()));
...
LValue isOutOfBounds = m_out.aboveOrEqual(originalIndex, limit);
where the original "limit" is the number of arguments passed in by the caller.
If the original limit is less than numberOfArgumentsToSkip, the resultant limit
will be a large unsigned number. As a result, this will defeat the bounds check
that follows it.
Note: later on in compileGetMyArgumentByVal(), we have to adjust adjust the index
value by adding numberOfArgumentsToSkip to it, in order to determine the actual
entry in the arguments array to get.
The fix is to just add numberOfArgumentsToSkip to index upfront (instead of
subtracting it from limit), and doing an overflow speculation check on that
addition before doing the bounds check.
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileGetMyArgumentByVal):
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (227997 => 227998)
--- trunk/JSTests/ChangeLog 2018-02-02 07:09:25 UTC (rev 227997)
+++ trunk/JSTests/ChangeLog 2018-02-02 07:30:30 UTC (rev 227998)
@@ -1,3 +1,13 @@
+2018-02-01 Mark Lam <mark....@apple.com>
+
+ Fix broken bounds check in FTL's compileGetMyArgumentByVal().
+ https://bugs.webkit.org/show_bug.cgi?id=182419
+ <rdar://problem/37044945>
+
+ Reviewed by Saam Barati.
+
+ * stress/regress-182419.js: Added.
+
2018-02-01 Keith Miller <keith_mil...@apple.com>
Fix crashes due to mishandling custom sections.
Added: trunk/JSTests/stress/regress-182419.js (0 => 227998)
--- trunk/JSTests/stress/regress-182419.js (rev 0)
+++ trunk/JSTests/stress/regress-182419.js 2018-02-02 07:30:30 UTC (rev 227998)
@@ -0,0 +1,28 @@
+function assertEqual(actual, expected) {
+ if (actual != expected)
+ throw "FAILED: expect " + expected + ", actual " + actual;
+}
+
+function test(index1, index2) {
+ function baz(a, b, c, ...args) {
+ return [args.length, args[index1], args[index2]];
+ }
+ function jaz(...args) {
+ return baz.apply(null, args);
+ }
+ noInline(jaz);
+
+ function check() {
+ let [length, a, b] = jaz();
+ assertEqual(length, 0);
+ assertEqual(a, undefined);
+ assertEqual(b, undefined);
+ }
+
+ for (let i = 0; i < 20000; i++) {
+ check();
+ }
+}
+
+test(0, 1);
+test(0x7fffffff, 0);
Modified: trunk/Source/_javascript_Core/ChangeLog (227997 => 227998)
--- trunk/Source/_javascript_Core/ChangeLog 2018-02-02 07:09:25 UTC (rev 227997)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-02-02 07:30:30 UTC (rev 227998)
@@ -1,3 +1,32 @@
+2018-02-01 Mark Lam <mark....@apple.com>
+
+ Fix broken bounds check in FTL's compileGetMyArgumentByVal().
+ https://bugs.webkit.org/show_bug.cgi?id=182419
+ <rdar://problem/37044945>
+
+ Reviewed by Saam Barati.
+
+ In compileGetMyArgumentByVal(), it computes:
+ limit = m_out.sub(limit, m_out.constInt32(m_node->numberOfArgumentsToSkip()));
+ ...
+ LValue isOutOfBounds = m_out.aboveOrEqual(originalIndex, limit);
+
+ where the original "limit" is the number of arguments passed in by the caller.
+ If the original limit is less than numberOfArgumentsToSkip, the resultant limit
+ will be a large unsigned number. As a result, this will defeat the bounds check
+ that follows it.
+
+ Note: later on in compileGetMyArgumentByVal(), we have to adjust adjust the index
+ value by adding numberOfArgumentsToSkip to it, in order to determine the actual
+ entry in the arguments array to get.
+
+ The fix is to just add numberOfArgumentsToSkip to index upfront (instead of
+ subtracting it from limit), and doing an overflow speculation check on that
+ addition before doing the bounds check.
+
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::compileGetMyArgumentByVal):
+
2018-02-01 Keith Miller <keith_mil...@apple.com>
Fix crashes due to mishandling custom sections.
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (227997 => 227998)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2018-02-02 07:09:25 UTC (rev 227997)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2018-02-02 07:30:30 UTC (rev 227998)
@@ -4009,20 +4009,23 @@
LValue originalIndex = lowInt32(m_node->child2());
- LValue originalLimit;
+ LValue numberOfArgsIncludingThis;
if (inlineCallFrame && !inlineCallFrame->isVarargs())
- originalLimit = m_out.constInt32(inlineCallFrame->argumentCountIncludingThis);
+ numberOfArgsIncludingThis = m_out.constInt32(inlineCallFrame->argumentCountIncludingThis);
else {
VirtualRegister argumentCountRegister = AssemblyHelpers::argumentCount(inlineCallFrame);
- originalLimit = m_out.load32(payloadFor(argumentCountRegister));
+ numberOfArgsIncludingThis = m_out.load32(payloadFor(argumentCountRegister));
}
- LValue limit = m_out.sub(originalLimit, m_out.int32One);
-
- if (m_node->numberOfArgumentsToSkip())
- limit = m_out.sub(limit, m_out.constInt32(m_node->numberOfArgumentsToSkip()));
-
- LValue isOutOfBounds = m_out.aboveOrEqual(originalIndex, limit);
+ LValue numberOfArgs = m_out.sub(numberOfArgsIncludingThis, m_out.int32One);
+ LValue indexToCheck = originalIndex;
+ if (m_node->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.aboveOrEqual(indexToCheck, numberOfArgs);
LBasicBlock continuation = nullptr;
LBasicBlock lastNext = nullptr;
ValueFromBlock slowResult;
@@ -4035,14 +4038,10 @@
lastNext = m_out.appendTo(normalCase, continuation);
} else
- speculate(OutOfBounds, noValue(), 0, isOutOfBounds);
+ speculate(OutOfBounds, noValue(), nullptr, isOutOfBounds);
- LValue index = originalIndex;
- if (m_node->numberOfArgumentsToSkip())
- index = m_out.add(index, m_out.constInt32(m_node->numberOfArgumentsToSkip()));
-
- index = m_out.add(index, m_out.int32One);
-
+ LValue index = m_out.add(indexToCheck, m_out.int32One);
+
TypedPointer base;
if (inlineCallFrame) {
if (inlineCallFrame->argumentCountIncludingThis > 1)
@@ -4055,7 +4054,7 @@
LValue pointer = m_out.baseIndex(
base.value(), m_out.zeroExt(index, pointerType()), ScaleEight);
result = m_out.load64(TypedPointer(m_heaps.variables.atAnyIndex(), pointer));
- result = preciseIndexMask32(result, originalIndex, limit);
+ result = preciseIndexMask32(result, indexToCheck, numberOfArgs);
} else
result = m_out.constInt64(JSValue::encode(jsUndefined()));
