Test for var declarations in eval which conflict with let

Previously, name conflicts between var and let declarations were only
made into exceptions if they were visible at parse-time. This patch adds
runtime checks so that sloppy-mode direct eval can't introduce conflicting
var declarations. The change is implemented by traversing the scope chain
when a direct eval introduces a var declaration to look for conflicting
let declarations, up to the function boundary.

BUG=v8:4454
R=adamk
LOG=Y

Review URL: https://codereview.chromium.org/1382513003

Cr-Commit-Position: refs/heads/master@{#31211}
This commit is contained in:
littledan 2015-10-12 07:30:46 -07:00 committed by Commit bot
parent 9feb530594
commit d515e5138d
18 changed files with 404 additions and 73 deletions

View File

@ -264,7 +264,8 @@ Handle<Object> Context::Lookup(Handle<String> name,
}
// 1. Check global objects, subjects of with, and extension objects.
if ((context->IsNativeContext() || context->IsWithContext() ||
if ((context->IsNativeContext() ||
(context->IsWithContext() && ((flags & SKIP_WITH_CONTEXT) == 0)) ||
context->IsFunctionContext() || context->IsBlockContext()) &&
context->extension_receiver() != nullptr) {
Handle<JSReceiver> object(context->extension_receiver());
@ -384,7 +385,9 @@ Handle<Object> Context::Lookup(Handle<String> name,
}
// 3. Prepare to continue with the previous (next outermost) context.
if (context->IsNativeContext()) {
if (context->IsNativeContext() ||
((flags & STOP_AT_DECLARATION_SCOPE) != 0 &&
context->is_declaration_context())) {
follow_context_chain = false;
} else {
context = Handle<Context>(context->previous(), isolate);

View File

@ -13,11 +13,15 @@ namespace internal {
enum ContextLookupFlags {
FOLLOW_CONTEXT_CHAIN = 1,
FOLLOW_PROTOTYPE_CHAIN = 2,
FOLLOW_CONTEXT_CHAIN = 1 << 0,
FOLLOW_PROTOTYPE_CHAIN = 1 << 1,
STOP_AT_DECLARATION_SCOPE = 1 << 2,
SKIP_WITH_CONTEXT = 1 << 3,
DONT_FOLLOW_CHAINS = 0,
FOLLOW_CHAINS = FOLLOW_CONTEXT_CHAIN | FOLLOW_PROTOTYPE_CHAIN
FOLLOW_CHAINS = FOLLOW_CONTEXT_CHAIN | FOLLOW_PROTOTYPE_CHAIN,
LEXICAL_TEST =
FOLLOW_CONTEXT_CHAIN | STOP_AT_DECLARATION_SCOPE | SKIP_WITH_CONTEXT,
};

View File

@ -847,10 +847,8 @@ void FullCodeGenerator::VisitVariableDeclaration(
__ mov(r0, Operand(Smi::FromInt(0))); // Indicates no initial value.
}
__ Push(r2, r0);
__ CallRuntime(IsImmutableVariableMode(mode)
? Runtime::kDeclareReadOnlyLookupSlot
: Runtime::kDeclareLookupSlot,
2);
__ Push(Smi::FromInt(variable->DeclarationPropertyAttributes()));
__ CallRuntime(Runtime::kDeclareLookupSlot, 3);
break;
}
}
@ -906,7 +904,8 @@ void FullCodeGenerator::VisitFunctionDeclaration(
__ Push(r2);
// Push initial value for function declaration.
VisitForStackValue(declaration->fun());
__ CallRuntime(Runtime::kDeclareLookupSlot, 2);
__ Push(Smi::FromInt(variable->DeclarationPropertyAttributes()));
__ CallRuntime(Runtime::kDeclareLookupSlot, 3);
break;
}
}

View File

@ -851,10 +851,8 @@ void FullCodeGenerator::VisitVariableDeclaration(
// Pushing 0 (xzr) indicates no initial value.
__ Push(x2, xzr);
}
__ CallRuntime(IsImmutableVariableMode(mode)
? Runtime::kDeclareReadOnlyLookupSlot
: Runtime::kDeclareLookupSlot,
2);
__ Push(Smi::FromInt(variable->DeclarationPropertyAttributes()));
__ CallRuntime(Runtime::kDeclareLookupSlot, 3);
break;
}
}
@ -910,7 +908,8 @@ void FullCodeGenerator::VisitFunctionDeclaration(
__ Push(x2);
// Push initial value for function declaration.
VisitForStackValue(declaration->fun());
__ CallRuntime(Runtime::kDeclareLookupSlot, 2);
__ Push(Smi::FromInt(variable->DeclarationPropertyAttributes()));
__ CallRuntime(Runtime::kDeclareLookupSlot, 3);
break;
}
}

View File

@ -796,10 +796,9 @@ void FullCodeGenerator::VisitVariableDeclaration(
} else {
__ push(Immediate(Smi::FromInt(0))); // Indicates no initial value.
}
__ CallRuntime(IsImmutableVariableMode(mode)
? Runtime::kDeclareReadOnlyLookupSlot
: Runtime::kDeclareLookupSlot,
2);
__ push(
Immediate(Smi::FromInt(variable->DeclarationPropertyAttributes())));
__ CallRuntime(Runtime::kDeclareLookupSlot, 3);
break;
}
}
@ -851,7 +850,9 @@ void FullCodeGenerator::VisitFunctionDeclaration(
Comment cmnt(masm_, "[ FunctionDeclaration");
__ push(Immediate(variable->name()));
VisitForStackValue(declaration->fun());
__ CallRuntime(Runtime::kDeclareLookupSlot, 2);
__ push(
Immediate(Smi::FromInt(variable->DeclarationPropertyAttributes())));
__ CallRuntime(Runtime::kDeclareLookupSlot, 3);
break;
}
}

