Reland "[wasm] Implement table.init for interpreter"

This is a reland of 63608968

The previous CL failed on Windows, but it was a general bug. The
dropped_elem_segments was not being set on the instance properly in
cctests, so`table.init` instructions would fail by reading uninitialized
data.

I took this opportunity to also add an implementation of
`elem.drop` in the interpreter, and ported the JS tests for those too.

Original change's description:
> [wasm] Implement table.init for interpreter
>
> This also fixes CheckCallViaJS when a trap occurs. In that case, the
> trap callback is called instead of an exception being thrown, so if it
> isn't handled, a bogus result will be returned instead.
>
> Bug: v8:8965
> Change-Id: I560e89f353756df23c062fb8c9484d9971c19253
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1539078
> Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
> Commit-Queue: Ben Smith <binji@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#60473}

Bug: v8:8965
Change-Id: Ia547d9530b7ca67fde5bd94539f49153b796e82d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1547142
Commit-Queue: Ben Smith <binji@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60664}

src/wasm/wasm-interpreter.cc

@@ -1668,6 +1668,15 @@ class ThreadImpl {
     return true;
   }
 
+  bool CheckElemSegmentIsPassiveAndNotDropped(uint32_t index, pc_t pc) {
+    DCHECK_LT(index, module()->elem_segments.size());
+    if (instance_object_->dropped_elem_segments()[index]) {
+      DoTrap(kTrapElemSegmentDropped, pc);
+      return false;
+    }
+    return true;
+  }
+
   template <typename type, typename op_type>
   bool ExtractAtomicOpParams(Decoder* decoder, InterpreterCode* code,
                              Address& address, pc_t pc, int& len,
@@ -1781,6 +1790,32 @@ class ThreadImpl {
         len += imm.length;
         return ok;
       }
+      case kExprTableInit: {
+        TableInitImmediate<Decoder::kNoValidate> imm(decoder, code->at(pc));
+        len += imm.length;
+        if (!CheckElemSegmentIsPassiveAndNotDropped(imm.elem_segment_index,
+                                                    pc)) {
+          return false;
+        }
+        auto size = Pop().to<uint32_t>();
+        auto src = Pop().to<uint32_t>();
+        auto dst = Pop().to<uint32_t>();
+        HandleScope scope(isolate_);  // Avoid leaking handles.
+        bool ok = WasmInstanceObject::InitTableEntries(
+            instance_object_->GetIsolate(), instance_object_, imm.table.index,
+            imm.elem_segment_index, dst, src, size);
+        if (!ok) DoTrap(kTrapTableOutOfBounds, pc);
+        return ok;
+      }
+      case kExprElemDrop: {
+        ElemDropImmediate<Decoder::kNoValidate> imm(decoder, code->at(pc));
+        len += imm.length;
+        if (!CheckElemSegmentIsPassiveAndNotDropped(imm.index, pc)) {
+          return false;
+        }
+        instance_object_->dropped_elem_segments()[imm.index] = 1;
+        return true;
+      }
       case kExprTableCopy: {
         TableCopyImmediate<Decoder::kNoValidate> imm(decoder, code->at(pc));
         auto size = Pop().to<uint32_t>();

src/wasm/wasm-module.cc

@@ -27,6 +27,9 @@ namespace v8 {
 namespace internal {
 namespace wasm {
 
+// static
+const uint32_t WasmElemSegment::kNullIndex;
+
 WireBytesRef WasmModule::LookupFunctionName(const ModuleWireBytes& wire_bytes,
                                             uint32_t function_index) const {
   if (!function_names) {

test/cctest/wasm/test-run-wasm-bulk-memory.cc

@@ -392,6 +392,133 @@ void CheckTableCall(Isolate* isolate, Handle<WasmTableObject> table,
 }
 }  // namespace
 
+WASM_EXEC_TEST(TableInitElems) {
+  EXPERIMENTAL_FLAG_SCOPE(bulk_memory);
+  Isolate* isolate = CcTest::InitIsolateOnce();
+  HandleScope scope(isolate);
+  TestSignatures sigs;
+  WasmRunner<uint32_t, uint32_t, uint32_t, uint32_t> r(execution_tier);
+  const uint32_t kTableSize = 5;
+  std::vector<uint32_t> function_indexes;
+  const uint32_t sig_index = r.builder().AddSignature(sigs.i_v());
+
+  for (uint32_t i = 0; i < kTableSize; ++i) {
+    WasmFunctionCompiler& fn = r.NewFunction(sigs.i_v(), "f");
+    BUILD(fn, WASM_I32V_1(i));
+    fn.SetSigIndex(sig_index);
+    function_indexes.push_back(fn.function_index());
+  }
+
+  // Passive element segment has [f0, f1, f2, f3, f4, null].
+  function_indexes.push_back(WasmElemSegment::kNullIndex);
+
+  r.builder().AddIndirectFunctionTable(nullptr, kTableSize);
+  r.builder().AddPassiveElementSegment(function_indexes);
+
+  WasmFunctionCompiler& call = r.NewFunction(sigs.i_i(), "call");
+  BUILD(call, WASM_CALL_INDIRECT0(sig_index, WASM_GET_LOCAL(0)));
+  const uint32_t call_index = call.function_index();
+
+  BUILD(r,
+        WASM_TABLE_INIT(0, WASM_GET_LOCAL(0), WASM_GET_LOCAL(1),
+                        WASM_GET_LOCAL(2)),
+        kExprI32Const, 0);
+
+  auto table = handle(
+      WasmTableObject::cast(r.builder().instance_object()->tables().get(0)),
+      isolate);
+  const double null = 0xDEADBEEF;
+
+  CheckTableCall(isolate, table, r, call_index, null, null, null, null, null);
+
+  // 0 count is ok in bounds, and at end of regions.
+  r.CheckCallViaJS(0, 0, 0, 0);
+  r.CheckCallViaJS(0, kTableSize, 0, 0);
+  r.CheckCallViaJS(0, 0, kTableSize, 0);
+
+  // Test actual writes.
+  r.CheckCallViaJS(0, 0, 0, 1);
+  CheckTableCall(isolate, table, r, call_index, 0, null, null, null, null);
+  r.CheckCallViaJS(0, 0, 0, 2);
+  CheckTableCall(isolate, table, r, call_index, 0, 1, null, null, null);
+  r.CheckCallViaJS(0, 0, 0, 3);
+  CheckTableCall(isolate, table, r, call_index, 0, 1, 2, null, null);
+  r.CheckCallViaJS(0, 3, 0, 2);
+  CheckTableCall(isolate, table, r, call_index, 0, 1, 2, 0, 1);
+  r.CheckCallViaJS(0, 3, 1, 2);
+  CheckTableCall(isolate, table, r, call_index, 0, 1, 2, 1, 2);
+  r.CheckCallViaJS(0, 3, 2, 2);
+  CheckTableCall(isolate, table, r, call_index, 0, 1, 2, 2, 3);
+  r.CheckCallViaJS(0, 3, 3, 2);
+  CheckTableCall(isolate, table, r, call_index, 0, 1, 2, 3, 4);
+}
+
+WASM_EXEC_TEST(TableInitOob) {
+  EXPERIMENTAL_FLAG_SCOPE(bulk_memory);
+  Isolate* isolate = CcTest::InitIsolateOnce();
+  HandleScope scope(isolate);
+  TestSignatures sigs;
+  WasmRunner<uint32_t, uint32_t, uint32_t, uint32_t> r(execution_tier);
+  const uint32_t kTableSize = 5;
+  std::vector<uint32_t> function_indexes;
+  const uint32_t sig_index = r.builder().AddSignature(sigs.i_v());
+
+  for (uint32_t i = 0; i < kTableSize; ++i) {
+    WasmFunctionCompiler& fn = r.NewFunction(sigs.i_v(), "f");
+    BUILD(fn, WASM_I32V_1(i));
+    fn.SetSigIndex(sig_index);
+    function_indexes.push_back(fn.function_index());
+  }
+
+  r.builder().AddIndirectFunctionTable(nullptr, kTableSize);
+  r.builder().AddPassiveElementSegment(function_indexes);
+
+  WasmFunctionCompiler& call = r.NewFunction(sigs.i_i(), "call");
+  BUILD(call, WASM_CALL_INDIRECT0(sig_index, WASM_GET_LOCAL(0)));
+  const uint32_t call_index = call.function_index();
+
+  BUILD(r,
+        WASM_TABLE_INIT(0, WASM_GET_LOCAL(0), WASM_GET_LOCAL(1),
+                        WASM_GET_LOCAL(2)),
+        kExprI32Const, 0);
+
+  auto table = handle(
+      WasmTableObject::cast(r.builder().instance_object()->tables().get(0)),
+      isolate);
+  const double null = 0xDEADBEEF;
+
+  CheckTableCall(isolate, table, r, call_index, null, null, null, null, null);
+
+  // Write all values up to the out-of-bounds write.
+  r.CheckCallViaJS(0xDEADBEEF, 3, 0, 3);
+  CheckTableCall(isolate, table, r, call_index, null, null, null, 0, 1);
+
+  // Write all values up to the out-of-bounds read.
+  r.CheckCallViaJS(0xDEADBEEF, 0, 3, 3);
+  CheckTableCall(isolate, table, r, call_index, 3, 4, null, 0, 1);
+
+  // 0-count is oob.
+  r.CheckCallViaJS(0xDEADBEEF, kTableSize + 1, 0, 0);
+  r.CheckCallViaJS(0xDEADBEEF, 0, kTableSize + 1, 0);
+
+  r.CheckCallViaJS(0xDEADBEEF, 0, 0, 6);
+  r.CheckCallViaJS(0xDEADBEEF, 0, 1, 5);
+  r.CheckCallViaJS(0xDEADBEEF, 0, 2, 4);
+  r.CheckCallViaJS(0xDEADBEEF, 0, 3, 3);
+  r.CheckCallViaJS(0xDEADBEEF, 0, 4, 2);
+  r.CheckCallViaJS(0xDEADBEEF, 0, 5, 1);
+
+  r.CheckCallViaJS(0xDEADBEEF, 0, 0, 6);
+  r.CheckCallViaJS(0xDEADBEEF, 1, 0, 5);
+  r.CheckCallViaJS(0xDEADBEEF, 2, 0, 4);
+  r.CheckCallViaJS(0xDEADBEEF, 3, 0, 3);
+  r.CheckCallViaJS(0xDEADBEEF, 4, 0, 2);
+  r.CheckCallViaJS(0xDEADBEEF, 5, 0, 1);
+
+  r.CheckCallViaJS(0xDEADBEEF, 10, 0, 1);
+  r.CheckCallViaJS(0xDEADBEEF, 0, 10, 1);
+}
+
 WASM_EXEC_TEST(TableCopyElems) {
   EXPERIMENTAL_FLAG_SCOPE(bulk_memory);
   Isolate* isolate = CcTest::InitIsolateOnce();
@@ -567,6 +694,29 @@ WASM_EXEC_TEST(TableCopyOob1) {
   }
 }
 
+WASM_EXEC_TEST(ElemDropTwice) {
+  EXPERIMENTAL_FLAG_SCOPE(bulk_memory);
+  WasmRunner<uint32_t> r(execution_tier);
+  r.builder().AddIndirectFunctionTable(nullptr, 1);
+  r.builder().AddPassiveElementSegment({});
+  BUILD(r, WASM_ELEM_DROP(0), kExprI32Const, 0);
+
+  r.CheckCallViaJS(0);
+  r.CheckCallViaJS(0xDEADBEEF);
+}
+
+WASM_EXEC_TEST(ElemDropThenTableInit) {
+  EXPERIMENTAL_FLAG_SCOPE(bulk_memory);
+  WasmRunner<uint32_t> r(execution_tier);
+  r.builder().AddIndirectFunctionTable(nullptr, 1);
+  r.builder().AddPassiveElementSegment({});
+  BUILD(r, WASM_ELEM_DROP(0),
+        WASM_TABLE_INIT(0, WASM_I32V_1(0), WASM_I32V_1(0), WASM_I32V_1(0)),
+        kExprI32Const, 0);
+
+  r.CheckCallViaJS(0xDEADBEEF);
+}
+
 }  // namespace test_run_wasm_bulk_memory
 }  // namespace wasm
 }  // namespace internal

test/cctest/wasm/wasm-run-utils.cc

@@ -266,6 +266,21 @@ uint32_t TestingModuleBuilder::AddPassiveDataSegment(Vector<const byte> bytes) {
   return index;
 }
 
+uint32_t TestingModuleBuilder::AddPassiveElementSegment(
+    const std::vector<uint32_t>& entries) {
+  uint32_t index = static_cast<uint32_t>(test_module_->elem_segments.size());
+  DCHECK_EQ(index, dropped_elem_segments_.size());
+
+  test_module_->elem_segments.emplace_back();
+  auto& elem_segment = test_module_->elem_segments.back();
+  elem_segment.entries = entries;
+
+  // The vector pointers may have moved, so update the instance object.
+  dropped_elem_segments_.push_back(0);
+  instance_object_->set_dropped_elem_segments(dropped_elem_segments_.data());
+  return index;
+}
+
 CompilationEnv TestingModuleBuilder::CreateCompilationEnv() {
   // This is a hack so we don't need to call
   // trap_handler::IsTrapHandlerEnabled().

test/cctest/wasm/wasm-run-utils.h

@@ -191,6 +191,7 @@ class TestingModuleBuilder {
   uint32_t AddException(FunctionSig* sig);
 
   uint32_t AddPassiveDataSegment(Vector<const byte> bytes);
+  uint32_t AddPassiveElementSegment(const std::vector<uint32_t>& entries);
 
   WasmFunction* GetFunctionAt(int index) {
     return &test_module_->functions[index];
@@ -241,6 +242,7 @@ class TestingModuleBuilder {
   std::vector<Address> data_segment_starts_;
   std::vector<uint32_t> data_segment_sizes_;
   std::vector<byte> dropped_data_segments_;
+  std::vector<byte> dropped_elem_segments_;
 
   const WasmGlobal* AddGlobal(ValueType type);
 
@@ -450,18 +452,21 @@ class WasmRunner : public WasmRunnerBase {
       : WasmRunner(execution_tier, nullptr, "main", kNoRuntimeExceptionSupport,
                    lower_simd) {}
 
-  ReturnType Call(ParamTypes... p) {
-    DCHECK(compiled_);
-    if (interpret()) return CallInterpreter(p...);
-
-    ReturnType return_value = static_cast<ReturnType>(0xDEADBEEFDEADBEEF);
+  void SetUpTrapCallback() {
     WasmRunnerBase::trap_happened = false;
-
     auto trap_callback = []() -> void {
       WasmRunnerBase::trap_happened = true;
       set_trap_callback_for_testing(nullptr);
     };
     set_trap_callback_for_testing(trap_callback);
+  }
+
+  ReturnType Call(ParamTypes... p) {
+    DCHECK(compiled_);
+    if (interpret()) return CallInterpreter(p...);
+
+    ReturnType return_value = static_cast<ReturnType>(0xDEADBEEFDEADBEEF);
+    SetUpTrapCallback();
 
     wrapper_.SetInnerCode(builder_.GetFunctionCode(main_fn_index_));
     wrapper_.SetInstance(builder_.instance_object());
@@ -508,6 +513,7 @@ class WasmRunner : public WasmRunnerBase {
   void CheckCallApplyViaJS(double expected, uint32_t function_index,
                            Handle<Object>* buffer, int count) {
     Isolate* isolate = builder_.isolate();
+    SetUpTrapCallback();
     if (jsfuncs_.size() <= function_index) {
       jsfuncs_.resize(function_index + 1);
     }
@@ -520,7 +526,7 @@ class WasmRunner : public WasmRunnerBase {
         Execution::TryCall(isolate, jsfunc, global, count, buffer,
                            Execution::MessageHandling::kReport, nullptr);
 
-    if (retval.is_null()) {
+    if (retval.is_null() || WasmRunnerBase::trap_happened) {
       CHECK_EQ(expected, static_cast<double>(0xDEADBEEF));
     } else {
       Handle<Object> result = retval.ToHandleChecked();
@@ -539,7 +545,9 @@ class WasmRunner : public WasmRunnerBase {
 
   void CheckCallViaJS(double expected, ParamTypes... p) {
     Isolate* isolate = builder_.isolate();
-    Handle<Object> buffer[] = {isolate->factory()->NewNumber(p)...};
+    // MSVC doesn't allow empty arrays, so include a dummy at the end.
+    Handle<Object> buffer[] = {isolate->factory()->NewNumber(p)...,
+                               Handle<Object>()};
     CheckCallApplyViaJS(expected, function()->func_index, buffer, sizeof...(p));
   }
 

test/mjsunit/wasm/bulk-memory.js

@@ -165,19 +165,3 @@ function getMemoryFill(mem) {
   const instance = builder.instantiate();
   assertTraps(kTrapElemSegmentDropped, () => instance.exports.drop());
 })();
-
-(function TestElemDropTwice() {
-  const builder = new WasmModuleBuilder();
-  builder.setTableBounds(5, 5);
-  builder.addPassiveElementSegment([0, 0, 0]);
-  builder.addFunction('drop', kSig_v_v)
-      .addBody([
-        kNumericPrefix, kExprElemDrop,
-        0,  // Element segment index.
-      ])
-      .exportAs('drop');
-
-  const instance = builder.instantiate();
-  instance.exports.drop();
-  assertTraps(kTrapElemSegmentDropped, () => instance.exports.drop());
-})();

test/mjsunit/wasm/table-init.js

-// Copyright 2018 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-bulk-memory
-
-load("test/mjsunit/wasm/wasm-module-builder.js");
-
-function addFunction(builder, k) {
-  let m = builder.addFunction("", kSig_i_v)
-      .addBody([...wasmI32Const(k)]);
-  return m;
-}
-
-function addFunctions(builder, count) {
-  let o = {};
-  for (var i = 0; i < count; i++) {
-    let name = `f${i}`;
-    o[name] = addFunction(builder, i);
-    o[name].exportAs(name);
-  }
-  return o;
-}
-
-function assertTable(obj, ...elems) {
-  for (var i = 0; i < elems.length; i++) {
-    assertEquals(elems[i], obj.get(i));
-  }
-}
-
-(function TestTableInitInBounds() {
-  print(arguments.callee.name);
-  let builder = new WasmModuleBuilder();
-  let sig_v_iii = builder.addType(kSig_v_iii);
-  let kTableSize = 5;
-
-  builder.setTableBounds(kTableSize, kTableSize);
-  {
-    let o = addFunctions(builder, kTableSize);
-    builder.addPassiveElementSegment(
-       [o.f0.index, o.f1.index, o.f2.index, o.f3.index, o.f4.index, null]);
-  }
-
-  builder.addFunction("init0", sig_v_iii)
-    .addBody([
-      kExprGetLocal, 0,
-      kExprGetLocal, 1,
-      kExprGetLocal, 2,
-      kNumericPrefix, kExprTableInit, kSegmentZero, kTableZero])
-    .exportAs("init0");
-
-  builder.addExportOfKind("table", kExternalTable, 0);
-
-  let instance = builder.instantiate();
-  let x = instance.exports;
-
-  assertTable(x.table, null, null, null, null, null);
-
-  // 0 count is ok in bounds, and at end of regions.
-  x.init0(0, 0, 0);
-  x.init0(kTableSize, 0, 0);
-  x.init0(0, kTableSize, 0);
-
-  // test actual writes.
-  x.init0(0, 0, 1);
-  assertTable(x.table, x.f0, null, null, null, null);
-  x.init0(0, 0, 2);
-  assertTable(x.table, x.f0, x.f1, null, null, null);
-  x.init0(0, 0, 3);
-  assertTable(x.table, x.f0, x.f1, x.f2, null, null);
-  x.init0(3, 0, 2);
-  assertTable(x.table, x.f0, x.f1, x.f2, x.f0, x.f1);
-  x.init0(3, 1, 2);
-  assertTable(x.table, x.f0, x.f1, x.f2, x.f1, x.f2);
-  x.init0(3, 2, 2);
-  assertTable(x.table, x.f0, x.f1, x.f2, x.f2, x.f3);
-  x.init0(3, 3, 2);
-  assertTable(x.table, x.f0, x.f1, x.f2, x.f3, x.f4);
-
-  // test writing null
-  x.init0(0, 5, 1);
-  assertTable(x.table, null, x.f1, x.f2, x.f3, x.f4);
-})();
-
-(function TestTableInitOob() {
-  print(arguments.callee.name);
-  let builder = new WasmModuleBuilder();
-  let sig_v_iii = builder.addType(kSig_v_iii);
-  let kTableSize = 5;
-
-  builder.setTableBounds(kTableSize, kTableSize);
-  {
-    let o = addFunctions(builder, kTableSize);
-    builder.addPassiveElementSegment(
-       [o.f0.index, o.f1.index, o.f2.index, o.f3.index, o.f4.index]);
-  }
-
-  builder.addFunction("init0", sig_v_iii)
-    .addBody([
-      kExprGetLocal, 0,
-      kExprGetLocal, 1,
-      kExprGetLocal, 2,
-      kNumericPrefix, kExprTableInit, kSegmentZero, kTableZero])
-    .exportAs("init0");
-
-  builder.addExportOfKind("table", kExternalTable, 0);
-
-  let instance = builder.instantiate();
-  let x = instance.exports;
-
-  assertTable(x.table, null, null, null, null, null);
-
-  // Write all values up to the out-of-bounds write.
-  assertThrows(() => x.init0(3, 0, 3));
-  assertTable(x.table, null, null, null, x.f0, x.f1);
-
-  // Write all values up to the out-of-bounds read.
-  assertThrows(() => x.init0(0, 3, 3));
-  assertTable(x.table, x.f3, x.f4, null, x.f0, x.f1);
-
-  // 0-count is oob.
-  assertThrows(() => x.init0(kTableSize+1, 0, 0));
-  assertThrows(() => x.init0(0, kTableSize+1, 0));
-
-  assertThrows(() => x.init0(0, 0, 6));
-  assertThrows(() => x.init0(0, 1, 5));
-  assertThrows(() => x.init0(0, 2, 4));
-  assertThrows(() => x.init0(0, 3, 3));
-  assertThrows(() => x.init0(0, 4, 2));
-  assertThrows(() => x.init0(0, 5, 1));
-
-  assertThrows(() => x.init0(0, 0, 6));
-  assertThrows(() => x.init0(1, 0, 5));
-  assertThrows(() => x.init0(2, 0, 4));
-  assertThrows(() => x.init0(3, 0, 3));
-  assertThrows(() => x.init0(4, 0, 2));
-  assertThrows(() => x.init0(5, 0, 1));
-
-  assertThrows(() => x.init0(10, 0, 1));
-  assertThrows(() => x.init0(0, 10, 1));
-})();