[wasm] Add missing validation on Drop

Before dropping a value we should validate that there is indeed a value
on the stack.

R=jkummerow@chromium.org

Bug: chromium:1184964
Change-Id: Iec3ac061df2545717749e664b10c383765d67c9d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2739588Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73263}

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

@@ -3084,6 +3084,7 @@ class WasmFullDecoder : public WasmDecoder<validate> {
   }
 
   DECODE(Drop) {
+    Peek(0, 0);
     CALL_INTERFACE_IF_REACHABLE(Drop);
     Drop(1);
     return 1;
@@ -4802,14 +4803,14 @@ class WasmFullDecoder : public WasmDecoder<validate> {
   V8_INLINE void Drop(int count = 1) {
     DCHECK(!control_.empty());
     uint32_t limit = control_.back().stack_depth;
+    // TODO(wasm): This check is often redundant.
     if (V8_UNLIKELY(stack_size() < limit + count)) {
       // Popping past the current control start in reachable code.
       if (!VALIDATE(!control_.back().reachable())) {
         NotEnoughArgumentsError(0);
       }
       // Pop what we can.
-      stack_end_ -= std::min(count, static_cast<int>(stack_size() - limit));
-      return;
+      count = std::min(count, static_cast<int>(stack_size() - limit));
     }
     DCHECK_LE(stack_, stack_end_ - count);
     stack_end_ -= count;

test/mjsunit/regress/wasm/regress-1184964.js

+// Copyright 2021 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: --wasm-lazy-compilation
+
+load('test/mjsunit/wasm/wasm-module-builder.js');
+
+const builder = new WasmModuleBuilder();
+builder.addFunction('foo', kSig_v_v).addBody([kExprDrop]);
+assertThrows(() => builder.instantiate(), WebAssembly.CompileError);

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

@@ -4365,6 +4365,16 @@ TEST_F(FunctionBodyDecoderTest, Regress_1154439) {
   ExpectFailure(sigs.v_v(), {}, kAppendEnd, "local count too large");
 }
 
+TEST_F(FunctionBodyDecoderTest, DropOnEmptyStack) {
+  // Valid code:
+  ExpectValidates(sigs.v_v(), {kExprI32Const, 1, kExprDrop}, kAppendEnd);
+  // Invalid code (dropping from empty stack):
+  ExpectFailure(sigs.v_v(), {kExprDrop}, kAppendEnd,
+                "not enough arguments on the stack for drop");
+  // Valid code (dropping from empty stack in unreachable code):
+  ExpectValidates(sigs.v_v(), {kExprUnreachable, kExprDrop}, kAppendEnd);
+}
+
 class BranchTableIteratorTest : public TestWithZone {
  public:
   BranchTableIteratorTest() : TestWithZone() {}