929642 - Poison fn.caller and .arguments for builtin functions

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jason Orendorff [:jorendorff]]

http://people.mozilla.org/~jorendorff/es6-draft.html#sec-standard-built-in-ecmascript-objects

To cut a long story short:

> Every built-in Function object, F, ... has the properties ... defined by ...
> ... the AddRestrictedFunctionProperties (9.1.15.9) abstract operation ... .

We currently get this wrong.

Comment 1

[Nicolas B. Pierron [:nbp]]

I don't think we have a "callee", but the "caller" property is defined by the resolve hook which is lazily adding the property the first time we look for it.  So, yes we do not have this AddRestrictedFunctionProperties operation.

I am not sure to understand how this implies that we have to poison fn.caller.
Also I thought that fn.arguments was abandoned, is it added again in es6?

Comment 2

[André Bargull [:anba]]

s/callee/arguments -> I've updated the bug title to reflect the correct property name.

In ES6 .caller and .arguments on built-in functions work the same way they work in strict-mode functions. That means they're accessor properties with [[Get]] and [[Set]] throwing a TypeError. So not just abandoned, non-working properties, but instead strictly prohibited properties.

Comment 3

[Jeff Walden [:Waldo]]

Attachment: Possible patch

https://tbpl.mozilla.org/?tree=Try&rev=1cfcdeaefe5f

Comment 4

[Jeff Walden [:Waldo]]

Comment on attachment 8483496 [details] [diff] [review]
Possible patch

Close enough to passing try to be reviewable.  One test to fix, but an easy one, just needed to annotate a jit-test as expected to throw.

https://tbpl.mozilla.org/?tree=Try&rev=a1b57986abdc

Comment 6

[Till Schneidereit [:till]]

Comment on attachment 8483496 [details] [diff] [review]
Possible patch

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

Reasonable enough.

::: js/src/jsfun.cpp
@@ +118,5 @@
>  static bool
>  ArgumentsRestrictions(JSContext *cx, HandleFunction fun)
>  {
> +    // Throw if the function is a builtin (note: this doesn't include asm.js),
> +    // a strict mode function (FIXME: needs work handle strict asm.js functions

Do we have a bug for that? If not, file one, please. In any case, add the bug number here.
Also, nit: "needs work *to* handle"

@@ +215,5 @@
>  static bool
>  CallerRestrictions(JSContext *cx, HandleFunction fun)
>  {
> +    // Throw if the function is a builtin (note: this doesn't include asm.js),
> +    // a strict mode function (FIXME: needs work handle strict asm.js functions

Same comments as above.

Comment 7

[Jeff Walden [:Waldo]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0a6ef3cea4bc

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0a6ef3cea4bc