View File

@ -851,10 +851,8 @@ void FullCodeGenerator::VisitVariableDeclaration(
__ mov(a0, zero_reg); // Smi::FromInt(0) indicates no initial value.
}
__ Push(a2, a0);
__ CallRuntime(IsImmutableVariableMode(mode)
? Runtime::kDeclareReadOnlyLookupSlot
: Runtime::kDeclareLookupSlot,
2);
__ Push(Smi::FromInt(variable->DeclarationPropertyAttributes()));
__ CallRuntime(Runtime::kDeclareLookupSlot, 3);
break;
}
}
@ -910,7 +908,8 @@ void FullCodeGenerator::VisitFunctionDeclaration(
__ Push(a2);
// Push initial value for function declaration.
VisitForStackValue(declaration->fun());
__ CallRuntime(Runtime::kDeclareLookupSlot, 2);
__ Push(Smi::FromInt(variable->DeclarationPropertyAttributes()));
__ CallRuntime(Runtime::kDeclareLookupSlot, 3);
break;
}
}

View File

@ -849,10 +849,8 @@ void FullCodeGenerator::VisitVariableDeclaration(
__ mov(a0, zero_reg); // Smi::FromInt(0) indicates no initial value.
}
__ Push(a2, a0);
__ CallRuntime(IsImmutableVariableMode(mode)
? Runtime::kDeclareReadOnlyLookupSlot
: Runtime::kDeclareLookupSlot,
2);
__ Push(Smi::FromInt(variable->DeclarationPropertyAttributes()));
__ CallRuntime(Runtime::kDeclareLookupSlot, 3);
break;
}
}
@ -908,7 +906,8 @@ void FullCodeGenerator::VisitFunctionDeclaration(
__ Push(a2);
// Push initial value for function declaration.
VisitForStackValue(declaration->fun());
__ CallRuntime(Runtime::kDeclareLookupSlot, 2);
__ Push(Smi::FromInt(variable->DeclarationPropertyAttributes()));
__ CallRuntime(Runtime::kDeclareLookupSlot, 3);
break;
}
}

View File

@ -817,10 +817,10 @@ void FullCodeGenerator::VisitVariableDeclaration(
__ LoadSmiLiteral(r3, Smi::FromInt(0)); // Indicates no initial value.
__ Push(r5, r3);
}
__ CallRuntime(IsImmutableVariableMode(mode)
? Runtime::kDeclareReadOnlyLookupSlot
: Runtime::kDeclareLookupSlot,
2);
__ LoadSmiLiteral(
r3, Smi::FromInt(variable->DeclarationPropertyAttributes()));
__ Push(r3);
__ CallRuntime(Runtime::kDeclareLookupSlot, 3);
break;
}
}
@ -871,7 +871,10 @@ void FullCodeGenerator::VisitFunctionDeclaration(
__ Push(r5);
// Push initial value for function declaration.
VisitForStackValue(declaration->fun());
__ CallRuntime(Runtime::kDeclareLookupSlot, 2);
__ LoadSmiLiteral(
r5, Smi::FromInt(variable->DeclarationPropertyAttributes()));
__ Push(r5);
__ CallRuntime(Runtime::kDeclareLookupSlot, 3);
break;
}
}

