Revert "[wasm][bug] Fix a couple of bugs in validation of unreachable code"

This reverts commit 4a037f871e.

Reason for revert: Bot failures, including MSVC compilation.

Original change's description:
> [wasm][bug] Fix a couple of bugs in validation of unreachable code
>
> Changes:
> - SetBlockType now instantiates the block's start merge with values of
>   the correct type in unreachable code.
> - EnsureStackArguments now keeps the existing stack values and moves
>   them over the new bottom values.
> - Drop stack size validation in Drop().
> - Add new tests in unreachable-validation.js.
>
> Change-Id: Ie68b3d9abb0a41d1623d4a123fb526e71941c4e7
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2902733
> Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
> Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#74650}

Change-Id: Icb16af9a8ed16e593fe345ab727b992d9c9b1500
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2905597
Auto-Submit: Manos Koukoutos <manoskouk@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#74657}

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

@@ -3577,18 +3577,13 @@ class WasmFullDecoder : public WasmDecoder<validate> {
     InitMerge(&c->end_merge, imm.out_arity(), [pc, &imm](uint32_t i) {
       return Value{pc, imm.out_type(i)};
     });
-    InitMerge(&c->start_merge, imm.in_arity(), [pc, &imm, args](uint32_t i) {
+    InitMerge(&c->start_merge, imm.in_arity(),
-      // The merge needs to be instantiated with Values of the correct type even
+              [args](uint32_t i) { return args[i]; });
-      // in the presence of bottom values (i.e. in unreachable code). Since
-      // bottom Values will never be used for code generation, we can safely
-      // instantiate new ones in that case.
-      return args[i].type != kWasmBottom ? args[i] : Value{pc, imm.in_type(i)};
-    });
   }
 
   V8_INLINE void EnsureStackArguments(int count) {
     uint32_t limit = control_.back().stack_depth;
-    if (V8_LIKELY(stack_size() >= count + limit)) return;
+    if (stack_size() >= count + limit) return;
     EnsureStackArguments_Slow(count, limit);
   }
 
@@ -3597,21 +3592,15 @@ class WasmFullDecoder : public WasmDecoder<validate> {
       int index = count - stack_size() - 1;
       NotEnoughArgumentsError(index);
     }
-    // Silently create unreachable values out of thin air underneath the
+    // Silently create unreachable values out of thin air. Since we push them
-    // existing stack values. To do so, we have to move existing stack values
+    // onto the stack, while conceptually we should be inserting them under
-    // upwards in the stack, then instantiate the new Values as
+    // any existing elements, we have to avoid validation failures that would
-    // {UnreachableValue}.
+    // be caused by finding non-unreachable values in the wrong slot, so we
-    int current_values = stack_size() - limit;
+    // replace the entire current scope's values.
-    int additional_values = count - current_values;
+    Drop(static_cast<int>(stack_size() - limit));
-    DCHECK_GT(additional_values, 0);
+    EnsureStackSpace(count + limit - stack_size());
-    EnsureStackSpace(additional_values);
+    while (stack_size() < count + limit) {
-    stack_end_ += additional_values;
+      Push(UnreachableValue(this->pc_));
-    Value* stack_base = stack_value(current_values + additional_values);
-    for (int i = current_values - 1; i >= 0; i--) {
-      stack_base[additional_values + i] = stack_base[i];
-    }
-    for (int i = 0; i < additional_values; i++) {
-      stack_base[i] = UnreachableValue(this->pc_);
     }
   }
 
@@ -3626,8 +3615,6 @@ class WasmFullDecoder : public WasmDecoder<validate> {
     }
     return args;
   }
-  // Drops a number of stack elements equal to the {sig}'s parameter count (0 if
-  // {sig} is null), or all of them if less are present.
   V8_INLINE void DropArgs(const FunctionSig* sig) {
     int count = sig ? static_cast<int>(sig->parameter_count()) : 0;
     Drop(count);
@@ -3643,8 +3630,6 @@ class WasmFullDecoder : public WasmDecoder<validate> {
     }
     return args;
   }
-  // Drops a number of stack elements equal to the struct's field count, or all
-  // of them if less are present.
   V8_INLINE void DropArgs(const StructType* type) {
     Drop(static_cast<int>(type->field_count()));
   }
@@ -4839,20 +4824,22 @@ class WasmFullDecoder : public WasmDecoder<validate> {
     }
   }
 
