Bug 153536 – [JSC] adjustFrameAndStackInOSRExitCompilerThunk() can trash values in FTL

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Benjamin Poulain]

[JSC] adjustFrameAndStackInOSRExitCompilerThunk() can trash values in FTL

[Benjamin Poulain]

Created attachment 269982 [details]
Patch

[Saam Barati]

Comment on attachment 269982 [details]
Patch

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

r=me

> Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.h:48
> +

It might be worth adding a comment here saying we can't trash the macro assembler scratch registers in the code we emit here.

> Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.h:107
> +    if (isFTLOSRExit)
> +        jit.pushToSave(GPRInfo::regT1);

It might be worth a comment saying that we do this because the previous push will be overwritten by the old top-of-the-stack.
Or you could make it explicit by doing something like:
unsigned stackOffset = MacroAssembler::pushToSaveByteOffset * (isFTLOSRExit ? 2 : 1)
subPtr(offset, SP)
storePtr(regT1, SP)

[Saam Barati]

also see discussion here for potential alternative implementations
that take into account generic unwind exits and utilize volatile
registers in situations where we know we've arrived at OSR exit
code from the genericUnwind path and therefore can use volatile
registers:

https://bugs.webkit.org/show_bug.cgi?id=151406

[Benjamin Poulain]

Created attachment 270053 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 270053 [details]
Patch for landing

Clearing flags on attachment: 270053

Committed r195707: <http://trac.webkit.org/changeset/195707>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.