[wasm] Fix error message for missing stack arguments

We currently could produce the error message 'not enough arguments on
the stack for block, expected 0 more'. This CL fixes this by printing
the available number of arguments and the needed number, and adds
DCHECKs to catch similar miscomputations in the future.

It also adds a new test that produced the broken error before, and
includes the expected failure message in a few more tests for
robustness.

R=manoskouk@chromium.org

Change-Id: Ia08863889ae36ae0a05d96d36e92295b7159a01e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3194264Reviewed-by: Manos Koukoutos <manoskouk@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#77167}

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

@@ -3633,8 +3633,7 @@ class WasmFullDecoder : public WasmDecoder<validate, decoding_mode> {
 
   V8_NOINLINE int EnsureStackArguments_Slow(int count, uint32_t limit) {
     if (!VALIDATE(control_.back().unreachable())) {
-      int index = count - stack_size() - 1;
-      NotEnoughArgumentsError(index);
+      NotEnoughArgumentsError(count, stack_size() - limit);
     }
     // Silently create unreachable values out of thin air underneath the
     // existing stack values. To do so, we have to move existing stack values
@@ -5118,10 +5117,13 @@ class WasmFullDecoder : public WasmDecoder<validate, decoding_mode> {
     PopTypeError(index, val, ("type " + expected.name()).c_str());
   }
 
-  V8_NOINLINE void NotEnoughArgumentsError(int index) {
+  V8_NOINLINE void NotEnoughArgumentsError(int needed, int actual) {
+    DCHECK_LT(0, needed);
+    DCHECK_LE(0, actual);
+    DCHECK_LT(actual, needed);
     this->DecodeError(
-        "not enough arguments on the stack for %s, expected %d more",
-        SafeOpcodeNameAt(this->pc_), index + 1);
+        "not enough arguments on the stack for %s (need %d, got %d)",
+        SafeOpcodeNameAt(this->pc_), needed, actual);
   }
 
   V8_INLINE Value Peek(int depth, int index, ValueType expected) {
@@ -5140,7 +5142,7 @@ class WasmFullDecoder : public WasmDecoder<validate, decoding_mode> {
       // Peeking past the current control start in reachable code.
       if (!VALIDATE(decoding_mode == kFunctionBody &&
                     control_.back().unreachable())) {
-        NotEnoughArgumentsError(index);
+        NotEnoughArgumentsError(depth + 1, stack_size() - limit);
       }
       return UnreachableValue(this->pc_);
     }

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

@@ -706,21 +706,33 @@ TEST_F(FunctionBodyDecoderTest, BlockType) {
 }
 
 TEST_F(FunctionBodyDecoderTest, BlockType_fail) {
-  ExpectFailure(sigs.i_i(), {WASM_BLOCK_L(WASM_I64V_1(0))});
-  ExpectFailure(sigs.i_i(), {WASM_BLOCK_F(WASM_F32(0.0))});
-  ExpectFailure(sigs.i_i(), {WASM_BLOCK_D(WASM_F64(1.1))});
-
-  ExpectFailure(sigs.l_l(), {WASM_BLOCK_I(WASM_ZERO)});
-  ExpectFailure(sigs.l_l(), {WASM_BLOCK_F(WASM_F32(0.0))});
-  ExpectFailure(sigs.l_l(), {WASM_BLOCK_D(WASM_F64(1.1))});
-
-  ExpectFailure(sigs.f_ff(), {WASM_BLOCK_I(WASM_ZERO)});
-  ExpectFailure(sigs.f_ff(), {WASM_BLOCK_L(WASM_I64V_1(0))});
-  ExpectFailure(sigs.f_ff(), {WASM_BLOCK_D(WASM_F64(1.1))});
-
-  ExpectFailure(sigs.d_dd(), {WASM_BLOCK_I(WASM_ZERO)});
-  ExpectFailure(sigs.d_dd(), {WASM_BLOCK_L(WASM_I64V_1(0))});
-  ExpectFailure(sigs.d_dd(), {WASM_BLOCK_F(WASM_F32(0.0))});
+  ExpectFailure(sigs.i_i(), {WASM_BLOCK_L(WASM_I64V_1(0))}, kAppendEnd,
+                "type error in fallthru[0]");
+  ExpectFailure(sigs.i_i(), {WASM_BLOCK_F(WASM_F32(0.0))}, kAppendEnd,
+                "type error in fallthru[0]");
+  ExpectFailure(sigs.i_i(), {WASM_BLOCK_D(WASM_F64(1.1))}, kAppendEnd,
+                "type error in fallthru[0]");
+
+  ExpectFailure(sigs.l_l(), {WASM_BLOCK_I(WASM_ZERO)}, kAppendEnd,
+                "type error in fallthru[0]");
+  ExpectFailure(sigs.l_l(), {WASM_BLOCK_F(WASM_F32(0.0))}, kAppendEnd,
+                "type error in fallthru[0]");
+  ExpectFailure(sigs.l_l(), {WASM_BLOCK_D(WASM_F64(1.1))}, kAppendEnd,
+                "type error in fallthru[0]");
+
+  ExpectFailure(sigs.f_ff(), {WASM_BLOCK_I(WASM_ZERO)}, kAppendEnd,
+                "type error in fallthru[0]");
+  ExpectFailure(sigs.f_ff(), {WASM_BLOCK_L(WASM_I64V_1(0))}, kAppendEnd,
+                "type error in fallthru[0]");
+  ExpectFailure(sigs.f_ff(), {WASM_BLOCK_D(WASM_F64(1.1))}, kAppendEnd,
+                "type error in fallthru[0]");
+
+  ExpectFailure(sigs.d_dd(), {WASM_BLOCK_I(WASM_ZERO)}, kAppendEnd,
+                "type error in fallthru[0]");
+  ExpectFailure(sigs.d_dd(), {WASM_BLOCK_L(WASM_I64V_1(0))}, kAppendEnd,
+                "type error in fallthru[0]");
+  ExpectFailure(sigs.d_dd(), {WASM_BLOCK_F(WASM_F32(0.0))}, kAppendEnd,
+                "type error in fallthru[0]");
 }
 
 TEST_F(FunctionBodyDecoderTest, BlockF32) {
@@ -3033,16 +3045,29 @@ TEST_F(FunctionBodyDecoderTest, MultiValBlock1) {
   ExpectValidates(
       sigs.i_ii(),
       {WASM_BLOCK_X(sig0, WASM_LOCAL_GET(0), WASM_LOCAL_GET(1)), kExprI32Add});
-  ExpectFailure(sigs.i_ii(), {WASM_BLOCK_X(sig0, WASM_NOP), kExprI32Add});
-  ExpectFailure(sigs.i_ii(),
-                {WASM_BLOCK_X(sig0, WASM_LOCAL_GET(0)), kExprI32Add});
+  ExpectFailure(sigs.i_ii(), {WASM_BLOCK_X(sig0, WASM_NOP), kExprI32Add},
+                kAppendEnd,
+                "expected 2 elements on the stack for fallthru, found 0");
+  ExpectFailure(
+      sigs.i_ii(), {WASM_BLOCK_X(sig0, WASM_LOCAL_GET(0)), kExprI32Add},
+      kAppendEnd, "expected 2 elements on the stack for fallthru, found 1");
   ExpectFailure(sigs.i_ii(),
                 {WASM_BLOCK_X(sig0, WASM_LOCAL_GET(0), WASM_LOCAL_GET(1),
                               WASM_LOCAL_GET(0)),
-                 kExprI32Add});
+                 kExprI32Add},
+                kAppendEnd,
+                "expected 2 elements on the stack for fallthru, found 3");
   ExpectFailure(
       sigs.i_ii(),
-      {WASM_BLOCK_X(sig0, WASM_LOCAL_GET(0), WASM_LOCAL_GET(1)), kExprF32Add});
+      {WASM_BLOCK_X(sig0, WASM_LOCAL_GET(0), WASM_LOCAL_GET(1)), kExprF32Add},
+      kAppendEnd, "f32.add[1] expected type f32, found block of type i32");
+
+  byte sig1 = builder.AddSignature(sigs.v_i());
+  ExpectFailure(
+      sigs.v_i(),
+      {WASM_LOCAL_GET(0), WASM_BLOCK(WASM_BLOCK_X(sig1, WASM_UNREACHABLE))},
+      kAppendEnd,
+      "not enough arguments on the stack for block (need 1, got 0)");
 }
 
 TEST_F(FunctionBodyDecoderTest, MultiValBlock2) {
@@ -3992,8 +4017,8 @@ TEST_F(FunctionBodyDecoderTest, GCStruct) {
                 {WASM_STRUCT_NEW_WITH_RTT(struct_type_index,
                                           WASM_RTT_CANON(struct_type_index))},
                 kAppendEnd,
-                "not enough arguments on the stack for struct.new_with_rtt, "
-                "expected 1 more");
+                "not enough arguments on the stack for struct.new_with_rtt "
+                "(need 2, got 1)");
   // Too many arguments.
   ExpectFailure(
       &sig_r_v,
@@ -4127,8 +4152,8 @@ TEST_F(FunctionBodyDecoderTest, GCArray) {
                 {WASM_I32V(10), WASM_RTT_CANON(array_type_index),
                  WASM_GC_OP(kExprArrayNewWithRtt), array_type_index},
                 kAppendEnd,
-                "not enough arguments on the stack for array.new_with_rtt, "
-                "expected 1 more");
+                "not enough arguments on the stack for array.new_with_rtt "
+                "(need 3, got 2)");
   // Mistyped initializer.
   ExpectFailure(&sig_r_v,
                 {WASM_ARRAY_NEW_WITH_RTT(

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

@@ -1156,7 +1156,7 @@ TEST_F(WasmModuleVerifyTest, ArrayInitInitExpr) {
                                         WASM_I32V(30), WASM_RTT_CANON(0)))};
   EXPECT_FAILURE_WITH_MSG(
       length_error,
-      "not enough arguments on the stack for array.init, expected 7 more");
+      "not enough arguments on the stack for array.init (need 11, got 4)");
 }
 
 TEST_F(WasmModuleVerifyTest, EmptyStruct) {