[wasm] Don't store imported WasmCode pointers in code table

When processing imports of an instance, we were storing pointers to
exported (and re-imported) wasm functions in the code table of the
importing module. This is dangerous since imports are instance specific.
Avoid ever storing call targets for imports in the NativeModule.
Instead, read the call targets from the imports table of the instance.

R=mstarzinger@chromium.org

Bug: chromium:843563

Change-Id: Id9f43a6c127025a5feaa81b2be75c001bc0bea81
Reviewed-on: https://chromium-review.googlesource.com/1065774
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53256}

src/wasm/module-compiler.cc

@@ -871,7 +871,9 @@ bool compile_lazy(const WasmModule* module) {
 }
 
 void FlushICache(const wasm::NativeModule* native_module) {
-  for (uint32_t i = 0, e = native_module->function_count(); i < e; ++i) {
+  for (uint32_t i = native_module->num_imported_functions(),
+                e = native_module->function_count();
+       i < e; ++i) {
     const wasm::WasmCode* code = native_module->code(i);
     if (code == nullptr) continue;
     Assembler::FlushICache(code->instructions().start(),
@@ -905,7 +907,9 @@ void RecordStats(const wasm::WasmCode* code, Counters* counters) {
 }
 
 void RecordStats(const wasm::NativeModule* native_module, Counters* counters) {
-  for (uint32_t i = 0, e = native_module->function_count(); i < e; ++i) {
+  for (uint32_t i = native_module->num_imported_functions(),
+                e = native_module->function_count();
+       i < e; ++i) {
     const wasm::WasmCode* code = native_module->code(i);
     if (code != nullptr) RecordStats(code, counters);
   }
@@ -2125,9 +2129,6 @@ int InstanceBuilder::ProcessImports(Handle<WasmInstanceObject> instance) {
           ImportedFunctionEntry entry(instance, func_index);
           Address imported_target = imported_function->GetWasmCallTarget();
           entry.set_wasm_to_wasm(*imported_instance, imported_target);
-          // TODO(clemensh): Remove this. NativeModule must be instance
-          // independent.
-          native_module->set_code(func_index, imported_function->GetWasmCode());
         } else {
           // The imported function is a callable.
           Handle<JSReceiver> js_receiver(JSReceiver::cast(*value), isolate_);
@@ -2651,20 +2652,20 @@ void InstanceBuilder::LoadTableSegments(Handle<WasmInstanceObject> instance) {
 
         // Update the local dispatch table first.
         uint32_t sig_id = module_->signature_ids[function->sig_index];
-        Address call_target =
-            native_module->GetCallTargetForFunction(func_index);
-
+        WasmInstanceObject* target_instance = *instance;
+        Address call_target;
         const bool is_import = func_index < module_->num_imported_functions;
         if (is_import) {
-          // Imported functions have the target instance put into the IFT.
-          WasmInstanceObject* target_instance =
-              ImportedFunctionEntry(instance, func_index).instance();
-          IndirectFunctionTableEntry(instance, table_index)
-              .set(sig_id, target_instance, call_target);
+          // For imported calls, take target instance and address from the
+          // import table.
+          ImportedFunctionEntry entry(instance, func_index);
+          target_instance = entry.instance();
+          call_target = entry.target();
         } else {
-          IndirectFunctionTableEntry(instance, table_index)
-              .set(sig_id, *instance, call_target);
+          call_target = native_module->GetCallTargetForFunction(func_index);
         }
+        IndirectFunctionTableEntry(instance, table_index)
+            .set(sig_id, target_instance, call_target);
 
         if (!table_instance.table_object.is_null()) {
           // Update the table object's other dispatch tables.

src/wasm/wasm-code-manager.h

@@ -252,12 +252,16 @@ class V8_EXPORT_PRIVATE NativeModule final {
 
   WasmCode* code(uint32_t index) const {
     DCHECK_LT(index, function_count());
+    DCHECK_LE(num_imported_functions(), index);
     return code_table_[index];
   }
 
-  void set_code(uint32_t index, WasmCode* wasm_code) {
+  // TODO(clemensh): Remove this method once we have the jump table
+  // (crbug.com/v8/7758).
+  void SetCodeForTesting(uint32_t index, WasmCode* code) {
     DCHECK_LT(index, function_count());
-    code_table_[index] = wasm_code;
+    DCHECK_LE(num_imported_functions(), index);
+    code_table_[index] = code;
   }
 
   bool has_code(uint32_t index) const {

src/wasm/wasm-debug.cc

@@ -553,20 +553,16 @@ wasm::InterpreterHandle* GetInterpreterHandleOrNull(WasmDebugInfo* debug_info) {
   return Managed<wasm::InterpreterHandle>::cast(handle_obj)->raw();
 }
 
-int GetNumFunctions(WasmInstanceObject* instance) {
-  size_t num_functions =
-      instance->module_object()->shared()->module()->functions.size();
-  DCHECK_GE(kMaxInt, num_functions);
-  return static_cast<int>(num_functions);
-}
-
 Handle<FixedArray> GetOrCreateInterpretedFunctions(
     Isolate* isolate, Handle<WasmDebugInfo> debug_info) {
   Handle<Object> obj(debug_info->interpreted_functions(), isolate);
   if (!obj->IsUndefined(isolate)) return Handle<FixedArray>::cast(obj);
 
-  Handle<FixedArray> new_arr = isolate->factory()->NewFixedArray(
-      GetNumFunctions(debug_info->wasm_instance()));
+  int num_functions = debug_info->wasm_instance()
+                          ->compiled_module()
+                          ->GetNativeModule()
+                          ->function_count();
+  Handle<FixedArray> new_arr = isolate->factory()->NewFixedArray(num_functions);
   debug_info->set_interpreted_functions(*new_arr);
   return new_arr;
 }
@@ -603,9 +599,12 @@ void RedirectCallsitesInInstance(Isolate* isolate, WasmInstanceObject* instance,
                                  CodeRelocationMap* map) {
   DisallowHeapAllocation no_gc;
   // Redirect all calls in wasm functions.
-  for (uint32_t i = 0, e = GetNumFunctions(instance); i < e; ++i) {
-    wasm::WasmCode* code =
-        instance->compiled_module()->GetNativeModule()->code(i);
+  wasm::NativeModule* native_module =
+      instance->compiled_module()->GetNativeModule();
+  for (uint32_t i = native_module->num_imported_functions(),
+                e = native_module->function_count();
+       i < e; ++i) {
+    wasm::WasmCode* code = native_module->code(i);
     RedirectCallsitesInCode(isolate, code, map);
   }
   // TODO(6668): Find instances that imported our code and also patch those.

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

@@ -3463,8 +3463,9 @@ TEST(Liftoff_prologue) {
     CHECK_EQ(10, r.Call(1, 2, 3, 4));
 
     // Update the native_module to contain the "optimized" code ({sub_locals}).
-    native_module->set_code(add_compiler.function_index(),
-                            native_module->code(sub_compiler.function_index()));
+    native_module->SetCodeForTesting(
+        add_compiler.function_index(),
+        native_module->code(sub_compiler.function_index()));
 
     // Second run should execute {add_locals}, which should detect that
     // the code was updated, and run {sub_locals}.