Reland [dict-proto] C++ implementation of SwissNameDictionary, pt. 10

This is a reland of
https://chromium-review.googlesource.com/c/v8/v8/+/2744138. It
shortens the runtime of the Copy and EnumerationOrder tests in
cctest/test-swiss-name-dictionary-csa for TSAN and CFI builds, as
compared to the original version.

Original description:

This CL is part of a series that adds the C++ implementation of
SwissNameDictionary, a deterministic property backing store based on
Swiss Tables.

This CL adds the actual tests for SwissNameDictionary, defined in
test-swiss-name-dictionary-shared-tests.h, using the infrastructure
in test-swiss-name-dictionary-infra.[h|cc].

Bug: v8:11388
Change-Id: Ia3f83f6e27be80bfdd63c2cb868638dc90d24cbc
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2778416
Commit-Queue: Frank Emrich <emrich@google.com>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73589}

test/cctest/cctest.h

@@ -89,6 +89,19 @@ class JSHeapBroker;
   static void Test##Name()
 #endif
 
+// Similar to TEST, but used when test definitions appear as members of a
+// (probably parameterized) class. This allows re-using the given tests multiple
+// times. For this to work, the following conditions must hold:
+//   1. The class has a template parameter named kTestFileName of type  char
+//      const*, which is instantiated with __FILE__ at the *use site*, in order
+//      to correctly associate the tests with the test suite using them.
+//   2. To actually execute the tests, create an instance of the class
+//      containing the MEMBER_TESTs.
+#define MEMBER_TEST(Name)                                   \
+  CcTest register_test_##Name =                             \
+      CcTest(Test##Name, kTestFileName, #Name, true, true); \
+  static void Test##Name()
+
 #define EXTENSION_LIST(V)                                                      \
   V(GC_EXTENSION,       "v8/gc")                                               \
   V(PRINT_EXTENSION,    "v8/print")                                            \

test/cctest/test-swiss-name-dictionary-csa.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "test/cctest/test-swiss-name-dictionary-infra.h"
+#include "test/cctest/test-swiss-name-dictionary-shared-tests.h"
 
 namespace v8 {
 namespace internal {
@@ -312,6 +313,13 @@ void CSATestRunner::CheckAgainstReference() {
   CHECK(table->EqualsForTesting(*reference_));
 }
 
+// Executes the tests defined in test-swiss-name-dictionary-shared-tests.h as if
+// they were defined in this file, using the CSATestRunner. See comments in
+// test-swiss-name-dictionary-shared-tests.h and in
+// swiss-name-dictionary-infra.h for details.
+const char kCSATestFileName[] = __FILE__;
+SharedSwissTableTests<CSATestRunner, kCSATestFileName> execute_shared_tests_csa;
+
 }  // namespace test_swiss_hash_table
 }  // namespace internal
 }  // namespace v8

test/cctest/test-swiss-name-dictionary-infra.cc

