From 9d52960e4d097d2503d9cda7b36c1716dcec01da Mon Sep 17 00:00:00 2001 From: Wenyong Huang Date: Wed, 7 Dec 2022 16:43:04 +0800 Subject: [PATCH] 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. --- core/iwasm/aot/aot_runtime.c | 22 +++++- core/iwasm/aot/aot_runtime.h | 4 + core/iwasm/common/wasm_c_api.c | 80 ++++++++++++++++---- core/iwasm/interpreter/wasm.h | 1 - core/iwasm/interpreter/wasm_interp_classic.c | 11 ++- core/iwasm/interpreter/wasm_interp_fast.c | 11 ++- core/iwasm/interpreter/wasm_runtime.c | 11 ++- core/iwasm/interpreter/wasm_runtime.h | 12 +++ 8 files changed, 128 insertions(+), 24 deletions(-) diff --git a/core/iwasm/aot/aot_runtime.c b/core/iwasm/aot/aot_runtime.c index 1077c3af..133c5e38 100644 --- a/core/iwasm/aot/aot_runtime.c +++ b/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; diff --git a/core/iwasm/aot/aot_runtime.h b/core/iwasm/aot/aot_runtime.h index d02af3cb..a6371729 100644 --- a/core/iwasm/aot/aot_runtime.h +++ b/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 { diff --git a/core/iwasm/common/wasm_c_api.c b/core/iwasm/common/wasm_c_api.c index f6212d5d..619e0229 100644 --- a/core/iwasm/common/wasm_c_api.c +++ b/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]; diff --git a/core/iwasm/interpreter/wasm.h b/core/iwasm/interpreter/wasm.h index 9c6a2ce3..58305a7f 100644 --- a/core/iwasm/interpreter/wasm.h +++ b/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 { diff --git a/core/iwasm/interpreter/wasm_interp_classic.c b/core/iwasm/interpreter/wasm_interp_classic.c index 45ef4d56..39c23f9f 100644 --- a/core/iwasm/interpreter/wasm_interp_classic.c +++ b/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]; diff --git a/core/iwasm/interpreter/wasm_interp_fast.c b/core/iwasm/interpreter/wasm_interp_fast.c index df356d56..3109b0c8 100644 --- a/core/iwasm/interpreter/wasm_interp_fast.c +++ b/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]; diff --git a/core/iwasm/interpreter/wasm_runtime.c b/core/iwasm/interpreter/wasm_runtime.c index 8e8f96b5..080a0eea 100644 --- a/core/iwasm/interpreter/wasm_runtime.c +++ b/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; diff --git a/core/iwasm/interpreter/wasm_runtime.h b/core/iwasm/interpreter/wasm_runtime.h index 3146b18f..1ebac5f3 100644 --- a/core/iwasm/interpreter/wasm_runtime.h +++ b/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;