Bug 153186 – FTL doesn't do proper spilling for exception handling when GetById/Snippets go to slow path

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

[Saam Barati]

Michael was investigating a bug he found while doing the new JSC calling convention work
and it turns out to be a latent bug in FTL try/catch machinery.
After I looked at the code again, I realized that what I had previously
written is wrong in a subtle way. The FTL callOperation machinery will remove
its result register from the set of registers it needs to spill. This is not
correct when we have try/catch. We may want to do value recovery on the value
of the result prior to the call. The case that we were solving before was
when the resultRegister == baseRegister in a GetById, or left/rightRegister == resultRegister
in a Snippet. This code is correct in wanting to spill in that case, but that case is
just a partial subset of the rule saying we should spill anytime the result is a register
we might do value recovery on.

[Saam Barati]

Created attachment 269176 [details]
patch

[Saam Barati]

Comment on attachment 269176 [details]
patch

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

> Source/JavaScriptCore/ftl/FTLCompile.cpp:666
> +                // on it in the OSR exit. This is because the callOperation(.) machinery doesn't
> +                // ever spill the result value. It actually takes care to never spill the result
> +                // because it overwrites it with the result of the call. But, with exceptions and
> +                // OSR exits, we may need the result value during OSR exit, so we take care to spill
> +                // it now.

changed this locally to:
                // We take care to always spill the result whenever we need to do value recovery
                // on it in the OSR exit. This is because the callOperation(.) machinery doesn't
                // ever spill the result value. It actually takes care to never spill the result
                // because it overwrites it with the result of the call. But, with exceptions and
                // OSR exits, we may need the result value prior to the call during OSR exit.
                // We take care to mark it for spillage now.

[Michael Saboff]

Comment on attachment 269176 [details]
patch

r=me

[Saam Barati]

landed in:
http://trac.webkit.org/changeset/195238