From 16ff4874adfe10ff427e4edd8b37d58106a1dac2 Mon Sep 17 00:00:00 2001
From: Toon Verwaest <[email protected]>
Date: Fri, 18 Sep 2026 13:27:10 +0200
Subject: [PATCH] [maglev] Do not cache the_hole in loaded_context_constants_

When a kNotAssigned (let/const) context variable is accessed in a try
block before initialization, LdaImmutableContextSlot emits a
LoadContextSlotNoCells and caches it in loaded_context_constants_
before ThrowReferenceErrorIfHole runs. When ThrowReferenceErrorIfHole
merges KnownNodeAspects into the catch handler (both during graph
building in VisitThrowReferenceErrorIfHole and during KNA recomputation
in ProcessThrowingNode), the uninitialized load (which evaluates to
the_hole on the exception path) was being merged into the catch block's
loaded_context_constants_. If the catch block resumes a generator that
initializes the variable and then reads it again, Maglev reused the
stale load from before initialization.

Clear the cached slot in loaded_context_constants_ before merging into
the exception handler in VisitThrowReferenceErrorIfHole and
RecomputeKnownNodeAspectsProcessor::ProcessThrowingNode, restoring it on
the non-throwing fallthrough path. Also erase any cached value in
SetContextCachedValue when storing the_hole (e.g. during loop TDZ
re-initialization) instead of caching RootConstant(kTheHoleValue).

This general fix handles both let and const variables in inverted
generators, allowing us to revert the parser/preparser/ScopeInfo
workaround from commits e935736811d and 37e1db4a5e8.

Bug: 559932543
TAG=agy
CONV=1b7d9654-2731-4157-8abc-cd9414138332

Change-Id: I5d4aaf955e586b688960228e9c8acd240987c1ff
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/8419542
Reviewed-by: Leszek Swirski <[email protected]>
Commit-Queue: Toon Verwaest <[email protected]>
Auto-Submit: Toon Verwaest <[email protected]>
Cr-Commit-Position: refs/heads/main@{#109939}
---
diff --git a/src/ast/ast.h b/src/ast/ast.h
index 21ed80e..b036f59 100644
--- a/src/ast/ast.h
+++ b/src/ast/ast.h
@@ -1578,13 +1578,6 @@
     bit_field_ = IsNewTargetField::update(bit_field_, true);
   }
 
-  bool is_inside_try_catch() const {
-    return IsInsideTryCatchField::decode(bit_field_);
-  }
-  void set_is_inside_try_catch() {
-    bit_field_ = IsInsideTryCatchField::update(bit_field_, true);
-  }
-
   HoleCheckMode hole_check_mode() const {
     HoleCheckMode mode = HoleCheckModeField::decode(bit_field_);
     DCHECK_IMPLIES(mode == HoleCheckMode::kRequired,
@@ -1658,7 +1651,6 @@
                   IsResolvedField::encode(false) |
                   IsRemovedFromUnresolvedField::encode(false) |
                   IsHomeObjectField::encode(false) |
-                  IsInsideTryCatchField::encode(false) |
                   HoleCheckModeField::encode(HoleCheckMode::kElided);
   }
 
@@ -1669,8 +1661,7 @@
   using IsRemovedFromUnresolvedField = IsResolvedField::Next<bool, 1>;
   using IsNewTargetField = IsRemovedFromUnresolvedField::Next<bool, 1>;
   using IsHomeObjectField = IsNewTargetField::Next<bool, 1>;
-  using IsInsideTryCatchField = IsHomeObjectField::Next<bool, 1>;
-  using HoleCheckModeField = IsInsideTryCatchField::Next<HoleCheckMode, 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 6f77706..eb6e4c4 100644
--- a/src/ast/scopes.cc
+++ b/src/ast/scopes.cc
@@ -471,10 +471,6 @@
       continue;
     }
 
-    if (parse_info && IsGeneratorFunction(scope_info->function_kind())) {
-      parse_info->set_has_generator_in_scope_chain();
-    }
-
     if (scope_info->scope_type() == FUNCTION_SCOPE) {
       outer_scope = zone->New<DeclarationScope>(
           zone, FUNCTION_SCOPE, ast_value_factory, handle(scope_info, isolate));
@@ -622,11 +618,6 @@
   return static_cast<const DeclarationScope*>(this);
 }
 
-FunctionKind Scope::scope_closure_function_kind() const {
-  if (!scope_info_.is_null()) return scope_info_->function_kind();
-  return GetClosureScope()->function_kind();
-}
-
 ModuleScope* Scope::AsModuleScope() {
   SBXCHECK(is_module_scope());
   return static_cast<ModuleScope*>(this);
@@ -1043,37 +1034,6 @@
   }
 }
 
