Commit 3825d796 authored by Sathya Gunasekaran's avatar Sathya Gunasekaran Committed by Commit Bot

[class] Throw error on accessing invalid private fields

Report an error during scope analysis if we're unable to find a
variable proxy for the given private field. This can happen if we try
to access a private field that was not defined or if we're outside
the class scope.

This doesn't correctly throw an early error when pre parsing a top
level function because we don't track it's variables.

Bug: v8:5368
Change-Id: I0a1193fe0ae213c0732fae5d435e150852a8d87d
Reviewed-on: https://chromium-review.googlesource.com/892093Reviewed-by: 's avatarAdam Klein <adamk@chromium.org>
Reviewed-by: 's avatarRoss McIlroy <rmcilroy@chromium.org>
Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51082}
parent 537e89a3
......@@ -1529,6 +1529,11 @@ class VariableProxy final : public Expression {
HoleCheckModeField::update(bit_field_, HoleCheckMode::kRequired);
}
bool is_private_field() const { return IsPrivateField::decode(bit_field_); }
void set_is_private_field() {
bit_field_ = IsPrivateField::update(bit_field_, true);
}
// Bind this proxy to the variable var.
void BindTo(Variable* var);
......@@ -1548,7 +1553,8 @@ class VariableProxy final : public Expression {
bit_field_ |= IsThisField::encode(variable_kind == THIS_VARIABLE) |
IsAssignedField::encode(false) |
IsResolvedField::encode(false) |
HoleCheckModeField::encode(HoleCheckMode::kElided);
HoleCheckModeField::encode(HoleCheckMode::kElided) |
IsPrivateField::encode(false);
}
explicit VariableProxy(const VariableProxy* copy_from);
......@@ -1560,6 +1566,7 @@ class VariableProxy final : public Expression {
class IsNewTargetField : public BitField<bool, IsResolvedField::kNext, 1> {};
class HoleCheckModeField
: public BitField<HoleCheckMode, IsNewTargetField::kNext, 1> {};
class IsPrivateField : public BitField<bool, HoleCheckModeField::kNext, 1> {};
union {
const AstRawString* raw_name_; // if !is_resolved_
......
......@@ -643,7 +643,7 @@ void DeclarationScope::AttachOuterScopeInfo(ParseInfo* info, Isolate* isolate) {
}
}
void DeclarationScope::Analyze(ParseInfo* info) {
bool DeclarationScope::Analyze(ParseInfo* info) {
RuntimeCallTimerScope runtimeTimer(
info->runtime_call_stats(),
info->on_background_thread()
......@@ -681,7 +681,7 @@ void DeclarationScope::Analyze(ParseInfo* info) {
info->consumed_preparsed_scope_data()->RestoreScopeAllocationData(scope);
}
scope->AllocateVariables(info);
if (!scope->AllocateVariables(info)) return false;
#ifdef DEBUG
if (info->is_native() ? FLAG_print_builtin_scopes : FLAG_print_scopes) {
......@@ -691,6 +691,8 @@ void DeclarationScope::Analyze(ParseInfo* info) {
scope->CheckScopePositions();
scope->CheckZones();
#endif
return true;
}
void DeclarationScope::DeclareThis(AstValueFactory* ast_value_factory) {
......@@ -1342,13 +1344,18 @@ Declaration* Scope::CheckLexDeclarationsConflictingWith(
return nullptr;
}
void DeclarationScope::AllocateVariables(ParseInfo* info) {
bool DeclarationScope::AllocateVariables(ParseInfo* info) {
// Module variables must be allocated before variable resolution
// to ensure that UpdateNeedsHoleCheck() can detect import variables.
if (is_module_scope()) AsModuleScope()->AllocateModuleVariables();
ResolveVariablesRecursively(info);
if (!ResolveVariablesRecursively(info)) {
DCHECK(info->pending_error_handler()->has_pending_error());
return false;
}
AllocateVariablesRecursively();
return true;
}
bool Scope::AllowsLazyParsingWithoutUnresolvedVariables(
......@@ -1811,7 +1818,8 @@ Variable* Scope::NonLocal(const AstRawString* name, VariableMode mode) {
return var;
}
Variable* Scope::LookupRecursive(VariableProxy* proxy, Scope* outer_scope_end) {
Variable* Scope::LookupRecursive(ParseInfo* info, VariableProxy* proxy,
Scope* outer_scope_end) {
DCHECK_NE(outer_scope_end, this);
// Short-cut: whenever we find a debug-evaluate scope, just look everything up
// dynamically. Debug-evaluate doesn't properly create scope info for the
......@@ -1834,6 +1842,15 @@ Variable* Scope::LookupRecursive(VariableProxy* proxy, Scope* outer_scope_end) {
// We may just be trying to find all free variables. In that case, don't
// declare them in the outer scope.
if (!is_script_scope()) return nullptr;
if (proxy->is_private_field()) {
info->pending_error_handler()->ReportMessageAt(
proxy->position(), proxy->position() + 1,
MessageTemplate::kInvalidPrivateFieldAccess, proxy->raw_name(),
kSyntaxError);
return nullptr;
}
// No binding has been found. Declare a variable on the global object.
return AsDeclarationScope()->DeclareDynamicGlobal(proxy->raw_name(),
NORMAL_VARIABLE);
......@@ -1841,7 +1858,7 @@ Variable* Scope::LookupRecursive(VariableProxy* proxy, Scope* outer_scope_end) {
DCHECK(!is_script_scope());
var = outer_scope_->LookupRecursive(proxy, outer_scope_end);
var = outer_scope_->LookupRecursive(info, proxy, outer_scope_end);
// The variable could not be resolved statically.
if (var == nullptr) return var;
......@@ -1899,11 +1916,16 @@ Variable* Scope::LookupRecursive(VariableProxy* proxy, Scope* outer_scope_end) {
return var;
}
void Scope::ResolveVariable(ParseInfo* info, VariableProxy* proxy) {
bool Scope::ResolveVariable(ParseInfo* info, VariableProxy* proxy) {
DCHECK(info->script_scope()->is_script_scope());
DCHECK(!proxy->is_resolved());
Variable* var = LookupRecursive(proxy, nullptr);
Variable* var = LookupRecursive(info, proxy, nullptr);
if (var == nullptr) {
DCHECK(proxy->is_private_field());
return false;
}
ResolveTo(info, proxy, var);
return true;
}
namespace {
......@@ -1999,7 +2021,7 @@ void Scope::ResolveTo(ParseInfo* info, VariableProxy* proxy, Variable* var) {
proxy->BindTo(var);
}
void Scope::ResolveVariablesRecursively(ParseInfo* info) {
bool Scope::ResolveVariablesRecursively(ParseInfo* info) {
DCHECK(info->script_scope()->is_script_scope());
// Lazy parsed declaration scopes are already partially analyzed. If there are
// unresolved references remaining, they just need to be resolved in outer
......@@ -2008,7 +2030,11 @@ void Scope::ResolveVariablesRecursively(ParseInfo* info) {
DCHECK_EQ(variables_.occupancy(), 0);
for (VariableProxy* proxy = unresolved_; proxy != nullptr;
proxy = proxy->next_unresolved()) {
Variable* var = outer_scope()->LookupRecursive(proxy, nullptr);
Variable* var = outer_scope()->LookupRecursive(info, proxy, nullptr);
if (var == nullptr) {
DCHECK(proxy->is_private_field());
return false;
}
if (!var->is_dynamic()) {
var->set_is_used();
var->ForceContextAllocation();
......@@ -2019,15 +2045,16 @@ void Scope::ResolveVariablesRecursively(ParseInfo* info) {
// Resolve unresolved variables for this scope.
for (VariableProxy* proxy = unresolved_; proxy != nullptr;
proxy = proxy->next_unresolved()) {
ResolveVariable(info, proxy);
if (!ResolveVariable(info, proxy)) return false;
}
// Resolve unresolved variables for inner scopes.
for (Scope* scope = inner_scope_; scope != nullptr;
scope = scope->sibling_) {
scope->ResolveVariablesRecursively(info);
if (!scope->ResolveVariablesRecursively(info)) return false;
}
}
return true;
}
VariableProxy* Scope::FetchFreeVariables(DeclarationScope* max_outer_scope,
......@@ -2050,7 +2077,7 @@ VariableProxy* Scope::FetchFreeVariables(DeclarationScope* max_outer_scope,
next = proxy->next_unresolved();
DCHECK(!proxy->is_resolved());
Variable* var =
lookup->LookupRecursive(proxy, max_outer_scope->outer_scope());
lookup->LookupRecursive(info, proxy, max_outer_scope->outer_scope());
if (var == nullptr) {
proxy->set_next_unresolved(stack);
stack = proxy;
......
......@@ -587,10 +587,11 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
// scope, and stopping when reaching the outer_scope_end scope. If the code is
// executed because of a call to 'eval', the context parameter should be set
// to the calling context of 'eval'.
Variable* LookupRecursive(VariableProxy* proxy, Scope* outer_scope_end);
Variable* LookupRecursive(ParseInfo* info, VariableProxy* proxy,
Scope* outer_scope_end);
void ResolveTo(ParseInfo* info, VariableProxy* proxy, Variable* var);
void ResolveVariable(ParseInfo* info, VariableProxy* proxy);
void ResolveVariablesRecursively(ParseInfo* info);
MUST_USE_RESULT bool ResolveVariable(ParseInfo* info, VariableProxy* proxy);
MUST_USE_RESULT bool ResolveVariablesRecursively(ParseInfo* info);
// Finds free variables of this scope. This mutates the unresolved variables
// list along the way, so full resolution cannot be done afterwards.
......@@ -849,7 +850,12 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
// Compute top scope and allocate variables. For lazy compilation the top
// scope only contains the single lazily compiled function, so this
// doesn't re-allocate variables repeatedly.
static void Analyze(ParseInfo* info);
//
// Returns false if private fields can not be resolved and
// ParseInfo's pending_error_handler will be populated with an
// error. Otherwise, returns true.
MUST_USE_RESULT
static bool Analyze(ParseInfo* info);
// To be called during parsing. Do just enough scope analysis that we can
// discard the Scope contents for lazily compiled functions. In particular,
......@@ -920,7 +926,9 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
// In the case of code compiled and run using 'eval', the context
// parameter is the context in which eval was called. In all other
// cases the context parameter is an empty handle.
void AllocateVariables(ParseInfo* info);
//
// Returns false if private fields can not be resolved.
bool AllocateVariables(ParseInfo* info);
void SetDefaults();
......
......@@ -761,6 +761,21 @@ CompilationJob::Status FinalizeOptimizedCompilationJob(CompilationJob* job,
return CompilationJob::FAILED;
}
bool FailWithPendingException(Isolate* isolate, ParseInfo* parse_info,
Compiler::ClearExceptionFlag flag) {
if (flag == Compiler::CLEAR_EXCEPTION) {
isolate->clear_pending_exception();
} else if (!isolate->has_pending_exception()) {
if (parse_info->pending_error_handler()->has_pending_error()) {
parse_info->pending_error_handler()->ReportErrors(
isolate, parse_info->script(), parse_info->ast_value_factory());
} else {
isolate->StackOverflow();
}
}
return false;
}
MaybeHandle<SharedFunctionInfo> FinalizeTopLevel(
ParseInfo* parse_info, Isolate* isolate, CompilationJob* outer_function_job,
CompilationJobList* inner_function_jobs) {
......@@ -782,7 +797,8 @@ MaybeHandle<SharedFunctionInfo> FinalizeTopLevel(
// Finalize compilation of the unoptimized bytecode or asm-js data.
if (!FinalizeUnoptimizedCode(parse_info, isolate, shared_info,
outer_function_job, inner_function_jobs)) {
if (!isolate->has_pending_exception()) isolate->StackOverflow();
FailWithPendingException(isolate, parse_info,
Compiler::ClearExceptionFlag::KEEP_EXCEPTION);
return MaybeHandle<SharedFunctionInfo>();
}
......@@ -824,7 +840,8 @@ MaybeHandle<SharedFunctionInfo> CompileToplevel(ParseInfo* parse_info,
std::unique_ptr<CompilationJob> outer_function_job(GenerateUnoptimizedCode(
parse_info, isolate->allocator(), &inner_function_jobs));
if (!outer_function_job) {
if (!isolate->has_pending_exception()) isolate->StackOverflow();
FailWithPendingException(isolate, parse_info,
Compiler::ClearExceptionFlag::KEEP_EXCEPTION);
return MaybeHandle<SharedFunctionInfo>();
}
......@@ -832,16 +849,6 @@ MaybeHandle<SharedFunctionInfo> CompileToplevel(ParseInfo* parse_info,
&inner_function_jobs);
}
bool FailWithPendingException(Isolate* isolate,
Compiler::ClearExceptionFlag flag) {
if (flag == Compiler::CLEAR_EXCEPTION) {
isolate->clear_pending_exception();
} else if (!isolate->has_pending_exception()) {
isolate->StackOverflow();
}
return false;
}
} // namespace
// ----------------------------------------------------------------------------
......@@ -855,7 +862,7 @@ bool Compiler::Analyze(ParseInfo* parse_info) {
? RuntimeCallCounterId::kCompileBackgroundAnalyse
: RuntimeCallCounterId::kCompileAnalyse);
if (!Rewriter::Rewrite(parse_info)) return false;
DeclarationScope::Analyze(parse_info);
if (!DeclarationScope::Analyze(parse_info)) return false;
return true;
}
......@@ -885,18 +892,19 @@ bool Compiler::Compile(Handle<SharedFunctionInfo> shared_info,
TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"), "V8.CompileCode");
AggregatedHistogramTimerScope timer(isolate->counters()->compile_lazy());
// Set up parse info.
ParseInfo parse_info(shared_info);
parse_info.set_lazy_compile();
// Check if the compiler dispatcher has shared_info enqueued for compile.
CompilerDispatcher* dispatcher = isolate->compiler_dispatcher();
if (dispatcher->IsEnqueued(shared_info)) {
if (!dispatcher->FinishNow(shared_info)) {
return FailWithPendingException(isolate, flag);
return FailWithPendingException(isolate, &parse_info, flag);
}
return true;
}
// Set up parse info.
ParseInfo parse_info(shared_info);
parse_info.set_lazy_compile();
if (FLAG_preparser_scope_analysis) {
if (shared_info->HasPreParsedScopeData()) {
Handle<PreParsedScopeData> data(
......@@ -910,7 +918,7 @@ bool Compiler::Compile(Handle<SharedFunctionInfo> shared_info,
// Parse and update ParseInfo with the results.
if (!parsing::ParseFunction(&parse_info, shared_info, isolate)) {
return FailWithPendingException(isolate, flag);
return FailWithPendingException(isolate, &parse_info, flag);
}
// Generate the unoptimized bytecode or asm-js data.
......@@ -918,7 +926,7 @@ bool Compiler::Compile(Handle<SharedFunctionInfo> shared_info,
std::unique_ptr<CompilationJob> outer_function_job(GenerateUnoptimizedCode(
&parse_info, isolate->allocator(), &inner_function_jobs));
if (!outer_function_job) {
return FailWithPendingException(isolate, flag);
return FailWithPendingException(isolate, &parse_info, flag);
}
// Internalize ast values onto the heap.
......@@ -928,7 +936,7 @@ bool Compiler::Compile(Handle<SharedFunctionInfo> shared_info,
if (!FinalizeUnoptimizedCode(&parse_info, isolate, shared_info,
outer_function_job.get(),
&inner_function_jobs)) {
return FailWithPendingException(isolate, flag);
return FailWithPendingException(isolate, &parse_info, flag);
}
DCHECK(!isolate->has_pending_exception());
......
......@@ -121,10 +121,11 @@ void ScopeIterator::TryParseAndRetrieveScopes(ScopeIterator::Option option) {
CollectNonLocals(info.get(), scope);
}
if (!ignore_nested_scopes) {
DeclarationScope::Analyze(info.get());
DeclarationScope::AllocateScopeInfos(info.get(), isolate_,
AnalyzeMode::kDebugger);
RetrieveScopeChain(scope);
if (DeclarationScope::Analyze(info.get())) {
DeclarationScope::AllocateScopeInfos(info.get(), isolate_,
AnalyzeMode::kDebugger);
RetrieveScopeChain(scope);
}
}
} else {
// A failed reparse indicates that the preparser has diverged from the
......
......@@ -616,6 +616,7 @@ class ErrorUtils : public AllStatic {
"Invalid left-hand side expression in prefix operation") \
T(InvalidRegExpFlags, "Invalid flags supplied to RegExp constructor '%'") \
T(InvalidOrUnexpectedToken, "Invalid or unexpected token") \
T(InvalidPrivateFieldAccess, "Invalid private field '%'") \
T(JsonParseUnexpectedEOS, "Unexpected end of JSON input") \
T(JsonParseUnexpectedToken, "Unexpected token % in JSON at position %") \
T(JsonParseUnexpectedTokenNumber, "Unexpected number in JSON at position %") \
......
......@@ -3717,10 +3717,12 @@ ParserBase<Impl>::ParseMemberExpressionContinuation(ExpressionT expression,
ExpressionT key;
IdentifierT name;
if (allow_harmony_private_fields() && peek() == Token::PRIVATE_NAME) {
// TODO(gsathya): Validate that we are in a class body.
Consume(Token::PRIVATE_NAME);
name = impl()->GetSymbol();
key = impl()->ExpressionFromIdentifier(name, pos, InferName::kNo);
auto key_proxy =
impl()->ExpressionFromIdentifier(name, pos, InferName::kNo);
key_proxy->set_is_private_field();
key = key_proxy;
} else {
name = ParseIdentifierName(CHECK_OK);
key = factory()->NewStringLiteral(name, pos);
......
......@@ -888,7 +888,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
Literal* ExpressionFromLiteral(Token::Value token, int pos);
V8_INLINE Expression* ExpressionFromIdentifier(
V8_INLINE VariableProxy* ExpressionFromIdentifier(
const AstRawString* name, int start_position,
InferName infer = InferName::kYes) {
if (infer == InferName::kYes) {
......
......@@ -298,6 +298,14 @@ class PreParserExpression {
// and PreParser.
PreParserExpression* operator->() { return this; }
void set_is_private_field() {
if (variables_ != nullptr) {
DCHECK(IsIdentifier());
DCHECK_EQ(1, variables_->length());
variables_->first()->set_is_private_field();
}
}
// More dummy implementations of things PreParser doesn't need to track:
void SetShouldEagerCompile() {}
......
......@@ -750,7 +750,7 @@ TEST(PreParserScopeAnalysis) {
scope_with_skipped_functions));
// Do scope allocation (based on the preparsed scope data).
i::DeclarationScope::Analyze(&using_scope_data);
CHECK(i::DeclarationScope::Analyze(&using_scope_data));
// Parse the lazy function again eagerly to produce baseline data.
i::ParseInfo not_using_scope_data(shared);
......@@ -765,7 +765,7 @@ TEST(PreParserScopeAnalysis) {
scope_without_skipped_functions));
// Do normal scope allocation.
i::DeclarationScope::Analyze(&not_using_scope_data);
CHECK(i::DeclarationScope::Analyze(&not_using_scope_data));
// Verify that scope allocation gave the same results when parsing w/ the
// scope data (and skipping functions), and when parsing without.
......
......@@ -844,7 +844,7 @@ TEST(ScopeUsesArgumentsSuperThis) {
CHECK(i::parsing::ParseProgram(&info, isolate));
CHECK(i::Rewriter::Rewrite(&info));
info.ast_value_factory()->Internalize(isolate);
i::DeclarationScope::Analyze(&info);
CHECK(i::DeclarationScope::Analyze(&info));
i::DeclarationScope::AllocateScopeInfos(&info, isolate,
i::AnalyzeMode::kRegular);
CHECK_NOT_NULL(info.literal());
......@@ -10292,7 +10292,7 @@ TEST(LexicalLoopVariable) {
info.set_allow_lazy_parsing(false);
CHECK(i::parsing::ParseProgram(&info, isolate));
CHECK(i::Rewriter::Rewrite(&info));
i::DeclarationScope::Analyze(&info);
CHECK(i::DeclarationScope::Analyze(&info));
i::DeclarationScope::AllocateScopeInfos(&info, isolate,
i::AnalyzeMode::kRegular);
CHECK_NOT_NULL(info.literal());
......@@ -10303,8 +10303,8 @@ TEST(LexicalLoopVariable) {
test(info, script_scope);
};
// Check `let` loop variables is a stack local when not captured by an eval
// or closure within the area of the loop body.
// Check `let` loop variables is a stack local when not captured by
// an eval or closure within the area of the loop body.
const char* local_bindings[] = {
"function loop() {"
" for (let loop_var = 0; loop_var < 10; ++loop_var) {"
......@@ -10463,6 +10463,106 @@ TEST(LexicalLoopVariable) {
}
}
TEST(PrivateNamesSyntaxError) {
i::Isolate* isolate = CcTest::i_isolate();
i::HandleScope scope(isolate);
LocalContext env;
auto test = [isolate](const char* program, bool is_lazy) {
i::Factory* const factory = isolate->factory();
i::Handle<i::String> source =
factory->NewStringFromUtf8(i::CStrVector(program)).ToHandleChecked();
i::Handle<i::Script> script = factory->NewScript(source);
i::ParseInfo info(script);
info.set_allow_lazy_parsing(is_lazy);
i::FLAG_harmony_private_fields = true;
CHECK(i::parsing::ParseProgram(&info, isolate));
CHECK(i::Rewriter::Rewrite(&info));
CHECK(!i::DeclarationScope::Analyze(&info));
return info.pending_error_handler()->has_pending_error();
};
const char* data[] = {
"class A {"
" foo() { return this.#bar; }"
"}",
"let A = class {"
" foo() { return this.#bar; }"
"}",
"class A {"
" #foo; "
" bar() { return this.#baz; }"
"}",
"let A = class {"
" #foo; "
" bar() { return this.#baz; }"
"}",
"class A {"
" bar() {"
" class D { #baz = 1; };"
" return this.#baz;"
" }"
"}",
"let A = class {"
" bar() {"
" class D { #baz = 1; };"
" return this.#baz;"
" }"
"}",
"a.#bar",
"class Foo {};"
"Foo.#bar;",
"let Foo = class {};"
"Foo.#bar;",
"class Foo {};"
"(new Foo).#bar;",
"let Foo = class {};"
"(new Foo).#bar;",
"class Foo { #bar; };"
"(new Foo).#bar;",
"let Foo = class { #bar; };"
"(new Foo).#bar;",
"function t(){"
" class Foo { getA() { return this.#foo; } }"
"}",
"function t(){"
" return class { getA() { return this.#foo; } }"
"}",
};
// TODO(gsathya): The preparser does not track unresolved
// variables in top level function which fails this test.
const char* parser_data[] = {
"function t() {"
" return this.#foo;"
"}",
};
for (const char* source : data) {
CHECK(test(source, true));
CHECK(test(source, false));
}
for (const char* source : parser_data) {
CHECK(test(source, false));
}
}
} // namespace test_parsing
} // namespace internal
} // namespace v8
......@@ -11,9 +11,8 @@
// (a) check private field access on proxies.
// (b) throw reference error on missing private field access.
// (c) throw when private fields are set without being declared.
// (d) throw when private fields are accessed outside class bodies.
// (e) check that private field isn't called 'constructor'.
// (f) tests involving eval
// (d) check that private field isn't called 'constructor'.
// (e) tests involving eval
{
class C {
#a;
......
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