@@ -76,6 +76,13 @@ Handle<Name> CreateKeyWithHash(Isolate* isolate, KeyCache& keys,
       int fake_hash = actual_hash;
       if (key.h1_override) {
         uint32_t override_with = key.h1_override.value().value;
+
+        // We cannot override h1 with 0 unless we also override h2 with a
+        // non-zero value. Otherwise, the overall hash may become 0 (which is
+        // forbidden) based on the (nondeterminstic) choice of h2.
+        CHECK_IMPLIES(override_with == 0,
+                      key.h2_override && key.h2_override.value().value != 0);
+
         fake_hash = (override_with << swiss_table::kH2Bits) |
                     swiss_table::H2(actual_hash);
       }
@@ -83,8 +90,14 @@ Handle<Name> CreateKeyWithHash(Isolate* isolate, KeyCache& keys,
         // Unset  7 bits belonging to H2:
         fake_hash &= ~((1 << swiss_table::kH2Bits) - 1);
 
-        DCHECK_LT(key.h2_override.value().value, 1 << swiss_table::kH2Bits);
-        fake_hash |= swiss_table::H2(key.h2_override.value().value);
+        uint8_t override_with = key.h2_override.value().value;
+
+        // Same as above, but for h2: Prevent accidentally creating 0 fake hash.
+        CHECK_IMPLIES(override_with == 0,
+                      key.h1_override && key.h1_override.value().value != 0);
+
+        CHECK_LT(key.h2_override.value().value, 1 << swiss_table::kH2Bits);
+        fake_hash |= swiss_table::H2(override_with);
       }
 
       // Ensure that just doing a shift below is correct.
@@ -96,9 +109,10 @@ Handle<Name> CreateKeyWithHash(Isolate* isolate, KeyCache& keys,
 
       // Prepare what to put into the hash field.
       uint32_t hash_field = fake_hash << Name::kHashShift;
+      CHECK_NE(hash_field, 0);
 
       key_symbol->set_raw_hash_field(hash_field);
-      DCHECK_EQ(fake_hash, key_symbol->hash());
+      CHECK_EQ(fake_hash, key_symbol->hash());
     }
 
     return key_symbol;
@@ -110,8 +124,8 @@ Handle<Name> CreateKeyWithHash(Isolate* isolate, KeyCache& keys,
     // else w.r.t. hash faking when using this key before. If so, the test case
     // would make inconsistent assumptions about how the hashes should be faked
     // and be broken.
-    DCHECK_EQ(cached_info.h1_override, key.h1_override);
-    DCHECK_EQ(cached_info.h2_override, key.h2_override);
+    CHECK_EQ(cached_info.h1_override, key.h1_override);
+    CHECK_EQ(cached_info.h2_override, key.h2_override);
 
     return cached_info.key_symbol;
   }

test/cctest/test-swiss-name-dictionary-infra.h

@@ -28,7 +28,7 @@ using IndexOpt = base::Optional<InternalIndex>;
 static const ValueOpt kNoValue;
 static const PropertyDetailsOpt kNoDetails;
 static const base::Optional<int> kNoInt;
-static const IndexOpt kNoIndex;
+static const IndexOpt kIndexUnknown;
 
 static const std::vector<int> interesting_initial_capacities = {
     4,
@@ -38,6 +38,17 @@ static const std::vector<int> interesting_initial_capacities = {
     1 << (sizeof(uint16_t) * 8),
     1 << (sizeof(uint16_t) * 8 + 1)};
 
+// Use these as initial capacities for tests that may timeout on larger
+// capacities when sanitizers/CFI are enabled.
+// TODO(v8:11330) Revisit this once the actual CSA/Torque versions are run by
+// the test suite, which will speed things up.
+#if defined(THREAD_SANITIZER) || defined(V8_ENABLE_CONTROL_FLOW_INTEGRITY)
+static const std::vector<int> capacities_for_slow_tests = {4, 8, 16, 128, 1024};
+#else
+static const std::vector<int> capacities_for_slow_tests =
+    interesting_initial_capacities;
+#endif
+
 extern const std::vector<PropertyDetails> distinct_property_details;
 
 // Wrapping this in a struct makes the tests a bit more readable.

test/cctest/test-swiss-name-dictionary.cc

@@ -5,6 +5,7 @@
 #include "src/objects/swiss-name-dictionary-inl.h"
 #include "test/cctest/cctest.h"
 #include "test/cctest/test-swiss-name-dictionary-infra.h"
+#include "test/cctest/test-swiss-name-dictionary-shared-tests.h"
 
 namespace v8 {
 namespace internal {
@@ -213,6 +214,14 @@ TEST(SizeFor) {
   CHECK_EQ(SwissNameDictionary::SizeFor(8), size_8);
 }
 
+// Executes the tests defined in test-swiss-name-dictionary-shared-tests.h as if
+// they were defined in this file, using the RuntimeTestRunner. See comments in
+// test-swiss-name-dictionary-shared-tests.h and in
+// swiss-name-dictionary-infra.h for details.
+const char kRuntimeTestFileName[] = __FILE__;
+SharedSwissTableTests<RuntimeTestRunner, kRuntimeTestFileName>
+    execute_shared_tests_runtime;
+
 }  // namespace test_swiss_hash_table
 }  // namespace internal
 }  // namespace v8