-void Scope::MarkUnresolvedVariablesAsInsideTryCatch() {
-  // While proxy marking is generic, we only actually call this when nested in a
-  // generator because that's the only place we care about variables escaping
-  // try-catch blocks (for hole check elision and resume logic).
-  for (VariableProxy* proxy : unresolved_list_) {
-    proxy->set_is_inside_try_catch();
-  }
-  for (Scope* inner = inner_scope_; inner; inner = inner->sibling_) {
-    if (inner->is_closure_scope()) continue;
-    inner->MarkUnresolvedVariablesAsInsideTryCatch();
-  }
-}
-
-void Scope::Snapshot::MarkUnresolvedVariablesAsInsideTryCatch() {
-  // While proxy marking is generic, we only actually call this when nested in a
-  // generator because that's the only place we care about variables escaping
-  // try-catch blocks (for hole check elision and resume logic).
-  auto it = top_unresolved_;
-  auto end = outer_scope_->unresolved_list_.end();
-
-  while (it != end) {
-    (*it)->set_is_inside_try_catch();
-    ++it;
-  }
-  for (Scope* inner = outer_scope_->inner_scope_; inner != top_inner_scope_;
-       inner = inner->sibling_) {
-    if (inner->is_closure_scope()) continue;
-    inner->MarkUnresolvedVariablesAsInsideTryCatch();
-  }
-}
-
 Variable* Scope::LookupInScopeInfo(const AstRawString* name, Scope* cache) {
   DCHECK(!scope_info_.is_null());
   DCHECK(this->IsOuterScopeOf(cache));
@@ -1612,18 +1572,6 @@
   return scope->AsDeclarationScope();
 }
 
-bool Scope::HasOuterGenerator() const {
-  const Scope* scope = GetClosureScope()->outer_scope();
-  while (scope != nullptr) {
-    scope = scope->GetClosureScope();
-    if (IsGeneratorFunction(scope->AsDeclarationScope()->function_kind())) {
-      return true;
-    }
-    scope = scope->outer_scope();
-  }
-  return false;
-}
-
 bool Scope::NeedsScopeInfo() const {
   DCHECK(!already_resolved_);
   DCHECK(GetClosureScope()->ShouldEagerCompile());
@@ -1779,7 +1727,7 @@
         }
       } else {
         var->set_is_used();
-        UpdateVariableMaybeAssigned(var, proxy, scope);
+        if (proxy->is_assigned()) var->SetMaybeAssigned();
       }
     }
 
@@ -2535,36 +2483,11 @@
 
 }  // anonymous namespace
 
-void Scope::UpdateVariableMaybeAssigned(Variable* var, VariableProxy* proxy,
-                                        Scope* current_scope) {
-  if (proxy->is_assigned()) {
-    var->SetMaybeAssigned();
-    return;
-  }
-
-  if (proxy->is_inside_try_catch()) {
-    Variable* true_var = var;
-    while (true_var->has_local_if_not_shadowed()) {
-      true_var = true_var->local_if_not_shadowed();
-    }
-    if (!true_var->scope()->IsOuterScopeUpToClosureScopeOf(current_scope) &&
-        IsGeneratorFunction(true_var->scope()->scope_closure_function_kind())) {
-      // We treat variables captured by generator yields in a try-catch as
-      // maybe_assigned since the context allocation and assignment might be
-      // skipped when resuming from a yield.
-      // See test/mjsunit/maglev/context-inverted-generator2.js.
-      true_var->SetMaybeAssigned();
-    }
-  }
-}
-
 void Scope::ResolveTo(VariableProxy* proxy, Variable* var,
                       int access_position) {
   DCHECK_NOT_NULL(var);
   UpdateNeedsHoleCheck(var, proxy, this, access_position);
   proxy->BindTo(var);
-
-  UpdateVariableMaybeAssigned(var, proxy, this);
 }
 
 void Scope::ResolvePreparsedVariable(VariableProxy* proxy, Scope* scope,
@@ -2576,7 +2499,7 @@
       var->set_is_used();
       if (!var->is_dynamic()) {
         var->ForceContextAllocation();
-        UpdateVariableMaybeAssigned(var, proxy, scope);
+        if (proxy->is_assigned()) var->SetMaybeAssigned();
         return;
       }
     }
