1396050 - Support IndirectEval with NonSyntacticVariablesScope

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, enhancement, P3)

Description

[Ted Campbell [:tcampbell]]

Follow-up to Bug 1394490.

If evaluating |(0,eval)("this")| with a NonSyntacticVariblesEnvironment on the chain, we get the global instead of the current lexical this (ie the NonSyntacticVariables itself).

This should be fixed by passing the correct environment to IndirectEval.

(Once this is fixed, the test in js/src/jsapi-tests/testExecuteInNonSyntacticGlobal should be updated)

Comment 1

[Ted Campbell [:tcampbell]]

This also applies to |new Function()|.

One difficulty is finding the environment in the callee context. To deal with this, I propose we move the |Function| and |eval| bindings from the global to the NSVO in these cases. We can then use FunctionExtended slots (or other mechanism) to keep track of the NSVO the binding came from. This would give predictable results similar to the non-shared case where the IndirectEval happens in the context that the binding was read from.

Comment 3

[Ted Campbell [:tcampbell]]

Attachment: Bug 1396050 - Support IndirectEval per-JSMEnvironment

Review commit: https://reviewboard.mozilla.org/r/179098/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/179098/

Comment 4

[Ted Campbell [:tcampbell]]

Comment on attachment 8907411 [details]
Bug 1396050 - Support IndirectEval per-JSMEnvironment

Here is a rough prototype for supporting IndirectEval per-JSMEnvironment/NSVO. Similar would be done for |new Function| constructor.

I'm wondering what parts of this worry you the most and if you have ideas of other mechanisms to achieve the same.

This could form a stop-gap until we are able to remove eval/new Function usage entirely from JSMs. So this will be a game of balancing risks of this change versus changing all the code/tests that use eval.

Comment 5

[Ted Campbell [:tcampbell]]

Attachment: Bug 1396050 - Use per-JSMEnvironment eval/Function in JSM loader

When JSM global sharing is enabled, this will create a private copy of
eval and Function in the JSMEnvironment. Dynamic code in these will be
run in the context of the JSMEnvironment instead of the shared global.

This is provided for compatibiltiy with existing JSMs. Eventually
dynamic code will be disallowed in JSMs.

Review commit: https://reviewboard.mozilla.org/r/179970/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/179970/

Comment 6

[Ted Campbell [:tcampbell]]

Comment on attachment 8907411 [details]
Bug 1396050 - Support IndirectEval per-JSMEnvironment

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/179098/diff/1-2/

Comment 7

[Ted Campbell [:tcampbell]]

I put together this experiment and it seems to work but is hacky. Since Bug 1399997 is on track to unblock this, I'm closing this as WONTFIX and recommending we do Bug 1399997, and then eventually disable eval/Function altogether via CSP.