-  // Drop the top {count} stack elements, or all of them if less than {count}
-  // are present.
   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(!current_code_reachable_and_ok_)) {
+        NotEnoughArgumentsError(0);
+      }
       // Pop what we can.
       count = std::min(count, static_cast<int>(stack_size() - limit));
     }
     DCHECK_LE(stack_, stack_end_ - count);
     stack_end_ -= count;
   }
-  // Drop the top stack element if present. Takes a Value input for more
+  // For more descriptive call sites:
-  // descriptive call sites.
   V8_INLINE void Drop(const Value& /* unused */) { Drop(1); }
 
   enum StackElementsCountMode : bool {
@@ -4885,9 +4872,7 @@ class WasmFullDecoder : public WasmDecoder<validate> {
             : merge_type == kReturnMerge ? "return" : "fallthru";
     uint32_t arity = merge->arity;
     uint32_t actual = stack_size() - control_.back().stack_depth;
-    // Here we have to check for !unreachable(), because we need to typecheck as
+    if (V8_LIKELY(current_code_reachable_and_ok_)) {
-    // if the current code is reachable even if it is spec-only reachable.
-    if (V8_LIKELY(!control_.back().unreachable())) {
       if (V8_UNLIKELY(strict_count ? actual != drop_values + arity
                                    : actual < drop_values + arity)) {
         this->DecodeError("expected %u elements on the stack for %s, found %u",

test/mjsunit/wasm/unreachable-validation.js

@@ -11,10 +11,6 @@ let unittest = true;
 
 function run(expected, name, code) {
   let builder = new WasmModuleBuilder();
-  // Index 0
-  builder.addType(kSig_i_i);
-  // Index 1
-  builder.addType(kSig_i_ii);
   builder.addFunction("main", kSig_v_v).
     addBody(code);
   let buffer = builder.toBuffer();
@@ -40,8 +36,6 @@ let X = undefined;
 
 let nop = kExprNop;
 let iadd = kExprI32Add;
-let ieqz = kExprI32Eqz;
-let leqz = kExprI64Eqz;
 let unr = kExprUnreachable;
 let ret = kExprReturn;
 let br0 = [kExprBr, 0];
@@ -51,7 +45,6 @@ let brt1 = [kExprBrTable, 0, 1];
 let brt01 = [kExprBrTable, 1, 0, 1];
 let f32 = [kExprF32Const, 0, 0, 0, 0];
 let zero = [kExprI32Const, 0];
-let zero64 = [kExprI64Const, 0];
 let if_else_empty = [kExprIf, kWasmVoid, kExprElse, kExprEnd];
 let if_unr = [kExprIf, kWasmVoid, kExprUnreachable, kExprEnd];
 let if_else_unr = [kExprIf, kWasmVoid, kExprUnreachable, kExprElse, kExprUnreachable, kExprEnd];
@@ -60,8 +53,6 @@ let loop_unr = [kExprLoop, kWasmVoid, kExprUnreachable, kExprEnd];
 let block_block_unr = [kExprBlock, kWasmVoid, kExprBlock, kWasmVoid, kExprUnreachable, kExprEnd, kExprEnd];
 let block = [kExprBlock, kWasmVoid]
 let iblock = [kExprBlock, kWasmI32]
-let i_iblock = [kExprBlock, 0]
-let i_iiblock = [kExprBlock, 1]
 let fblock = [kExprBlock, kWasmF32]
 let end = kExprEnd;
 let drop = kExprDrop;
@@ -101,10 +92,6 @@ run(I, '(block U) iadd drop', [...block_unr, iadd, drop]);
 run(I, '(block (block U)) iadd drop', [...block_block_unr, iadd, drop]);
 run(I, '(loop U) iadd drop', [...loop_unr, iadd]);
 run(I, '(if 0 U U) iadd drop', [...zero, ...if_else_unr, iadd, drop]);
-run(I, 'U (i_iblock leqz)', [unr, ...i_iblock, leqz, end, drop]);
-run(V, 'U (i_iblock ieqz)', [unr, ...i_iblock, ieqz, end, drop]);
-run(I, 'U (iblock iadd)', [unr, ...iblock, iadd, end, drop]);
-run(I, 'U zero64 (i_iiblock iadd) drop', [unr, ...zero64, ...i_iiblock, iadd, end, drop])
 
 run(V, 'U 0 0 iadd drop', [unr, ...zero, ...zero, iadd, drop]);
 run(V, "(block U) 0 0 iadd drop", [...block_unr, ...zero, ...zero, iadd, drop]);