Yesterday Andreas asked me to figure out what was happening
with "this" and "with" in script scopes -- why that mozilla
test was failing before the latest patch.
Of course, before my patch entirely, each "eval" scope
declared a "this" binding, which was propagated to the
compiled code by the caller of eval, either via
Runtime_ResolvePossiblyDirectEval or via the various
%CompileString invocations in v8natives.js. "This" was
never resolved dynamically -- it always mapped to the
statically declared receiver in the closest declaration
scope, which also mapped to parameter -1.
After this patch, there are (broadly) two cases: in script
context, or in function context.
In function context, use of eval will mark all locals as
needing context allocation, so "this" ends up in the
context, and accesses go there. Use of "this" within "with"
will mark "this" as needing context allocation. Everything
works, there is a context object with a "this" binding.
In a script, usage of eval or other dynamic features is not
recorded:
// Inform the scope that the corresponding code contains an eval call.
void RecordEvalCall() { if (!is_script_scope()) scope_calls_eval_ = true;
}
So "this" not forced to be allocated in a context, in
general. e.g. on this file:
$ echo "eval('this')" > /tmp/eval.js
$ out/x64.debug/d8 --print-scopes /tmp/eval.js
global { // (0, 14)
// scope has trivial outer context
// 1 stack slots
// temporary vars:
TEMPORARY .result; // local[0]
// local vars:
VAR this; // parameter[-1]
DYNAMIC_GLOBAL eval; //
}
eval { // (0, 4)
// scope has trivial outer context
// scope uses 'this'
// 1 stack slots
// temporary vars:
TEMPORARY .result; // local[0]
}
Note that "this" is given stack allocation. And yet, how
does eval("this") actually work? The answer: by horrible
accident. :(((
You might want to finish breakfast, make sure all food is
not at risk of coming back up before reading on.
So, Runtime_CompileString or
Runtime_ResolvePossibleDirectEval end up calling
Compiler::GetFunctionFromEval, which takes a context and a
string. In script context, the context is usually the
native context, but it could be a script context if there
are context-allocated locals (possibly including "this"
itself). GetFunctionFromEval ends up calling
Parser::DoParseProgram, indirectly, and that declares a new
script scope, possibly unpacks other nested scopes from the
ScopeInfo, and then nests an eval scope.
The script scope declares a "this" binding. The eval scope
does not.
What happens next is where it gets bad.
"this" could be in the context, for example if the script
scope contains ()=>this. Or it could be just on the stack.
If "this" is context-allocated in the script, everything
works as you would think.
If "this" is stack-allocated in the script:
If eval code uses "this", it resolves to the "this"
binding in the script scope, which is at parameter[-1].
There happens to be a copy of "this" passed to the eval
code, as we mentioned before, so this happens to work --
even though it thinks it's getting the binding from the
script. Horrific.
Inside "with", in earlier versions of this patchset,
access to "this" was happening via dynamic lookup.
Again, this is not the case in master; in master, "this"
is always resolved statically. OK. But without "this"
in the context, the lookup will fail. That's the error
I was getting with mozilla code.
Arrow functions: normally arrow functions cause "this"
to be context-allocated in the script, and this works
fine. But if the arrow function is in "eval", then
scope resolution assumes that the variable was
contexted-allocated in the script (since it's outside
the arrow function scope), and "()=>this" residualizes
an access to context slot (depth=0,
slot=MIN_CONTEXT_SLOTS=4). But there is no script
context! So where does this map? To slot 4 in the
native context... which happens to be the global
proxy... so it just "works" :(((
In later versions of the patchset, we resolve "this"
statically even within "with". That makes the
eval('with({}) this') case be the same as
'eval("()=>this")()', and have the same unfortunate
possible aliasing of the native context.
I am not sure what The Right Solution is here. It sounds
like making all scripts have context-allocated "this" is the
cleanest thing, but I don't know what the perf impact would
be. An initial attempt to do this produced bootstrapping
failures.
I would like to plead for applying this patch as is and
fixing these terrible things in follow-ups, but I wonder if
this horrible accidental aliasing would not be exploitable
to gain access to the global object. In theory "this"
should always be allocated to either parameter[-1] or to
slot 4 in the context, so probably this is not possible.
Hard to make definitive statements here, though. Your
thoughts very much appreciated!
https://codereview.chromium.org/1097283003/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
