[wasm] Fix streaming decoding position reporting.

This fixes the streaming decoder to report the correct error position
for repeating code sections (i.e. only one code section per module).

R=clemensb@chromium.org

Change-Id: Ie02d704d74b4e051fa9b00288dd6d1e46e2418a5
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1890094
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#64654}

src/wasm/streaming-decoder.cc

@@ -370,6 +370,16 @@ std::unique_ptr<StreamingDecoder::DecodingState>
 StreamingDecoder::DecodeSectionID::Next(StreamingDecoder* streaming) {
   TRACE_STREAMING("DecodeSectionID: %s section\n",
                   SectionName(static_cast<SectionCode>(id_)));
+  if (id_ == SectionCode::kCodeSectionCode) {
+    // Explicitly check for multiple code sections as module decoder never
+    // sees the code section and hence cannot track this section.
+    if (streaming->code_section_processed_) {
+      // TODO(mstarzinger): This error message (and others in this class) is
+      // different for non-streaming decoding. Bring them in sync and test.
+      return streaming->Error("code section can only appear once");
+    }
+    streaming->code_section_processed_ = true;
+  }
   return std::make_unique<DecodeSectionLength>(id_, module_offset_);
 }
 
@@ -391,22 +401,13 @@ StreamingDecoder::DecodeSectionLength::NextWithValue(
     if (!streaming->ok()) return nullptr;
     // There is no payload, we go to the next section immediately.
     return std::make_unique<DecodeSectionID>(streaming->module_offset_);
-  } else {
-    if (section_id_ == SectionCode::kCodeSectionCode) {
-      // Explicitly check for multiple code sections as module decoder never
-      // sees the code section and hence cannot track this section.
-      if (streaming->code_section_processed_) {
-        // TODO(mstarzinger): This error message (and other in this class) is
-        // different for non-streaming decoding. Bring them in sync and test.
-        return streaming->Error("code section can only appear once");
-      }
-      streaming->code_section_processed_ = true;
-      // We reached the code section. All functions of the code section are put
-      // into the same SectionBuffer.
-      return std::make_unique<DecodeNumberOfFunctions>(buf);
-    }
-    return std::make_unique<DecodeSectionPayload>(buf);
   }
+  if (section_id_ == SectionCode::kCodeSectionCode) {
+    // We reached the code section. All functions of the code section are put
+    // into the same SectionBuffer.
+    return std::make_unique<DecodeNumberOfFunctions>(buf);
+  }
+  return std::make_unique<DecodeSectionPayload>(buf);
 }
 
 std::unique_ptr<StreamingDecoder::DecodingState>

test/mjsunit/wasm/streaming-error-position.js

@@ -8,18 +8,6 @@
 
 load('test/mjsunit/wasm/wasm-module-builder.js');
 
-function module(bytes) {
-  let buffer = bytes;
-  if (typeof buffer === 'string') {
-    buffer = new ArrayBuffer(bytes.length);
-    let view = new Uint8Array(buffer);
-    for (let i = 0; i < bytes.length; ++i) {
-      view[i] = bytes.charCodeAt(i);
-    }
-  }
-  return new WebAssembly.Module(buffer);
-}
-
 function testErrorPosition(bytes, pos, test_name) {
   assertThrowsAsync(
       WebAssembly.compile(bytes.trunc_buffer()), WebAssembly.CompileError,
@@ -299,7 +287,7 @@ function testErrorPosition(bytes, pos, test_name) {
   ]);
   bytes.emit_bytes([
       kCodeSectionCode,  // section id
-      6,                 // section length (too big)
+      6,                 // section length
       1,                 // functions count
       4,                 // body size
       0,                 // locals count
@@ -312,6 +300,45 @@ function testErrorPosition(bytes, pos, test_name) {
   testErrorPosition(bytes, pos, 'testInvalidCode');
 })();
 
+(function testCodeSectionRepeats() {
+  let bytes = new Binary;
+  bytes.emit_header();
+  bytes.emit_bytes([
+      kTypeSectionCode,       // section id
+      4,                      // section length
+      1,                      // number of types
+      kWasmFunctionTypeForm,  // type
+      0,                      // number of parameter
+      0                       // number of returns
+  ]);
+  bytes.emit_bytes([
+      kFunctionSectionCode,  // section id
+      2,                     // section length
+      1,                     // number of functions
+      0                      // signature index
+  ]);
+  bytes.emit_bytes([
+      kCodeSectionCode,  // section id
+      4,                 // section length
+      1,                 // functions count
+      2,                 // body size
+      0,                 // locals count
+      kExprEnd           // body
+  ]);
+  let pos = bytes.length;
+  bytes.emit_bytes([
+      kCodeSectionCode,  // section id (repeating)
+      4,                 // section length
+      1,                 // functions count
+      2,                 // body size
+      0,                 // locals count
+      kExprEnd           // body
+  ]);
+
+  // Find error at the second kCodeSectionCode.
+  testErrorPosition(bytes, pos, 'testCodeSectionRepeats');
+})();
+
 (function testCodeSectionSizeZero() {
   let bytes = new Binary;
   bytes.emit_header();
@@ -331,10 +358,10 @@ function testErrorPosition(bytes, pos, test_name) {
   ]);
   bytes.emit_bytes([
       kCodeSectionCode,  // section id
-      0,                 // section length (too big)
+      0,                 // section length (empty)
   ]);
 
-  // Find error at the index of kExprLocalGet.
+  // Find error at the code section length.
   let pos = bytes.length - 1;
   testErrorPosition(bytes, pos, 'testCodeSectionSizeZero');
 })();

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

@@ -608,14 +608,13 @@ TEST_F(WasmStreamingDecoderTest, TwoCodeSections) {
       0x1,                   // Number of Functions
       0x1,                   // Function Length
       0x0,                   // Function
-      kCodeSectionCode,      // Section ID      -- ERROR (where it should be)
-      0x3,                   // Section Length  -- ERROR (where it is reported)
+      kCodeSectionCode,      // Section ID      -- ERROR
+      0x3,                   // Section Length
       0x1,                   // Number of Functions
       0x1,                   // Function Length
       0x0,                   // Function
   };
-  // TODO(wasm): This should report at the second kCodeSectionCode.
-  ExpectFailure(ArrayVector(data), sizeof(data) - 4,
+  ExpectFailure(ArrayVector(data), sizeof(data) - 5,
                 "code section can only appear once");
 }
 
@@ -651,14 +650,13 @@ TEST_F(WasmStreamingDecoderTest, UnknownSectionSandwich) {
       0x1,                   // Name Length
       0x1,                   // Name
       0x0,                   // Content
-      kCodeSectionCode,      // Section ID     -- ERROR (where it should be)
-      0x3,                   // Section Length -- ERROR (where it is reported)
+      kCodeSectionCode,      // Section ID     -- ERROR
+      0x3,                   // Section Length
       0x1,                   // Number of Functions
       0x1,                   // Function Length
       0x0,                   // Function
   };
-  // TODO(wasm): This should report at the second kCodeSectionCode.
-  ExpectFailure(ArrayVector(data), sizeof(data) - 4,
+  ExpectFailure(ArrayVector(data), sizeof(data) - 5,
                 "code section can only appear once");
 }