Reland [wasm] Stop decoding operands after error.

The problem was that parts of Simd8x16ShuffleOperand were uninitialized.

Original message:

[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: I732bc23547dbe531019d81a4397d22165a26d46b
Reviewed-on: https://chromium-review.googlesource.com/833934Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50211}

src/wasm/decoder.h

@@ -58,6 +58,7 @@ 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;
@@ -165,6 +166,7 @@ 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;
@@ -312,7 +314,8 @@ 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;
-    const bool at_end = validate && pc >= end_;
+    DCHECK_LE(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,10 +246,11 @@ struct CallIndirectOperand {
   uint32_t table_index;
   uint32_t index;
   FunctionSig* sig = nullptr;
-  unsigned length;
+  unsigned length = 0;
   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",
@@ -338,7 +339,7 @@ template <Decoder::ValidateFlag validate>
 struct MemoryAccessOperand {
   uint32_t alignment;
   uint32_t offset;
-  unsigned length;
+  unsigned length = 0;
   inline MemoryAccessOperand(Decoder* decoder, const byte* pc,
                              uint32_t max_alignment) {
     unsigned alignment_length;
@@ -350,6 +351,7 @@ 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");
@@ -382,11 +384,12 @@ struct SimdShiftOperand {
 // Operand for SIMD S8x16 shuffle operations.
 template <Decoder::ValidateFlag validate>
 struct Simd8x16ShuffleOperand {
-  uint8_t shuffle[kSimd128Size];
+  uint8_t shuffle[kSimd128Size] = {0};
 
   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,36 +52,40 @@ 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))
+#define EXPECT_VERIFIES_C(sig, x) \
+  Verify(true, sigs.sig(), x, x + arraysize(x), kAppendEnd)
 
-#define EXPECT_FAILURE_C(sig, x) Verify(false, sigs.sig(), x, x + arraysize(x))
+#define EXPECT_FAILURE_C(sig, x) \
+  Verify(false, sigs.sig(), x, x + arraysize(x), kAppendEnd)
 
-#define EXPECT_VERIFIES_SC(sig, x) Verify(true, sig, x, x + arraysize(x))
+#define EXPECT_VERIFIES_SC(sig, x) \
+  Verify(true, sig, x, x + arraysize(x), kAppendEnd)
 
-#define EXPECT_FAILURE_SC(sig, x) Verify(false, sig, x, x + arraysize(x))
+#define EXPECT_FAILURE_SC(sig, x) \
+  Verify(false, sig, x, x + arraysize(x), kAppendEnd)
 
 #define EXPECT_VERIFIES_S(env, ...)                              \
   do {                                                           \
     static byte code[] = {__VA_ARGS__};                          \
-    Verify(true, env, code, code + arraysize(code)); \
+    Verify(true, env, code, code + arraysize(code), kAppendEnd); \
   } while (false)
 
 #define EXPECT_FAILURE_S(env, ...)                                \
   do {                                                            \
     static byte code[] = {__VA_ARGS__};                           \
-    Verify(false, env, code, code + arraysize(code)); \
+    Verify(false, env, code, code + arraysize(code), kAppendEnd); \
   } while (false)
 
 #define EXPECT_VERIFIES(sig, ...)                                    \
   do {                                                               \
     static const byte code[] = {__VA_ARGS__};                        \
-    Verify(true, sigs.sig(), code, code + sizeof(code)); \
+    Verify(true, sigs.sig(), code, code + sizeof(code), kAppendEnd); \
   } while (false)
 
 #define EXPECT_FAILURE(sig, ...)                                      \
   do {                                                                \
     static const byte code[] = {__VA_ARGS__};                         \
-    Verify(false, sigs.sig(), code, code + sizeof(code)); \
+    Verify(false, sigs.sig(), code, code + sizeof(code), kAppendEnd); \
   } while (false)
 
 class FunctionBodyDecoderTest : public TestWithZone {
@@ -98,18 +102,24 @@ class FunctionBodyDecoderTest : public TestWithZone {
     local_decls.AddLocals(count, type);
   }
 
-  void PrepareBytecode(const byte** startp, const byte** endp) {
+  enum AppendEnd : bool { kAppendEnd, kOmitEnd };
+
+  void PrepareBytecode(const byte** startp, const byte** endp,
+                       AppendEnd append_end) {
     const byte* start = *startp;
     const byte* end = *endp;
     size_t locals_size = local_decls.Size();
-    size_t total_size = end - start + locals_size + 1;
+    size_t total_size = end - start + locals_size;
+    if (append_end == kAppendEnd) ++total_size;
     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;
+    }
 
     *startp = buffer;
     *endp = buffer + total_size;
@@ -118,8 +128,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) {
-    PrepareBytecode(&start, &end);
+              const byte* end, AppendEnd append_end) {
+    PrepareBytecode(&start, &end, append_end);
 
     // Verify the code.
     DecodeResult result =
@@ -253,8 +263,8 @@ TEST_F(FunctionBodyDecoderTest, Int32Const1) {
 
 TEST_F(FunctionBodyDecoderTest, EmptyFunction) {
   byte code[] = {0};
-  Verify(true, sigs.v_v(), code, code);
-  Verify(false, sigs.i_i(), code, code);
+  Verify(true, sigs.v_v(), code, code, kAppendEnd);
+  Verify(false, sigs.i_i(), code, code, kAppendEnd);
 }
 
 TEST_F(FunctionBodyDecoderTest, IncompleteIf1) {
@@ -310,7 +320,9 @@ 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);
+    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);
   }
 }
 
@@ -496,7 +508,7 @@ TEST_F(FunctionBodyDecoderTest, BlockN) {
     buffer[0] = kExprBlock;
     buffer[1] = kLocalVoid;
     buffer[i + 2] = kExprEnd;
-    Verify(true, sigs.v_i(), buffer, buffer + i + 3);
+    Verify(true, sigs.v_i(), buffer, buffer + i + 3, kAppendEnd);
   }
 }
 
@@ -643,7 +655,8 @@ 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);
+    Verify(false, sigs.v_v(), code, code + i, kAppendEnd);
+    Verify(false, sigs.v_v(), code, code + i, kOmitEnd);
   }
 }
 
@@ -973,7 +986,8 @@ 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);
+    Verify(false, sigs.i_i(), kCode, kCode + len, kAppendEnd);
+    Verify(false, sigs.i_i(), kCode, kCode + len, kOmitEnd);
   }
 }
 
@@ -1566,6 +1580,40 @@ 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;
@@ -2139,7 +2187,8 @@ 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);
+    Verify(false, sigs.i_i(), code, code + len, kAppendEnd);
+    Verify(false, sigs.i_i(), code, code + len, kOmitEnd);
   }
 }
 
@@ -2616,7 +2665,7 @@ TEST_F(FunctionBodyDecoderTest, Regression709741) {
   byte code[] = {WASM_NOP};
   const byte* start = code;
   const byte* end = code + sizeof(code);
-  PrepareBytecode(&start, &end);
+  PrepareBytecode(&start, &end, kAppendEnd);
 
   for (const byte* i = start; i < end; i++) {
     DecodeResult result =