Fix rehashing of script compilation cache

The script compilation cache contains weak pointers to Script objects as
its keys. When doing a rehashing operation, any hash table needs the
ability to get the hash code for every entry in the table. However, if
the weak pointer was cleared from a key, there is no longer any way to
get the hash code for that entry.

In https://crrev.com/c/3597106 , I attempted to solve this problem by
deleting all entries whose keys contain cleared weak pointers prior to
rehashing, but the implementation has a bug: when resizing, the new
table is allocated after deleting the entries with cleared keys, so if
that allocation triggers a GC, the table can once again have entries
with cleared keys.

This could be solved in a variety of ways, such as:

1. Iterate the entries again and delete those with cleared keys, after
   allocating the new table but before calling Rehash() to copy data
   into that new table. This means we can't directly use
   HashTable::EnsureCapacity, which normally does both the allocation
   and the rehashing.
2. Return a bogus hash code for entries whose keys contain cleared weak
   pointers. This is simple but risks poor distribution of data after
   rehashing.
3. Implement custom rehashing which can avoid copying entries with
   cleared keys, rather than reusing the rehashing implementation from
   HashTable.
4. Include the hash value in every key, so a consistent hash value is
   available even after the weak Script pointer has been cleared.

The fourth option sounds like the lowest risk to me, so this change
implements that option.

Bug: v8:12808
Change-Id: I6b19b9c8af67dcfc31b74842ba581dd141e18845
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3654413
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#80637}

src/objects/compilation-cache-table-inl.h

@@ -57,6 +57,7 @@ void CompilationCacheTable::SetEvalFeedbackValueAt(InternalIndex entry,
 class ScriptCacheKey : public HashTableKey {
  public:
   enum Index {
+    kHash,
     kWeakScript,
     kEnd,
   };
@@ -112,11 +113,6 @@ uint32_t CompilationCacheShape::EvalHash(String source,
   return hash;
 }
 
-uint32_t CompilationCacheShape::ScriptHash(String source) {
-  uint32_t hash = source.EnsureHash();
-  return hash;
-}
-
 uint32_t CompilationCacheShape::HashForObject(ReadOnlyRoots roots,
                                               Object object) {
   // Eval: The key field contains the hash as a Number.
@@ -129,12 +125,19 @@ uint32_t CompilationCacheShape::HashForObject(ReadOnlyRoots roots,
 
   // Script.
   if (object.IsWeakFixedArray()) {
+    uint32_t result = static_cast<uint32_t>(Smi::ToInt(
+        WeakFixedArray::cast(object).Get(ScriptCacheKey::kHash).ToSmi()));
+#ifdef DEBUG
     base::Optional<String> script_key =
         ScriptCacheKey::SourceFromObject(object);
-    // Rehashing should only happen after we've removed all of the keys
-    // containing cleared weak refs.
-    DCHECK(script_key);
-    return ScriptHash(*script_key);
+    if (script_key) {
+      uint32_t source_hash;
+      if (script_key->TryGetHash(&source_hash)) {
+        DCHECK_EQ(result, source_hash);
+      }
+    }
+#endif
+    return result;
   }
 
   // Eval: See EvalCacheKey::ToHandle for the encoding.

src/objects/compilation-cache-table.cc

@@ -226,8 +226,13 @@ class CodeKey : public HashTableKey {
 }  // namespace
 
 ScriptCacheKey::ScriptCacheKey(Handle<String> source)
-    : HashTableKey(CompilationCacheShape::ScriptHash(*source)),
-      source_(source) {}
+    : HashTableKey(source->EnsureHash()), source_(source) {
+  // Hash values must fit within a Smi.
+  static_assert(Name::HashBits::kSize <= kSmiValueSize);
+  DCHECK_EQ(
+      static_cast<uint32_t>(Smi::ToInt(Smi::FromInt(static_cast<int>(Hash())))),
+      Hash());
+}
 
 bool ScriptCacheKey::IsMatch(Object other) {
   DisallowGarbageCollection no_gc;
@@ -241,6 +246,8 @@ Handle<Object> ScriptCacheKey::AsHandle(Isolate* isolate,
   // Any SharedFunctionInfo being stored in the script cache should have a
   // Script.
   DCHECK(shared->script().IsScript());
+  array->Set(kHash,
+             MaybeObject::FromObject(Smi::FromInt(static_cast<int>(Hash()))));
   array->Set(kWeakScript,
              MaybeObject::MakeWeak(MaybeObject::FromObject(shared->script())));
   return array;
@@ -469,12 +476,8 @@ void CompilationCacheTable::RemoveEntry(InternalIndex entry) {
   ElementRemoved();
 
   // This table does not shrink upon deletion. The script cache depends on that
-  // fact, in two ways:
-  // 1. EnsureScriptTableCapacity calls RemoveEntry, at a time when shrinking
-  //    the table would be counterproductive, and
-  // 2. CompilationCacheShape::HashForObject cannot produce a hash for keys that
-  //    contain cleared weak pointers, so rehashing must only occur right after
-  //    all such keys have been cleared.
+  // fact, because EnsureScriptTableCapacity calls RemoveEntry at a time when
+  // shrinking the table would be counterproductive.
 }
 
 }  // namespace internal

src/objects/compilation-cache-table.h

@@ -32,8 +32,6 @@ class CompilationCacheShape : public BaseShape<HashTableKey*> {
   static inline uint32_t EvalHash(String source, SharedFunctionInfo shared,
                                   LanguageMode language_mode, int position);
 
-  static inline uint32_t ScriptHash(String source);
-
   static inline uint32_t HashForObject(ReadOnlyRoots roots, Object object);
 
   static const int kPrefixSize = 0;