[wasm] Errors in names section do not fail the whole module.

R=ahaas@chromium.org
BUG=v8:5882

Review-Url: https://codereview.chromium.org/2657463003
Cr-Commit-Position: refs/heads/master@{#42629}

src/wasm/decoder.h

@@ -37,6 +37,12 @@ class Decoder {
         end_(end),
         error_pc_(nullptr),
         error_pt_(nullptr) {}
+  Decoder(const byte* start, const byte* pc, const byte* end)
+      : start_(start),
+        pc_(pc),
+        end_(end),
+        error_pc_(nullptr),
+        error_pt_(nullptr) {}
 
   virtual ~Decoder() {}
 
@@ -184,8 +190,13 @@ class Decoder {
 
   // Consume {size} bytes and send them to the bit bucket, advancing {pc_}.
   void consume_bytes(uint32_t size, const char* name = "skip") {
-    TRACE("  +%d  %-20s: %d bytes\n", static_cast<int>(pc_ - start_), name,
-          size);
+#if DEBUG
+    if (name) {
+      // Only trace if the name is not null.
+      TRACE("  +%d  %-20s: %d bytes\n", static_cast<int>(pc_ - start_), name,
+            size);
+    }
+#endif
     if (checkAvailable(size)) {
       pc_ += size;
     } else {

src/wasm/module-decoder.cc

@@ -610,22 +610,29 @@ class ModuleDecoder : public Decoder {
 
     // ===== Name section ====================================================
     if (section_iter.section_code() == kNameSectionCode) {
-      uint32_t functions_count = consume_u32v("functions count");
+      // TODO(titzer): find a way to report name errors as warnings.
+      // Use an inner decoder so that errors don't fail the outer decoder.
+      Decoder inner(start_, pc_, end_);
+      uint32_t functions_count = inner.consume_u32v("functions count");
 
-      for (uint32_t i = 0; ok() && i < functions_count; ++i) {
+      for (uint32_t i = 0; inner.ok() && i < functions_count; ++i) {
         uint32_t function_name_length = 0;
-        uint32_t name_offset = consume_string(&function_name_length, false);
+        uint32_t name_offset =
+            consume_string(inner, &function_name_length, false);
         uint32_t func_index = i;
         if (func_index < module->functions.size()) {
           module->functions[func_index].name_offset = name_offset;
           module->functions[func_index].name_length = function_name_length;
         }
 
-        uint32_t local_names_count = consume_u32v("local names count");
+        uint32_t local_names_count = inner.consume_u32v("local names count");
         for (uint32_t j = 0; ok() && j < local_names_count; j++) {
-          skip_string();
+          uint32_t length = inner.consume_u32v("string length");
+          inner.consume_bytes(length, "string");
         }
       }
+      // Skip the whole names section in the outer decoder.
+      consume_bytes(section_iter.payload_length(), nullptr);
       section_iter.advance();
     }
 
@@ -811,27 +818,26 @@ class ModuleDecoder : public Decoder {
     }
   }
 
+  uint32_t consume_string(uint32_t* length, bool validate_utf8) {
+    return consume_string(*this, length, validate_utf8);
+  }
+
   // Reads a length-prefixed string, checking that it is within bounds. Returns
   // the offset of the string, and the length as an out parameter.
-  uint32_t consume_string(uint32_t* length, bool validate_utf8) {
-    *length = consume_u32v("string length");
-    uint32_t offset = pc_offset();
-    const byte* string_start = pc_;
+  uint32_t consume_string(Decoder& decoder, uint32_t* length,
+                          bool validate_utf8) {
+    *length = decoder.consume_u32v("string length");
+    uint32_t offset = decoder.pc_offset();
+    const byte* string_start = decoder.pc();
     // Consume bytes before validation to guarantee that the string is not oob.
-    if (*length > 0) consume_bytes(*length, "string");
-    if (ok() && validate_utf8 &&
+    if (*length > 0) decoder.consume_bytes(*length, "string");
+    if (decoder.ok() && validate_utf8 &&
         !unibrow::Utf8::Validate(string_start, *length)) {
-      error(string_start, "no valid UTF-8 string");
+      decoder.error(string_start, "no valid UTF-8 string");
     }
     return offset;
   }
 
-  // Skips over a length-prefixed string, but checks that it is within bounds.
-  void skip_string() {
-    uint32_t length = consume_u32v("string length");
-    consume_bytes(length, "string");
-  }
-
   uint32_t consume_sig_index(WasmModule* module, FunctionSig** sig) {
     const byte* pos = pc_;
     uint32_t sig_index = consume_u32v("signature index");

test/mjsunit/wasm/names.js

+// Copyright 2017 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: --expose-wasm
+
+load('test/mjsunit/wasm/wasm-constants.js');
+load('test/mjsunit/wasm/wasm-module-builder.js');
+
+function toBytes(string) {
+  var a = new Array(string.length + 1);
+  a[0] = string.length;
+  for (i = 0; i < string.length; i++) {
+    a[i + 1] = string.charCodeAt(i);
+  }
+  return a;
+}
+
+(function TestEmptyNamesSection() {
+  print('TestEmptyNamesSection...');
+  var builder = new WasmModuleBuilder();
+
+  builder.addExplicitSection([kUnknownSectionCode, 6, ...toBytes('name'), 0]);
+
+  var buffer = builder.toBuffer();
+  assertTrue(WebAssembly.validate(buffer));
+  assertTrue((new WebAssembly.Module(buffer)) instanceof WebAssembly.Module);
+})();
+
+(function TestTruncatedNamesSection() {
+  print('TestTruncatedNamesSection...');
+  var builder = new WasmModuleBuilder();
+
+  builder.addExplicitSection([kUnknownSectionCode, 6, ...toBytes('name'), 1]);
+
+  var buffer = builder.toBuffer();
+  assertTrue(WebAssembly.validate(buffer));
+  assertTrue((new WebAssembly.Module(buffer)) instanceof WebAssembly.Module);
+})();
+
+(function TestBrokenNamesSection() {
+  print('TestBrokenNamesSection...');
+  var builder = new WasmModuleBuilder();
+
+  builder.addExplicitSection(
+      [kUnknownSectionCode, 7, ...toBytes('name'), 1, 100]);
+
+  var buffer = builder.toBuffer();
+  assertTrue(WebAssembly.validate(buffer));
+  assertTrue((new WebAssembly.Module(buffer)) instanceof WebAssembly.Module);
+})();