Commit dcd61b90 authored by littledan's avatar littledan Committed by Commit bot

Filter out synthetic variables from with scopes

This patch ensures that variables like .new_target aren't overwritable
using with scopes. It does this by ensuring that scope analysis does
not consider with scopes (or eval scopes) for such 'synthetic variables',
similarly to how the 'this' variable was already handled.
The patch also adds a DCHECK for the dynamic parallel to this case,
replacing a previous unreachable path for a particular instance.

BUG=v8:5405

Review-Url: https://codereview.chromium.org/2353623002
Cr-Commit-Position: refs/heads/master@{#39567}
parent bd078193
......@@ -261,8 +261,14 @@ Handle<Object> Context::Lookup(Handle<String> name, ContextLookupFlags flags,
object->IsJSContextExtensionObject()) {
maybe = JSReceiver::GetOwnPropertyAttributes(object, name);
} else if (context->IsWithContext()) {
// A with context will never bind "this".
if (name->Equals(*isolate->factory()->this_string())) {
// A with context will never bind "this", but debug-eval may look into
// a with context when resolving "this". Other synthetic variables such
// as new.target may be resolved as DYNAMIC_LOCAL due to bug v8:5405 ,
// skipping them here serves as a workaround until a more thorough
// fix can be applied.
// TODO(v8:5405): Replace this check with a DCHECK when resolution of
// of synthetic variables does not go through this code path.
if (ScopeInfo::VariableIsSynthetic(*name)) {
maybe = Just(ABSENT);
} else {
LookupIterator it(object, name, object);
......
......@@ -25,6 +25,4 @@ let log = [];
}
})();
// TODO(v8:5405): The log should actually be empty and not
// contain new.target.
assertArrayEquals(['new.target'], log);
assertArrayEquals([], log);
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment