Skip to content

Commit

Permalink
chore: cherry-pick 1 change from Release-0-M123 (#41631)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ppontes
ppontes committed Mar 21, 2024
1 parent 07f5075 commit d2177e4
Show file tree
Hide file tree
Showing 2 changed files with 277 additions and 0 deletions.
1 change: 1 addition & 0 deletions patches/v8/.patches
View file
@@ -1,2 +1,3 @@
chore_allow_customizing_microtask_policy_per_context.patch
deps_add_v8_object_setinternalfieldfornodecore.patch
merged_parser_fix_home_object_proxy_to_work_off-thread.patch
276 changes: 276 additions & 0 deletions patches/v8/merged_parser_fix_home_object_proxy_to_work_off-thread.patch
View file
@@ -0,0 +1,276 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Shu-yu Guo <syg@chromium.org>
Date: Thu, 7 Mar 2024 14:55:28 -0800
Subject: Merged: [parser] Fix home object proxy to work off-thread

Because the home object has special scope lookup rules due to class
heritage position, VariableProxies of the home object are currently
directly created on the correct scope during parsing. However, during
off-thread parsing the main thread is parked, and the correct scope
may try to dereference a main-thread Handle.

This CL moves the logic into ResolveVariable instead, which happens
during postprocessing, with the main thread unparked.

Fixed: chromium:327740539

(cherry picked from commit 8f477f936c9b9e6b4c9f35a8ccc5e65bd4cb7f4e)

Change-Id: Ia57c211e5d285f1a801ca1f95db02f7e199ccde9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5363633
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Reviewed-by: Deepti Gandluri <gdeepti@chromium.org>
Cr-Commit-Position: refs/branch-heads/12.3@{#18}
Cr-Branched-From: a86e1971579f4165123467fa6ad378e552536b43-refs/heads/12.3.219@{#1}
Cr-Branched-From: 21869f7f6f3e8f5a58a0b2e61e0f7412480230b1-refs/heads/main@{#92385}

diff --git a/src/ast/ast.h b/src/ast/ast.h
index d4b2f23dda1b387f4024462babcc4055d0416c87..5843b3f6a9698acc1d302a5293b6f529649b345a 100644
--- a/src/ast/ast.h
+++ b/src/ast/ast.h
@@ -1535,6 +1535,12 @@ class VariableProxy final : public Expression {
bit_field_ = IsRemovedFromUnresolvedField::update(bit_field_, true);
}

+ bool is_home_object() const { return IsHomeObjectField::decode(bit_field_); }
+
+ void set_is_home_object() {
+ bit_field_ = IsHomeObjectField::update(bit_field_, true);
+ }
+
// Provides filtered access to the unresolved variable proxy threaded list.
struct UnresolvedNext {
static VariableProxy** filter(VariableProxy** t) {
@@ -1566,6 +1572,7 @@ class VariableProxy final : public Expression {
bit_field_ |= IsAssignedField::encode(false) |
IsResolvedField::encode(false) |
IsRemovedFromUnresolvedField::encode(false) |
+ IsHomeObjectField::encode(false) |
HoleCheckModeField::encode(HoleCheckMode::kElided);
}

@@ -1575,7 +1582,8 @@ class VariableProxy final : public Expression {
using IsResolvedField = IsAssignedField::Next<bool, 1>;
using IsRemovedFromUnresolvedField = IsResolvedField::Next<bool, 1>;
using IsNewTargetField = IsRemovedFromUnresolvedField::Next<bool, 1>;
- using HoleCheckModeField = IsNewTargetField::Next<HoleCheckMode, 1>;
+ using IsHomeObjectField = IsNewTargetField::Next<bool, 1>;
+ using HoleCheckModeField = IsHomeObjectField::Next<HoleCheckMode, 1>;

union {
const AstRawString* raw_name_; // if !is_resolved_
diff --git a/src/ast/scopes.cc b/src/ast/scopes.cc
index 672440a2b4b9ad6262c49be50d03377b413452a6..6dfcd45cf208e58a2fc0cd18ba3b115bae35a0d5 100644
--- a/src/ast/scopes.cc
+++ b/src/ast/scopes.cc
@@ -491,7 +491,6 @@ Scope* Scope::DeserializeScopeChain(IsolateT* isolate, Zone* zone,
if (cache_scope_found) {
outer_scope->set_deserialized_scope_uses_external_cache();
} else {
- DCHECK(!cache_scope_found);
cache_scope_found =
outer_scope->is_declaration_scope() && !outer_scope->is_eval_scope();
}
@@ -970,9 +969,14 @@ Variable* Scope::LookupInScopeInfo(const AstRawString* name, Scope* cache) {
DCHECK(!cache->deserialized_scope_uses_external_cache());
// The case where where the cache can be another scope is when the cache scope
// is the last scope that doesn't use an external cache.
+ //
+ // The one exception to this is when looking up the home object, which may
+ // skip multiple scopes that don't use an external cache (e.g., several arrow
+ // functions).
DCHECK_IMPLIES(
cache != this,
- cache->outer_scope()->deserialized_scope_uses_external_cache());
+ cache->outer_scope()->deserialized_scope_uses_external_cache() ||
+ cache->GetHomeObjectScope() == this);
DCHECK_NULL(cache->variables_.Lookup(name));
DisallowGarbageCollection no_gc;

@@ -2282,7 +2286,33 @@ Variable* Scope::LookupSloppyEval(VariableProxy* proxy, Scope* scope,

void Scope::ResolveVariable(VariableProxy* proxy) {
DCHECK(!proxy->is_resolved());
- Variable* var = Lookup<kParsedScope>(proxy, this, nullptr);
+ Variable* var;
+ if (V8_UNLIKELY(proxy->is_home_object())) {
+ // VariableProxies of the home object cannot be resolved like a normal
+ // variable. Consider the case of a super.property usage in heritage
+ // position:
+ //
+ // class C extends super.foo { m() { super.bar(); } }
+ //
+ // The super.foo property access is logically nested under C's class scope,
+ // which also has a home object due to its own method m's usage of
+ // super.bar(). However, super.foo must resolve super in C's outer scope.
+ //
+ // Because of the above, start resolving home objects directly at the home
+ // object scope instead of the current scope.
+ Scope* scope = GetDeclarationScope()->GetHomeObjectScope();
+ DCHECK_NOT_NULL(scope);
+ if (scope->scope_info_.is_null()) {
+ var = Lookup<kParsedScope>(proxy, scope, nullptr);
+ } else {
+ Scope* entry_cache = scope->deserialized_scope_uses_external_cache()
+ ? GetNonEvalDeclarationScope()
+ : scope;
+ var = Lookup<kDeserializedScope>(proxy, scope, nullptr, entry_cache);
+ }
+ } else {
+ var = Lookup<kParsedScope>(proxy, this, nullptr);
+ }
DCHECK_NOT_NULL(var);
ResolveTo(proxy, var);
}
@@ -2752,48 +2782,6 @@ int Scope::ContextLocalCount() const {
(is_function_var_in_context ? 1 : 0);
}

-VariableProxy* Scope::NewHomeObjectVariableProxy(AstNodeFactory* factory,
- const AstRawString* name,
- int start_pos) {
- // VariableProxies of the home object cannot be resolved like a normal
- // variable. Consider the case of a super.property usage in heritage position:
- //
- // class C extends super.foo { m() { super.bar(); } }
- //
- // The super.foo property access is logically nested under C's class scope,
- // which also has a home object due to its own method m's usage of
- // super.bar(). However, super.foo must resolve super in C's outer scope.
- //
- // Because of the above, home object VariableProxies are always made directly
- // on the Scope that needs the home object instead of the innermost scope.
- DCHECK(needs_home_object());
- if (!scope_info_.is_null()) {
- // This is a lazy compile, so the home object's context slot is already
- // known.
- Variable* home_object = variables_.Lookup(name);
- if (home_object == nullptr) {
- VariableLookupResult lookup_result;
- int index = scope_info_->ContextSlotIndex(name->string(), &lookup_result);
- DCHECK_GE(index, 0);
- bool was_added;
- home_object = variables_.Declare(zone(), this, name, lookup_result.mode,
- NORMAL_VARIABLE, lookup_result.init_flag,
- lookup_result.maybe_assigned_flag,
- IsStaticFlag::kNotStatic, &was_added);
- DCHECK(was_added);
- home_object->AllocateTo(VariableLocation::CONTEXT, index);
- }
- return factory->NewVariableProxy(home_object, start_pos);
- }
- // This is not a lazy compile. Add the unresolved home object VariableProxy to
- // the unresolved list of the home object scope, which is not necessarily the
- // innermost scope.
- VariableProxy* proxy =
- factory->NewVariableProxy(name, NORMAL_VARIABLE, start_pos);
- AddUnresolved(proxy);
- return proxy;
-}
-
bool IsComplementaryAccessorPair(VariableMode a, VariableMode b) {
switch (a) {
case VariableMode::kPrivateGetterOnly:
diff --git a/src/ast/scopes.h b/src/ast/scopes.h
index b4c2e8b2136813985231a722c6dbcd26e2c17336..751aaee3d11ecc0da71e2171dd42ed4b85d00219 100644
--- a/src/ast/scopes.h
+++ b/src/ast/scopes.h
@@ -603,10 +603,6 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
needs_home_object_ = true;
}

- VariableProxy* NewHomeObjectVariableProxy(AstNodeFactory* factory,
- const AstRawString* name,
- int start_pos);
-
bool RemoveInnerScope(Scope* inner_scope) {
DCHECK_NOT_NULL(inner_scope);
if (inner_scope == inner_scope_) {
@@ -865,7 +861,7 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
FunctionKind function_kind() const { return function_kind_; }

// Inform the scope that the corresponding code uses "super".
- Scope* RecordSuperPropertyUsage() {
+ void RecordSuperPropertyUsage() {
DCHECK(IsConciseMethod(function_kind()) ||
IsAccessorFunction(function_kind()) ||
IsClassConstructor(function_kind()));
@@ -873,7 +869,6 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
Scope* home_object_scope = GetHomeObjectScope();
DCHECK_NOT_NULL(home_object_scope);
home_object_scope->set_needs_home_object();
- return home_object_scope;
}

bool uses_super_property() const { return uses_super_property_; }
diff --git a/src/parsing/parser-base.h b/src/parsing/parser-base.h
index cc899086d3b8ad0e09963a144a4e58de4d20788c..ac35090ca5d129c58c0e4fb31ee2e2456e202dce 100644
--- a/src/parsing/parser-base.h
+++ b/src/parsing/parser-base.h
@@ -3823,9 +3823,9 @@ ParserBase<Impl>::ParseSuperExpression() {
impl()->ReportMessage(MessageTemplate::kOptionalChainingNoSuper);
return impl()->FailureExpression();
}
- Scope* home_object_scope = scope->RecordSuperPropertyUsage();
+ scope->RecordSuperPropertyUsage();
UseThis();
- return impl()->NewSuperPropertyReference(home_object_scope, pos);
+ return impl()->NewSuperPropertyReference(pos);
}
// super() is only allowed in derived constructor. new super() is never
// allowed; it's reported as an error by
diff --git a/src/parsing/parser.cc b/src/parsing/parser.cc
index 662e487be695750b63e1713b5b76e5aff9daf70f..645d005e53f944665f269194366e3d559e3f53c6 100644
--- a/src/parsing/parser.cc
+++ b/src/parsing/parser.cc
@@ -297,18 +297,17 @@ Expression* Parser::NewThrowError(Runtime::FunctionId id,
return factory()->NewThrow(call_constructor, pos);
}

-Expression* Parser::NewSuperPropertyReference(Scope* home_object_scope,
- int pos) {
+Expression* Parser::NewSuperPropertyReference(int pos) {
const AstRawString* home_object_name;
if (IsStatic(scope()->GetReceiverScope()->function_kind())) {
home_object_name = ast_value_factory_->dot_static_home_object_string();
} else {
home_object_name = ast_value_factory_->dot_home_object_string();
}
- return factory()->NewSuperPropertyReference(
- home_object_scope->NewHomeObjectVariableProxy(factory(), home_object_name,
- pos),
- pos);
+
+ VariableProxy* proxy = NewUnresolved(home_object_name, pos);
+ proxy->set_is_home_object();
+ return factory()->NewSuperPropertyReference(proxy, pos);
}

SuperCallReference* Parser::NewSuperCallReference(int pos) {
diff --git a/src/parsing/parser.h b/src/parsing/parser.h
index cc397e198b718790911666e813960fa9d434b886..8f9d57868ffd609d9236fcfa1a550ff4d5de62bf 100644
--- a/src/parsing/parser.h
+++ b/src/parsing/parser.h
@@ -797,7 +797,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
return factory()->NewThisExpression(pos);
}

- Expression* NewSuperPropertyReference(Scope* home_object_scope, int pos);
+ Expression* NewSuperPropertyReference(int pos);
SuperCallReference* NewSuperCallReference(int pos);
Expression* NewTargetExpression(int pos);
Expression* ImportMetaExpression(int pos);
diff --git a/src/parsing/preparser.h b/src/parsing/preparser.h
index 2b81771464ba306154325d81f7f29e573f1adf16..9e8446a3481b22ff67b1c6a6f9a8b10e5d1839cb 100644
--- a/src/parsing/preparser.h
+++ b/src/parsing/preparser.h
@@ -1532,8 +1532,7 @@ class PreParser : public ParserBase<PreParser> {
return PreParserExpression::This();
}

- V8_INLINE PreParserExpression
- NewSuperPropertyReference(Scope* home_object_scope, int pos) {
+ V8_INLINE PreParserExpression NewSuperPropertyReference(int pos) {
return PreParserExpression::Default();
}

0 comments on commit d2177e4

Please sign in to comment.