@@ -2874,8 +2797,7 @@
 template <typename IsolateT>
 void Scope::AllocateScopeInfosRecursively(
     IsolateT* isolate, MaybeHandle<ScopeInfo> outer_scope,
-    std::unordered_map<int, IndirectHandle<ScopeInfo>>& scope_infos_to_reuse,
-    FunctionKind closure_function_kind) {
+    std::unordered_map<int, IndirectHandle<ScopeInfo>>& scope_infos_to_reuse) {
   DCHECK(scope_info_.is_null());
   MaybeHandle<ScopeInfo> next_outer_scope = outer_scope;
 
@@ -2897,8 +2819,7 @@
     it->second = {};
 #endif
   } else if (NeedsScopeInfo()) {
-    scope_info_ = ScopeInfo::Create(isolate, zone(), this, outer_scope,
-                                    closure_function_kind);
+    scope_info_ = ScopeInfo::Create(isolate, zone(), this, outer_scope);
 #ifdef DEBUG
     // Mark this ID as being used.
     scope_infos_to_reuse[UniqueIdInScript()] = {};
@@ -2922,12 +2843,8 @@
     }
     if (!scope->is_function_scope() ||
         scope->AsDeclarationScope()->ShouldEagerCompile()) {
-      FunctionKind inner_closure_kind =
-          scope->is_closure_scope()
-              ? scope->AsDeclarationScope()->function_kind()
-              : closure_function_kind;
-      scope->AllocateScopeInfosRecursively(
-          isolate, next_outer_scope, scope_infos_to_reuse, inner_closure_kind);
+      scope->AllocateScopeInfosRecursively(isolate, next_outer_scope,
+                                           scope_infos_to_reuse);
     } else {
       auto scope_it = scope_infos_to_reuse.find(scope->UniqueIdInScript());
       if (scope_it != scope_infos_to_reuse.end()) {
@@ -2945,14 +2862,12 @@
     AllocateScopeInfosRecursively<Isolate>(
         Isolate* isolate, MaybeHandle<ScopeInfo> outer_scope,
         std::unordered_map<int, IndirectHandle<ScopeInfo>>&
-            scope_infos_to_reuse,
-        FunctionKind closure_function_kind);
+            scope_infos_to_reuse);
 template EXPORT_TEMPLATE_DEFINE(V8_EXPORT_PRIVATE) void Scope::
     AllocateScopeInfosRecursively<LocalIsolate>(
         LocalIsolate* isolate, MaybeHandle<ScopeInfo> outer_scope,
         std::unordered_map<int, IndirectHandle<ScopeInfo>>&
-            scope_infos_to_reuse,
-        FunctionKind closure_function_kind);
+            scope_infos_to_reuse);
 
 void DeclarationScope::RecalcPrivateNameContextChain() {
   // The outermost scope in a class heritage expression is marked to skip the
@@ -3162,17 +3077,16 @@
     }
   }
 
-  scope->AllocateScopeInfosRecursively(
-      isolate, outer_scope, scope_infos_to_reuse,
-      scope->GetClosureScope()->function_kind());
+  scope->AllocateScopeInfosRecursively(isolate, outer_scope,
+                                       scope_infos_to_reuse);
 
   // The debugger expects all shared function infos to contain a scope info.
   // Since the top-most scope will end up in a shared function info, make sure
   // it has one, even if it doesn't need a scope info.
   // TODO(yangguo): Remove this requirement.
   if (scope->scope_info_.is_null()) {
-    scope->scope_info_ = ScopeInfo::Create(isolate, scope->zone(), scope,
-                                           outer_scope, scope->function_kind());
+    scope->scope_info_ =
+        ScopeInfo::Create(isolate, scope->zone(), scope, outer_scope);
   }
 }
 
diff --git a/src/ast/scopes.h b/src/ast/scopes.h
index 5a7effd..f85e742 100644
--- a/src/ast/scopes.h
+++ b/src/ast/scopes.h
@@ -121,10 +121,6 @@
   // to REPL_GLOBAL. Should only be called on REPL scripts.
   void RewriteReplGlobalVariables();
 
-  // Mark all unresolved variables added after taking the snapshot as inside a
-  // try/catch.
-  void MarkUnresolvedVariablesAsInsideTryCatch();
-
   class Snapshot final {
    public:
     inline explicit Snapshot(Scope* scope);
@@ -145,9 +141,6 @@
     }
 
     void Reparent(DeclarationScope* new_parent);
-    // Mark all unresolved variables added after taking the snapshot as inside a
-    // try/catch.
-    void MarkUnresolvedVariablesAsInsideTryCatch();
 
    private:
     Scope* outer_scope_;
@@ -580,12 +573,6 @@
   DeclarationScope* GetClosureScope();
   const DeclarationScope* GetClosureScope() const;
 
-  // Returns the function kind of the closure scope, even if the scope is fully
-  // deserialized and its closure scope isn't available.
-  FunctionKind scope_closure_function_kind() const;
-
-  bool HasOuterGenerator() const;
-
   // Returns true if this scope is an outer scope of the given scope, up to
   // and including the closure scope of the given scope.
   bool IsOuterScopeUpToClosureScopeOf(Scope* scope) const;
