Revert "[wasm] Stop decoding operands after error."

This reverts commit 6633ad56.

Reason for revert:
https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20arm64%20-%20sim%20-%20MSAN/builds/18850

Original change's description:
> [wasm] Stop decoding operands after error.
> 
> When we decode operands of WebAssembly instructions, we do not use the
> current pc but a pc of the instruction plus some offset. However, the
> pc of the instruction + offset can become invalid in case of a decoder
> error. Therefore we have to stop decoding operands explicitly in case
> of an error.
> 
> R=​clemensh@chromium.org
> 
> Bug: chromium:795131
> Change-Id: I3b7b45782c71a70364adf930bee3e94a1be88fea
> Reviewed-on: https://chromium-review.googlesource.com/832867
> Commit-Queue: Andreas Haas <ahaas@chromium.org>
> Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#50196}

TBR=ahaas@chromium.org,clemensh@chromium.org

Change-Id: I5a67f77285fdedc7f4645f8efaaf0087b4046011
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:795131
Reviewed-on: https://chromium-review.googlesource.com/832650Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50199}

src/wasm/decoder.h

@@ -58,7 +58,6 @@ class Decoder {
 
   inline bool validate_size(const byte* pc, uint32_t length, const char* msg) {
     DCHECK_LE(start_, pc);
-    DCHECK_LE(pc, end_);
     if (V8_UNLIKELY(length > static_cast<uint32_t>(end_ - pc))) {
       error(pc, msg);
       return false;
@@ -166,7 +165,6 @@ class Decoder {
 
   // Check that at least {size} bytes exist between {pc_} and {end_}.
   bool checkAvailable(uint32_t size) {
-    DCHECK_LE(pc_, end_);
     if (V8_UNLIKELY(size > static_cast<uint32_t>(end_ - pc_))) {
       errorf(pc_, "expected %u bytes, fell off end", size);
       return false;
@@ -314,8 +312,7 @@ class Decoder {
     static_assert(byte_index < kMaxLength, "invalid template instantiation");
     constexpr int shift = byte_index * 7;
     constexpr bool is_last_byte = byte_index == kMaxLength - 1;
-    DCHECK_LE(pc, end_);
-    const bool at_end = validate && pc == end_;
+    const bool at_end = validate && pc >= end_;
     byte b = 0;
     if (!at_end) {
       DCHECK_LT(pc, end_);

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

@@ -246,11 +246,10 @@ struct CallIndirectOperand {
   uint32_t table_index;
   uint32_t index;
   FunctionSig* sig = nullptr;
-  unsigned length = 0;
+  unsigned length;
   inline CallIndirectOperand(Decoder* decoder, const byte* pc) {
     unsigned len = 0;
     index = decoder->read_u32v<validate>(pc + 1, &len, "signature index");
-    if (!VALIDATE(decoder->ok())) return;
     table_index = decoder->read_u8<validate>(pc + 1 + len, "table index");
     if (!VALIDATE(table_index == 0)) {
       decoder->errorf(pc + 1 + len, "expected table index 0, found %u",
@@ -339,7 +338,7 @@ template <Decoder::ValidateFlag validate>
 struct MemoryAccessOperand {
   uint32_t alignment;
   uint32_t offset;
-  unsigned length = 0;
+  unsigned length;
   inline MemoryAccessOperand(Decoder* decoder, const byte* pc,
                              uint32_t max_alignment) {
     unsigned alignment_length;
@@ -351,7 +350,6 @@ struct MemoryAccessOperand {
                       "actual alignment is %u",
                       max_alignment, alignment);
     }
-    if (!VALIDATE(decoder->ok())) return;
     unsigned offset_length;
     offset = decoder->read_u32v<validate>(pc + 1 + alignment_length,
                                           &offset_length, "offset");
@@ -389,7 +387,6 @@ struct Simd8x16ShuffleOperand {
   inline Simd8x16ShuffleOperand(Decoder* decoder, const byte* pc) {
     for (uint32_t i = 0; i < kSimd128Size; ++i) {
       shuffle[i] = decoder->read_u8<validate>(pc + 2 + i, "shuffle");
-      if (!VALIDATE(decoder->ok())) return;
     }
   }
 };

test/unittests/wasm/function-body-decoder-unittest.cc

@@ -52,40 +52,36 @@ static const WasmOpcode kInt32BinopOpcodes[] = {
 #define WASM_BRV_IF_ZERO(depth, val) \
   val, WASM_ZERO, kExprBrIf, static_cast<byte>(depth)
 
-#define EXPECT_VERIFIES_C(sig, x) \
-  Verify(true, sigs.sig(), x, x + arraysize(x), kAppendEnd)
+#define EXPECT_VERIFIES_C(sig, x) Verify(true, sigs.sig(), x, x + arraysize(x))
 
-#define EXPECT_FAILURE_C(sig, x) \
-  Verify(false, sigs.sig(), x, x + arraysize(x), kAppendEnd)
+#define EXPECT_FAILURE_C(sig, x) Verify(false, sigs.sig(), x, x + arraysize(x))
 
-#define EXPECT_VERIFIES_SC(sig, x) \
-  Verify(true, sig, x, x + arraysize(x), kAppendEnd)
+#define EXPECT_VERIFIES_SC(sig, x) Verify(true, sig, x, x + arraysize(x))
 
-#define EXPECT_FAILURE_SC(sig, x) \
-  Verify(false, sig, x, x + arraysize(x), kAppendEnd)
+#define EXPECT_FAILURE_SC(sig, x) Verify(false, sig, x, x + arraysize(x))
 
-#define EXPECT_VERIFIES_S(env, ...)                              \
-  do {                                                           \
-    static byte code[] = {__VA_ARGS__};                          \
-    Verify(true, env, code, code + arraysize(code), kAppendEnd); \
+#define EXPECT_VERIFIES_S(env, ...)                  \
+  do {                                               \
+    static byte code[] = {__VA_ARGS__};              \
+    Verify(true, env, code, code + arraysize(code)); \
   } while (false)
 
-#define EXPECT_FAILURE_S(env, ...)                                \
-  do {                                                            \
-    static byte code[] = {__VA_ARGS__};                           \
-    Verify(false, env, code, code + arraysize(code), kAppendEnd); \
+#define EXPECT_FAILURE_S(env, ...)                    \
+  do {                                                \
+    static byte code[] = {__VA_ARGS__};               \
+    Verify(false, env, code, code + arraysize(code)); \
   } while (false)
 
-#define EXPECT_VERIFIES(sig, ...)                                    \
-  do {                                                               \
-    static const byte code[] = {__VA_ARGS__};                        \
-    Verify(true, sigs.sig(), code, code + sizeof(code), kAppendEnd); \
+#define EXPECT_VERIFIES(sig, ...)                        \
+  do {                                                   \
+    static const byte code[] = {__VA_ARGS__};            \
+    Verify(true, sigs.sig(), code, code + sizeof(code)); \
   } while (false)
 
-#define EXPECT_FAILURE(sig, ...)                                      \
-  do {                                                                \
-    static const byte code[] = {__VA_ARGS__};                         \
-    Verify(false, sigs.sig(), code, code + sizeof(code), kAppendEnd); \
+#define EXPECT_FAILURE(sig, ...)                          \
+  do {                                                    \
+    static const byte code[] = {__VA_ARGS__};             \
+    Verify(false, sigs.sig(), code, code + sizeof(code)); \
   } while (false)
 
 class FunctionBodyDecoderTest : public TestWithZone {
@@ -102,24 +98,18 @@ class FunctionBodyDecoderTest : public TestWithZone {
     local_decls.AddLocals(count, type);
   }
 
-  enum AppendEnd : bool { kAppendEnd, kOmitEnd };
-
-  void PrepareBytecode(const byte** startp, const byte** endp,
-                       AppendEnd append_end) {
+  void PrepareBytecode(const byte** startp, const byte** endp) {
     const byte* start = *startp;
     const byte* end = *endp;
     size_t locals_size = local_decls.Size();
-    size_t total_size = end - start + locals_size;
-    if (append_end == kAppendEnd) ++total_size;
+    size_t total_size = end - start + locals_size + 1;
     byte* buffer = static_cast<byte*>(zone()->New(total_size));
     // Prepend the local decls to the code.
     local_decls.Emit(buffer);
     // Emit the code.
     memcpy(buffer + locals_size, start, end - start);
-    if (append_end == kAppendEnd) {
-      // Append an extra end opcode.
-      buffer[total_size - 1] = kExprEnd;
-    }
+    // Append an extra end opcode.
+    buffer[total_size - 1] = kExprEnd;
 
     *startp = buffer;
     *endp = buffer + total_size;
@@ -128,8 +118,8 @@ class FunctionBodyDecoderTest : public TestWithZone {
   // Prepends local variable declarations and renders nice error messages for
   // verification failures.
   void Verify(bool expected_success, FunctionSig* sig, const byte* start,
-              const byte* end, AppendEnd append_end) {
-    PrepareBytecode(&start, &end, append_end);
+              const byte* end) {
+    PrepareBytecode(&start, &end);
 
     // Verify the code.
     DecodeResult result =
@@ -263,8 +253,8 @@ TEST_F(FunctionBodyDecoderTest, Int32Const1) {
 
 TEST_F(FunctionBodyDecoderTest, EmptyFunction) {
   byte code[] = {0};
-  Verify(true, sigs.v_v(), code, code, kAppendEnd);
-  Verify(false, sigs.i_i(), code, code, kAppendEnd);
+  Verify(true, sigs.v_v(), code, code);
+  Verify(false, sigs.i_i(), code, code);
 }
 
 TEST_F(FunctionBodyDecoderTest, IncompleteIf1) {
@@ -320,9 +310,7 @@ TEST_F(FunctionBodyDecoderTest, Int32Const_off_end) {
   byte code[] = {kExprI32Const, 0xAA, 0xBB, 0xCC, 0x44};
 
   for (int size = 1; size <= 4; size++) {
-    Verify(false, sigs.i_i(), code, code + size, kAppendEnd);
-    // Should also fail without the trailing 'end' opcode.
-    Verify(false, sigs.i_i(), code, code + size, kOmitEnd);
+    Verify(false, sigs.i_i(), code, code + size);
   }
 }
 
@@ -508,7 +496,7 @@ TEST_F(FunctionBodyDecoderTest, BlockN) {
     buffer[0] = kExprBlock;
     buffer[1] = kLocalVoid;
     buffer[i + 2] = kExprEnd;
-    Verify(true, sigs.v_i(), buffer, buffer + i + 3, kAppendEnd);
+    Verify(true, sigs.v_i(), buffer, buffer + i + 3);
   }
 }
 
@@ -655,8 +643,7 @@ TEST_F(FunctionBodyDecoderTest, BlockN_off_end) {
   byte code[] = {WASM_BLOCK(kExprNop, kExprNop, kExprNop, kExprNop)};
   EXPECT_VERIFIES_C(v_v, code);
   for (size_t i = 1; i < arraysize(code); i++) {
-    Verify(false, sigs.v_v(), code, code + i, kAppendEnd);
-    Verify(false, sigs.v_v(), code, code + i, kOmitEnd);
+    Verify(false, sigs.v_v(), code, code + i);
   }
 }
 
@@ -986,8 +973,7 @@ TEST_F(FunctionBodyDecoderTest, If_off_end) {
   static const byte kCode[] = {
       WASM_IF_ELSE(WASM_GET_LOCAL(0), WASM_GET_LOCAL(0), WASM_GET_LOCAL(0))};
   for (size_t len = 3; len < arraysize(kCode); len++) {
-    Verify(false, sigs.i_i(), kCode, kCode + len, kAppendEnd);
-    Verify(false, sigs.i_i(), kCode, kCode + len, kOmitEnd);
+    Verify(false, sigs.i_i(), kCode, kCode + len);
   }
 }
 
@@ -1580,40 +1566,6 @@ TEST_F(FunctionBodyDecoderTest, IndirectCallsWithoutTableCrash) {
                                             WASM_I32V_2(72)));
 }
 
-TEST_F(FunctionBodyDecoderTest, IncompleteIndirectCall) {
-  FunctionSig* sig = sigs.i_i();
-  TestModuleBuilder builder;
-  builder.InitializeFunctionTable();
-  module = builder.module();
-
-  static byte code[] = {kExprCallIndirect};
-  Verify(false, sig, code, code + arraysize(code), kOmitEnd);
-}
-
-TEST_F(FunctionBodyDecoderTest, IncompleteStore) {
-  FunctionSig* sig = sigs.i_i();
-  TestModuleBuilder builder;
-  builder.InitializeMemory();
-  builder.InitializeFunctionTable();
-  module = builder.module();
-
-  static byte code[] = {kExprI32StoreMem};
-  Verify(false, sig, code, code + arraysize(code), kOmitEnd);
-}
-
-TEST_F(FunctionBodyDecoderTest, IncompleteS8x16Shuffle) {
-  EXPERIMENTAL_FLAG_SCOPE(simd);
-  FunctionSig* sig = sigs.i_i();
-  TestModuleBuilder builder;
-  builder.InitializeMemory();
-  builder.InitializeFunctionTable();
-  module = builder.module();
-
-  static byte code[] = {kSimdPrefix,
-                        static_cast<byte>(kExprS8x16Shuffle & 0xff)};
-  Verify(false, sig, code, code + arraysize(code), kOmitEnd);
-}
-
 TEST_F(FunctionBodyDecoderTest, SimpleImportCalls) {
   FunctionSig* sig = sigs.i_i();
   TestModuleBuilder builder;
@@ -2187,8 +2139,7 @@ TEST_F(FunctionBodyDecoderTest, BrTable2b) {
 TEST_F(FunctionBodyDecoderTest, BrTable_off_end) {
   static byte code[] = {B1(WASM_BR_TABLE(WASM_GET_LOCAL(0), 0, BR_TARGET(0)))};
   for (size_t len = 1; len < sizeof(code); len++) {
-    Verify(false, sigs.i_i(), code, code + len, kAppendEnd);
-    Verify(false, sigs.i_i(), code, code + len, kOmitEnd);
+    Verify(false, sigs.i_i(), code, code + len);
   }
 }
 
@@ -2665,7 +2616,7 @@ TEST_F(FunctionBodyDecoderTest, Regression709741) {
   byte code[] = {WASM_NOP};
   const byte* start = code;
   const byte* end = code + sizeof(code);
-  PrepareBytecode(&start, &end, kAppendEnd);
+  PrepareBytecode(&start, &end);
 
   for (const byte* i = start; i < end; i++) {
     DecodeResult result =