Chakracore: Functions evaluated in strict mode in console does not prevent implicit variables from being created in Edge

[infinnie]

Evaluating

(function(){"use strict";aa=233})()

in Edge causes a global variable aa to be created, instead of an error being thrown.

User agent:

Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/64.0.3282.140 Safari/537.36 Edge/17.17134

[fatcerberus]

Okay so this works as expected when the code above is run normally:

@/test.js:1 [global code]
(ssj) l
->    1 (function(){"use strict";aa=233})()
      2 debugger;

@/test.js:1 [global code]
(ssj) s
uncaught: ReferenceError: Variable undefined in strict mode
   at Anonymous function (@/test.js:1:26)
   at Global code (@/test.js:1:2)
-> # 0: [anonymous function], at @/test.js:1
1 (function(){"use strict";aa=233})()

However I can reproduce it with an eval (debugger eval in this case):

@/test.js:1 [anonymous function]
(ssj) e (function(){"use strict";aa=233})()
= "undefined"

@/test.js:1 [anonymous function]
(ssj) e aa
= 233

[fatcerberus]

The plot thickens. It appears this doesn't actually create a global variable in the usual sense:

@/test.js:1 [anonymous function]
(ssj) e aa
= 233

@/test.js:1 [anonymous function]
(ssj) e global.aa
= "undefined"

[infinnie]

Yes, something like the eval-context.

[infinnie]

Sorry for my having commented on the wrong issue 😿

[kfarnung]

@infinnie Can you provide a full-fledge repro? We weren't seeing this behavior in our console host (ch.exe), so more information would help narrow it down.

/cc @dilijev

[infinnie]

@kfarnung See @fatcerberus’ comment.

[kfarnung]

@infinnie We'd like to see a repro scenario in the browser, if possible.

Running the following script in ch.exe:

(function(){'use strict';aa=233;})();
print(aa);

> ch test2.js
ReferenceError: Variable undefined in strict mode
   at Anonymous function (C:\Users\kfarnung\test2.js:1:26)
   at Global code (C:\Users\kfarnung\test2.js:1:2)

Which I believe is the expected behavior.

[fatcerberus]

@kfarnung The issue is specifically when the code is run using eval (or JsDiagEvaluate), it doesn’t repro when run as program code.

[kfarnung]

I'm still seeing correct behavior with or without eval:

eval("(function(){'use strict';aa=233;})();");
print(aa);

> ch .\test2.js
ReferenceError: Variable undefined in strict mode
   at Anonymous function (eval code:1:26)
   at eval code (eval code:1:2)
   at Global code (C:\Users\kfarnung\test2.js:1:1)

@infinnie can you provide more context? How is the code in question being evaluated?

[infinnie]

Something wrong with the console? Typing directly into a console will not report any error, but an error would be reported when appending the same code to a <script> tag and then appending that script to the body, or even eval()-ing it in that script tag.

SCRIPT5042: Variable undefined in strict mode

[fatcerberus]

I bet the issue is with JsDiagEvaluate, then.

[infinnie]

So what’s the mysterious debugger-eval? @fatcerberus

[fatcerberus]

@infinnie The e command in my SSj debugger uses JsDiagEvaluate and it triggers this bug, too.

[MSLaguana]

I believe that if you use an indirect eval that it may also reproduce in ch.

[MSLaguana]

I might be mis remembering; I can see it with this:

"use strict";
var lave = eval;
lave('(function () {aa = 123;})()');
console.log(aa);

but I'm not sure how strictness is supposed to propagate within eval; if I add some "use strict"; inside the eval, then it gives the expected error.

[fatcerberus]

@MSLaguana I can't reproduce using ch.exe and pure JavaScript eval (not even indirect) which is very frustrating. Best I can tell the issue only manifests when one executes the code in the OP using JsDiagEvaluate.

[fatcerberus]

Repro in Edge console:

> (function(){"use strict";aa=233})()
< undefined
> aa
< 233
> (Function('return this'))().aa
< undefined

[MSLaguana]

Ok; Finally getting a chance to dig into what's going on here, and it seems like the specific cause of this issue is that the function defined in the console environment isn't considered to be strict mode. Still trying to work out why that is at the moment.

[fatcerberus]

@MSLaguana See my repro above; It doesn't actually create a property on the global object, which is odd. Nonetheless the variable is accessible by simply eval'ing its name. I don't think it's as simple as not running the code in strict mode, something more insidious might be going on.

[MSLaguana]

There's a few things going on; one is that we have some additional scopes injected while debugging so variables don't end up on the global, but they end up in some near-global scope that follows the debugging session around. But this particular issue here is actually because the function isn't in strict mode; I was following the execution of the interpreter and when it goes to execute the bytecode (which is a strict bytecode operator) it checks the function's environment and is told that it isn't strict. I could be wrong, but I think that's leading to this sloppy behavior being considered acceptable

[fatcerberus]

What I don't understand is that, normally in sloppy mode, any assignment to an undeclared variable automatically becomes global -- but in this case it doesn't. It instead seems to stop at the "near-global" scope you describe, which is bizarre.

[MSLaguana]

Here we go, this is the specific reason for this behavior:

https://github.com/Microsoft/ChakraCore/blob/master/lib/Runtime/Language/JavascriptOperators.cpp#L3073-L3106

None of this is specced out from what I understand, but at one point a decision was made that when console-eval-ing scripts it shouldn't "accidentally" pollute the global object, so running x = 1 shouldn't make global.x == 1. That's what leaed to this "console scope" being added just above the global, and that's where these undeclared variables are getting set.

@kfarnung has been looking at undoing some of this since there seems to be a consensus in the browser space that you do in fact pollute the global by assigning to things from inside debuggers, and it looks like the code I highlighted above has been removed in his branch, so with his changes it looks like this behavior will match intuition.

[fatcerberus]

I think I would prefer it to pollute the global scope over the current behavior, myself. Ideally when eval’ing code in the debugger I expect it to work exactly as though the same code were inserted into the source at the point execution is paused—or at least as close to that behavior as possible.

[kfarnung]

Thanks for digging in @MSLaguana! I suspected the behavior was related, but didn't have a chance to dive in. I think the changes I have should be relatively close to landing, I'll take another pass and see what's needed to land it.

FWIW the current evaluate behavior deviates significantly from Chrome and Firefox, creating a bunch of complexity along the way. It will be nice to remove a bunch of code and get a more desirable experience in the process.