636635 - Function("return function() { eval(''); return anonymous; }")()() should throw a ReferenceError

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jeff Walden [:Waldo]]

We give Function()-created functions the name "anonymous" for stringification purposes.  That behavior's pretty much an ipse dixit at this point.  But as a side effect, this makes the name "anonymous", if not optimized, evaluate to the Function()-created function in the function's code.

Here's correct behavior from jsc:

[jwalden@find-waldo-now ~]$ run-jsc
> Function("return function() { eval(''); return anonymous; }")    
function anonymous() { return function() { eval(''); return anonymous; }
}
> Function("return function() { eval(''); return anonymous; }")()  
function () { eval(''); return anonymous; }
> Function("return function() { eval(''); return anonymous; }")()()
Exception: ReferenceError: Can't find variable: anonymous

Here's our incorrect behavior:

[jwalden@find-waldo-now ~]$ ~/moz/js-tm/js/src/dbg/js
js> Function("return function() { eval(''); return anonymous; }")
(function anonymous() {return function () {eval("");return anonymous;};})
js> Function("return function() { eval(''); return anonymous; }")()
(function () {eval("");return anonymous;})
js> Function("return function() { eval(''); return anonymous; }")()()
(function anonymous() {return function () {eval("");return anonymous;};})
js> Function("return function() { eval(''); return anonymous; }")()()()
(function () {eval("");return anonymous;})

Thanks to jorendorff for pointing out the key to this when inquiring about how a function in Function() code could have an "Object" (DeclEnv, but clasp->name is "Object") on its scope chain.

Comment 1

[Jeff Walden [:Waldo]]

Attachment: Non-working patch (changes Function().name)

Spurred on by atom/displayAtom discussion today, I had this idea I could set "anonymous" as the display name and that that would turn off Function()'s "anonymous" name appearing in the scope chain.  It does.  But it also makes Function().name === "".  Spur-of-the-moment hack fail, back on the back burner again.

Comment 2

[Tooru Fujisawa [:arai]]

Attachment: Do not create named lambda binding for a function created by Function constructor.

This patch is based on bug 755821.

We create named lambda scope in Parser::finishFunctionScopes, when |funbox->function()->isNamedLambda()| is true.
this condition matches to a function created by Function ctor, that's the reason why it gets wrong scope with the name binding.
So, added extra parameter |isStandaloneFunction|, and avoid creating the scope if it's true.

Also added a testcase for binding with JS::CompileFunction in testFunctionBinding.cpp.
The test passes with and without this patch, so it keeps current behavior (no binding is created)

Comment 3

[Tooru Fujisawa [:arai]]

Green on try run except known failure in tc-M-V

https://treeherder.mozilla.org/#/jobs?repo=try&revision=51ea841cde56511df3fad7e152e5add65e4334b7

Comment 4

[Till Schneidereit [:till]]

Comment on attachment 8813443 [details] [diff] [review]
Do not create named lambda binding for a function created by Function constructor.

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

I'm not too happy about the "isStandaloneFunction" parameter name. I also don't have any suggestions for a better name, so let's go with that.

Comment 5

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1e932a9badfac50e6dcfa4a4da395c7644cbc73a
Bug 636635 - Do not create named lambda binding for a function created by Function constructor. r=till

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1e932a9badfa