[wasm] Include prefix byte in length of bytes read

read_prefixed_byte is used mostly to read an entire prefixed opcode, it
writes the number of bytes of the opcode index (without prefix byte) to
the out param length. Change it so it writes the total number of bytes
(including the prefix byte), as that is what most callers want (they add
1 after calling read_prefixed_byte).

Bug: v8:10810
Change-Id: I914190ecae62e3547652accdc05d1cef3686fff4
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2476678Reviewed-by: Andreas Haas <ahaas@chromium.org>
Commit-Queue: Zhi An Ng <zhin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70551}

src/wasm/decoder.h

@@ -138,8 +138,8 @@ class Decoder {
   }
 
   // Reads a prefixed-opcode, possibly with variable-length index.
-  // The length param is set to the number of bytes this index is encoded with.
-  // For most cases (non variable-length), it will be 1.
+  // `length` is set to the number of bytes that make up this opcode,
+  // *including* the prefix byte. For most opcodes, it will be 2.
   template <ValidateFlag validate>
   WasmOpcode read_prefixed_opcode(const byte* pc, uint32_t* length,
                                   const char* name = "prefixed opcode") {
@@ -147,6 +147,7 @@ class Decoder {
 
     // Prefixed opcodes all use LEB128 encoding.
     index = read_u32v<validate>(pc + 1, length, "prefixed opcode index");
+    *length += 1;  // Prefix byte.
     // Only support opcodes that go up to 0xFF (when decoded). Anything
     // bigger will need 1 more byte, and the '<< 8' below will be wrong.
     if (validate && V8_UNLIKELY(index > 0xff)) {

src/wasm/function-body-decoder-impl.h

@@ -1671,7 +1671,6 @@ class WasmDecoder : public Decoder {
       case kNumericPrefix: {
         uint32_t length = 0;
         opcode = decoder->read_prefixed_opcode<validate>(pc, &length);
-        length++;  // Prefix byte.
         switch (opcode) {
           case kExprI32SConvertSatF32:
           case kExprI32UConvertSatF32:
@@ -1728,48 +1727,47 @@ class WasmDecoder : public Decoder {
 #define DECLARE_OPCODE_CASE(name, opcode, sig) case kExpr##name:
           FOREACH_SIMD_0_OPERAND_OPCODE(DECLARE_OPCODE_CASE)
 #undef DECLARE_OPCODE_CASE
-          return 1 + length;
+          return length;
 #define DECLARE_OPCODE_CASE(name, opcode, sig) case kExpr##name:
           FOREACH_SIMD_1_OPERAND_OPCODE(DECLARE_OPCODE_CASE)
 #undef DECLARE_OPCODE_CASE
-          return 2 + length;
+          return length + 1;
 #define DECLARE_OPCODE_CASE(name, opcode, sig) case kExpr##name:
           FOREACH_SIMD_MEM_OPCODE(DECLARE_OPCODE_CASE)
 #undef DECLARE_OPCODE_CASE
           {
-            MemoryAccessImmediate<validate> imm(decoder, pc + length + 1,
+            MemoryAccessImmediate<validate> imm(decoder, pc + length,
                                                 UINT32_MAX);
-            return 1 + length + imm.length;
+            return length + imm.length;
           }
           case kExprS128LoadMem32Zero:
           case kExprS128LoadMem64Zero: {
-            MemoryAccessImmediate<validate> imm(decoder, pc + length + 1,
+            MemoryAccessImmediate<validate> imm(decoder, pc + length,
                                                 UINT32_MAX);
-            return 1 + length + imm.length;
+            return length + imm.length;
           }
           case kExprS128Load8Lane:
           case kExprS128Load16Lane:
           case kExprS128Load32Lane:
           case kExprS128Load64Lane: {
-            MemoryAccessImmediate<validate> imm(decoder, pc + length + 1,
+            MemoryAccessImmediate<validate> imm(decoder, pc + length,
                                                 UINT32_MAX);
             // 1 more byte for lane index immediate.
-            return 1 + length + imm.length + 1;
+            return length + imm.length + 1;
           }
           // Shuffles require a byte per lane, or 16 immediate bytes.
           case kExprS128Const:
           case kExprI8x16Shuffle:
-            return 1 + length + kSimd128Size;
+            return length + kSimd128Size;
           default:
             decoder->DecodeError(pc, "invalid SIMD opcode");
-            return 1 + length;
+            return length;
         }
       }
       case kAtomicPrefix: {
         uint32_t length = 0;
         opcode = decoder->read_prefixed_opcode<validate>(pc, &length,
                                                          "atomic_index");
-        length++;  // Prefix byte.
         switch (opcode) {
 #define DECLARE_OPCODE_CASE(name, opcode, sig) case kExpr##name:
           FOREACH_ATOMIC_OPCODE(DECLARE_OPCODE_CASE)
@@ -1794,7 +1792,6 @@ class WasmDecoder : public Decoder {
         uint32_t length = 0;
         opcode =
             decoder->read_prefixed_opcode<validate>(pc, &length, "gc_index");
-        length++;  // Prefix byte.
         switch (opcode) {
           case kExprStructNewWithRtt:
           case kExprStructNewDefault: {
@@ -3033,8 +3030,7 @@ class WasmFullDecoder : public WasmDecoder<validate> {
       CHECK_PROTOTYPE_OPCODE(bulk_memory);
     }
     trace_msg->AppendOpcode(full_opcode);
-    // Prefix byte (1) + leb opcode bytes.
-    return DecodeNumericOpcode(full_opcode, 1 + opcode_length);
+    return DecodeNumericOpcode(full_opcode, opcode_length);
   }
 
   DECODE(Simd) {
@@ -3044,8 +3040,7 @@ class WasmFullDecoder : public WasmDecoder<validate> {
         this->pc_, &opcode_length);
     if (!VALIDATE(this->ok())) return 0;
     trace_msg->AppendOpcode(full_opcode);
-    // Prefix byte (1) + leb opcode bytes.
-    return DecodeSimdOpcode(full_opcode, 1 + opcode_length);
+    return DecodeSimdOpcode(full_opcode, opcode_length);
   }
 
   DECODE(Atomic) {
@@ -3054,8 +3049,7 @@ class WasmFullDecoder : public WasmDecoder<validate> {
     WasmOpcode full_opcode = this->template read_prefixed_opcode<validate>(
         this->pc_, &opcode_length, "atomic index");
     trace_msg->AppendOpcode(full_opcode);
-    // Prefix byte (1) + leb opcode bytes.
-    return DecodeAtomicOpcode(full_opcode, 1 + opcode_length);
+    return DecodeAtomicOpcode(full_opcode, opcode_length);
   }
 
   DECODE(GC) {
@@ -3064,8 +3058,7 @@ class WasmFullDecoder : public WasmDecoder<validate> {
     WasmOpcode full_opcode = this->template read_prefixed_opcode<validate>(
         this->pc_, &opcode_length, "gc index");
     trace_msg->AppendOpcode(full_opcode);
-    // Prefix byte (1) + leb opcode bytes.
-    return DecodeGCOpcode(full_opcode, 1 + opcode_length);
+    return DecodeGCOpcode(full_opcode, opcode_length);
   }
 
 #define SIMPLE_PROTOTYPE_CASE(name, opc, sig) \

src/wasm/module-decoder.cc

@@ -1745,8 +1745,8 @@ class ModuleDecoderImpl : public Decoder {
             return {};
           }
 
-          Simd128Immediate<validate> imm(this, pc() + len + 1);
-          len += 1 + kSimd128Size;
+          Simd128Immediate<validate> imm(this, pc() + len);
+          len += kSimd128Size;
           stack.emplace_back(imm.value);
           break;
         }
@@ -1759,8 +1759,8 @@ class ModuleDecoderImpl : public Decoder {
             case kExprRttCanon: {
               HeapTypeImmediate<validate> imm(enabled_features_, this,
                                               pc() + 2);
-              len += 1 + imm.length;
-              if (!Validate(pc() + 2, imm)) return {};
+              len += imm.length;
+              if (!Validate(pc() + len, imm)) return {};
               stack.push_back(
                   WasmInitExpr::RttCanon(imm.type.representation()));
               break;
@@ -1768,8 +1768,8 @@ class ModuleDecoderImpl : public Decoder {
             case kExprRttSub: {
               HeapTypeImmediate<validate> imm(enabled_features_, this,
                                               pc() + 2);
-              len += 1 + imm.length;
-              if (!Validate(pc() + 2, imm)) return {};
+              len += imm.length;
+              if (!Validate(pc() + len, imm)) return {};
               if (stack.empty()) {
                 error(pc(), "calling rtt.sub without arguments");
                 return {};

test/common/wasm/wasm-interpreter.cc

@@ -3031,13 +3031,12 @@ class WasmInterpreterInternals {
       byte orig = code->start[pc];
       WasmOpcode opcode = static_cast<WasmOpcode>(orig);
 
-      // If the opcode is a prefix, read the suffix and add the extra length to
-      // 'len'.
       if (WasmOpcodes::IsPrefixOpcode(opcode)) {
         uint32_t prefixed_opcode_length = 0;
         opcode = decoder.read_prefixed_opcode<Decoder::kNoValidation>(
             code->at(pc), &prefixed_opcode_length);
-        len += prefixed_opcode_length;
+        // read_prefixed_opcode includes the prefix byte, overwrite len.
+        len = prefixed_opcode_length;
       }
 
       // If max is 0, break. If max is positive (a limit is set), decrement it.