Bug 182419 – Fix broken bounds check in FTL's compileGetMyArgumentByVal().

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Lam]

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.

<rdar://problem/37044945>

[Mark Lam]

Created attachment 332937 [details]
proposed patch.

[Saam Barati]

Comment on attachment 332937 [details]
proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=332937&action=review

r=me

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:4023
> +            CheckValue* check = m_out.speculateAdd(indexToCheck, m_out.constInt32(m_node->numberOfArgumentsToSkip()));

It’d be great to get a test that triggers this overflow

[Mark Lam]

Comment on attachment 332937 [details]
proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=332937&action=review

Thanks for the review.

>> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:4023
>> +            CheckValue* check = m_out.speculateAdd(indexToCheck, m_out.constInt32(m_node->numberOfArgumentsToSkip()));
> 
> It’d be great to get a test that triggers this overflow

I've added this case to the test.

[Mark Lam]

Created attachment 332938 [details]
patch for landing.

[Mark Lam]

Landed in r227998: <http://trac.webkit.org/r227998>.