[parser] Skipping inner funcs: accurately record NeedsHomeObject
Inner functions which called eval, and were the kind of functions that can use `super`, were erroneously not marked as "uses_super_property", leading to downstream crashes when the runtime tried to load the [[HomeObject]] from them. This patch eliminates the public Scope::uses_super_property() API and ensures that callers always call Scope::NeedsHomeObject() instead. This is a minimal fix designed for easy merging; it's likely that in the long run we should remove most mentions of "uses super property" and replace them with "needs home object" for clarity. Bug: v8:5516, chromium:774994 Change-Id: Id269dd33e35bd40f6b59a3d3e19330687afa64f8 Reviewed-on: https://chromium-review.googlesource.com/721879 Reviewed-by: Marja Hölttä <marja@chromium.org> Commit-Queue: Adam Klein <adamk@chromium.org> Cr-Commit-Position: refs/heads/master@{#48619}
This commit is contained in:
parent
50f7455cd9
commit
94a71d7c45
@ -1739,8 +1739,8 @@ void Scope::Print(int n) {
|
||||
if (is_declaration_scope() && AsDeclarationScope()->calls_sloppy_eval()) {
|
||||
Indent(n1, "// scope calls sloppy 'eval'\n");
|
||||
}
|
||||
if (is_declaration_scope() && AsDeclarationScope()->uses_super_property()) {
|
||||
Indent(n1, "// scope uses 'super' property\n");
|
||||
if (is_declaration_scope() && AsDeclarationScope()->NeedsHomeObject()) {
|
||||
Indent(n1, "// scope needs home object\n");
|
||||
}
|
||||
if (inner_scope_calls_eval_) Indent(n1, "// inner scope calls 'eval'\n");
|
||||
if (is_declaration_scope()) {
|
||||
|
@ -669,14 +669,13 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
|
||||
|
||||
// Inform the scope that the corresponding code uses "super".
|
||||
void RecordSuperPropertyUsage() {
|
||||
DCHECK((IsConciseMethod(function_kind()) ||
|
||||
IsAccessorFunction(function_kind()) ||
|
||||
IsClassConstructor(function_kind())));
|
||||
DCHECK(IsConciseMethod(function_kind()) ||
|
||||
IsAccessorFunction(function_kind()) ||
|
||||
IsClassConstructor(function_kind()));
|
||||
scope_uses_super_property_ = true;
|
||||
}
|
||||
// Does this scope access "super" property (super.foo).
|
||||
bool uses_super_property() const { return scope_uses_super_property_; }
|
||||
|
||||
// Does this scope access "super" property (super.foo).
|
||||
bool NeedsHomeObject() const {
|
||||
return scope_uses_super_property_ ||
|
||||
(inner_scope_calls_eval_ && (IsConciseMethod(function_kind()) ||
|
||||
|
@ -2801,7 +2801,7 @@ Parser::LazyParsingResult Parser::SkipFunction(
|
||||
DCHECK(log_);
|
||||
log_->LogFunction(function_scope->start_position(),
|
||||
function_scope->end_position(), *num_parameters,
|
||||
language_mode(), function_scope->uses_super_property(),
|
||||
language_mode(), function_scope->NeedsHomeObject(),
|
||||
logger->num_inner_functions());
|
||||
}
|
||||
return kLazyParsingComplete;
|
||||
|
@ -193,7 +193,7 @@ void ProducedPreParsedScopeData::DataGatheringScope::MarkFunctionAsSkippable(
|
||||
produced_preparsed_scope_data_->parent_->AddSkippableFunction(
|
||||
function_scope_->start_position(), end_position,
|
||||
function_scope_->num_parameters(), num_inner_functions,
|
||||
function_scope_->language_mode(), function_scope_->uses_super_property());
|
||||
function_scope_->language_mode(), function_scope_->NeedsHomeObject());
|
||||
}
|
||||
|
||||
void ProducedPreParsedScopeData::AddSkippableFunction(
|
||||
|
@ -864,8 +864,14 @@ TEST(ScopeUsesArgumentsSuperThis) {
|
||||
!scope->AsDeclarationScope()->is_arrow_scope()) {
|
||||
CHECK_NOT_NULL(scope->AsDeclarationScope()->arguments());
|
||||
}
|
||||
CHECK_EQ((source_data[i].expected & SUPER_PROPERTY) != 0,
|
||||
scope->AsDeclarationScope()->uses_super_property());
|
||||
if (IsClassConstructor(scope->AsDeclarationScope()->function_kind())) {
|
||||
CHECK_EQ((source_data[i].expected & SUPER_PROPERTY) != 0 ||
|
||||
(source_data[i].expected & EVAL) != 0,
|
||||
scope->AsDeclarationScope()->NeedsHomeObject());
|
||||
} else {
|
||||
CHECK_EQ((source_data[i].expected & SUPER_PROPERTY) != 0,
|
||||
scope->AsDeclarationScope()->NeedsHomeObject());
|
||||
}
|
||||
if ((source_data[i].expected & THIS) != 0) {
|
||||
// Currently the is_used() flag is conservative; all variables in a
|
||||
// script scope are marked as used.
|
||||
|
34
test/mjsunit/regress/regress-crbug-774994.js
Normal file
34
test/mjsunit/regress/regress-crbug-774994.js
Normal file
@ -0,0 +1,34 @@
|
||||
// Copyright 2017 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: --preparser-scope-analysis
|
||||
|
||||
function f() {
|
||||
new class extends Object {
|
||||
constructor() {
|
||||
eval("super(); super.__f_10();");
|
||||
}
|
||||
}
|
||||
}
|
||||
assertThrows(f, TypeError);
|
||||
|
||||
function g() {
|
||||
let obj = {
|
||||
m() {
|
||||
eval("super.foo()");
|
||||
}
|
||||
}
|
||||
obj.m();
|
||||
}
|
||||
assertThrows(g, TypeError);
|
||||
|
||||
function h() {
|
||||
let obj = {
|
||||
get m() {
|
||||
eval("super.foo()");
|
||||
}
|
||||
}
|
||||
obj.m;
|
||||
}
|
||||
assertThrows(h, TypeError);
|
Loading…
Reference in New Issue
Block a user