From c137b2d4d937e2a7a2d116fcc9fbcb1afb150e92 Mon Sep 17 00:00:00 2001
From: Jeff Walden <jwalden@mit.edu>
Date: Fri, 12 Nov 2010 14:33:46 -0800
Subject: [PATCH] Bug 610350 - Assigning to a named function's name in strict
 mode code should throw.  r=brendan

---
 js/src/jsemit.cpp                             | 27 ++++++++----
 js/src/jsparse.cpp                            | 13 +++++-
 .../ecma_5/strict/assign-to-callee-name.js    | 42 +++++++++++++++++++
 js/src/tests/ecma_5/strict/jstests.list       |  1 +
 4 files changed, 75 insertions(+), 8 deletions(-)
 create mode 100644 js/src/tests/ecma_5/strict/assign-to-callee-name.js

diff --git a/js/src/jsemit.cpp b/js/src/jsemit.cpp
index 0582becfc66..badb45b80e9 100644
--- a/js/src/jsemit.cpp
+++ b/js/src/jsemit.cpp
@@ -2500,15 +2500,28 @@ BindNameToSlot(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn)
             JS_ASSERT((cg->fun()->flags & JSFUN_LAMBDA) && atom == cg->fun()->atom);
 
             /*
-             * Leave pn->pn_op == JSOP_NAME if cg->fun() is heavyweight, as we
-             * cannot be sure cg->fun() is not something of the form:
+             * Leave pn->pn_op == JSOP_NAME if cg->fun is heavyweight to
+             * address two cases: a new binding introduced by eval, and
+             * assignment to the name in strict mode.
              *
-             *   var ff = (function f(s) { eval(s); return f; });
+             *   var fun = (function f(s) { eval(s); return f; });
+             *   assertEq(fun("var f = 42"), 42);
              *
-             * where a caller invokes ff("var f = 42"). The result returned for
-             * such an invocation must be 42, since the callee name is
-             * lexically bound in an outer declarative environment from the
-             * function's activation. See jsfun.cpp:call_resolve.
+             * ECMAScript specifies that a function expression's name is bound
+             * in a lexical environment distinct from that used to bind its
+             * named parameters, the arguments object, and its variables.  The
+             * new binding for "var f = 42" shadows the binding for the
+             * function itself, so the name of the function will not refer to
+             * the function.
+             *
+             *    (function f() { "use strict"; f = 12; })();
+             *
+             * Outside strict mode, assignment to a function expression's name
+             * has no effect.  But in strict mode, this attempt to mutate an
+             * immutable binding must throw a TypeError.  We implement this by
+             * not optimizing such assignments and by marking such functions as
+             * heavyweight, ensuring that the function name is represented in
+             * the scope chain so that assignment will throw a TypeError.
              */
             JS_ASSERT(op != JSOP_DELNAME);
             if (!(cg->flags & TCF_FUN_HEAVYWEIGHT)) {
diff --git a/js/src/jsparse.cpp b/js/src/jsparse.cpp
index 84ee92a16f6..05a1ab36878 100644
--- a/js/src/jsparse.cpp
+++ b/js/src/jsparse.cpp
@@ -3843,9 +3843,20 @@ NoteLValue(JSContext *cx, JSParseNode *pn, JSTreeContext *tc, uintN dflag = PND_
 
     pn->pn_dflags |= dflag;
 
+    /*
+     * Both arguments and the enclosing function's name are immutable bindings
+     * in ES5, so assignments to them must do nothing or throw a TypeError
+     * depending on code strictness.  Assignment to arguments is a syntax error
+     * in strict mode and thus cannot happen.  Outside strict mode, we optimize
+     * away assignment to the function name.  For assignment to function name
+     * to fail in strict mode, we must have a binding for it in the scope
+     * chain; we ensure this happens by making such functions heavyweight.
+     */
     JSAtom *lname = pn->pn_atom;
-    if (lname == cx->runtime->atomState.argumentsAtom)
+    if (lname == cx->runtime->atomState.argumentsAtom ||
+        (tc->inFunction() && lname == tc->fun()->atom)) {
         tc->flags |= TCF_FUN_HEAVYWEIGHT;
+    }
 }
 
 #if JS_HAS_DESTRUCTURING
diff --git a/js/src/tests/ecma_5/strict/assign-to-callee-name.js b/js/src/tests/ecma_5/strict/assign-to-callee-name.js
new file mode 100644
index 00000000000..3c2efba2e2b
--- /dev/null
+++ b/js/src/tests/ecma_5/strict/assign-to-callee-name.js
@@ -0,0 +1,42 @@
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/
+ */
+
+var gTestfile = 'assign-to-callee-name.js';
+var BUGNUMBER = 610350;
+var summary =
+  "Assigning to a function expression's name within that function should " +
+  "throw a TypeError in strict mode code";
+
+print(BUGNUMBER + ": " + summary);
+
+/**************
+ * BEGIN TEST *
+ **************/
+
+var f = function assignSelfStrict() { "use strict"; assignSelfStrict = 12; };
+
+try
+{
+  var r = f();
+  throw new Error("should have thrown a TypeError, returned " + r);
+}
+catch (e)
+{
+  assertEq(e instanceof TypeError, true,
+           "didn't throw a TypeError: " + e);
+}
+
+var assignSelf = 42;
+var f2 = function assignSelf() { assignSelf = 12; };
+
+f2(); // shouldn't throw, does nothing
+assertEq(assignSelf, 42);
+
+/******************************************************************************/
+
+if (typeof reportCompare === "function")
+  reportCompare(true, true);
+
+print("All tests passed!");
diff --git a/js/src/tests/ecma_5/strict/jstests.list b/js/src/tests/ecma_5/strict/jstests.list
index 6de46ec717d..1594c0f9a9f 100644
--- a/js/src/tests/ecma_5/strict/jstests.list
+++ b/js/src/tests/ecma_5/strict/jstests.list
@@ -37,4 +37,5 @@ script regress-532254.js
 script regress-532041.js
 script unbrand-this.js
 script this-for-function-expression-recursion.js
+script assign-to-callee-name.js
 script directive-prologue-01.js