[wasm] Parse DataCount section for bulk-memory

The bulk-memory proposal adds a new DataCount section that declares the
number of data segments that are expected to be seen in the Data
section. This is similar to the way the number of functions is split
between the Function and Code sections.

The DataCount section occurs before the Code section, so we can do
single-pass validation of the new `memory.init` and `memory.drop`
instructions, which have data segment indices as immediates.

Bug: v8:7747
Change-Id: Ibc5a7ee9336dbc5d0fd667572c42cb065c048e00
Reviewed-on: https://chromium-review.googlesource.com/c/1352792
Commit-Queue: Ben Smith <binji@chromium.org>
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57951}

src/wasm/module-decoder.cc

@@ -83,6 +83,8 @@ const char* SectionName(SectionCode code) {
       return "Data";
     case kExceptionSectionCode:
       return "Exception";
+    case kDataCountSectionCode:
+      return "DataCount";
     case kNameSectionCode:
       return kNameString;
     case kSourceMappingURLSectionCode:
@@ -331,6 +333,40 @@ class ModuleDecoderImpl : public Decoder {
 #undef BYTES
   }
 
+  bool CheckSectionOrder(SectionCode section_code,
+                         SectionCode prev_section_code,
+                         SectionCode next_section_code) {
+    if (next_ordered_section_ > next_section_code) {
+      errorf(pc(), "The %s section must appear before the %s section",
+             SectionName(section_code), SectionName(next_section_code));
+      return false;
+    }
+    if (next_ordered_section_ <= prev_section_code) {
+      next_ordered_section_ = prev_section_code + 1;
+    }
+    return true;
+  }
+
+  bool CheckUnorderedSection(SectionCode section_code) {
+    if (has_seen_unordered_section(section_code)) {
+      errorf(pc(), "Multiple %s sections not allowed",
+             SectionName(section_code));
+      return false;
+    }
+    set_seen_unordered_section(section_code);
+
+    switch (section_code) {
+      case kDataCountSectionCode:
+        return CheckSectionOrder(section_code, kElementSectionCode,
+                                 kCodeSectionCode);
+      case kExceptionSectionCode:
+        return CheckSectionOrder(section_code, kImportSectionCode,
+                                 kExportSectionCode);
+      default:
+        UNREACHABLE();
+    }
+  }
+
   void DecodeSection(SectionCode section_code, Vector<const uint8_t> bytes,
                      uint32_t offset, bool verify_functions = true) {
     if (failed()) return;
@@ -349,20 +385,12 @@ class ModuleDecoderImpl : public Decoder {
     switch (section_code) {
       case kUnknownSectionCode:
         break;
+      case kDataCountSectionCode:
       case kExceptionSectionCode:
-        // Note: kExceptionSectionCode > kExportSectionCode, but must appear
-        // before the export (and code) section, as well as after the import
-        // section. Hence, treat it as a special case.
-        if (seen_unordered_sections_ & (1 << kExceptionSectionCode)) {
-          errorf(pc(), "Multiple exception sections not allowed");
-          return;
-        } else if (next_ordered_section_ > kExportSectionCode) {
-          errorf(pc(), "Exception section must appear before export section");
-          return;
-        } else if (next_ordered_section_ <= kImportSectionCode) {
-          next_ordered_section_ = kImportSectionCode + 1;
-        }
-        seen_unordered_sections_ |= 1 << kExceptionSectionCode;
+        // Note: These sections have a section code that is numerically
+        // out-of-order with respect to their required location. So they are
+        // treated as a special case.
+        if (!CheckUnorderedSection(section_code)) return;
         break;
       case kSourceMappingURLSectionCode:
         // sourceMappingURL is a custom section and currently can occur anywhere
@@ -420,6 +448,13 @@ class ModuleDecoderImpl : public Decoder {
       case kSourceMappingURLSectionCode:
         DecodeSourceMappingURLSection();
         break;
+      case kDataCountSectionCode:
+        if (enabled_features_.bulk_memory) {
+          DecodeDataCountSection();
+        } else {
+          errorf(pc(), "unexpected section: %s", SectionName(section_code));
+        }
+        break;
       case kExceptionSectionCode:
         if (enabled_features_.eh) {
           DecodeExceptionSection();
@@ -833,9 +868,21 @@ class ModuleDecoderImpl : public Decoder {
     }
   }
 
+  bool CheckDataSegmentsCount(uint32_t data_segments_count) {
+    if (has_seen_unordered_section(kDataCountSectionCode) &&
+        data_segments_count != num_declared_data_segments_) {
+      errorf(pc(), "data segments count %u mismatch (%u expected)",
+             data_segments_count, num_declared_data_segments_);
+      return false;
+    }
+    return true;
+  }
+
   void DecodeDataSection() {
     uint32_t data_segments_count =
         consume_count("data segments count", kV8MaxWasmDataSegments);
+    if (!CheckDataSegmentsCount(data_segments_count)) return;
+
     module_->data_segments.reserve(data_segments_count);
     for (uint32_t i = 0; ok() && i < data_segments_count; ++i) {
       const byte* pos = pc();
@@ -879,8 +926,8 @@ class ModuleDecoderImpl : public Decoder {
   void DecodeNameSection() {
     // TODO(titzer): find a way to report name errors as warnings.
     // ignore all but the first occurrence of name section.
-    if (!(seen_unordered_sections_ & (1 << kNameSectionCode))) {
-      seen_unordered_sections_ |= 1 << kNameSectionCode;
+    if (!has_seen_unordered_section(kNameSectionCode)) {
+      set_seen_unordered_section(kNameSectionCode);
       // Use an inner decoder so that errors don't fail the outer decoder.
       Decoder inner(start_, pc_, end_, buffer_offset_);
       // Decode all name subsections.
@@ -910,16 +957,21 @@ class ModuleDecoderImpl : public Decoder {
     Decoder inner(start_, pc_, end_, buffer_offset_);
     WireBytesRef url = wasm::consume_string(inner, true, "module name");
     if (inner.ok() &&
-        !(seen_unordered_sections_ & (1 << kSourceMappingURLSectionCode))) {
+        !has_seen_unordered_section(kSourceMappingURLSectionCode)) {
       const byte* url_start =
           inner.start() + inner.GetBufferRelativeOffset(url.offset());
       module_->source_map_url.assign(reinterpret_cast<const char*>(url_start),
                                      url.length());
-      seen_unordered_sections_ |= 1 << kSourceMappingURLSectionCode;
+      set_seen_unordered_section(kSourceMappingURLSectionCode);
     }
     consume_bytes(static_cast<uint32_t>(end_ - start_), nullptr);
   }
 
+  void DecodeDataCountSection() {
+    num_declared_data_segments_ =
+        consume_count("data segments count", kV8MaxWasmDataSegments);
+  }
+
   void DecodeExceptionSection() {
     uint32_t exception_count =
         consume_count("exception count", kV8MaxWasmExceptions);
@@ -933,25 +985,32 @@ class ModuleDecoderImpl : public Decoder {
     }
   }
 
-  ModuleResult FinishDecoding(bool verify_functions = true) {
-    if (ok()) {
-      // The declared vs. defined function count is normally checked when
-      // decoding the code section, but we have to check it here too in case the
-      // code section is absent.
-      if (module_->num_declared_functions != 0) {
-        DCHECK_LT(module_->num_imported_functions, module_->functions.size());
-        // We know that the code section has been decoded if the first
-        // non-imported function has its code set.
-        if (!module_->functions[module_->num_imported_functions]
-                 .code.is_set()) {
-          errorf(pc(), "function count is %u, but code section is absent",
-                 module_->num_declared_functions);
-        }
+  bool CheckMismatchedCounts() {
+    // The declared vs. defined function count is normally checked when
+    // decoding the code section, but we have to check it here too in case the
+    // code section is absent.
+    if (module_->num_declared_functions != 0) {
+      DCHECK_LT(module_->num_imported_functions, module_->functions.size());
+      // We know that the code section has been decoded if the first
+      // non-imported function has its code set.
+      if (!module_->functions[module_->num_imported_functions].code.is_set()) {
+        errorf(pc(), "function count is %u, but code section is absent",
+               module_->num_declared_functions);
+        return false;
       }
+    }
+    // Perform a similar check for the DataCount and Data sections, where data
+    // segments are declared but the Data section is absent.
+    if (!CheckDataSegmentsCount(
+            static_cast<uint32_t>(module_->data_segments.size()))) {
+      return false;
+    }
+    return true;
+  }
 
-      if (ok()) {
-        CalculateGlobalOffsets(module_.get());
-      }
+  ModuleResult FinishDecoding(bool verify_functions = true) {
+    if (ok() && CheckMismatchedCounts()) {
+      CalculateGlobalOffsets(module_.get());
     }
     ModuleResult result = toResult(std::move(module_));
     if (verify_functions && result.ok() && intermediate_result_.failed()) {
@@ -1061,6 +1120,16 @@ class ModuleDecoderImpl : public Decoder {
                 "not enough bits");
   VoidResult intermediate_result_;
   ModuleOrigin origin_;
+  // The number of data segments as specified by the DataCount section.
+  uint32_t num_declared_data_segments_ = 0;
+
+  bool has_seen_unordered_section(SectionCode section_code) {
+    return seen_unordered_sections_ & (1 << section_code);
+  }
+
+  void set_seen_unordered_section(SectionCode section_code) {
+    seen_unordered_sections_ |= 1 << section_code;
+  }
 
   uint32_t off(const byte* ptr) {
     return static_cast<uint32_t>(ptr - start_) + buffer_offset_;

src/wasm/wasm-constants.h

@@ -71,14 +71,19 @@ enum SectionCode : int8_t {
   kElementSectionCode = 9,     // Elements section
   kCodeSectionCode = 10,       // Function code
   kDataSectionCode = 11,       // Data segments
-  kExceptionSectionCode = 12,  // Exception (aka. event) section
-  kNameSectionCode = 13,       // Name section (encoded as a string)
-  kSourceMappingURLSectionCode = 14,  // Source Map URL section
+  kExceptionSectionCode = 12,  // Exception section
+  kDataCountSectionCode = 13,  // Number of data segments
+
+  // The following sections are custom sections, and are identified using a
+  // string rather than an integer. Their enumeration values are not guaranteed
+  // to be consistent.
+  kNameSectionCode,              // Name section (encoded as a string)
+  kSourceMappingURLSectionCode,  // Source Map URL section
 
   // Helper values
   kFirstSectionInModule = kTypeSectionCode,
   kLastKnownModuleSection = kSourceMappingURLSectionCode,
-  kFirstUnorderedSection = kNameSectionCode,
+  kFirstUnorderedSection = kExceptionSectionCode,
 };
 
 // Binary encoding of name section kinds.

test/unittests/wasm/module-decoder-unittest.cc

@@ -572,7 +572,8 @@ TEST_F(WasmModuleVerifyTest, ExceptionSectionAfterExport) {
 
   WASM_FEATURE_SCOPE(eh);
   ModuleResult result = DecodeModule(data, data + sizeof(data));
-  EXPECT_NOT_OK(result, "Exception section must appear before export section");
+  EXPECT_NOT_OK(result,
+                "The Exception section must appear before the Export section");
 }
 
 TEST_F(WasmModuleVerifyTest, ExceptionSectionBeforeImport) {
@@ -2270,6 +2271,99 @@ TEST_F(WasmModuleVerifyTest, PassiveElementSegment) {
   EXPECT_OFF_END_FAILURE(data, arraysize(data) - 5);
 }
 
+TEST_F(WasmModuleVerifyTest, DataCountSectionCorrectPlacement) {
+  static const byte data[] = {SECTION(Element, ENTRY_COUNT(0)),
+                              SECTION(DataCount, ENTRY_COUNT(0)),
+                              SECTION(Code, ENTRY_COUNT(0))};
+  EXPECT_FAILURE(data);
+  WASM_FEATURE_SCOPE(bulk_memory);
+  EXPECT_VERIFIES(data);
+}
+
+TEST_F(WasmModuleVerifyTest, DataCountSectionAfterCode) {
+  static const byte data[] = {SECTION(Code, ENTRY_COUNT(0)),
+                              SECTION(DataCount, ENTRY_COUNT(0))};
+  WASM_FEATURE_SCOPE(bulk_memory);
+  ModuleResult result = DecodeModule(data, data + sizeof(data));
+  EXPECT_NOT_OK(result,
+                "The DataCount section must appear before the Code section");
+}
+
+TEST_F(WasmModuleVerifyTest, DataCountSectionBeforeElement) {
+  static const byte data[] = {SECTION(DataCount, ENTRY_COUNT(0)),
+                              SECTION(Element, ENTRY_COUNT(0))};
+  WASM_FEATURE_SCOPE(bulk_memory);
+  ModuleResult result = DecodeModule(data, data + sizeof(data));
+  EXPECT_NOT_OK(result, "unexpected section: Element");
+}
+
+TEST_F(WasmModuleVerifyTest, DataCountSectionAfterStartBeforeElement) {
+  STATIC_ASSERT(kStartSectionCode + 1 == kElementSectionCode);
+  static const byte data[] = {
+      // We need the start section for this test, but the start section must
+      // reference a valid function, which requires the type and function
+      // sections too.
+      SIGNATURES_SECTION(1, SIG_ENTRY_v_v),  // Type section.
+      FUNCTION_SIGNATURES_SECTION(1, 0),     // Function section.
+
+      SECTION(Start, U32V_1(0)),           // Start section.
+      SECTION(DataCount, ENTRY_COUNT(0)),  // DataCount section.
+      SECTION(Element, ENTRY_COUNT(0))     // Element section.
+  };
+  WASM_FEATURE_SCOPE(bulk_memory);
+  ModuleResult result = DecodeModule(data, data + sizeof(data));
+  EXPECT_NOT_OK(result, "unexpected section: Element");
+}
+
+TEST_F(WasmModuleVerifyTest, MultipleDataCountSections) {
+  static const byte data[] = {SECTION(DataCount, ENTRY_COUNT(0)),
+                              SECTION(DataCount, ENTRY_COUNT(0))};
+  WASM_FEATURE_SCOPE(bulk_memory);
+  ModuleResult result = DecodeModule(data, data + sizeof(data));
+  EXPECT_NOT_OK(result, "Multiple DataCount sections not allowed");
+}
+
+TEST_F(WasmModuleVerifyTest, DataCountSegmentCountMatch) {
+  static const byte data[] = {
+      SECTION(Memory, ENTRY_COUNT(1), 0, 1),  // Memory section.
+      SECTION(DataCount, ENTRY_COUNT(1)),     // DataCount section.
+      SECTION(Data, ENTRY_COUNT(1), LINEAR_MEMORY_INDEX_0,  // Data section.
+              WASM_INIT_EXPR_I32V_1(12), ADD_COUNT('h', 'i'))};
+
+  EXPECT_FAILURE(data);
+  WASM_FEATURE_SCOPE(bulk_memory);
+  EXPECT_VERIFIES(data);
+}
+
+TEST_F(WasmModuleVerifyTest, DataCountSegmentCount_greater) {
+  static const byte data[] = {
+      SECTION(Memory, ENTRY_COUNT(1), 0, 1),  // Memory section.
+      SECTION(DataCount, ENTRY_COUNT(3)),     // DataCount section.
+      SECTION(Data, ENTRY_COUNT(0))};         // Data section.
+  WASM_FEATURE_SCOPE(bulk_memory);
+  ModuleResult result = DecodeModule(data, data + sizeof(data));
+  EXPECT_NOT_OK(result, "data segments count 0 mismatch (3 expected)");
+}
+
+TEST_F(WasmModuleVerifyTest, DataCountSegmentCount_less) {
+  static const byte data[] = {
+      SECTION(Memory, ENTRY_COUNT(1), 0, 1),  // Memory section.
+      SECTION(DataCount, ENTRY_COUNT(0)),     // DataCount section.
+      SECTION(Data, ENTRY_COUNT(1), LINEAR_MEMORY_INDEX_0,  // Data section.
+              WASM_INIT_EXPR_I32V_1(12), ADD_COUNT('a', 'b', 'c'))};
+  WASM_FEATURE_SCOPE(bulk_memory);
+  ModuleResult result = DecodeModule(data, data + sizeof(data));
+  EXPECT_NOT_OK(result, "data segments count 1 mismatch (0 expected)");
+}
+
+TEST_F(WasmModuleVerifyTest, DataCountSegmentCount_omitted) {
+  static const byte data[] = {SECTION(Memory, ENTRY_COUNT(1), 0, 1),
+                              SECTION(DataCount, ENTRY_COUNT(1))};
+  WASM_FEATURE_SCOPE(bulk_memory);
+  ModuleResult result = DecodeModule(data, data + sizeof(data));
+  EXPECT_NOT_OK(result, "data segments count 0 mismatch (1 expected)");
+}
+
 #undef WASM_FEATURE_SCOPE
 #undef WASM_FEATURE_SCOPE_VAL
 #undef EXPECT_INIT_EXPR