[wasm-gc] Fix node order for array.new length check

Operator::kEliminatable has the unfortunate consequence that depending
on surrounding code, the allocating builtin call could get scheduled
before the max length check, causing a crash instead of a trap.

Fixed: chromium:1239954
Change-Id: Ice2e3e4f67e8fce44a886c0079e0e31f124c02b0
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3103315Reviewed-by: Georg Neis <neis@chromium.org>
Reviewed-by: Manos Koukoutos <manoskouk@chromium.org>
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#76385}

src/compiler/wasm-compiler.cc

@@ -5549,9 +5549,12 @@ Node* WasmGraphBuilder::ArrayNewWithRtt(uint32_t array_index,
               position);
   wasm::ValueType element_type = type->element_type();
   Builtin stub = ChooseArrayAllocationBuiltin(element_type, initial_value);
-  Node* a =
-      gasm_->CallBuiltin(stub, Operator::kEliminatable, rtt, length,
-                         Int32Constant(element_type.element_size_bytes()));
+  // Do NOT mark this as Operator::kEliminatable, because that would cause the
+  // Call node to have no control inputs, which means it could get scheduled
+  // before the check/trap above.
+  Node* a = gasm_->CallBuiltin(
+      stub, Operator::kNoDeopt | Operator::kNoThrow, rtt, length,
+      Int32Constant(element_type.element_size_bytes()));
   if (initial_value != nullptr) {
     // TODO(manoskouk): If the loop is ever removed here, we have to update
     // ArrayNewWithRtt() in graph-builder-interface.cc to not mark the current

test/mjsunit/regress/wasm/regress-1239954.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: --experimental-wasm-gc
+
+d8.file.execute('test/mjsunit/wasm/wasm-module-builder.js');
+
+var builder = new WasmModuleBuilder();
+
+let array_index = builder.addArray(kWasmI64, true);
+let sig_index = builder.addType(kSig_v_v);
+
+let main = builder.addFunction("main", kSig_v_i);
+let other = builder.addFunction("other", sig_index).addBody([]);
+
+let table = builder.addTable(kWasmAnyFunc, 1, 1);
+builder.addActiveElementSegment(
+    0,                         // table
+    WasmInitExpr.I32Const(0),  // offset
+    [1]);                      // values
+
+main.addBody([
+    kExprI64Const, 0x33,
+    kExprLocalGet, 0,
+    kGCPrefix, kExprRttCanon, array_index,
+    kGCPrefix, kExprArrayNewWithRtt, array_index,
+    kExprDrop,
+    kExprI32Const, 0,
+    kExprCallIndirect, sig_index, table.index,
+]).exportFunc();
+
+var instance = builder.instantiate();
+
+assertThrows(
+    () => instance.exports.main(1<<29), WebAssembly.RuntimeError,
+    'requested new array is too large');