[code] Extend comments, use better terms to describe metadata

This addresses comments from [0] by extending comments to also
describe embedded builtins in code.h, and by improving language
around various meaning of 'metadata':

- The Code object's metadata section is still called 'metadata'.
- The embedded blob's table of layout descriptions for builtins is
  now called 'layout descriptions'.
- The embedded blob's data section (containing hashes and layout
  descriptions) is now called 'data' section.

[0] chromium-review.googlesource.com/c/v8/v8/+/2491025

Bug: v8:11036
Change-Id: Ibe84fddb9784cc5d3b66482612dcdb7a2e8d14ae
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2501284
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70793}

src/execution/isolate.h

@@ -1437,16 +1437,16 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
 
   static const uint8_t* CurrentEmbeddedBlobCode();
   static uint32_t CurrentEmbeddedBlobCodeSize();
-  static const uint8_t* CurrentEmbeddedBlobMetadata();
-  static uint32_t CurrentEmbeddedBlobMetadataSize();
+  static const uint8_t* CurrentEmbeddedBlobData();
+  static uint32_t CurrentEmbeddedBlobDataSize();
   static bool CurrentEmbeddedBlobIsBinaryEmbedded();
 
   // These always return the same result as static methods above, but don't
   // access the global atomic variable (and thus *might be* slightly faster).
   const uint8_t* embedded_blob_code() const;
   uint32_t embedded_blob_code_size() const;
-  const uint8_t* embedded_blob_metadata() const;
-  uint32_t embedded_blob_metadata_size() const;
+  const uint8_t* embedded_blob_data() const;
+  uint32_t embedded_blob_data_size() const;
 
   void set_array_buffer_allocator(v8::ArrayBuffer::Allocator* allocator) {
     array_buffer_allocator_ = allocator;
@@ -1920,13 +1920,13 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
   void TearDownEmbeddedBlob();
 
   void SetEmbeddedBlob(const uint8_t* code, uint32_t code_size,
-                       const uint8_t* metadata, uint32_t metadata_size);
+                       const uint8_t* data, uint32_t data_size);
   void ClearEmbeddedBlob();
 
   const uint8_t* embedded_blob_code_ = nullptr;
   uint32_t embedded_blob_code_size_ = 0;
-  const uint8_t* embedded_blob_metadata_ = nullptr;
-  uint32_t embedded_blob_metadata_size_ = 0;
+  const uint8_t* embedded_blob_data_ = nullptr;
+  uint32_t embedded_blob_data_size_ = 0;
 
   v8::ArrayBuffer::Allocator* array_buffer_allocator_ = nullptr;
   std::shared_ptr<v8::ArrayBuffer::Allocator> array_buffer_allocator_shared_;

src/objects/code.h

