[torque] Generate verifiers for any indexed field

This change updates verifier generation to:
- Fix a bug I introduced in https://crrev.com/c/2047399 that caused
  values within struct-typed fields to not get verified
- Support indexed fields with start offsets that are not known at
  compile time
- Support indexed fields with complex length expressions

Bug: v8:7793
Change-Id: I5ae8803fce59abae0989fcb094bd9692cd88e38e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2461456
Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70490}

src/diagnostics/objects-debug.cc

@@ -1183,6 +1183,7 @@ void SmallOrderedHashTable<Derived>::SmallOrderedHashTableVerify(
   }
 }
 void SmallOrderedHashMap::SmallOrderedHashMapVerify(Isolate* isolate) {
+  TorqueGeneratedClassVerifiers::SmallOrderedHashMapVerify(*this, isolate);
   SmallOrderedHashTable<SmallOrderedHashMap>::SmallOrderedHashTableVerify(
       isolate);
   for (int entry = NumberOfElements(); entry < NumberOfDeletedElements();
@@ -1195,6 +1196,7 @@ void SmallOrderedHashMap::SmallOrderedHashMapVerify(Isolate* isolate) {
 }
 
 void SmallOrderedHashSet::SmallOrderedHashSetVerify(Isolate* isolate) {
+  TorqueGeneratedClassVerifiers::SmallOrderedHashSetVerify(*this, isolate);
   SmallOrderedHashTable<SmallOrderedHashSet>::SmallOrderedHashTableVerify(
       isolate);
   for (int entry = NumberOfElements(); entry < NumberOfDeletedElements();
@@ -1204,28 +1206,12 @@ void SmallOrderedHashSet::SmallOrderedHashSetVerify(Isolate* isolate) {
       CHECK(val.IsTheHole(isolate));
     }
   }
-
-  // Eventually Torque-generated offset computations could replace the ones
-  // implemented in C++. For now, just make sure they match. This could help
-  // ensure that the class definitions in C++ and Torque don't diverge.
-  intptr_t offset;
-  intptr_t length;
-  std::tie(std::ignore, offset, length) =
-      TqRuntimeFieldSliceSmallOrderedHashSetDataTable(isolate, *this);
-  CHECK_EQ(offset, DataTableStartOffset());
-  CHECK_EQ(length, Capacity());
-  std::tie(std::ignore, offset, length) =
-      TqRuntimeFieldSliceSmallOrderedHashSetHashTable(isolate, *this);
-  CHECK_EQ(offset, GetBucketsStartOffset());
-  CHECK_EQ(length, NumberOfBuckets());
-  std::tie(std::ignore, offset, length) =
-      TqRuntimeFieldSliceSmallOrderedHashSetChainTable(isolate, *this);
-  CHECK_EQ(offset, GetChainTableOffset());
-  CHECK_EQ(length, Capacity());
 }
 
 void SmallOrderedNameDictionary::SmallOrderedNameDictionaryVerify(
     Isolate* isolate) {
+  TorqueGeneratedClassVerifiers::SmallOrderedNameDictionaryVerify(*this,
+                                                                  isolate);
   SmallOrderedHashTable<
       SmallOrderedNameDictionary>::SmallOrderedHashTableVerify(isolate);
   for (int entry = NumberOfElements(); entry < NumberOfDeletedElements();

src/objects/ordered-hash-table.tq

@@ -23,7 +23,6 @@ extern class SmallOrderedHashTable extends HeapObject
 extern macro SmallOrderedHashSetMapConstant(): Map;
 const kSmallOrderedHashSetMap: Map = SmallOrderedHashSetMapConstant();
 
-@noVerifier
 extern class SmallOrderedHashSet extends SmallOrderedHashTable {
   number_of_elements: uint8;
   number_of_deleted_elements: uint8;
@@ -62,7 +61,6 @@ struct HashMapEntry {
 extern macro SmallOrderedHashMapMapConstant(): Map;
 const kSmallOrderedHashMapMap: Map = SmallOrderedHashMapMapConstant();
 
-@noVerifier
 extern class SmallOrderedHashMap extends SmallOrderedHashTable {
   number_of_elements: uint8;
   number_of_deleted_elements: uint8;
@@ -99,7 +97,6 @@ struct NameDictionaryEntry {
   property_details: Smi|TheHole;
 }
 
-@noVerifier
 extern class SmallOrderedNameDictionary extends SmallOrderedHashTable {
   hash: int32;
   number_of_elements: uint8;

src/torque/cc-generator.cc

@@ -239,13 +239,6 @@ void CCGenerator::EmitInstruction(const CallCsaMacroInstruction& instruction,
     }
   }
 
-  if (ExternMacro* extern_macro = ExternMacro::DynamicCast(instruction.macro)) {
-    out() << "TorqueRuntimeMacroShims::"
-          << extern_macro->external_assembler_name() << "::";
-  } else {
-    out() << "TqRuntime";
-  }
-
   out() << instruction.macro->CCName() << "(isolate";
   if (!args.empty()) out() << ", ";
   PrintCommaSeparatedList(out(), args);

src/torque/declarable.h

@@ -322,8 +322,17 @@ class Callable : public Scope {
     return !ShouldBeInlined(output_type);
   }
 
+  static std::string PrefixNameForCCOutput(const std::string& name) {
+    // If a Torque macro requires a C++ runtime function to be generated, then
+    // the generated function begins with this prefix to avoid any naming
+    // collisions with the generated CSA function for the same macro.
+    return "TqRuntime" + name;
+  }
+
   // Name to use in runtime C++ code.
-  virtual const std::string& CCName() const { return ExternalName(); }
+  virtual std::string CCName() const {
+    return PrefixNameForCCOutput(ExternalName());
+  }
 
  protected:
   Callable(Declarable::Kind kind, std::string external_name,
@@ -389,6 +398,11 @@ class ExternMacro : public Macro {
     return external_assembler_name_;
   }
 
+  std::string CCName() const override {
+    return "TorqueRuntimeMacroShims::" + external_assembler_name() +
+           "::" + ExternalName();
+  }
+
  private:
   friend class Declarations;
   ExternMacro(const std::string& name, std::string external_assembler_name,
@@ -404,10 +418,11 @@ class TorqueMacro : public Macro {
  public:
   DECLARE_DECLARABLE_BOILERPLATE(TorqueMacro, TorqueMacro)
   bool IsExportedToCSA() const { return exported_to_csa_; }
-  const std::string& CCName() const override {
+  std::string CCName() const override {
     // Exported functions must have unique and C++-friendly readable names, so
     // prefer those wherever possible.
-    return IsExportedToCSA() ? ReadableName() : ExternalName();
+    return PrefixNameForCCOutput(IsExportedToCSA() ? ReadableName()
+                                                   : ExternalName());
   }
 
  protected:

src/torque/implementation-visitor.cc

@@ -314,12 +314,11 @@ void ImplementationVisitor::VisitMacroCommon(Macro* macro) {
   bool can_return = return_type != TypeOracle::GetNeverType();
   bool has_return_value =
       can_return && return_type != TypeOracle::GetVoidType();
-  const char* prefix = output_type_ == OutputType::kCC ? "TqRuntime" : "";
 
-  GenerateMacroFunctionDeclaration(header_out(), prefix, macro);
+  GenerateMacroFunctionDeclaration(header_out(), macro);
   header_out() << ";\n";
 
-  GenerateMacroFunctionDeclaration(source_out(), prefix, macro);
+  GenerateMacroFunctionDeclaration(source_out(), macro);
   source_out() << " {\n";
 
   if (output_type_ == OutputType::kCC) {
@@ -1668,10 +1667,10 @@ void ImplementationVisitor::GenerateImplementation(const std::string& dir) {
   WriteFile(dir + "/runtime-macros.cc", runtime_macros_cc_.str());
 }
 
-void ImplementationVisitor::GenerateMacroFunctionDeclaration(
-    std::ostream& o, const std::string& macro_prefix, Macro* macro) {
+void ImplementationVisitor::GenerateMacroFunctionDeclaration(std::ostream& o,
+                                                             Macro* macro) {
   GenerateFunctionDeclaration(
-      o, macro_prefix,
+      o, "",
       output_type_ == OutputType::kCC ? macro->CCName() : macro->ExternalName(),
       macro->signature(), macro->parameter_names());
 }
@@ -4621,10 +4620,9 @@ namespace {
 // Generate verification code for a single piece of class data, which might be
 // nested within a struct or might be a single element in an indexed field (or
 // both).
-void GenerateFieldValueVerifier(const std::string& class_name,
-                                const Field& class_field,
-                                const Field& leaf_field, size_t struct_offset,
-                                std::string field_size,
+void GenerateFieldValueVerifier(const std::string& class_name, bool indexed,
+                                std::string offset, const Field& leaf_field,
+                                std::string indexed_field_size,
                                 std::ostream& cc_contents) {
   const Type* field_type = leaf_field.name_and_type.type;
 
@@ -4633,17 +4631,15 @@ void GenerateFieldValueVerifier(const std::string& class_name,
   const char* object_type = maybe_object ? "MaybeObject" : "Object";
   const char* verify_fn =
       maybe_object ? "VerifyMaybeObjectPointer" : "VerifyPointer";
-  std::string index_offset = std::to_string(struct_offset);
-  if (class_field.index) {
-    index_offset += " + i * " + field_size;
+  if (indexed) {
+    offset += " + i * " + indexed_field_size;
   }
   // Name the local var based on the field name for nicer CHECK output.
   const std::string value = leaf_field.name_and_type.name + "__value";
 
   // Read the field.
   cc_contents << "    " << object_type << " " << value << " = TaggedField<"
-              << object_type << ", " << *class_field.offset << ">::load(o, "
-              << index_offset << ");\n";
+              << object_type << ">::load(o, " << offset << ");\n";
 
   // Call VerifyPointer or VerifyMaybeObjectPointer on it.
   cc_contents << "    " << object_type << "::" << verify_fn << "(isolate, "
@@ -4674,49 +4670,49 @@ void GenerateClassFieldVerifier(const std::string& class_name,
   // Do not verify if the field may be uninitialized.
   if (TypeOracle::GetUninitializedType()->IsSubtypeOf(field_type)) return;
 
+  std::string field_start_offset;
   if (f.index) {
-    base::Optional<NameAndType> array_length =
-        ExtractSimpleFieldArraySize(class_type, *f.index);
-    if (!array_length) {
-      Error("Cannot generate verifier for array field with complex length.")
-          .Position((*f.index)->pos)
-          .Throw();
-    }
-
-    std::string length_field_offset =
-        class_name + "::k" + CamelifyString(array_length->name) + "Offset";
-    cc_contents << "  for (int i = 0; i < ";
-    if (array_length->type == TypeOracle::GetSmiType()) {
-      // We already verified the index field because it was listed earlier, so
-      // we can assume it's safe to read here.
-      cc_contents << "TaggedField<Smi, " << length_field_offset
-                  << ">::load(o).value()";
-    } else {
-      const Type* constexpr_version = array_length->type->ConstexprVersion();
-      if (constexpr_version == nullptr) {
-        Error("constexpr representation for type ",
-              array_length->type->ToString(),
-              " is required due to usage as index")
-            .Position(f.pos);
-      }
-      cc_contents << "o.ReadField<" << constexpr_version->GetGeneratedTypeName()
-                  << ">(" << length_field_offset << ")";
-    }
-    cc_contents << "; ++i) {\n";
+    field_start_offset = f.name_and_type.name + "__offset";
+    std::string length = f.name_and_type.name + "__length";
+    cc_contents << "  intptr_t " << field_start_offset << ", " << length
+                << ";\n";
+    cc_contents << "  std::tie(std::ignore, " << field_start_offset << ", "
+                << length << ") = "
+                << Callable::PrefixNameForCCOutput(
+                       class_type.GetSliceMacroName(f))
+                << "(isolate, o);\n";
+
+    // Slices use intptr, but TaggedField<T>.load() uses int, so verify that
+    // such a cast is valid.
+    cc_contents << "  CHECK_EQ(" << field_start_offset << ", static_cast<int>("
+                << field_start_offset << "));\n";
+    cc_contents << "  CHECK_EQ(" << length << ", static_cast<int>(" << length
+                << "));\n";
+    field_start_offset = "static_cast<int>(" + field_start_offset + ")";
+    length = "static_cast<int>(" + length + ")";
+
+    cc_contents << "  for (int i = 0; i < " << length << "; ++i) {\n";
   } else {
+    // Non-indexed fields have known offsets.
+    field_start_offset = std::to_string(*f.offset);
     cc_contents << "  {\n";
   }
 
   if (auto struct_type = field_type->StructSupertype()) {
-    for (const Field& field : (*struct_type)->fields()) {
-      if (field_type->IsSubtypeOf(TypeOracle::GetTaggedType())) {
-        GenerateFieldValueVerifier(class_name, f, field, *field.offset,
-                                   std::to_string((*struct_type)->PackedSize()),
-                                   cc_contents);
+    for (const Field& struct_field : (*struct_type)->fields()) {
+      if (struct_field.name_and_type.type->IsSubtypeOf(
+              TypeOracle::GetTaggedType())) {
+        GenerateFieldValueVerifier(
+            class_name, f.index.has_value(),
+            field_start_offset + " + " + std::to_string(*struct_field.offset),
+            struct_field, std::to_string((*struct_type)->PackedSize()),
+            cc_contents);
       }
     }
   } else {
-    GenerateFieldValueVerifier(class_name, f, f, 0, "kTaggedSize", cc_contents);
+    GenerateFieldValueVerifier(class_name, f.index.has_value(),
+                               field_start_offset, f, "kTaggedSize",
+                               cc_contents);
   }
 
   cc_contents << "  }\n";
@@ -4744,6 +4740,7 @@ void ImplementationVisitor::GenerateClassVerifiers(
                    "\"torque-generated/internal-class-definitions-inl.h\"\n";
     cc_contents << "#include "
                    "\"torque-generated/exported-class-definitions-inl.h\"\n";
+    cc_contents << "#include \"torque-generated/runtime-macros.h\"\n";
 
     IncludeObjectMacrosScope object_macros(cc_contents);
 

src/torque/implementation-visitor.h

@@ -729,7 +729,6 @@ class ImplementationVisitor {
                                 Block* false_block);
 
   void GenerateMacroFunctionDeclaration(std::ostream& o,
-                                        const std::string& macro_prefix,
                                         Macro* macro);
   std::vector<std::string> GenerateFunctionDeclaration(
       std::ostream& o, const std::string& macro_prefix, const std::string& name,