718022 - can we further nerf f.arguments?

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Luke Wagner [:luke]]

Even with bug 592927, f.arguments still causes undue complexity (and gets in the way of the ideal solution for bug 659577).  Fortunately, other browsers (chrome and ie) "break" f.arguments even more than we currently do (scare quotes b/c f.arguments is unspecified in ES3 and specified-to-be-a-poison-pill in ES5 strict).  Already, in IE9, Chrome, and FF4+, 

  function g() { f.arguments[0] = 42; }
  function f(arg) { g(); return arg; }

f(10) returns 10.  In Chrome:

  function g() { f.arguments[0] = 42; }
  function f(arg) { g(); return arguments[0]; }

f(10) returns 10 when called from crankshaft.  In IE9:

  function g() { return f.arguments[0]; }
  function f(arg) { arg = 42; return g(); }

f(10) returns 10.

The conditions where f.arguments breaks is obviously pretty irregular.  Based on this and the fact that we didn't have a single reported breakage resulting from bug 592927, I doubt much code can be depending on these aliasing subtleties.  Thus, I propose the following nerfing extension: f.arguments creates a new non-aliasing copy of the actual arguments, always.

(If noone objects) the patch for this should be simple and back-out-able so we can use the same "send it down the chutes and back it out if it breaks the web" model as, say, the single-threaded runtime assert.

Comment 1

[Luke Wagner [:luke]]

Attachment: preparation patch (shouldn't have observable change)

Comment 2

[Luke Wagner [:luke]]

Attachment: nerf 1-liner (and fix tests)

I'll wait a bit longer for comment before requesting review on these.

Comment 3

[Luke Wagner [:luke]]

Green on try

Comment 4

[Luke Wagner [:luke]]

Attachment: simpler and more direct patch

This patch nerfs a case the original missed by just doing what comment 0 said directly.  It also adds a comment.

Comment 5

[Luke Wagner [:luke]]

Attachment: simpler and more direct patch

Oops, missed a test in src/tests (overwritten args are ignored).

Comment 6

[Jeff Walden [:Waldo]]

Comment on attachment 588949 [details] [diff] [review]
simpler and more direct patch

Review of attachment 588949 [details] [diff] [review]:
-----------------------------------------------------------------

It's really really early and my sleep schedule's all shot to pieces (hopefully to be put back together during the BOS->SFO flight, priming me for a very early night's sleep that has me waking up at a normal time tomorrow), but I think I can review such a short patch without too much danger of footgunning.

Comment 7

[Luke Wagner [:luke]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/96fe039c4685

Comment 8

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/96fe039c4685