View File

@ -809,10 +809,8 @@ void FullCodeGenerator::VisitVariableDeclaration(
} else {
__ Push(Smi::FromInt(0)); // Indicates no initial value.
}
__ CallRuntime(IsImmutableVariableMode(mode)
? Runtime::kDeclareReadOnlyLookupSlot
: Runtime::kDeclareLookupSlot,
2);
__ Push(Smi::FromInt(variable->DeclarationPropertyAttributes()));
__ CallRuntime(Runtime::kDeclareLookupSlot, 3);
break;
}
}
@ -865,7 +863,8 @@ void FullCodeGenerator::VisitFunctionDeclaration(
Comment cmnt(masm_, "[ FunctionDeclaration");
__ Push(variable->name());
VisitForStackValue(declaration->fun());
__ CallRuntime(Runtime::kDeclareLookupSlot, 2);
__ Push(Smi::FromInt(variable->DeclarationPropertyAttributes()));
__ CallRuntime(Runtime::kDeclareLookupSlot, 3);
break;
}
}

View File

@ -793,10 +793,8 @@ void FullCodeGenerator::VisitVariableDeclaration(
} else {
__ push(Immediate(Smi::FromInt(0))); // Indicates no initial value.
}
__ CallRuntime(IsImmutableVariableMode(mode)
? Runtime::kDeclareReadOnlyLookupSlot
: Runtime::kDeclareLookupSlot,
2);
__ push(Immediate(variable->DeclarationPropertyAttributes()));
__ CallRuntime(Runtime::kDeclareLookupSlot, 3);
break;
}
}
@ -844,7 +842,8 @@ void FullCodeGenerator::VisitFunctionDeclaration(
Comment cmnt(masm_, "[ FunctionDeclaration");
__ push(Immediate(variable->name()));
VisitForStackValue(declaration->fun());
__ CallRuntime(Runtime::kDeclareLookupSlot, 2);
__ push(Immediate(variable->DeclarationPropertyAttributes()));
__ CallRuntime(Runtime::kDeclareLookupSlot, 3);
break;
}
}

View File

@ -2114,6 +2114,7 @@ Variable* Parser::Declare(Declaration* declaration,
var = new (zone()) Variable(declaration_scope, name, mode, kind,
declaration->initialization(), kNotAssigned);
var->AllocateTo(VariableLocation::LOOKUP, -1);
var->SetFromEval();
resolve = true;
}

View File

@ -11,23 +11,28 @@
// Ecma-262 3rd 8.6.1
enum PropertyAttributes {
NONE = v8::None,
READ_ONLY = v8::ReadOnly,
DONT_ENUM = v8::DontEnum,
DONT_DELETE = v8::DontDelete,
NONE = v8::None,
READ_ONLY = v8::ReadOnly,
DONT_ENUM = v8::DontEnum,
DONT_DELETE = v8::DontDelete,
SEALED = DONT_DELETE,
FROZEN = SEALED | READ_ONLY,
SEALED = DONT_DELETE,
FROZEN = SEALED | READ_ONLY,
STRING = 8, // Used to filter symbols and string names
SYMBOLIC = 16,
PRIVATE_SYMBOL = 32,
STRING = 8, // Used to filter symbols and string names
SYMBOLIC = 16,
PRIVATE_SYMBOL = 32,
DONT_SHOW = DONT_ENUM | SYMBOLIC | PRIVATE_SYMBOL,
ABSENT = 64 // Used in runtime to indicate a property is absent.
DONT_SHOW = DONT_ENUM | SYMBOLIC | PRIVATE_SYMBOL,
ABSENT = 64, // Used in runtime to indicate a property is absent.
// ABSENT can never be stored in or returned from a descriptor's attributes
// bitfield. It is only used as a return value meaning the attributes of
// a non-existent property.
// When creating a property, EVAL_DECLARED used to indicate that the property
// came from a sloppy-mode direct eval, and certain checks need to be done.
// Cannot be stored in or returned from a descriptor's attributes bitfield.
EVAL_DECLARED = 128
};

View File

