Fix wasm-c-api import func link issue in wasm_instance_new (#1787)

When a wasm module is duplicated instantiated with wasm_instance_new,
the function import info of the previous instantiation may be overwritten by
the later instantiation, which may cause unexpected behavior.

Store the function import info into the module instance to fix the issue.

Author: wenyongh

core/iwasm/aot/aot_runtime.c

@@ -987,7 +987,7 @@ aot_instantiate(AOTModule *module, bool is_sub_inst, uint32 stack_size,
         (uint64)module->memory_count * sizeof(AOTMemoryInstance);
     uint64 total_size, table_size = 0;
     uint8 *p;
-    uint32 i;
+    uint32 i, extra_info_offset;
 
     /* Check heap size */
     heap_size = align_uint(heap_size, 8);
@@ -1015,6 +1015,11 @@ aot_instantiate(AOTModule *module, bool is_sub_inst, uint32 stack_size,
     }
     total_size += table_size;
 
+    /* The offset of AOTModuleInstanceExtra, make it 8-byte aligned */
+    total_size = (total_size + 7LL) & ~7LL;
+    extra_info_offset = (uint32)total_size;
+    total_size += sizeof(AOTModuleInstanceExtra);
+
     /* Allocate module instance, global data, table data and heap data */
     if (!(module_inst =
               runtime_malloc(total_size, error_buf, error_buf_size))) {
@@ -1023,6 +1028,8 @@ aot_instantiate(AOTModule *module, bool is_sub_inst, uint32 stack_size,
 
     module_inst->module_type = Wasm_Module_AoT;
     module_inst->module = (void *)module;
+    module_inst->e =
+        (WASMModuleInstanceExtra *)((uint8 *)module_inst + extra_info_offset);
 
     /* Initialize global info */
     p = (uint8 *)module_inst + module_inst_struct_size
@@ -1179,6 +1186,10 @@ aot_deinstantiate(AOTModuleInstance *module_inst, bool is_sub_inst)
     if (module_inst->exec_env_singleton)
         wasm_exec_env_destroy((WASMExecEnv *)module_inst->exec_env_singleton);
 
+    if (((AOTModuleInstanceExtra *)module_inst->e)->c_api_func_imports)
+        wasm_runtime_free(
+            ((AOTModuleInstanceExtra *)module_inst->e)->c_api_func_imports);
+
     wasm_runtime_free(module_inst);
 }
 
@@ -1750,6 +1761,10 @@ aot_invoke_native(WASMExecEnv *exec_env, uint32 func_idx, uint32 argc,
     AOTModuleInstance *module_inst =
         (AOTModuleInstance *)wasm_runtime_get_module_inst(exec_env);
     AOTModule *aot_module = (AOTModule *)module_inst->module;
+    AOTModuleInstanceExtra *module_inst_extra =
+        (AOTModuleInstanceExtra *)module_inst->e;
+    CApiFuncImport *c_api_func_import =
+        module_inst_extra->c_api_func_imports + func_idx;
     uint32 *func_type_indexes = module_inst->func_type_indexes;
     uint32 func_type_idx = func_type_indexes[func_idx];
     AOTFuncType *func_type = aot_module->func_types[func_type_idx];
@@ -1764,6 +1779,9 @@ aot_invoke_native(WASMExecEnv *exec_env, uint32 func_idx, uint32 argc,
     bh_assert(func_idx < aot_module->import_func_count);
 
     import_func = aot_module->import_funcs + func_idx;
+    if (import_func->call_conv_wasm_c_api)
+        func_ptr = c_api_func_import->func_ptr_linked;
+
     if (!func_ptr) {
         snprintf(buf, sizeof(buf),
                  "failed to call unlinked import function (%s, %s)",
@@ -1776,7 +1794,7 @@ aot_invoke_native(WASMExecEnv *exec_env, uint32 func_idx, uint32 argc,
     if (import_func->call_conv_wasm_c_api) {
         ret = wasm_runtime_invoke_c_api_native(
             (WASMModuleInstanceCommon *)module_inst, func_ptr, func_type, argc,
-            argv, import_func->wasm_c_api_with_env, attachment);
+            argv, c_api_func_import->with_env_arg, c_api_func_import->env_arg);
     }
     else if (!import_func->call_conv_raw) {
         signature = import_func->signature;

core/iwasm/aot/aot_runtime.h

@@ -73,6 +73,10 @@ typedef struct AOTFunctionInstance {
     } u;
 } AOTFunctionInstance;
 
+typedef struct AOTModuleInstanceExtra {
+    CApiFuncImport *c_api_func_imports;
+} AOTModuleInstanceExtra;
+
 #if defined(OS_ENABLE_HW_BOUND_CHECK) && defined(BH_PLATFORM_WINDOWS)
 /* clang-format off */
 typedef struct AOTUnwindInfo {

core/iwasm/common/wasm_c_api.c

@@ -4282,15 +4282,13 @@ interp_link_func(const wasm_instance_t *inst, const WASMModule *module_interp,
         return false;
 
     imported_func_interp->u.function.call_conv_wasm_c_api = true;
-    imported_func_interp->u.function.wasm_c_api_with_env = import->with_env;
-    if (import->with_env) {
+    /* only set func_ptr_linked to avoid unlink warning during instantiation,
+       func_ptr_linked, with_env and env will be stored in module instance's
+       c_api_func_imports later and used when calling import function */
+    if (import->with_env)
         imported_func_interp->u.function.func_ptr_linked = import->u.cb_env.cb;
-        imported_func_interp->u.function.attachment = import->u.cb_env.env;
-    }
-    else {
+    else
         imported_func_interp->u.function.func_ptr_linked = import->u.cb;
-        imported_func_interp->u.function.attachment = NULL;
-    }
     import->func_idx_rt = func_idx_rt;
 
     return true;
@@ -4496,15 +4494,13 @@ aot_link_func(const wasm_instance_t *inst, const AOTModule *module_aot,
         return false;
 
     import_aot_func->call_conv_wasm_c_api = true;
-    import_aot_func->wasm_c_api_with_env = import->with_env;
-    if (import->with_env) {
+    /* only set func_ptr_linked to avoid unlink warning during instantiation,
+       func_ptr_linked, with_env and env will be stored in module instance's
+       c_api_func_imports later and used when calling import function */
+    if (import->with_env)
         import_aot_func->func_ptr_linked = import->u.cb_env.cb;
-        import_aot_func->attachment = import->u.cb_env.env;
-    }
-    else {
+    else
         import_aot_func->func_ptr_linked = import->u.cb;
-        import_aot_func->attachment = NULL;
-    }
     import->func_idx_rt = import_func_idx_rt;
 
     return true;
@@ -4718,10 +4714,12 @@ wasm_instance_new_with_args(wasm_store_t *store, const wasm_module_t *module,
 {
     char sub_error_buf[128] = { 0 };
     char error_buf[256] = { 0 };
-    uint32 import_count = 0;
     bool import_count_verified = false;
     wasm_instance_t *instance = NULL;
-    uint32 i = 0;
+    WASMModuleInstance *inst_rt;
+    CApiFuncImport *func_import = NULL, **p_func_imports = NULL;
+    uint32 i = 0, import_count = 0, import_func_count = 0;
+    uint64 total_size;
     bool processed = false;
 
     bh_assert(singleton_engine);
@@ -4804,6 +4802,56 @@ wasm_instance_new_with_args(wasm_store_t *store, const wasm_module_t *module,
         goto failed;
     }
 
+    inst_rt = (WASMModuleInstance *)instance->inst_comm_rt;
+#if WASM_ENABLE_INTERP != 0
+    if (instance->inst_comm_rt->module_type == Wasm_Module_Bytecode) {
+        p_func_imports = &inst_rt->e->c_api_func_imports;
+        import_func_count = inst_rt->module->import_function_count;
+    }
+#endif
+#if WASM_ENABLE_AOT != 0
+    if (instance->inst_comm_rt->module_type == Wasm_Module_AoT) {
+        p_func_imports =
+            &((AOTModuleInstanceExtra *)inst_rt->e)->c_api_func_imports;
+        import_func_count = ((AOTModule *)inst_rt->module)->import_func_count;
+    }
+#endif
+    bh_assert(p_func_imports);
+
+    /* create the c-api func import list */
+    total_size = (uint64)sizeof(CApiFuncImport) * import_func_count;
+    if (total_size > 0
+        && !(*p_func_imports = func_import = malloc_internal(total_size))) {
+        snprintf(sub_error_buf, sizeof(sub_error_buf),
+                 "Failed to create wasm-c-api func imports");
+        goto failed;
+    }
+
+    /* fill in c-api func import list */
+    for (i = 0; i < import_count; i++) {
+        wasm_func_t *func_host;
+        wasm_extern_t *in;
+
+        in = imports->data[i];
+        if (wasm_extern_kind(in) != WASM_EXTERN_FUNC)
+            continue;
+
+        func_host = wasm_extern_as_func(in);
+
+        func_import->with_env_arg = func_host->with_env;
+        if (func_host->with_env) {
+            func_import->func_ptr_linked = func_host->u.cb_env.cb;
+            func_import->env_arg = func_host->u.cb_env.env;
+        }
+        else {
+            func_import->func_ptr_linked = func_host->u.cb;
+            func_import->env_arg = NULL;
+        }
+
+        func_import++;
+    }
+    bh_assert((uint32)(func_import - *p_func_imports) == import_func_count);
+
     /* fill with inst */
     for (i = 0; imports && imports->data && i < (uint32)import_count; ++i) {
         wasm_extern_t *import = imports->data[i];

core/iwasm/interpreter/wasm.h

@@ -189,7 +189,6 @@ typedef struct WASMFunctionImport {
     WASMFunction *import_func_linked;
 #endif
     bool call_conv_wasm_c_api;
-    bool wasm_c_api_with_env;
 } WASMFunctionImport;
 
 typedef struct WASMGlobalImport {

core/iwasm/interpreter/wasm_interp_classic.c

@@ -838,6 +838,7 @@ wasm_interp_call_func_native(WASMModuleInstance *module_inst,
                              WASMInterpFrame *prev_frame)
 {
     WASMFunctionImport *func_import = cur_func->u.func_import;
+    CApiFuncImport *c_api_func_import = NULL;
     unsigned local_cell_num = 2;
     WASMInterpFrame *frame;
     uint32 argv_ret[2], cur_func_index;
@@ -858,7 +859,13 @@ wasm_interp_call_func_native(WASMModuleInstance *module_inst,
 
     cur_func_index = (uint32)(cur_func - module_inst->e->functions);
     bh_assert(cur_func_index < module_inst->module->import_function_count);
-    native_func_pointer = module_inst->import_func_ptrs[cur_func_index];
+    if (!func_import->call_conv_wasm_c_api) {
+        native_func_pointer = module_inst->import_func_ptrs[cur_func_index];
+    }
+    else {
+        c_api_func_import = module_inst->e->c_api_func_imports + cur_func_index;
+        native_func_pointer = c_api_func_import->func_ptr_linked;
+    }
 
     if (!native_func_pointer) {
         snprintf(buf, sizeof(buf),
@@ -872,7 +879,7 @@ wasm_interp_call_func_native(WASMModuleInstance *module_inst,
         ret = wasm_runtime_invoke_c_api_native(
             (WASMModuleInstanceCommon *)module_inst, native_func_pointer,
             func_import->func_type, cur_func->param_cell_num, frame->lp,
-            func_import->wasm_c_api_with_env, func_import->attachment);
+            c_api_func_import->with_env_arg, c_api_func_import->env_arg);
         if (ret) {
             argv_ret[0] = frame->lp[0];
             argv_ret[1] = frame->lp[1];

core/iwasm/interpreter/wasm_interp_fast.c

@@ -902,6 +902,7 @@ wasm_interp_call_func_native(WASMModuleInstance *module_inst,
                              WASMInterpFrame *prev_frame)
 {
     WASMFunctionImport *func_import = cur_func->u.func_import;
+    CApiFuncImport *c_api_func_import = NULL;
     unsigned local_cell_num = 2;
     WASMInterpFrame *frame;
     uint32 argv_ret[2], cur_func_index;
@@ -921,7 +922,13 @@ wasm_interp_call_func_native(WASMModuleInstance *module_inst,
 
     cur_func_index = (uint32)(cur_func - module_inst->e->functions);
     bh_assert(cur_func_index < module_inst->module->import_function_count);
-    native_func_pointer = module_inst->import_func_ptrs[cur_func_index];
+    if (!func_import->call_conv_wasm_c_api) {
+        native_func_pointer = module_inst->import_func_ptrs[cur_func_index];
+    }
+    else {
+        c_api_func_import = module_inst->e->c_api_func_imports + cur_func_index;
+        native_func_pointer = c_api_func_import->func_ptr_linked;
+    }
 
     if (!native_func_pointer) {
         char buf[128];
@@ -936,7 +943,7 @@ wasm_interp_call_func_native(WASMModuleInstance *module_inst,
         ret = wasm_runtime_invoke_c_api_native(
             (WASMModuleInstanceCommon *)module_inst, native_func_pointer,
             func_import->func_type, cur_func->param_cell_num, frame->lp,
-            func_import->wasm_c_api_with_env, func_import->attachment);
+            c_api_func_import->with_env_arg, c_api_func_import->env_arg);
         if (ret) {
             argv_ret[0] = frame->lp[0];
             argv_ret[1] = frame->lp[1];

core/iwasm/interpreter/wasm_runtime.c

@@ -1948,6 +1948,9 @@ wasm_deinstantiate(WASMModuleInstance *module_inst, bool is_sub_inst)
         os_mutex_destroy(&module_inst->e->mem_lock);
 #endif
 
+    if (module_inst->e->c_api_func_imports)
+        wasm_runtime_free(module_inst->e->c_api_func_imports);
+
     wasm_runtime_free(module_inst);
 }
 
@@ -2849,6 +2852,7 @@ llvm_jit_invoke_native(WASMExecEnv *exec_env, uint32 func_idx, uint32 argc,
     WASMType *func_type;
     void *func_ptr;
     WASMFunctionImport *import_func;
+    CApiFuncImport *c_api_func_import = NULL;
     const char *signature;
     void *attachment;
     char buf[96];
@@ -2870,6 +2874,11 @@ llvm_jit_invoke_native(WASMExecEnv *exec_env, uint32 func_idx, uint32 argc,
     bh_assert(func_idx < module->import_function_count);
 
     import_func = &module->import_functions[func_idx].u.function;
+    if (import_func->call_conv_wasm_c_api) {
+        c_api_func_import = module_inst->e->c_api_func_imports + func_idx;
+        func_ptr = c_api_func_import->func_ptr_linked;
+    }
+
     if (!func_ptr) {
         snprintf(buf, sizeof(buf),
                  "failed to call unlinked import function (%s, %s)",
@@ -2882,7 +2891,7 @@ llvm_jit_invoke_native(WASMExecEnv *exec_env, uint32 func_idx, uint32 argc,
     if (import_func->call_conv_wasm_c_api) {
         ret = wasm_runtime_invoke_c_api_native(
             (WASMModuleInstanceCommon *)module_inst, func_ptr, func_type, argc,
-            argv, import_func->wasm_c_api_with_env, attachment);
+            argv, c_api_func_import->with_env_arg, c_api_func_import->env_arg);
     }
     else if (!import_func->call_conv_raw) {
         signature = import_func->signature;

core/iwasm/interpreter/wasm_runtime.h

@@ -192,6 +192,16 @@ typedef struct WASMExportMemInstance {
     WASMMemoryInstance *memory;
 } WASMExportMemInstance;
 
+/* wasm-c-api import function info */
+typedef struct CApiFuncImport {
+    /* host func pointer after linked */
+    void *func_ptr_linked;
+    /* whether the host func has env argument */
+    bool with_env_arg;
+    /* the env argument of the host func */
+    void *env_arg;
+} CApiFuncImport;
+
 /* Extra info of WASM module instance for interpreter/jit mode */
 typedef struct WASMModuleInstanceExtra {
     WASMGlobalInstance *globals;
@@ -205,6 +215,8 @@ typedef struct WASMModuleInstanceExtra {
     WASMFunctionInstance *free_function;
     WASMFunctionInstance *retain_function;
 
+    CApiFuncImport *c_api_func_imports;
+
 #if WASM_ENABLE_SHARED_MEMORY != 0
     /* lock for shared memory atomic operations */
     korp_mutex mem_lock;