@@ -814,8 +801,6 @@
                                     bool force_context_allocation = false);
   static void ResolvePreparsedVariable(VariableProxy* proxy, Scope* scope,
                                        Scope* end);
-  static void UpdateVariableMaybeAssigned(Variable* var, VariableProxy* proxy,
-                                          Scope* current_scope);
   void ResolveTo(VariableProxy* proxy, Variable* var, int access_position);
   void ResolveVariable(VariableProxy* proxy);
   V8_WARN_UNUSED_RESULT bool ResolveVariablesRecursively(Scope* end);
@@ -846,8 +831,7 @@
   template <typename IsolateT>
   void AllocateScopeInfosRecursively(
       IsolateT* isolate, MaybeHandle<ScopeInfo> outer_scope,
-      std::unordered_map<int, IndirectHandle<ScopeInfo>>& scope_infos_to_reuse,
-      FunctionKind closure_function_kind);
+      std::unordered_map<int, IndirectHandle<ScopeInfo>>& scope_infos_to_reuse);
 
   // Construct a scope based on the scope info.
   Scope(Zone* zone, ScopeType type, AstValueFactory* ast_value_factory,
diff --git a/src/maglev/maglev-graph-builder.cc b/src/maglev/maglev-graph-builder.cc
index 6afff35..cf70bc1 100644
--- a/src/maglev/maglev-graph-builder.cc
+++ b/src/maglev/maglev-graph-builder.cc
@@ -15627,9 +15627,34 @@
                                 GetConstant(name));
     case Tribool::kFalse:
       return ReduceResult::Done();
-    case Tribool::kMaybe:
+    case Tribool::kMaybe: {
       DCHECK(value->is_tagged());
-      return AddNewNode<ThrowReferenceErrorIfHole>({value}, name);
+      // Temporarily clear the cached constant value before emitting
+      // ThrowReferenceErrorIfHole so that known_node_aspects() merged into the
+      // exception handler (the catch block) does not cache this load. On the
+      // exception path, the value is guaranteed to be the_hole, and the catch
+      // block may resume a generator that initializes the const variable.
+      // Restore the cached value afterward for the non-throwing fallthrough
+      // path where the value is known not to be the_hole.
+      // TODO(verwaest): Look into making loaded_context_constants_ monotonic,
+      // e.g. by folding the hole check into the context load rather than
+      // temporarily clearing the cached constant here.
+      ValueNode** cached_slot = nullptr;
+      if (auto* load = value->TryCast<LoadContextSlotNoCells>();
+          load && load->maybe_assigned() == kNotAssigned) {
+        ValueNode*& slot = known_node_aspects().GetContextCachedValue(
+            load->input(0).node(), load->offset(), kNotAssigned);
+        if (slot == value) {
+          cached_slot = &slot;
+          *cached_slot = nullptr;
+        }
+      }
+      ReduceResult res = AddNewNode<ThrowReferenceErrorIfHole>({value}, name);
+      if (cached_slot) {
+        *cached_slot = value;
+      }
+      return res;
+    }
   }
   UNREACHABLE();
 }
diff --git a/src/maglev/maglev-kna-processor.h b/src/maglev/maglev-kna-processor.h
index a188285..3b5d133 100644
--- a/src/maglev/maglev-kna-processor.h
+++ b/src/maglev/maglev-kna-processor.h
@@ -172,7 +172,33 @@
       if (mark_handler_reachable) {
         tracker_.MarkReachable(exception_handler);
       }
+      // Temporarily clear the cached constant value before merging into the
+      // exception handler so that the catch block does not cache this load. On
+      // the exception path, the value is guaranteed to be the_hole, and the
+      // catch block may resume a generator that initializes the variable.
+      // Restore the cached value afterward for the non-throwing fallthrough
+      // path where the value is known not to be the_hole.
... (file diff truncated)

diff --git a/test/mjsunit/maglev/context-inverted-generator3.js b/test/mjsunit/maglev/context-inverted-generator3.js
new file mode 100644
index 0000000..3a91ef8
--- /dev/null
+++ b/test/mjsunit/maglev/context-inverted-generator3.js
@@ -0,0 +1,30 @@
+// Copyright 2026 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: --allow-natives-syntax
+
+function* f() {
+  yield function g() {
+    try { test } catch (e) { gen.next(); return test }
+  }
+  const test = 10;
+}
+
+var gen = f();
+var g = gen.next().value;
+%PrepareFunctionForOptimization(g);
+assertEquals(10, g());
+
+gen = f();
+g = gen.next().value;
+assertEquals(10, g());
+
+gen = f();
+g = gen.next().value;
+assertEquals(10, g());
+
+gen = f();
+g = gen.next().value;
+%OptimizeMaglevOnNextCall(g);
+assertEquals(10, g());