@ -223,10 +223,22 @@ Object* DeclareLookupSlot(Isolate* isolate, Handle<String> name,
int index;
PropertyAttributes attributes;
ContextLookupFlags flags = DONT_FOLLOW_CHAINS;
BindingFlags binding_flags;
Handle<Object> holder =
context->Lookup(name, flags, &index, &attributes, &binding_flags);
if ((attr & EVAL_DECLARED) != 0) {
// Check for a conflict with a lexically scoped variable
context_arg->Lookup(name, LEXICAL_TEST, &index, &attributes,
&binding_flags);
if (attributes != ABSENT &&
(binding_flags == MUTABLE_CHECK_INITIALIZED ||
binding_flags == IMMUTABLE_CHECK_INITIALIZED)) {
return ThrowRedeclarationError(isolate, name);
}
attr = static_cast<PropertyAttributes>(attr & ~EVAL_DECLARED);
}
Handle<Object> holder = context->Lookup(name, DONT_FOLLOW_CHAINS, &index,
&attributes, &binding_flags);
if (holder.is_null()) {
// In case of JSProxy, an exception might have been thrown.
if (isolate->has_pending_exception()) return isolate->heap()->exception();
@ -307,21 +319,14 @@ Object* DeclareLookupSlot(Isolate* isolate, Handle<String> name,
RUNTIME_FUNCTION(Runtime_DeclareLookupSlot) {
HandleScope scope(isolate);
DCHECK_EQ(2, args.length());
DCHECK_EQ(3, args.length());
CONVERT_ARG_HANDLE_CHECKED(String, name, 0);
CONVERT_ARG_HANDLE_CHECKED(Object, initial_value, 1);
CONVERT_ARG_HANDLE_CHECKED(Smi, property_attributes, 2);
return DeclareLookupSlot(isolate, name, initial_value, NONE);
}
RUNTIME_FUNCTION(Runtime_DeclareReadOnlyLookupSlot) {
HandleScope scope(isolate);
DCHECK_EQ(2, args.length());
CONVERT_ARG_HANDLE_CHECKED(String, name, 0);
CONVERT_ARG_HANDLE_CHECKED(Object, initial_value, 1);
return DeclareLookupSlot(isolate, name, initial_value, READ_ONLY);
PropertyAttributes attributes =
static_cast<PropertyAttributes>(property_attributes->value());
return DeclareLookupSlot(isolate, name, initial_value, attributes);
}

View File

@ -566,8 +566,7 @@ namespace internal {
F(DeclareGlobals, 2, 1) \
F(InitializeVarGlobal, 3, 1) \
F(InitializeConstGlobal, 2, 1) \
F(DeclareLookupSlot, 2, 1) \
F(DeclareReadOnlyLookupSlot, 2, 1) \
F(DeclareLookupSlot, 3, 1) \
F(InitializeLegacyConstLookupSlot, 3, 1) \
F(NewSloppyArguments_Generic, 1, 1) \
F(NewStrictArguments_Generic, 1, 1) \

View File

@ -44,6 +44,7 @@ Variable::Variable(Scope* scope, const AstRawString* name, VariableMode mode,
strong_mode_reference_start_position_(RelocInfo::kNoPosition),
strong_mode_reference_end_position_(RelocInfo::kNoPosition),
local_if_not_shadowed_(NULL),
is_from_eval_(false),
force_context_allocation_(false),
is_used_(false),
initialization_flag_(initialization_flag),

View File

@ -124,6 +124,8 @@ class Variable: public ZoneObject {
index_ = index;
}
void SetFromEval() { is_from_eval_ = true; }
static int CompareIndex(Variable* const* v, Variable* const* w);
void RecordStrongModeReference(int start_position, int end_position) {
@ -144,6 +146,16 @@ class Variable: public ZoneObject {
int strong_mode_reference_end_position() const {
return strong_mode_reference_end_position_;
}
PropertyAttributes DeclarationPropertyAttributes() const {
int property_attributes = NONE;
if (IsImmutableVariableMode(mode_)) {
property_attributes |= READ_ONLY;
}
if (is_from_eval_) {
property_attributes |= EVAL_DECLARED;
}
return static_cast<PropertyAttributes>(property_attributes);
}
private:
Scope* scope_;
@ -165,6 +177,9 @@ class Variable: public ZoneObject {
// binding scope (exclusive).
Variable* local_if_not_shadowed_;
// True if this variable is introduced by a sloppy eval
bool is_from_eval_;
// Usage info.
bool force_context_allocation_; // set by variable resolver
bool is_used_;

View File

@ -0,0 +1,109 @@
// Copyright 2015 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --harmony-sloppy --harmony-sloppy-let --harmony-sloppy-function
// Var-let conflict in a function throws, even if the var is in an eval
let caught = false;
// Throws at the top level of a function
try {
(function() {
let x = 1;
eval('const x = 2');
})()
} catch (e) {
caught = true;
}
assertTrue(caught);
// If the eval is in its own block scope, throws
caught = false;
try {
(function() {
let y = 1;
{ eval('const y = 2'); }
})()
} catch (e) {
caught = true;
}
assertTrue(caught);
// If the let is in its own block scope, with the eval, throws
caught = false
try {
(function() {
{
let x = 1;
eval('const x = 2');
}
})();
} catch (e) {
caught = true;
}
assertTrue(caught);
// Legal if the let is no longer visible
caught = false
try {
(function() {
{
let x = 1;
}
eval('const x = 2');
})();
} catch (e) {
caught = true;
}
assertFalse(caught);
// In global scope
caught = false;
try {
let z = 1;
eval('const z = 2');
} catch (e) {
caught = true;
}
assertTrue(caught);
// Let declarations beyond a function boundary don't conflict
caught = false;
try {
let a = 1;
(function() {
eval('const a');
})();
} catch (e) {
caught = true;
}
assertFalse(caught);
// var across with doesn't conflict
caught = false;
try {
(function() {
with ({x: 1}) {
eval("const x = 2;");
}
})();
} catch (e) {
caught = true;
}
assertFalse(caught);
// var can still conflict with let across a with
caught = false;
try {
(function() {
let x;
with ({x: 1}) {
eval("const x = 2;");
}
})();
} catch (e) {
caught = true;
}
assertTrue(caught);

View File

@ -0,0 +1,191 @@
// Copyright 2015 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --harmony-sloppy --harmony-sloppy-let --harmony-sloppy-function --no-legacy-const
// Var-let conflict in a function throws, even if the var is in an eval
let caught = false;
// Throws at the top level of a function
try {
(function() {
let x = 1;
eval('var x = 2');
})()
} catch (e) {
caught = true;
}
assertTrue(caught);
// If the eval is in its own block scope, throws
caught = false;
try {
(function() {
let y = 1;
{ eval('var y = 2'); }
})()
} catch (e) {
caught = true;
}
assertTrue(caught);
// If the let is in its own block scope, with the eval, throws
caught = false
try {
(function() {
{
let x = 1;
eval('var x = 2');
}
})();
} catch (e) {
caught = true;
}
assertTrue(caught);
// Legal if the let is no longer visible
caught = false
try {
(function() {
{
let x = 1;
}
eval('var x = 2');
})();
} catch (e) {
caught = true;
}
assertFalse(caught);
// All the same works for const:
// Throws at the top level of a function
try {
(function() {
const x = 1;
eval('var x = 2');
})();
} catch (e) {
caught = true;
}
assertTrue(caught);
// If the eval is in its own block scope, throws
caught = false;
try {
(function() {
const y = 1;
{ eval('var y = 2'); }
})();
} catch (e) {
caught = true;
}
assertTrue(caught);
// If the const is in its own block scope, with the eval, throws
caught = false
try {
(function() {
{
const x = 1;
eval('var x = 2');
}
})();
} catch (e) {
caught = true;
}
assertTrue(caught);
// Legal if the const is no longer visible
caught = false
try {
(function() {
{
const x = 1;
}
eval('var x = 2');
})();
} catch (e) {
caught = true;
}
assertFalse(caught);
// In global scope
caught = false;
try {
let z = 1;
eval('var z = 2');
} catch (e) {
caught = true;
}
assertTrue(caught);
// Let declarations beyond a function boundary don't conflict
caught = false;
try {
let a = 1;
(function() {
eval('var a');
})();
} catch (e) {
caught = true;
}
assertFalse(caught);
// var across with doesn't conflict
caught = false;
try {
(function() {
with ({x: 1}) {
eval("var x = 2;");
}
})();
} catch (e) {
caught = true;
}
assertFalse(caught);
// var can still conflict with let across a with
caught = false;
try {
(function() {
let x;
with ({x: 1}) {
eval("var x = 2;");
}
})();
} catch (e) {
caught = true;
}
assertTrue(caught);
// Functions declared in eval also conflict
caught = false
try {
(function() {
{
let x = 1;
eval('function x() {}');
}
})();
} catch (e) {
caught = true;
}
assertTrue(caught);
// TODO(littledan): Hoisting x out of the block should be
// prevented in this case BUG(v8:4479)
caught = false
try {
(function() {
{
let x = 1;
eval('{ function x() {} }');
}
})();
} catch (e) {
caught = true;
}
// TODO(littledan): switch to assertTrue when bug is fixed
assertTrue(caught);