Bug 180366 – We need to leave room on the top of the stack for the FTL TailCall slow path so it doesn't overwrite things we want to retrieve when doing a stack walk when throwing an exception

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

[Saam Barati]

Otherwise, the slow path branch will overwrite things on the stack before calling into the slow path C call. Once we're in the C call, we realize we're actually going to throw an exception because we're trying to call something that isn't callable. Then we'll throw an exception, which does a stack walk. This stack walk will read the stack to recover the callee of inlined frames, which might have been overwritten with garbage already.

[Saam Barati]

I'm wondering if this is needed in the DFG as well. I suspect it is, but I have yet to prove it with a crashing test.

[Saam Barati]

<rdar://problem/35685877>

[Saam Barati]

(In reply to Saam Barati from comment #1)
> I'm wondering if this is needed in the DFG as well. I suspect it is, but I
> have yet to prove it with a crashing test.

The DFG already handles exactly this via Graph's m_parameterSlots which tracks the maximum outgoing call parameter count.

[Saam Barati]

Created attachment 328368 [details]
patch

[Mark Lam]

Comment on attachment 328368 [details]
patch

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

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:6994
> +        m_proc.requestCallArgAreaSizeInBytes(
> +            WTF::roundUpToMultipleOf(stackAlignmentBytes(), (CallFrame::headerSizeInRegisters + numArgs) * sizeof(EncodedJSValue)));

AirCode::requestCallArgAreaSizeInBytes() already takes care of aligning to stackAlignmentBytes().  So, no need to do that here.

[Saam Barati]

Comment on attachment 328368 [details]
patch

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

>> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:6994
>> +            WTF::roundUpToMultipleOf(stackAlignmentBytes(), (CallFrame::headerSizeInRegisters + numArgs) * sizeof(EncodedJSValue)));
> 
> AirCode::requestCallArgAreaSizeInBytes() already takes care of aligning to stackAlignmentBytes().  So, no need to do that here.

I'm going to keep the rounding in to stay consistent w/ other places that do this in FTLLower

[Michael Saboff]

Comment on attachment 328368 [details]
patch

r=me

[Saam Barati]

Created attachment 328376 [details]
patch for landing

[WebKit Commit Bot]

Comment on attachment 328376 [details]
patch for landing

Clearing flags on attachment: 328376

Committed r225492: <https://trac.webkit.org/changeset/225492>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.