Browse Source

chore: cherry-pick 1 change from Release-0-M123 (#41631)

Pedro Pontes 1 year ago
parent
commit
d2177e4d08

+ 1 - 0
patches/v8/.patches

@@ -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 - 0
patches/v8/merged_parser_fix_home_object_proxy_to_work_off-thread.patch

@@ -0,0 +1,276 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Shu-yu Guo <[email protected]>
+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 <[email protected]>
+Reviewed-by: Deepti Gandluri <[email protected]>
+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();
+   }
+