@@ -106,6 +106,28 @@ class Code : public HeapObject {
   // raw accessors (e.g. raw_instruction_start) always refer to the on-heap
   // Code object, while camel-case accessors (e.g. InstructionStart) may refer
   // to an off-heap area in the case of embedded builtins.
+  //
+  // Embedded builtins are on-heap Code objects, with an out-of-line body
+  // section. The on-heap Code object contains an essentially empty body
+  // section, while accessors, as mentioned above, redirect to the off-heap
+  // area. Metadata table offsets remain relative to MetadataStart(), i.e. they
+  // point into the off-heap metadata section. The off-heap layout is described
+  // in detail in the EmbeddedData class, but at a high level one can assume a
+  // dedicated, out-of-line, instruction and metadata section for each embedded
+  // builtin *in addition* to the on-heap Code object:
+  //
+  //  +--------------------------+  <-- InstructionStart()
+  //  |   off-heap instructions  |
+  //  |           ...            |
+  //  +--------------------------+  <-- InstructionEnd()
+  //
+  //  +--------------------------+  <-- MetadataStart() (MS)
+  //  |    off-heap metadata     |
+  //  |           ...            |  <-- MS + handler_table_offset()
+  //  |                          |  <-- MS + constant_pool_offset()
+  //  |                          |  <-- MS + code_comments_offset()
+  //  |                          |  <-- MS + unwinding_info_offset()
+  //  +--------------------------+  <-- MetadataEnd()
 
   // TODO(jgruber,v8:11036): Update once no longer true for embedded builtins.
   static constexpr bool kOnHeapBodyIsContiguous = true;

src/runtime/runtime-debug.cc

@@ -859,7 +859,7 @@ RUNTIME_FUNCTION(Runtime_ProfileCreateSnapshotDataBlob) {
   {
     i::EmbeddedData d = i::EmbeddedData::FromBlob();
     PrintF("Embedded blob is %d bytes\n",
-           static_cast<int>(d.code_size() + d.metadata_size()));
+           static_cast<int>(d.code_size() + d.data_size()));
   }
 
   FreeCurrentEmbeddedBlob();

src/snapshot/embedded/embedded-data.h

@@ -32,11 +32,10 @@ class InstructionStream final : public AllStatic {
   // mksnapshot. Otherwise, off-heap code is embedded directly into the binary.
   static void CreateOffHeapInstructionStream(Isolate* isolate, uint8_t** code,
                                              uint32_t* code_size,
-                                             uint8_t** metadata,
-                                             uint32_t* metadata_size);
+                                             uint8_t** data,
+                                             uint32_t* data_size);
   static void FreeOffHeapInstructionStream(uint8_t* code, uint32_t code_size,
-                                           uint8_t* metadata,
-                                           uint32_t metadata_size);
+                                           uint8_t* data, uint32_t data_size);
 };
 
 class EmbeddedData final {
@@ -46,27 +45,26 @@ class EmbeddedData final {
   static EmbeddedData FromBlob() {
     return EmbeddedData(Isolate::CurrentEmbeddedBlobCode(),
                         Isolate::CurrentEmbeddedBlobCodeSize(),
-                        Isolate::CurrentEmbeddedBlobMetadata(),
-                        Isolate::CurrentEmbeddedBlobMetadataSize());
+                        Isolate::CurrentEmbeddedBlobData(),
+                        Isolate::CurrentEmbeddedBlobDataSize());
   }
 
   static EmbeddedData FromBlob(Isolate* isolate) {
-    return EmbeddedData(isolate->embedded_blob_code(),
-                        isolate->embedded_blob_code_size(),
-                        isolate->embedded_blob_metadata(),
-                        isolate->embedded_blob_metadata_size());
+    return EmbeddedData(
+        isolate->embedded_blob_code(), isolate->embedded_blob_code_size(),
+        isolate->embedded_blob_data(), isolate->embedded_blob_data_size());
   }
 
   const uint8_t* code() const { return code_; }
   uint32_t code_size() const { return code_size_; }
-  const uint8_t* metadata() const { return metadata_; }
-  uint32_t metadata_size() const { return metadata_size_; }
+  const uint8_t* data() const { return data_; }
+  uint32_t data_size() const { return data_size_; }
 
   void Dispose() {
     delete[] code_;
     code_ = nullptr;
-    delete[] metadata_;
-    metadata_ = nullptr;
+    delete[] data_;
+    data_ = nullptr;
   }
 
   Address InstructionStartOfBuiltin(int i) const;
@@ -95,47 +93,45 @@ class EmbeddedData final {
 
   size_t CreateEmbeddedBlobHash() const;
   size_t EmbeddedBlobHash() const {
-    return *reinterpret_cast<const size_t*>(metadata_ +
-                                            EmbeddedBlobHashOffset());
+    return *reinterpret_cast<const size_t*>(data_ + EmbeddedBlobHashOffset());
   }
 
   size_t IsolateHash() const {
-    return *reinterpret_cast<const size_t*>(metadata_ + IsolateHashOffset());
+    return *reinterpret_cast<const size_t*>(data_ + IsolateHashOffset());
   }
 
   // Blob layout information for a single instruction stream. Corresponds
   // roughly to Code object layout (see the instruction and metadata area).
-  // TODO(jgruber): With the addition of metadata sections in Code objects,
-  // naming here has become confusing. Metadata refers to both this struct
-  // and the Code section, and the embedded instruction area currently
-  // contains both Code's instruction and metadata areas. Fix it.
-  struct Metadata {
+  struct LayoutDescription {
     // The offset and (unpadded) length of this builtin's instruction area
     // from the start of the embedded code section.
     uint32_t instruction_offset;
     uint32_t instruction_length;
     // The offset and (unpadded) length of this builtin's metadata area
     // from the start of the embedded code section.
-    // TODO(jgruber,v8:11036): Move this to the embedded metadata area.
     uint32_t metadata_offset;
     uint32_t metadata_length;
   };
-  STATIC_ASSERT(offsetof(Metadata, instruction_offset) == 0 * kUInt32Size);
-  STATIC_ASSERT(offsetof(Metadata, instruction_length) == 1 * kUInt32Size);
-  STATIC_ASSERT(offsetof(Metadata, metadata_offset) == 2 * kUInt32Size);
-  STATIC_ASSERT(offsetof(Metadata, metadata_length) == 3 * kUInt32Size);
-  STATIC_ASSERT(sizeof(Metadata) == 4 * kUInt32Size);
+  STATIC_ASSERT(offsetof(LayoutDescription, instruction_offset) ==
+                0 * kUInt32Size);
+  STATIC_ASSERT(offsetof(LayoutDescription, instruction_length) ==
+                1 * kUInt32Size);
+  STATIC_ASSERT(offsetof(LayoutDescription, metadata_offset) ==
+                2 * kUInt32Size);
+  STATIC_ASSERT(offsetof(LayoutDescription, metadata_length) ==
+                3 * kUInt32Size);
+  STATIC_ASSERT(sizeof(LayoutDescription) == 4 * kUInt32Size);
 
   // The layout of the blob is as follows:
   //
-  // metadata:
+  // data:
   // [0] hash of the remaining blob
   // [1] hash of embedded-blob-relevant heap objects
-  // [2] metadata of instruction stream 0
-  // ... metadata
+  // [2] layout description of instruction stream 0
+  // ... layout descriptions
   //
   // code:
-  // [0] instruction streams 0
+  // [0] instruction stream 0
   // ... instruction streams
 
   static constexpr uint32_t kTableSize = Builtins::builtin_count;
@@ -145,30 +141,27 @@ class EmbeddedData final {
     return EmbeddedBlobHashOffset() + EmbeddedBlobHashSize();
   }
   static constexpr uint32_t IsolateHashSize() { return kSizetSize; }
-  static constexpr uint32_t MetadataTableOffset() {
+  static constexpr uint32_t LayoutDescriptionTableOffset() {
     return IsolateHashOffset() + IsolateHashSize();
   }
-  static constexpr uint32_t MetadataTableSize() {
-    return sizeof(struct Metadata) * kTableSize;
+  static constexpr uint32_t LayoutDescriptionTableSize() {
+    return sizeof(struct LayoutDescription) * kTableSize;
   }
   static constexpr uint32_t RawCodeOffset() { return 0; }
 
  private:
-  EmbeddedData(const uint8_t* code, uint32_t code_size, const uint8_t* metadata,
-               uint32_t metadata_size)
-      : code_(code),
-        code_size_(code_size),
-        metadata_(metadata),
-        metadata_size_(metadata_size) {
+  EmbeddedData(const uint8_t* code, uint32_t code_size, const uint8_t* data,
+               uint32_t data_size)
+      : code_(code), code_size_(code_size), data_(data), data_size_(data_size) {
     DCHECK_NOT_NULL(code);
     DCHECK_LT(0, code_size);
-    DCHECK_NOT_NULL(metadata);
-    DCHECK_LT(0, metadata_size);
+    DCHECK_NOT_NULL(data);
+    DCHECK_LT(0, data_size);
   }
 
-  const Metadata* Metadata() const {
-    return reinterpret_cast<const struct Metadata*>(metadata_ +
-                                                    MetadataTableOffset());
+  const LayoutDescription* LayoutDescription() const {
+    return reinterpret_cast<const struct LayoutDescription*>(
+        data_ + LayoutDescriptionTableOffset());
   }
   const uint8_t* RawCode() const { return code_ + RawCodeOffset(); }
 
@@ -185,9 +178,11 @@ class EmbeddedData final {
   const uint8_t* code_;
   uint32_t code_size_;
 
-  // This is metadata for the code.
-  const uint8_t* metadata_;
-  uint32_t metadata_size_;
+  // The data section contains both descriptions of the code section (hashes,
+  // offsets, sizes) and metadata describing Code objects (see
+  // Code::MetadataStart()).
+  const uint8_t* data_;
+  uint32_t data_size_;
 };
 
 }  // namespace internal

src/snapshot/embedded/embedded-empty.cc

@@ -10,22 +10,22 @@
 
 extern "C" const uint8_t* v8_Default_embedded_blob_code_;
 extern "C" uint32_t v8_Default_embedded_blob_code_size_;
-extern "C" const uint8_t* v8_Default_embedded_blob_metadata_;
-extern "C" uint32_t v8_Default_embedded_blob_metadata_size_;
+extern "C" const uint8_t* v8_Default_embedded_blob_data_;
+extern "C" uint32_t v8_Default_embedded_blob_data_size_;
 
 const uint8_t* v8_Default_embedded_blob_code_ = nullptr;
 uint32_t v8_Default_embedded_blob_code_size_ = 0;
-const uint8_t* v8_Default_embedded_blob_metadata_ = nullptr;
-uint32_t v8_Default_embedded_blob_metadata_size_ = 0;
+const uint8_t* v8_Default_embedded_blob_data_ = nullptr;
+uint32_t v8_Default_embedded_blob_data_size_ = 0;
 
 #ifdef V8_MULTI_SNAPSHOTS
 extern "C" const uint8_t* v8_Trusted_embedded_blob_code_;
 extern "C" uint32_t v8_Trusted_embedded_blob_code_size_;
-extern "C" const uint8_t* v8_Trusted_embedded_blob_metadata_;
-extern "C" uint32_t v8_Trusted_embedded_blob_metadata_size_;
+extern "C" const uint8_t* v8_Trusted_embedded_blob_data_;
+extern "C" uint32_t v8_Trusted_embedded_blob_data_size_;
 
 const uint8_t* v8_Trusted_embedded_blob_code_ = nullptr;
 uint32_t v8_Trusted_embedded_blob_code_size_ = 0;
-const uint8_t* v8_Trusted_embedded_blob_metadata_ = nullptr;
-uint32_t v8_Trusted_embedded_blob_metadata_size_ = 0;
+const uint8_t* v8_Trusted_embedded_blob_data_ = nullptr;
+uint32_t v8_Trusted_embedded_blob_data_size_ = 0;
 #endif

src/snapshot/embedded/embedded-file-writer.cc

@@ -177,15 +177,14 @@ void EmbeddedFileWriter::WriteFileEpilogue(PlatformEmbeddedFileWriterBase* w,
                               EmbeddedBlobCodeDataSymbol().c_str());
     w->Newline();
 
-    i::EmbeddedVector<char, kTemporaryStringLength>
-        embedded_blob_metadata_symbol;
-    i::SNPrintF(embedded_blob_metadata_symbol, "v8_%s_embedded_blob_metadata_",
+    i::EmbeddedVector<char, kTemporaryStringLength> embedded_blob_data_symbol;
+    i::SNPrintF(embedded_blob_data_symbol, "v8_%s_embedded_blob_data_",
                 embedded_variant_);
 
-    w->Comment("Pointer to the beginning of the embedded blob metadata.");
+    w->Comment("Pointer to the beginning of the embedded blob data section.");
     w->AlignToDataAlignment();
-    w->DeclarePointerToSymbol(embedded_blob_metadata_symbol.begin(),
-                              EmbeddedBlobMetadataDataSymbol().c_str());
+    w->DeclarePointerToSymbol(embedded_blob_data_symbol.begin(),
+                              EmbeddedBlobDataDataSymbol().c_str());
     w->Newline();
   }
 
@@ -202,13 +201,12 @@ void EmbeddedFileWriter::WriteFileEpilogue(PlatformEmbeddedFileWriterBase* w,
     w->Newline();
 
     i::EmbeddedVector<char, kTemporaryStringLength>
-        embedded_blob_metadata_size_symbol;
-    i::SNPrintF(embedded_blob_metadata_size_symbol,
-                "v8_%s_embedded_blob_metadata_size_", embedded_variant_);
+        embedded_blob_data_size_symbol;
+    i::SNPrintF(embedded_blob_data_size_symbol,
+                "v8_%s_embedded_blob_data_size_", embedded_variant_);
 
-    w->Comment("The size of the embedded blob metadata in bytes.");
-    w->DeclareUint32(embedded_blob_metadata_size_symbol.begin(),
-                     blob->metadata_size());
+    w->Comment("The size of the embedded blob data section in bytes.");
+    w->DeclareUint32(embedded_blob_data_size_symbol.begin(), blob->data_size());
     w->Newline();
   }
 

src/snapshot/embedded/embedded-file-writer.h

@@ -109,7 +109,7 @@ class EmbeddedFileWriter : public EmbeddedFileWriterInterface {
 
     WriteFilePrologue(writer.get());
     WriteExternalFilenames(writer.get());
-    WriteMetadataSection(writer.get(), blob);
+    WriteDataSection(writer.get(), blob);
     WriteInstructionStreams(writer.get(), blob);
     WriteFileEpilogue(writer.get(), blob);
 
@@ -161,23 +161,22 @@ class EmbeddedFileWriter : public EmbeddedFileWriterInterface {
     return std::string{embedded_blob_code_data_symbol.begin()};
   }
 
-  std::string EmbeddedBlobMetadataDataSymbol() const {
+  std::string EmbeddedBlobDataDataSymbol() const {
     i::EmbeddedVector<char, kTemporaryStringLength>
-        embedded_blob_metadata_data_symbol;
-    i::SNPrintF(embedded_blob_metadata_data_symbol,
-                "v8_%s_embedded_blob_metadata_data_", embedded_variant_);
-    return std::string{embedded_blob_metadata_data_symbol.begin()};
+        embedded_blob_data_data_symbol;
+    i::SNPrintF(embedded_blob_data_data_symbol,
+                "v8_%s_embedded_blob_data_data_", embedded_variant_);
+    return std::string{embedded_blob_data_data_symbol.begin()};
   }
 
-  void WriteMetadataSection(PlatformEmbeddedFileWriterBase* w,
-                            const i::EmbeddedData* blob) const {
-    w->Comment("The embedded blob metadata starts here.");
+  void WriteDataSection(PlatformEmbeddedFileWriterBase* w,
+                        const i::EmbeddedData* blob) const {
+    w->Comment("The embedded blob data section starts here.");
     w->SectionRoData();
     w->AlignToDataAlignment();
-    w->DeclareLabel(EmbeddedBlobMetadataDataSymbol().c_str());
+    w->DeclareLabel(EmbeddedBlobDataDataSymbol().c_str());
 
-    WriteBinaryContentsAsInlineAssembly(w, blob->metadata(),
-                                        blob->metadata_size());
+    WriteBinaryContentsAsInlineAssembly(w, blob->data(), blob->data_size());
   }
 
   void WriteBuiltin(PlatformEmbeddedFileWriterBase* w,