[parser] Remove Scope::zone_ field in favour of VariableMap::zone()

... which gets the zone from its ZoneAllocationPolicy instance.

This recovers memory regression caused by adding an AllocationPolicy
instance into TemplateHashMapImpl and therefore to VariableMap.

Bug: v8:10572
Change-Id: I7962b49e5f2669307e58b3ed7b1f29bab1c42cad
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2298002Reviewed-by: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68879}

src/ast/scopes.cc

@@ -37,12 +37,16 @@ namespace internal {
 VariableMap::VariableMap(Zone* zone)
     : ZoneHashMap(8, ZoneAllocationPolicy(zone)) {}
 
+VariableMap::VariableMap(const VariableMap& other, Zone* zone)
+    : ZoneHashMap(other, ZoneAllocationPolicy(zone)) {}
+
 Variable* VariableMap::Declare(Zone* zone, Scope* scope,
                                const AstRawString* name, VariableMode mode,
                                VariableKind kind,
                                InitializationFlag initialization_flag,
                                MaybeAssignedFlag maybe_assigned_flag,
                                IsStaticFlag is_static_flag, bool* was_added) {
+  DCHECK_EQ(zone, allocator().zone());
   // AstRawStrings are unambiguous, i.e., the same string is always represented
   // by the same AstRawString*.
   // FIXME(marja): fix the type of Lookup.
@@ -88,18 +92,12 @@ Variable* VariableMap::Lookup(const AstRawString* name) {
 // Implementation of Scope
 
 Scope::Scope(Zone* zone)
-    : zone_(zone),
-      outer_scope_(nullptr),
-      variables_(zone),
-      scope_type_(SCRIPT_SCOPE) {
+    : outer_scope_(nullptr), variables_(zone), scope_type_(SCRIPT_SCOPE) {
   SetDefaults();
 }
 
 Scope::Scope(Zone* zone, Scope* outer_scope, ScopeType scope_type)
-    : zone_(zone),
-      outer_scope_(outer_scope),
-      variables_(zone),
-      scope_type_(scope_type) {
+    : outer_scope_(outer_scope), variables_(zone), scope_type_(scope_type) {
   DCHECK_NE(SCRIPT_SCOPE, scope_type);
   SetDefaults();
   set_language_mode(outer_scope->language_mode());
@@ -190,8 +188,7 @@ ClassScope::ClassScope(Isolate* isolate, Zone* zone,
 }
 
 Scope::Scope(Zone* zone, ScopeType scope_type, Handle<ScopeInfo> scope_info)
-    : zone_(zone),
-      outer_scope_(nullptr),
+    : outer_scope_(nullptr),
       variables_(zone),
       scope_info_(scope_info),
       scope_type_(scope_type) {
@@ -224,8 +221,7 @@ DeclarationScope::DeclarationScope(Zone* zone, ScopeType scope_type,
 
 Scope::Scope(Zone* zone, const AstRawString* catch_variable_name,
              MaybeAssignedFlag maybe_assigned, Handle<ScopeInfo> scope_info)
-    : zone_(zone),
-      outer_scope_(nullptr),
+    : outer_scope_(nullptr),
       variables_(zone),
       scope_info_(scope_info),
       scope_type_(CATCH_SCOPE) {
@@ -1522,21 +1518,22 @@ void DeclarationScope::ResetAfterPreparsing(AstValueFactory* ast_value_factory,
   has_rest_ = false;
   function_ = nullptr;
 
-  DCHECK_NE(zone_, ast_value_factory->zone());
-  zone_->ReleaseMemory();
+  DCHECK_NE(zone(), ast_value_factory->zone());
+  // Make sure this scope and zone aren't used for allocation anymore.
+  {
+    // Get the zone, while variables_ is still valid
+    Zone* zone = this->zone();
+    variables_.Invalidate();
+    zone->ReleaseMemory();
+  }
 
   if (aborted) {
     // Prepare scope for use in the outer zone.
-    zone_ = ast_value_factory->zone();
-    variables_.Reset(ZoneAllocationPolicy(zone_));
+    variables_ = VariableMap(ast_value_factory->zone());
     if (!IsArrowFunction(function_kind_)) {
       has_simple_parameters_ = true;
       DeclareDefaultFunctionVariables(ast_value_factory);
     }
-  } else {
-    // Make sure this scope isn't used for allocation anymore.
-    zone_ = nullptr;
-    variables_.Invalidate();
   }
 
 #ifdef DEBUG

src/ast/scopes.h

@@ -41,6 +41,15 @@ using UnresolvedList =
 class VariableMap : public ZoneHashMap {
  public:
   explicit VariableMap(Zone* zone);
+  VariableMap(const VariableMap& other, Zone* zone);
+
+  VariableMap(VariableMap&& other) V8_NOEXCEPT : ZoneHashMap(std::move(other)) {
+  }
+
+  VariableMap& operator=(VariableMap&& other) V8_NOEXCEPT {
+    static_cast<ZoneHashMap&>(*this) = std::move(other);
+    return *this;
+  }
 
   Variable* Declare(Zone* zone, Scope* scope, const AstRawString* name,
                     VariableMode mode, VariableKind kind,
@@ -51,6 +60,8 @@ class VariableMap : public ZoneHashMap {
   V8_EXPORT_PRIVATE Variable* Lookup(const AstRawString* name);
   void Remove(Variable* var);
   void Add(Variable* var);
+
+  Zone* zone() const { return allocator().zone(); }
 };
 
 class Scope;
@@ -168,7 +179,7 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
   // Assumes outer_scope_ is non-null.
   void ReplaceOuterScope(Scope* outer_scope);
 
-  Zone* zone() const { return zone_; }
+  Zone* zone() const { return variables_.zone(); }
 
   void SetMustUsePreparseData() {
     if (must_use_preparsed_scope_data_) {
@@ -701,8 +712,6 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
   friend class ScopeTestHelper;
   friend Zone;
 
-  Zone* zone_;
-
   // Scope tree.
   Scope* outer_scope_;  // the immediately enclosing outer scope, or nullptr
   Scope* inner_scope_;  // an inner scope of this scope
@@ -899,11 +908,13 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
   }
   bool is_being_lazily_parsed() const { return is_being_lazily_parsed_; }
 #endif
+
   void set_zone(Zone* zone) {
 #ifdef DEBUG
     needs_migration_ = true;
 #endif
-    zone_ = zone;
+    // Migrate variables_' backing store to new zone.
+    variables_ = VariableMap(variables_, zone);
   }
 
   // ---------------------------------------------------------------------------
@@ -1261,7 +1272,7 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
 
   V8_INLINE RareData* EnsureRareData() {
     if (rare_data_ == nullptr) {
-      rare_data_ = zone_->New<RareData>();
+      rare_data_ = zone()->New<RareData>();
     }
     return rare_data_;
   }
@@ -1443,7 +1454,7 @@ class V8_EXPORT_PRIVATE ClassScope : public Scope {
   V8_INLINE RareData* EnsureRareData() {
     if (GetRareData() == nullptr) {
       rare_data_and_is_parsing_heritage_.SetPointer(
-          zone_->New<RareData>(zone_));
+          zone()->New<RareData>(zone()));
     }
     return GetRareData();
   }

src/base/hashmap.h

@@ -36,17 +36,34 @@ class TemplateHashMapImpl {
 
   // initial_capacity is the size of the initial hash map;
   // it must be a power of 2 (and thus must not be 0).
-  TemplateHashMapImpl(uint32_t capacity = kDefaultHashMapCapacity,
-                      MatchFun match = MatchFun(),
-                      AllocationPolicy allocator = AllocationPolicy());
+  explicit TemplateHashMapImpl(uint32_t capacity = kDefaultHashMapCapacity,
+                               MatchFun match = MatchFun(),
+                               AllocationPolicy allocator = AllocationPolicy());
 
   // Clones the given hashmap and creates a copy with the same entries.
-  TemplateHashMapImpl(const TemplateHashMapImpl<Key, Value, MatchFun,
-                                                AllocationPolicy>* original,
-                      AllocationPolicy allocator = AllocationPolicy());
+  explicit TemplateHashMapImpl(const TemplateHashMapImpl* original,
+                               AllocationPolicy allocator = AllocationPolicy());
+
+  TemplateHashMapImpl(TemplateHashMapImpl&& other) V8_NOEXCEPT
+      : allocator_(other.allocator_) {
+    *this = std::move(other);
+  }
 
   ~TemplateHashMapImpl();
 
+  TemplateHashMapImpl& operator=(TemplateHashMapImpl&& other) V8_NOEXCEPT {
+    map_ = other.map_;
+    capacity_ = other.capacity_;
+    occupancy_ = other.occupancy_;
+    match_ = other.match_;
+    allocator_ = other.allocator_;
+
+    other.map_ = nullptr;
+    other.occupancy_ = 0;
+    other.capacity_ = 0;
+    return *this;
+  }
+
   // If an entry with matching key is found, returns that entry.
   // Otherwise, nullptr is returned.
   Entry* Lookup(const Key& key, uint32_t hash) const;
@@ -75,6 +92,7 @@ class TemplateHashMapImpl {
   // Empties the map and makes it unusable for allocation.
   void Invalidate() {
     AllocationPolicy::Delete(map_);
+    allocator_ = AllocationPolicy();
     map_ = nullptr;
     occupancy_ = 0;
     capacity_ = 0;
@@ -99,10 +117,7 @@ class TemplateHashMapImpl {
   Entry* Start() const;
   Entry* Next(Entry* entry) const;
 
-  void Reset(AllocationPolicy allocator) {
-    Initialize(capacity_);
-    occupancy_ = 0;
-  }
+  AllocationPolicy allocator() const { return allocator_; }
 
  protected:
   void Initialize(uint32_t capacity);
@@ -392,13 +407,13 @@ class CustomMatcherTemplateHashMapImpl
  public:
   using MatchFun = bool (*)(void*, void*);
 
-  CustomMatcherTemplateHashMapImpl(
+  explicit CustomMatcherTemplateHashMapImpl(
       MatchFun match, uint32_t capacity = Base::kDefaultHashMapCapacity,
       AllocationPolicy allocator = AllocationPolicy())
       : Base(capacity, HashEqualityThenKeyMatcher<void*, MatchFun>(match),
              allocator) {}
 
-  CustomMatcherTemplateHashMapImpl(
+  explicit CustomMatcherTemplateHashMapImpl(
       const CustomMatcherTemplateHashMapImpl<AllocationPolicy>* original,
       AllocationPolicy allocator = AllocationPolicy())
       : Base(original, allocator) {}
@@ -428,9 +443,23 @@ class PointerTemplateHashMapImpl
                                    AllocationPolicy>;
 
  public:
-  PointerTemplateHashMapImpl(uint32_t capacity = Base::kDefaultHashMapCapacity,
-                             AllocationPolicy allocator = AllocationPolicy())
+  explicit PointerTemplateHashMapImpl(
+      uint32_t capacity = Base::kDefaultHashMapCapacity,
+      AllocationPolicy allocator = AllocationPolicy())
       : Base(capacity, KeyEqualityMatcher<void*>(), allocator) {}
+
+  PointerTemplateHashMapImpl(const PointerTemplateHashMapImpl& other,
+                             AllocationPolicy allocator = AllocationPolicy())
+      : Base(&other, allocator) {}
+
+  PointerTemplateHashMapImpl(PointerTemplateHashMapImpl&& other) V8_NOEXCEPT
+      : Base(std::move(other)) {}
+
+  PointerTemplateHashMapImpl& operator=(PointerTemplateHashMapImpl&& other)
+      V8_NOEXCEPT {
+    static_cast<Base&>(*this) = std::move(other);
+    return *this;
+  }
 };
 
 using HashMap = PointerTemplateHashMapImpl<DefaultAllocationPolicy>;
@@ -473,8 +502,8 @@ class TemplateHashMap
     friend class TemplateHashMap;
   };
 
-  TemplateHashMap(MatchFun match,
-                  AllocationPolicy allocator = AllocationPolicy())
+  explicit TemplateHashMap(MatchFun match,
+                           AllocationPolicy allocator = AllocationPolicy())
       : Base(Base::kDefaultHashMapCapacity,
              HashEqualityThenKeyMatcher<void*, MatchFun>(match), allocator) {}
 

src/zone/zone.h

@@ -194,6 +194,8 @@ class ZoneObject {
 // structures to allocate themselves and their elements in the Zone.
 class ZoneAllocationPolicy final {
  public:
+  // Creates unusable allocation policy.
+  ZoneAllocationPolicy() : zone_(nullptr) {}
   explicit ZoneAllocationPolicy(Zone* zone) : zone_(zone) {}
   void* New(size_t size) { return zone()->New(size); }
   static void Delete(void* pointer) {}