[wasm] Keep instances of imported code alive

If one wasm instance imports an exported function of another instance,
we unwrap the js-to-wasm wrapper of the export and use the underlying
code object directly. However, the code object does not keep the wasm
instance alive. It is only connected via a WeakCell.
With this CL, we explicitly store a FixedArray of all wasm instances
from which we imported functions to keep them alive at least as long as
the instance which imports the code.

R=mtrofin@chromium.org, ahaas@chromium.org
BUG=chromium:734345

Change-Id: I8dcfc9a4ea2d791a62d8cb7255039e481c50bdfd
Reviewed-on: https://chromium-review.googlesource.com/539738Reviewed-by: Mircea Trofin <mtrofin@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46062}

src/wasm/module-compiler.cc

@@ -530,16 +530,22 @@ bool in_bounds(uint32_t offset, uint32_t size, uint32_t upper) {
   return offset + size <= upper && offset + size >= offset;
 }
 
-Handle<Code> CompileImportWrapper(Isolate* isolate, int index, FunctionSig* sig,
-                                  Handle<JSReceiver> target,
-                                  Handle<String> module_name,
-                                  MaybeHandle<String> import_name,
-                                  ModuleOrigin origin) {
+using WasmInstanceMap =
+    IdentityMap<Handle<WasmInstanceObject>, FreeStoreAllocationPolicy>;
+
+Handle<Code> UnwrapOrCompileImportWrapper(
+    Isolate* isolate, int index, FunctionSig* sig, Handle<JSReceiver> target,
+    Handle<String> module_name, MaybeHandle<String> import_name,
+    ModuleOrigin origin, WasmInstanceMap* imported_instances) {
   WasmFunction* other_func = GetWasmFunctionForImportWrapper(isolate, target);
   if (other_func) {
     if (!sig->Equals(other_func->sig)) return Handle<Code>::null();
     // Signature matched. Unwrap the import wrapper and return the raw wasm
     // function code.
+    // Remember the wasm instance of the import. We have to keep it alive.
+    Handle<WasmInstanceObject> imported_instance(
+        Handle<WasmExportedFunction>::cast(target)->instance(), isolate);
+    imported_instances->Set(imported_instance, imported_instance);
     return UnwrapImportWrapper(target);
   }
   // No wasm function or being debugged. Compile a new wrapper for the new
@@ -1285,6 +1291,7 @@ int InstanceBuilder::ProcessImports(Handle<FixedArray> code_table,
                                     Handle<WasmInstanceObject> instance) {
   int num_imported_functions = 0;
   int num_imported_tables = 0;
+  WasmInstanceMap imported_wasm_instances(isolate_->heap());
   for (int index = 0; index < static_cast<int>(module_->import_table.size());
        ++index) {
     WasmImport& import = module_->import_table[index];
@@ -1316,10 +1323,10 @@ int InstanceBuilder::ProcessImports(Handle<FixedArray> code_table,
           return -1;
         }
 
-        Handle<Code> import_wrapper = CompileImportWrapper(
+        Handle<Code> import_wrapper = UnwrapOrCompileImportWrapper(
             isolate_, index, module_->functions[import.index].sig,
             Handle<JSReceiver>::cast(value), module_name, import_name,
-            module_->get_origin());
+            module_->get_origin(), &imported_wasm_instances);
         if (import_wrapper.is_null()) {
           ReportLinkError("imported function does not match the expected type",
                           index, module_name, import_name);
@@ -1474,6 +1481,19 @@ int InstanceBuilder::ProcessImports(Handle<FixedArray> code_table,
         break;
     }
   }
+
+  if (!imported_wasm_instances.empty()) {
+    WasmInstanceMap::IteratableScope iteratable_scope(&imported_wasm_instances);
+    Handle<FixedArray> instances_array = isolate_->factory()->NewFixedArray(
+        imported_wasm_instances.size(), TENURED);
+    instance->set_directly_called_instances(*instances_array);
+    int index = 0;
+    for (auto it = iteratable_scope.begin(), end = iteratable_scope.end();
+         it != end; ++it, ++index) {
+      instances_array->set(index, ***it);
+    }
+  }
+
   return num_imported_functions;
 }
 

src/wasm/wasm-objects.cc

@@ -540,6 +540,8 @@ DEFINE_OPTIONAL_OBJ_ACCESSORS(WasmInstanceObject, debug_info, kDebugInfo,
                               WasmDebugInfo)
 DEFINE_OPTIONAL_OBJ_ACCESSORS(WasmInstanceObject, instance_wrapper,
                               kWasmMemInstanceWrapper, WasmInstanceWrapper)
+DEFINE_OPTIONAL_OBJ_ACCESSORS(WasmInstanceObject, directly_called_instances,
+                              kDirectlyCalledInstances, FixedArray)
 
 WasmModuleObject* WasmInstanceObject::module_object() {
   return *compiled_module()->wasm_module();

src/wasm/wasm-objects.h

@@ -132,6 +132,8 @@ class WasmInstanceObject : public JSObject {
     kGlobalsArrayBuffer,
     kDebugInfo,
     kWasmMemInstanceWrapper,
+    // FixedArray of wasm instances whose code we imported (to keep them alive).
+    kDirectlyCalledInstances,
     kFieldCount
   };
 
@@ -143,6 +145,7 @@ class WasmInstanceObject : public JSObject {
   DECLARE_OPTIONAL_ACCESSORS(memory_object, WasmMemoryObject);
   DECLARE_OPTIONAL_ACCESSORS(debug_info, WasmDebugInfo);
   DECLARE_OPTIONAL_ACCESSORS(instance_wrapper, WasmInstanceWrapper);
+  DECLARE_OPTIONAL_ACCESSORS(directly_called_instances, FixedArray);
 
   WasmModuleObject* module_object();
   V8_EXPORT_PRIVATE wasm::WasmModule* module();

test/mjsunit/regress/wasm/regression-734345.js

+// Copyright 2017 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: --expose-gc
+
+load('test/mjsunit/wasm/wasm-constants.js');
+load('test/mjsunit/wasm/wasm-module-builder.js');
+
+builder1 = new WasmModuleBuilder();
+builder1.addFunction('exp1', kSig_v_v).addBody([kExprUnreachable]).exportFunc();
+
+builder2 = new WasmModuleBuilder();
+builder2.addImport('imp', 'imp', kSig_v_v);
+builder2.addFunction('call_imp', kSig_v_v)
+    .addBody([kExprCallFunction, 0])
+    .exportFunc();
+
+export1 = builder1.instantiate().exports.exp1;
+export2 = builder2.instantiate({imp: {imp: export1}}).exports.call_imp;
+export1 = undefined;
+
+let a = [0];
+for (i = 0; i < 10; ++i) {
+  a = a.concat(new Array(i).fill(i));
+  assertThrows(() => export2(), WebAssembly.RuntimeError);
+  gc();
+}