Commit cb7279da authored by marja's avatar marja Committed by Commit bot

[strong] Check strong mode free variables against the global object.

Gather references to unbound variables where the reference (VariableProxy) is
inside strong mode. Check them against the global object when a script is bound
to a context (during compilation).

This CL only checks unbound variables which are not inside lazy functions - TBD
how do we solve that; alternatives: add developer mode which disables laziness /
do the check whenever lazy functions are really compiled.

BUG=v8:3956
LOG=N

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

Cr-Commit-Position: refs/heads/master@{#27422}
parent bb880058
......@@ -22,6 +22,7 @@
#include "src/bootstrapper.h"
#include "src/code-stubs.h"
#include "src/compiler.h"
#include "src/contexts.h"
#include "src/conversions-inl.h"
#include "src/counters.h"
#include "src/cpu-profiler.h"
......@@ -36,6 +37,7 @@
#include "src/messages.h"
#include "src/natives.h"
#include "src/parser.h"
#include "src/pending-compilation-error-handler.h"
#include "src/profile-generator-inl.h"
#include "src/property.h"
#include "src/property-details.h"
......@@ -1497,9 +1499,36 @@ Local<Script> UnboundScript::BindToCurrentContext() {
i::Handle<i::HeapObject>::cast(Utils::OpenHandle(this));
i::Handle<i::SharedFunctionInfo>
function_info(i::SharedFunctionInfo::cast(*obj), obj->GetIsolate());
i::Isolate* isolate = obj->GetIsolate();
i::ScopeInfo* scope_info = function_info->scope_info();
i::Handle<i::JSReceiver> global(isolate->native_context()->global_object());
for (int i = 0; i < scope_info->StrongModeFreeVariableCount(); ++i) {
i::Handle<i::String> name_string(scope_info->StrongModeFreeVariableName(i));
i::ScriptContextTable::LookupResult result;
i::Handle<i::ScriptContextTable> script_context_table(
isolate->native_context()->script_context_table());
if (!i::ScriptContextTable::Lookup(script_context_table, name_string,
&result)) {
i::Handle<i::Name> name(scope_info->StrongModeFreeVariableName(i));
Maybe<bool> has = i::JSReceiver::HasProperty(global, name);
if (has.IsJust() && !has.FromJust()) {
i::PendingCompilationErrorHandler pending_error_handler_;
pending_error_handler_.ReportMessageAt(
scope_info->StrongModeFreeVariableStartPosition(i),
scope_info->StrongModeFreeVariableEndPosition(i),
"strong_unbound_global", name_string, i::kReferenceError);
i::Handle<i::Script> script(i::Script::cast(function_info->script()));
pending_error_handler_.ThrowPendingError(isolate, script);
isolate->ReportPendingMessages();
isolate->OptionalRescheduleException(true);
return Local<Script>();
}
}
}
i::Handle<i::JSFunction> function =
obj->GetIsolate()->factory()->NewFunctionFromSharedFunctionInfo(
function_info, obj->GetIsolate()->native_context());
function_info, isolate->native_context());
return ToApiHandle<Script>(function);
}
......@@ -1947,7 +1976,9 @@ MaybeLocal<Script> ScriptCompiler::Compile(Local<Context> context,
i::Handle<i::SharedFunctionInfo> result(raw_result, isolate);
Local<UnboundScript> generic = ToApiHandle<UnboundScript>(result);
if (generic.IsEmpty()) return Local<Script>();
RETURN_ESCAPED(generic->BindToCurrentContext());
Local<Script> bound = generic->BindToCurrentContext();
if (bound.IsEmpty()) return Local<Script>();
RETURN_ESCAPED(bound);
}
......
......@@ -172,6 +172,7 @@ var kMessages = {
strong_for_in: ["In strong mode, 'for'-'in' loops are deprecated, use 'for'-'of' instead"],
strong_empty: ["In strong mode, empty sub-statements are deprecated, make them explicit with '{}' instead"],
strong_use_before_declaration: ["In strong mode, declaring variable '", "%0", "' before its use is required"],
strong_unbound_global: ["In strong mode, using an undeclared global variable '", "%0", "' is not allowed"],
strong_super_call_missing: ["In strong mode, invoking the super constructor in a subclass is required"],
strong_super_call_duplicate: ["In strong mode, invoking the super constructor multiple times is deprecated"],
strong_super_call_nested: ["In strong mode, invoking the super constructor nested inside another statement or expression is deprecated"],
......
......@@ -4312,6 +4312,10 @@ class ScopeInfo : public FixedArray {
// exposed to the user in a debugger.
bool LocalIsSynthetic(int var);
String* StrongModeFreeVariableName(int var);
int StrongModeFreeVariableStartPosition(int var);
int StrongModeFreeVariableEndPosition(int var);
// Lookup support for serialized scope info. Returns the
// the stack slot index for a given slot name if the slot is
// present; otherwise returns a value < 0. The name must be an internalized
......@@ -4364,11 +4368,12 @@ class ScopeInfo : public FixedArray {
// 3. The number of non-parameter variables allocated on the stack.
// 4. The number of non-parameter and parameter variables allocated in the
// context.
#define FOR_EACH_NUMERIC_FIELD(V) \
V(Flags) \
V(ParameterCount) \
V(StackLocalCount) \
V(ContextLocalCount)
#define FOR_EACH_NUMERIC_FIELD(V) \
V(Flags) \
V(ParameterCount) \
V(StackLocalCount) \
V(ContextLocalCount) \
V(StrongModeFreeVariableCount)
#define FIELD_ACCESSORS(name) \
void Set##name(int value) { \
......@@ -4415,7 +4420,12 @@ class ScopeInfo : public FixedArray {
// the context locals in ContextLocalNameEntries. One slot is used per
// context local, so in total this part occupies ContextLocalCount()
// slots in the array.
// 5. FunctionNameEntryIndex:
// 5. StrongModeFreeVariableNameEntries:
// Stores the names of strong mode free variables.
// 6. StrongModeFreeVariablePositionEntries:
// Stores the locations (start and end position) of strong mode free
// variables.
// 7. FunctionNameEntryIndex:
// If the scope belongs to a named function expression this part contains
// information about the function variable. It always occupies two array
// slots: a. The name of the function variable.
......@@ -4424,6 +4434,8 @@ class ScopeInfo : public FixedArray {
int StackLocalEntriesIndex();
int ContextLocalNameEntriesIndex();
int ContextLocalInfoEntriesIndex();
int StrongModeFreeVariableNameEntriesIndex();
int StrongModeFreeVariablePositionEntriesIndex();
int FunctionNameEntryIndex();
// Location of the function variable for named function expressions.
......
......@@ -3666,6 +3666,10 @@ bool CheckAndDeclareArrowParameter(ParserTraits* traits, Expression* expression,
return false;
}
// When the variable was seen, it was recorded as unresolved in the outer
// scope. But it's really not unresolved.
scope->outer_scope()->RemoveUnresolved(expression->AsVariableProxy());
scope->DeclareParameter(raw_name, VAR);
++(*num_params);
return true;
......
......@@ -17,7 +17,7 @@ void PendingCompilationErrorHandler::ThrowPendingError(Isolate* isolate,
if (!has_pending_error_) return;
MessageLocation location(script, start_position_, end_position_);
Factory* factory = isolate->factory();
bool has_arg = arg_ != NULL || char_arg_ != NULL;
bool has_arg = arg_ != NULL || char_arg_ != NULL || !handle_arg_.is_null();
Handle<FixedArray> elements = factory->NewFixedArray(has_arg ? 1 : 0);
if (arg_ != NULL) {
Handle<String> arg_string = arg_->string();
......@@ -26,6 +26,8 @@ void PendingCompilationErrorHandler::ThrowPendingError(Isolate* isolate,
Handle<String> arg_string =
factory->NewStringFromUtf8(CStrVector(char_arg_)).ToHandleChecked();
elements->set(0, *arg_string);
} else if (!handle_arg_.is_null()) {
elements->set(0, *handle_arg_);
}
isolate->debug()->OnCompileError(script);
......
......@@ -7,13 +7,12 @@
#include "src/base/macros.h"
#include "src/globals.h"
#include "src/handles.h"
namespace v8 {
namespace internal {
class AstRawString;
template <class T>
class Handle;
class Isolate;
class Script;
......@@ -56,6 +55,20 @@ class PendingCompilationErrorHandler {
error_type_ = error_type;
}
void ReportMessageAt(int start_position, int end_position,
const char* message, Handle<String> arg,
ParseErrorType error_type = kSyntaxError) {
if (has_pending_error_) return;
has_pending_error_ = true;
start_position_ = start_position;
end_position_ = end_position;
message_ = message;
char_arg_ = nullptr;
arg_ = nullptr;
handle_arg_ = arg;
error_type_ = error_type;
}
bool has_pending_error() const { return has_pending_error_; }
void ThrowPendingError(Isolate* isolate, Handle<Script> script);
......@@ -67,6 +80,7 @@ class PendingCompilationErrorHandler {
const char* message_;
const AstRawString* arg_;
const char* char_arg_;
Handle<String> handle_arg_;
ParseErrorType error_type_;
DISALLOW_COPY_AND_ASSIGN(PendingCompilationErrorHandler);
......
......@@ -18,9 +18,14 @@ Handle<ScopeInfo> ScopeInfo::Create(Isolate* isolate, Zone* zone,
// Collect stack and context locals.
ZoneList<Variable*> stack_locals(scope->StackLocalCount(), zone);
ZoneList<Variable*> context_locals(scope->ContextLocalCount(), zone);
scope->CollectStackAndContextLocals(&stack_locals, &context_locals);
ZoneList<Variable*> strong_mode_free_variables(0, zone);
scope->CollectStackAndContextLocals(&stack_locals, &context_locals,
&strong_mode_free_variables);
const int stack_local_count = stack_locals.length();
const int context_local_count = context_locals.length();
const int strong_mode_free_variable_count =
strong_mode_free_variables.length();
// Make sure we allocate the correct amount.
DCHECK(scope->StackLocalCount() == stack_local_count);
DCHECK(scope->ContextLocalCount() == context_local_count);
......@@ -49,9 +54,10 @@ Handle<ScopeInfo> ScopeInfo::Create(Isolate* isolate, Zone* zone,
const bool has_function_name = function_name_info != NONE;
const int parameter_count = scope->num_parameters();
const int length = kVariablePartIndex
+ parameter_count + stack_local_count + 2 * context_local_count
+ (has_function_name ? 2 : 0);
const int length = kVariablePartIndex + parameter_count + stack_local_count +
2 * context_local_count +
3 * strong_mode_free_variable_count +
(has_function_name ? 2 : 0);
Factory* factory = isolate->factory();
Handle<ScopeInfo> scope_info = factory->NewScopeInfo(length);
......@@ -71,6 +77,7 @@ Handle<ScopeInfo> ScopeInfo::Create(Isolate* isolate, Zone* zone,
scope_info->SetParameterCount(parameter_count);
scope_info->SetStackLocalCount(stack_local_count);
scope_info->SetContextLocalCount(context_local_count);
scope_info->SetStrongModeFreeVariableCount(strong_mode_free_variable_count);
int index = kVariablePartIndex;
// Add parameters.
......@@ -113,6 +120,25 @@ Handle<ScopeInfo> ScopeInfo::Create(Isolate* isolate, Zone* zone,
scope_info->set(index++, Smi::FromInt(value));
}
DCHECK(index == scope_info->StrongModeFreeVariableNameEntriesIndex());
for (int i = 0; i < strong_mode_free_variable_count; ++i) {
scope_info->set(index++, *strong_mode_free_variables[i]->name());
}
DCHECK(index == scope_info->StrongModeFreeVariablePositionEntriesIndex());
for (int i = 0; i < strong_mode_free_variable_count; ++i) {
// Unfortunately, the source code positions are stored as int even though
// int32_t would be enough (given the maximum source code length).
Handle<Object> start_position = factory->NewNumberFromInt(
static_cast<int32_t>(strong_mode_free_variables[i]
->strong_mode_reference_start_position()));
scope_info->set(index++, *start_position);
Handle<Object> end_position = factory->NewNumberFromInt(
static_cast<int32_t>(strong_mode_free_variables[i]
->strong_mode_reference_end_position()));
scope_info->set(index++, *end_position);
}
// If present, add the function variable name and its index.
DCHECK(index == scope_info->FunctionNameEntryIndex());
if (has_function_name) {
......@@ -285,6 +311,35 @@ bool ScopeInfo::LocalIsSynthetic(int var) {
}
String* ScopeInfo::StrongModeFreeVariableName(int var) {
DCHECK(0 <= var && var < StrongModeFreeVariableCount());
int info_index = StrongModeFreeVariableNameEntriesIndex() + var;
return String::cast(get(info_index));
}
int ScopeInfo::StrongModeFreeVariableStartPosition(int var) {
DCHECK(0 <= var && var < StrongModeFreeVariableCount());
int info_index = StrongModeFreeVariablePositionEntriesIndex() + var * 2;
int32_t value = 0;
bool ok = get(info_index)->ToInt32(&value);
USE(ok);
DCHECK(ok);
return value;
}
int ScopeInfo::StrongModeFreeVariableEndPosition(int var) {
DCHECK(0 <= var && var < StrongModeFreeVariableCount());
int info_index = StrongModeFreeVariablePositionEntriesIndex() + var * 2 + 1;
int32_t value = 0;
bool ok = get(info_index)->ToInt32(&value);
USE(ok);
DCHECK(ok);
return value;
}
int ScopeInfo::StackSlotIndex(String* name) {
DCHECK(name->IsInternalizedString());
if (length() > 0) {
......@@ -432,11 +487,23 @@ int ScopeInfo::ContextLocalInfoEntriesIndex() {
}
int ScopeInfo::FunctionNameEntryIndex() {
int ScopeInfo::StrongModeFreeVariableNameEntriesIndex() {
return ContextLocalInfoEntriesIndex() + ContextLocalCount();
}
int ScopeInfo::StrongModeFreeVariablePositionEntriesIndex() {
return StrongModeFreeVariableNameEntriesIndex() +
StrongModeFreeVariableCount();
}
int ScopeInfo::FunctionNameEntryIndex() {
return StrongModeFreeVariablePositionEntriesIndex() +
2 * StrongModeFreeVariableCount();
}
int ContextSlotCache::Hash(Object* data, String* name) {
// Uses only lower 32 bits if pointers are larger.
uintptr_t addr_hash =
......
......@@ -586,8 +586,9 @@ class VarAndOrder {
};
void Scope::CollectStackAndContextLocals(ZoneList<Variable*>* stack_locals,
ZoneList<Variable*>* context_locals) {
void Scope::CollectStackAndContextLocals(
ZoneList<Variable*>* stack_locals, ZoneList<Variable*>* context_locals,
ZoneList<Variable*>* strong_mode_free_variables) {
DCHECK(stack_locals != NULL);
DCHECK(context_locals != NULL);
......@@ -621,6 +622,11 @@ void Scope::CollectStackAndContextLocals(ZoneList<Variable*>* stack_locals,
p != NULL;
p = variables_.Next(p)) {
Variable* var = reinterpret_cast<Variable*>(p->value);
if (strong_mode_free_variables && var->has_strong_mode_reference() &&
var->mode() == DYNAMIC_GLOBAL) {
strong_mode_free_variables->Add(var, zone());
}
if (var->is_used()) {
vars.Add(VarAndOrder(var, p->order), zone());
}
......@@ -1107,6 +1113,12 @@ bool Scope::ResolveVariable(ParseInfo* info, VariableProxy* proxy,
DCHECK(var != NULL);
if (proxy->is_assigned()) var->set_maybe_assigned();
if (is_strong(language_mode())) {
// Record that the variable is referred to from strong mode. Also, record
// the position.
var->RecordStrongModeReference(proxy->position(), proxy->end_position());
}
proxy->BindTo(var);
return true;
......
......@@ -414,8 +414,9 @@ class Scope: public ZoneObject {
// Collect stack and context allocated local variables in this scope. Note
// that the function variable - if present - is not collected and should be
// handled separately.
void CollectStackAndContextLocals(ZoneList<Variable*>* stack_locals,
ZoneList<Variable*>* context_locals);
void CollectStackAndContextLocals(
ZoneList<Variable*>* stack_locals, ZoneList<Variable*>* context_locals,
ZoneList<Variable*>* strong_mode_free_variables = nullptr);
// Current number of var or const locals.
int num_var_or_const() { return num_var_or_const_; }
......
......@@ -42,6 +42,9 @@ Variable::Variable(Scope* scope, const AstRawString* name, VariableMode mode,
location_(UNALLOCATED),
index_(-1),
initializer_position_(RelocInfo::kNoPosition),
has_strong_mode_reference_(false),
strong_mode_reference_start_position_(RelocInfo::kNoPosition),
strong_mode_reference_end_position_(RelocInfo::kNoPosition),
local_if_not_shadowed_(NULL),
force_context_allocation_(false),
is_used_(false),
......
......@@ -128,6 +128,25 @@ class Variable: public ZoneObject {
static int CompareIndex(Variable* const* v, Variable* const* w);
void RecordStrongModeReference(int start_position, int end_position) {
// Record the earliest reference to the variable. Used in error messages for
// strong mode references to undeclared variables.
if (has_strong_mode_reference_ &&
strong_mode_reference_start_position_ < start_position)
return;
has_strong_mode_reference_ = true;
strong_mode_reference_start_position_ = start_position;
strong_mode_reference_end_position_ = end_position;
}
bool has_strong_mode_reference() const { return has_strong_mode_reference_; }
int strong_mode_reference_start_position() const {
return strong_mode_reference_start_position_;
}
int strong_mode_reference_end_position() const {
return strong_mode_reference_end_position_;
}
private:
Scope* scope_;
const AstRawString* name_;
......@@ -136,6 +155,11 @@ class Variable: public ZoneObject {
Location location_;
int index_;
int initializer_position_;
// Tracks whether the variable is bound to a VariableProxy which is in strong
// mode, and if yes, the source location of the reference.
bool has_strong_mode_reference_;
int strong_mode_reference_start_position_;
int strong_mode_reference_end_position_;
// If this field is set, this variable references the stored locally bound
// variable, but it might be shadowed by variable bindings introduced by
......
......@@ -5638,3 +5638,153 @@ TEST(ArrowFunctionASIErrors) {
RunParserSyncTest(context_data, data, kError, NULL, 0, always_flags,
arraysize(always_flags));
}
TEST(StrongModeFreeVariablesDeclaredByPreviousScript) {
i::FLAG_strong_mode = true;
v8::V8::Initialize();
v8::HandleScope scope(CcTest::isolate());
v8::Context::Scope context_scope(v8::Context::New(CcTest::isolate()));
v8::TryCatch try_catch;
// Introduce a bunch of variables, in all language modes.
const char* script1 =
"var my_var1 = 0; \n"
"function my_func1() { } \n"
"const my_const1 = 0; \n";
CompileRun(v8_str(script1));
CHECK(!try_catch.HasCaught());
const char* script2 =
"\"use strict\"; \n"
"let my_var2 = 0; \n"
"function my_func2() { } \n"
"const my_const2 = 0 \n";
CompileRun(v8_str(script2));
CHECK(!try_catch.HasCaught());
const char* script3 =
"\"use strong\"; \n"
"let my_var3 = 0; \n"
"function my_func3() { } \n"
"const my_const3 = 0; \n";
CompileRun(v8_str(script3));
CHECK(!try_catch.HasCaught());
// Sloppy eval introduces variables in the surrounding scope.
const char* script4 =
"eval('var my_var4 = 0;') \n"
"eval('function my_func4() { }') \n"
"eval('const my_const4 = 0;') \n";
CompileRun(v8_str(script4));
CHECK(!try_catch.HasCaught());
// Test that referencing these variables work.
const char* script5 =
"\"use strong\"; \n"
"my_var1; \n"
"my_func1; \n"
"my_const1; \n"
"my_var2; \n"
"my_func2; \n"
"my_const2; \n"
"my_var3; \n"
"my_func3; \n"
"my_const3; \n"
"my_var4; \n"
"my_func4; \n"
"my_const4; \n";
CompileRun(v8_str(script5));
CHECK(!try_catch.HasCaught());
}
TEST(StrongModeFreeVariablesDeclaredByLanguage) {
i::FLAG_strong_mode = true;
v8::V8::Initialize();
v8::HandleScope scope(CcTest::isolate());
v8::Context::Scope context_scope(v8::Context::New(CcTest::isolate()));
v8::TryCatch try_catch;
const char* script1 =
"\"use strong\"; \n"
"Math; \n"
"RegExp; \n";
CompileRun(v8_str(script1));
CHECK(!try_catch.HasCaught());
}
TEST(StrongModeFreeVariablesDeclaredInGlobalPrototype) {
i::FLAG_strong_mode = true;
v8::V8::Initialize();
v8::HandleScope scope(CcTest::isolate());
v8::Context::Scope context_scope(v8::Context::New(CcTest::isolate()));
v8::TryCatch try_catch;
const char* script1 = "this.__proto__.my_var = 0;\n";
CompileRun(v8_str(script1));
CHECK(!try_catch.HasCaught());
const char* script2 =
"\"use strong\"; \n"
"my_var; \n";
CompileRun(v8_str(script2));
CHECK(!try_catch.HasCaught());
}
TEST(StrongModeFreeVariablesNotDeclared) {
i::FLAG_strong_mode = true;
v8::V8::Initialize();
v8::HandleScope scope(CcTest::isolate());
v8::Context::Scope context_scope(v8::Context::New(CcTest::isolate()));
v8::TryCatch try_catch;
// Test that referencing unintroduced variables in sloppy mode is ok.
const char* script1 =
"if (false) { \n"
" not_there1; \n"
"} \n";
CompileRun(v8_str(script1));
CHECK(!try_catch.HasCaught());
// But not in strong mode.
{
const char* script2 =
"\"use strong\"; \n"
"if (false) { \n"
" not_there2; \n"
"} \n";
v8::TryCatch try_catch2;
v8::Script::Compile(v8_str(script2));
CHECK(try_catch2.HasCaught());
v8::String::Utf8Value exception(try_catch2.Exception());
CHECK_EQ(0,
strcmp(
"ReferenceError: In strong mode, using an undeclared global "
"variable 'not_there2' is not allowed",
*exception));
}
// Check that the variable reference is detected inside a strong function too,
// even if the script scope is not strong.
{
const char* script3 =
"(function not_lazy() { \n"
" \"use strong\"; \n"
" if (false) { \n"
" not_there3; \n"
" } \n"
"})(); \n";
v8::TryCatch try_catch2;
v8::Script::Compile(v8_str(script3));
CHECK(try_catch2.HasCaught());
v8::String::Utf8Value exception(try_catch2.Exception());
CHECK_EQ(0,
strcmp(
"ReferenceError: In strong mode, using an undeclared global "
"variable 'not_there3' is not allowed",
*exception));
}
}
......@@ -208,6 +208,9 @@ function assertThrowsHelper(code) {
let func5 = (p1, p2) => { p1; p2; };
func5();
let func5b = p1 => p1;
func5b();
function func6() {
var1, var2a, var2b, var2c;
}
......
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