1325473 - A TypeError should be thrown when accessing 'arguments' or 'caller' on any of the new function types

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[zirak]

Attachment: Detect newer-style functions when accessing 'arguments' or 'caller'

User Agent: Mozilla/5.0 (Windows NT 10.0.14393; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/57.0.2950.4 Safari/537.36

Steps to reproduce:

Section 16.2 of the ES spec, "Forbidden Extensions", specifies that newer-type functions should behave the same way as strict-mode functions when one accesses their 'arguments' and 'caller' properties, i.e. throw a TypeError.
By "newer-type functions", my intention is:
* Arrow functions
* Async functions
* Classes
* Generators
* Methods

In other words:

	function strictFoo() { "use strict" }
	strictFoo.arguments // TypeError, as expected

	async function asyncFoo() {}
	asyncFoo.arguments // Should throw

You can see the committee notes here: https://github.com/rwaldron/tc39-notes/blob/46d2396e02fd73121b5985d5a0fafbcdbf9c9072/es6/2014-07/jul-29.md#conclusionresolution-5
The parallel v8 bug: https://bugs.chromium.org/p/v8/issues/detail?id=3946

Attached is a patch with a few test cases.

Thanks for your time!

Comment 1

[Shu-yu Guo [:shu]]

Comment on attachment 8821303 [details] [diff] [review]
Detect newer-style functions when accessing 'arguments' or 'caller'

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

Thank you for the patch and good find!

I'd like to see a new version of the test. Everything else looks good.

::: js/src/jsfun.cpp
@@ +129,3 @@
>      return IsAsmJSStrictModeModuleOrFunction(fun);
>  }
> +static bool IsNewerTypeFunction(JSFunction* fun) {

Style nit: the SpiderMonkey style guide would do

static bool
IsNewerTypeFunction(JSFunction* fun)
{
  ...
}

@@ +146,4 @@
>      // a strict mode function, or a bound function.
>      // TODO (bug 1057208): ensure semantics are correct for all possible
>      // pairings of callee/caller.
> +    if (fun->isBuiltin() || IsFunctionInStrictMode(fun) || fun->isBoundFunction() || IsNewerTypeFunction(fun)) {

Style nit: SpiderMonkey limits columns to 100 characters wide, and, for multi-line conditions, the { goes on its own line. So this would look something like:

if (fun->isBuiltin() ||
    IsFunctionInStrictMode(fun) ||
    fun->isBoundFunction() ||
    IsNewerTypeFunction(fun))
{
  ...
}

@@ +233,4 @@
>      // a strict mode function, or a bound function.
>      // TODO (bug 1057208): ensure semantics are correct for all possible
>      // pairings of callee/caller.
> +    if (fun->isBuiltin() || IsFunctionInStrictMode(fun) || fun->isBoundFunction() || IsNewerTypeFunction(fun)) {

Ditto.

::: js/src/tests/ecma_6/extensions/newer-type-functions-caller-arguments.js
@@ +6,5 @@
> +// * Async functions
> +// * Generators
> +// * Method definitions
> +// Note that classes run in strict mode by default, so they are covered by the
> +// first clause.

As discussed on IRC, the reason we need to throw here is:

1. ES 9.2.7 AddRestrictedFunctionProperties adds setters to Function.prototype that throw on setting "caller" and "arguments".
2. ES 16.2 "Forbidden Extensions" forbids adding own properties for basically all functions except sloppy functions introduced via |function () { ... }|.
3. So implementations are allowed to add own properties named "caller" and "arguments" to sloppy functions that shadow the setters that throw. But, for all other kinds of functions, they are disallowed and thus must throw.

@@ +59,5 @@
> +
> +function isProperTypeError(e) {
> +    return e instanceof TypeError &&
> +        e.message === "'caller', 'callee', and 'arguments' properties may not be accessed on strict mode functions or the arguments objects for calls to them";
> +}

No need to test for the exact message.

For most of this test, you can replace the handwritten try-catches with things like

assertThrowsInstanceOf(() => f.arguments, TypeError)

Comment 2

[zirak]

Attachment: Fix review in comment #1

Addressed shu's review.

Comment 3

[Shu-yu Guo [:shu]]

Comment on attachment 8821369 [details] [diff] [review]
Fix review in comment #1

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

Looks great, thanks.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7dd9c8a64775
"A TypeError should be thrown when accessing 'arguments' or 'caller' on any of the new function types". r=shu

Comment 5

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/7dd9c8a64775