Add machinery to support arguments escaping to safe builtins

Add machinery to support arguments escaping to safe builtins

BUG=
R=verwaest@chromium.org, rossberg@chromium.org

[kbalazs, 2014-06-18 20:26:55]

Could you please tell more about the motivation of this change?

[p.antonov, 2014-06-19 08:12:47]

On 2014/06/18 20:26:55, kbalazs wrote:
> Could you please tell more about the motivation of this change?

To support some JavaScript idioms in crankshaft e.g. [].slice.call(arguments).
This was the original patch https://codereview.chromium.org/327173002/ which was
asked to split into 3 patches, of which this patch is the second. The first
patch is here https://codereview.chromium.org/335683002/

[Toon Verwaest, 2014-06-23 13:50:59]

This seems really brittle. I'm not comfortable landing this. I don't really have
an idea for an alternative though.

Btw, your first CL inlining .call got reverted due to breakage. You may want to
fix that and re-upload.

[p.antonov, 2014-10-17 14:01:50]

On 2014/06/23 13:50:59, Toon Verwaest wrote:
> This seems really brittle. I'm not comfortable landing this. I don't really
have
> an idea for an alternative though.

Has anything changed with regard to this?

[Toon Verwaest, 2014-10-20 10:44:28]

No. This is just way too error-prone.

[p.antonov, 2014-10-21 08:19:03]

On 2014/10/20 10:44:28, Toon Verwaest wrote:
> No. This is just way too error-prone.

I mean do you still have no ideas of a different approach that wouldn't be so
brittle? Maybe entering something like SafeArgumentsValueContext when visiting
the argument nodes for a .call - or is that too specific to be worth
implementing?

[Toon Verwaest, 2014-10-23 11:19:20]

Honestly I'd rather wait for Turbofan to catch up. There we can do proper escape
analysis on the arguments object. Then we can just support passing it to slice.

The reason why I find it brittle is that, suppose we implement full slice with
support for slice range in Crankshaft. The arguments object isn't passed as the
receiver, but as one of the range ends. That would do an implicit ToNumber on
the arguments object, which internally calls valueOf on arguments, which leaks
the object.

Of course currently we don't do any of that, so it could be safe for now. I'm
not sure it's, at this time, worth the investment and possible fallout.

[p.antonov, 2014-10-28 14:50:31]

I see. However, before your reply I had another proposal worked out. It is not
as brittle but it's more specific (only works when arguments is the receiver,
however that seems to be the case almost always). Patch here:
http://pastebin.com/r3VbDSk0

Here's how it would look with ArraySlice implementation:
http://pastebin.com/R1924p4i There's some code specific to the case when
arguments is receiver but most of the code
could probably be extracted out if ArraySlice is implemented as a normal inlined
built-in as well to avoid code duplication.

What do you think?

[p.antonov, 2014-11-05 11:19:20]

ping

[Jakob Kummerow, 2014-11-06 17:14:19]

As Toon said, we're not making non-essential changes to Hydrogen any more, as
we're working towards replacing it with TurboFan.

The time investment to review complex changes, or think about alternatives, and
possibly investigate crashes is just not worth it.

I'm sorry about the work you've invested in this, and would like to emphasize
that we do welcome your contributions; however the Crankshaft pipeline is not
the right target for anyone's efforts at this point. The TurboFan code has a
much longer-term future ahead of it, and some parts of it, like the
platform-specific backends, are pretty close to production use. If you'd like to
help there, please make sure to coordinate with the people you see making
changes to it.

[Laszlo Gombos, 2015-05-08 11:49:09]

Closing it based on comment #msg11.