[string] Add checks for correct hash values in heap verification

- Check that internalized strings always have a computed hash value.
- Check that ThinStrings never have a forwarding index.
- Add a simple test of various property access with
  --always-use-string-forwarding-table to make the CF aware of the flag.

Change-Id: Ie047c9f635d5e0ed999208ec3379ef09c395b3f5
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3717988Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Patrick Thier <pthier@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81303}

src/diagnostics/objects-debug.cc

@@ -856,6 +856,7 @@ void String::StringVerify(Isolate* isolate) {
   CHECK(length() >= 0 && length() <= Smi::kMaxValue);
   CHECK_IMPLIES(length() == 0, *this == ReadOnlyRoots(isolate).empty_string());
   if (IsInternalizedString()) {
+    CHECK(HasHashCode());
     CHECK(!ObjectInYoungGeneration(*this));
   }
 }
@@ -874,6 +875,7 @@ void ConsString::ConsStringVerify(Isolate* isolate) {
 
 void ThinString::ThinStringVerify(Isolate* isolate) {
   TorqueGeneratedClassVerifiers::ThinStringVerify(*this, isolate);
+  CHECK(!HasForwardingIndex());
   CHECK(actual().IsInternalizedString());
   CHECK(actual().IsSeqString() || actual().IsExternalString());
 }

src/objects/string-table.cc

@@ -433,8 +433,11 @@ namespace {
 void SetInternalizedReference(Isolate* isolate, String string,
                               String internalized) {
   // TODO(v8:12007): Support external strings.
+  DCHECK(!string.IsThinString());
+  DCHECK(internalized.IsInternalizedString());
   if ((string.IsShared() || FLAG_always_use_string_forwarding_table) &&
       !string.IsExternalString()) {
+    DCHECK(!internalized.HasForwardingIndex());
     uint32_t field = string.raw_hash_field();
     // Don't use the forwarding table for strings that have an integer index.
     // Using the hash field for the integer index is more beneficial than

test/cctest/test-shared-strings.cc

@@ -93,11 +93,14 @@ UNINITIALIZED_TEST(InPlaceInternalizableStringsAreShared) {
   CHECK(!young_two_byte_seq->InSharedHeap());
 
   // Internalized strings are shared.
+  uint64_t seed = HashSeed(i_isolate1);
   Handle<String> one_byte_intern = factory1->NewOneByteInternalizedString(
-      base::OneByteVector(raw_one_byte), 1);
+      base::OneByteVector(raw_one_byte),
+      StringHasher::HashSequentialString<char>(raw_one_byte, 3, seed));
   CHECK(one_byte_intern->InSharedHeap());
-  Handle<String> two_byte_intern =
-      factory1->NewTwoByteInternalizedString(two_byte, 1);
+  Handle<String> two_byte_intern = factory1->NewTwoByteInternalizedString(
+      two_byte,
+      StringHasher::HashSequentialString<uint16_t>(raw_two_byte, 3, seed));
   CHECK(two_byte_intern->InSharedHeap());
 }
 

test/mjsunit/string-forwarding-table.js

+// Copyright 2022 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: --always-use-string-forwarding-table
+
+// The main purpose of this test is to make ClusterFuzz aware of the flag and
+// provide some interesting input.
+
+const long_key = 'key1234567890abcdefg';
+const substr_key = long_key.substring(3,17);
+const consstr_key = 'key' + 1234567890 + 'abcdefg';
+const integer_index = long_key.substring(3,8);
+
+// Test object with in-place properties.
+{
+  let obj = {
+    'key1234567890abcdefg': 'long_key_value',
+    '1234567890abcd': 'substr_value',
+    12345: 'integer_index'
+  };
+
+  assertEquals('long_key_value', obj[long_key]);
+  assertEquals('substr_value', obj[substr_key]);
+  assertEquals('long_key_value', obj[consstr_key]);
+  assertEquals('integer_index', obj[integer_index]);
+}
+
+// Test object with dictionary properties.
+{
+  let obj = [];
+  for (let i = 0; i < 100; ++i) {
+    obj[i] = i;
+    obj['XXX' + i] = 'XXX' + i;
+  }
+
+  obj['key1234567890abcdefg'] = 'long_key_value';
+  obj['1234567890abcd'] = 'substr_value';
+  obj[12345] = 'integer_index';
+
+  assertEquals('long_key_value', obj[long_key]);
+  assertEquals('substr_value', obj[substr_key]);
+  assertEquals('long_key_value', obj[consstr_key]);
+  assertEquals('integer_index', obj[integer_index]);
+}

test/unittests/compiler/types-unittest.cc

@@ -274,7 +274,9 @@ class TypesTest : public TestWithNativeContextAndZone {
     CHECK(T.Constant(s1).Is(T.InternalizedString));
     const base::uc16 two_byte[1] = {0x2603};
     Handle<String> s2 = fac->NewTwoByteInternalizedString(
-        base::Vector<const base::uc16>(two_byte, 1), 1);
+        base::Vector<const base::uc16>(two_byte, 1),
+        StringHasher::HashSequentialString<uint16_t>(two_byte, 1,
+                                                     HashSeed(isolate())));
     CHECK(T.Constant(s2).Is(T.InternalizedString));
 
     // Typing of special constants